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

nix-prefetch-git: use fetchgit's naming heuristic #12597

Merged
merged 1 commit into from
Feb 13, 2016

Conversation

zimbatm
Copy link
Member

@zimbatm zimbatm commented Jan 24, 2016

This commit fixes #6651.

Before this change the nix-prefetch-git script would use a different store
name than nix's fetchgit function. Because of that it was not possible to
use nix-prefetch-git as a way to pre-populate the store (for example when
the user it using private git dependencies that needs access to the ssh agent)

@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @bjornfor, @nbp and @rbvermaa to be potential reviewers

@jagajaga
Copy link
Member

Do we need the second commit?

@zimbatm
Copy link
Member Author

zimbatm commented Jan 24, 2016

@jagajaga it belongs to a second PR, I'll remove it

@zimbatm zimbatm force-pushed the nix-prefetch-git-name branch from 611f799 to 35fe889 Compare January 24, 2016 21:46
@zimbatm
Copy link
Member Author

zimbatm commented Jan 24, 2016

Now part of #12603

This commit fixes NixOS#6651.

Before this change the `nix-prefetch-git` script would use a different store
name than nix's `fetchgit` function. Because of that it was not possible to
use `nix-prefetch-git` as a way to pre-populate the store (for example when
the user it using private git dependencies that needs access to the ssh agent)
@zimbatm zimbatm force-pushed the nix-prefetch-git-name branch from 35fe889 to 02f5a01 Compare February 13, 2016 14:40
@zimbatm
Copy link
Member Author

zimbatm commented Feb 13, 2016

@jagajaga @copumpkin merge ?

@zimbatm zimbatm closed this Feb 13, 2016
@zimbatm zimbatm deleted the nix-prefetch-git-name branch February 13, 2016 14:46
@zimbatm zimbatm restored the nix-prefetch-git-name branch February 13, 2016 14:48
@zimbatm zimbatm reopened this Feb 13, 2016
@copumpkin
Copy link
Member

Looks good, thanks

copumpkin added a commit that referenced this pull request Feb 13, 2016
nix-prefetch-git: use fetchgit's naming heuristic
@copumpkin copumpkin merged commit e63a3a6 into NixOS:master Feb 13, 2016
@zimbatm zimbatm deleted the nix-prefetch-git-name branch February 13, 2016 16:41
@domenkozar
Copy link
Member

To be honest, I'd like to revert this, see NixOS/nix#904 (comment) for reasoning

@zimbatm
Copy link
Member Author

zimbatm commented May 19, 2016

I'm not sure about the reasoning, do you mean that we are now getting different derivations between fetchgit and nix-prefetch-git because of this change ?

Would it be possible to implement nix-prefetch-git using nix-repl + fetchgit and get rid of the difference once and for all ?

@domenkozar
Copy link
Member

They use the same script (minus some wrappers). I'd like to get the same hash as for builtins.fetchgit and builtins.fetchTarball

@zimbatm
Copy link
Member Author

zimbatm commented May 19, 2016

I wouldn't be upset if you reverted the commit since the goal was to achieve exactly that :)

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

Successfully merging this pull request may close these issues.

nix-prefetch-git produces different store paths from fetchgit
5 participants