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

fetchgitPrivate: Remove fetcher #73406

Merged
merged 1 commit into from Nov 14, 2019

Conversation

adisbladis
Copy link
Member

@adisbladis adisbladis commented Nov 14, 2019

Since Nix 2.0 we have builtins.fetchGit which is a much better
option since it runs in the evaluator and has access to the regular
users ssh keys.

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @

Since Nix 2.0 we have `builtins.fetchGit` which is a much better
option since it runs in the evaluator and has access to the regular
users ssh keys.
Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only mention is the package itself

[09:33:24] jon@jon-desktop /home/jon/projects/nixpkgs (master)
$ rg fetchgitPrivate
pkgs/top-level/all-packages.nix
253:  fetchgitPrivate = callPackage ../build-support/fetchgit/private.nix { };

@jonringer jonringer merged commit e578b84 into NixOS:master Nov 14, 2019
@c0bw3b
Copy link
Contributor

c0bw3b commented Nov 14, 2019

I guess there may be some private (sic) use of it out of nixpkgs. Maybe a mention of the alternative in 20.03 release note would be good.

@jonringer
Copy link
Contributor

probably more correct, would be make an alias of builtins.fetchgit

@takeda
Copy link
Contributor

takeda commented Dec 19, 2019

@adisbladis So I just learned that this removal happened, I used fetchgitPrivate in combination with buildGoPackage like this:

  buildGoPackage = pkgs.buildGoPackage.override {
    go = pkgs.go_1_13;
    fetchgit = pkgs.fetchgitPrivate;
  };

like this and it allowed me to use nix with packages that had private repos, any idea how I can do that without it? If there's a way to do it with buildGoModule that would be even better.

@jonringer
Copy link
Contributor

@takeda

  buildGoPackage = pkgs.buildGoPackage.override {
    go = pkgs.go_1_13;
    fetchgit = builtins.fetchGit;
  };

@takeda
Copy link
Contributor

takeda commented Dec 19, 2019

@jonringer I tried it, but unfortunately that doesn't work.

fetchGit doesn't expect sha256 parameter, when I write a wrapper that that removes that parameter I also need to rewrite that rev parameter is mapped to ref. But then there is an issue because I need to distinguish between hash and a tag. With a tag it supposed to be "refs/tags/${x.rev}" (example of repo: github.com/BurntSushi/toml rev v0.3.1), the problem though is when hash is given I need to provide that hash supposed to be provided as rev parameter, which makes things more complex. To make things worse, hashes generated by vgo2nix are short (I guess because go.mod only stores short version), but builtins.fetchGit expects the entire length. Example of repo: github.com/PagerDuty/go-pagerduty rev 94ee1c55dbdb.

This is where I'm right now.

pkgs.fetchgitPrivate was a drop in replacement and just worked without issues.

@jonringer
Copy link
Contributor

your package might be private, but the dependencies should be public (unless go implemented private repo references that I'm unaware of (I don't mess much with go)). just use builtins.fetchGit on the src of your private go package, instead of editing how all go packages are built

@takeda
Copy link
Contributor

takeda commented Dec 19, 2019

Go internally uses git for fetching. There are two ways to make it work with private repos:

One way (most convenient) to make it work with private repo is to add this to the ~/.gitconfig configuration:

[url "ssh://git@github.com/"]
        insteadOf = https://github.com/

Then it will fall back to ssh and be able to access all private repos.

(this is also how fetchgitPrivate was working by passing ssh-agent socket)

Another way is to create .netrc file (I see that's what fetchFromGithub supports now) but it requires to generate a token and it is not as straight forward.

@jonringer
Copy link
Contributor

beyond me, sorry ;(

@takeda
Copy link
Contributor

takeda commented Dec 19, 2019

Would this be a good reason to restore fetchgitPrivate?

Alternatively is there a way for builtins.fetchGit have a wrapper that could be a drop-in replacement for fetchgit? Or (maybe even better) fetchgit use builtins.fetchGit? In last case that would make fetchgit and fetchFromGithub automatically work with private repos.

@tekeri
Copy link
Contributor

tekeri commented Jan 20, 2020

As a side note, I use fetchgitPrivate as much as possible. Because:

  • building on remote server with fast connection,
  • suffers low-speed Wi-Fi,
  • fetchGit is slow since cloning and uploading some big and frequently changing repositories takes extra time.

I wish we could have fetch and build remotely with local ssh keys...

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/why-doesnt-fetchfromgithub-support-submodules-with-private/6600/3

myme added a commit to myme/node2nix that referenced this pull request Apr 27, 2020
fetchgitPrivate was removed in
NixOS/nixpkgs#73406

Apparently builtins.fetchGit has support for user ssh keys, etc.

This is a temporary hack, which seems to work for a simple use-case.
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

7 participants