Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fetchers: use a meaningful name #63596

Closed
wants to merge 1 commit into from

Conversation

layus
Copy link
Member

@layus layus commented Jun 20, 2019

Motivation for this change

This adds a meaningful name to source packages fetched with fetch*
helper functions. The idea is to guess a good name based on the repo
name and the revision. When the revision is a hash, it is truncated to
length 10.

To get an idea of the generated names, please look at
https://gist.github.com/layus/7b176dd31b87a30be274d6de40f658b8. This was
generated with nox-review and also hints at the massive rebuild incurred.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@layus layus requested review from edolstra and nbp as code owners June 20, 2019 23:49
@layus layus mentioned this pull request Jun 20, 2019
10 tasks
@7c6f434c
Copy link
Member

Looks good. I think there was the idea that upstream repo names sometimes change (which causes a rebuild if the repo name is included into the source derivation name). Should we check how often this happens in practice in Nixpkgs?

@layus
Copy link
Member Author

layus commented Jun 21, 2019

Looks good. I think there was the idea that upstream repo names sometimes change (which causes a rebuild if the repo name is included into the source derivation name). Should we check how often this happens in practice in Nixpkgs?

For me it looks like a no-brainer, but there is indeed a small risk. This risk is mostly alleviated by the hydra cache, so that the name will only really need to change when version changes, and in that case you should exêct changes in the sources themselves.

@edolstra
Copy link
Member

edolstra commented Jun 21, 2019

No, we explicitly moved to source to prevent variation between various fetchers and spurious rebuilds. See c3255fe and NixOS/nix@65b5f17.

@edolstra edolstra closed this Jun 21, 2019
@edolstra
Copy link
Member

A concrete example of why we don't want to include the repo name: it would cause different store paths for the same commits from the nixpkgs and nixpkgs-channels repositories.

@layus
Copy link
Member Author

layus commented Jun 23, 2019

It makes sense in the current state of Nix. This is the kind of workaround/limitation that a CAS implementation would fix.
Thanks for explaining further BTW.

@vcunat
Copy link
Member

vcunat commented Jun 23, 2019

Can you point out how it would be fixed? These basically are content-addressable already, and we keep the names the same so that the addresses are the same more often.

@layus
Copy link
Member Author

layus commented Jun 23, 2019

In this case, there is a confusion on whether the name should be part of the content. In a real CAS, it clearly should not. In this case, for good reasons, we are trying to mimic a CAS for these source paths, but we deprive ourselves from some ways to know where it comes from, that we would have in CASes as I see them.

Speaking of which, we could further narrow the gap from a real CAS by having /nix/store/<hash>-nix-2.0.1-source.drv producing /nix/store/<hash2>-source but I am not aware of any way to decouple the .drv name from the output path name.

BTW, "cas" may be a better name than "source", as it hints at the reason why we `constantify' it.

@vcunat
Copy link
Member

vcunat commented Jun 23, 2019

Well, I meant it confused me that in the first post you desire more human-meaningful information in the name and in the last it seems the other way around. I suppose you assume we'd have some kind of meta-data passed along (while excluded from content) that would contain extra information like human-meaningful name? (And/or information about one or more of its possible *.drv.)

@layus
Copy link
Member Author

layus commented Jun 24, 2019

Well, I meant it confused me that in the first post you desire more human-meaningful information in the name and in the last it seems the other way around. I suppose you assume we'd have some kind of meta-data passed along (while excluded from content) that would contain extra information like human-meaningful name? (And/or information about one or more of its possible *.drv.)

That's it !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants