NixOS / nix

Nix, the purely functional package manager
https://nixos.org/
GNU Lesser General Public License v2.1
12.84k stars 1.52k forks source link

Add option to use github.com instead of api.github.com (which may be blocked) for `github:` flake ref #5409

Open bew opened 3 years ago

bew commented 3 years ago

Is your feature request related to a problem? Please describe. My company recently blocked access to https://api.github.com, but we can still access most parts of https://github.com.

When referencing flake inputs with github:numtide/devshell (for example), the download URL uses the blocked URL 😢. Thus downloading the tarballs fails.

Related code: https://github.com/NixOS/nix/blob/fda4efff873730f0065cf35f0bc5b36983d675c7/src/libfetchers/github.cc#L260-L267 and https://github.com/NixOS/nix/blob/fda4efff873730f0065cf35f0bc5b36983d675c7/src/libfetchers/github.cc#L243-L247

It also blocks my use of nix run foo#bar where foo is a flake reference to a github branch, it tries to access https://api.github.com/repos/nixos/nixpkgs/commits/nixpkgs-unstable but fails..

Related code: (thanks @Kha for the find) https://github.com/NixOS/nix/blob/37cc50f2c88b50086a1a8ed20631d1427d445b7e/src/libfetchers/github.cc#L246-L247

Describe the solution you'd like A config option to choose github.com host to download tarballs.

For example, the equivalent of: https://api.github.com/repos/numtide/devshell/tarball/4b5ac7cf7d9a1cc60b965bb51b59922f2210cbc7 is: https://github.com/numtide/devshell/tarball/4b5ac7cf7d9a1cc60b965bb51b59922f2210cbc7 (which does work at my company)

Describe alternatives you've considered

Use git+https://github.com/numtide/devshell for my own dependencies. ⚠️ BUT if one of my dependency uses a github:... flake reference, I'm back to square one and I can't use that dependency.

Additional context nix --version: nix (Nix) 2.4pre20210326_dd77f71

roberth commented 3 years ago

I don't oppose making this configurable. Please don't change the default though. Certain auth tokens don't work with the github.com archive endpoints.

Kha commented 3 years ago

What about rate limits, do those apply only to the former URL? If yes, that would be a good reason to dynamically fall back to the latter. Nix shouldn't fail to fetch a github input simply because there have been 60 other API requests from the same public IP (e.g. a whole office, university, ...) in the same hour.

edit: there is in fact a comment in the source in that vein: https://github.com/NixOS/nix/blob/37cc50f2c88b50086a1a8ed20631d1427d445b7e/src/libfetchers/github.cc#L262-L263

Note that when not specifying a rev, there is another API request: https://github.com/NixOS/nix/blob/37cc50f2c88b50086a1a8ed20631d1427d445b7e/src/libfetchers/github.cc#L246-L247 But the tarball request certainly is the more important one.

bew commented 3 years ago

And I just realized it also blocks my use of nix run unstable#foobar where unstable is a flake reference to nixpkgs's unstable branch, it tries to access https://api.github.com/repos/nixos/nixpkgs/commits/nixpkgs-unstable but fails. 😢

This is the 2nd thing you found @Kha in https://github.com/NixOS/nix/issues/5409#issuecomment-949519816 It's definitetly also important!

Kha commented 3 years ago

I think for this we would have to fall back to the regular Git fetcher. Though overriding the unpinned nixpkgs default registry flake is a good idea in general IMO!

stale[bot] commented 2 years ago

I marked this as stale due to inactivity. → More info

bew commented 2 years ago

Still relevant

SuperSandro2000 commented 2 years ago

I don't oppose making this configurable. Please don't change the default though. Certain auth tokens don't work with the github.com archive endpoints.

What should we do instead then? api.github.com has a very strict rate limit of 60 per hour (needs to be confirmed) which is constantly hit in places where the IP is shared like a hack space. Wouldn't it work to use the https+git scheme to get nix to use the token?

roberth commented 2 years ago

60 per hour (needs to be confirmed)

curl -I https://api.github.com/repos/NixOS/nixpkgs/commits/master
[...]
x-ratelimit-limit: 60
x-ratelimit-remaining: 57
x-ratelimit-reset: 1662303760
x-ratelimit-resource: core
x-ratelimit-used: 3
[...]

and (having just passed the reset mark, by coincidence)

curl -I https://api.github.com/repos/NixOS/nixpkgs/tarball/master
x-ratelimit-limit: 60
x-ratelimit-remaining: 60
x-ratelimit-reset: 1662304737
x-ratelimit-used: 0
x-ratelimit-resource: core

use the https+git scheme

You can't count on that fetcher giving the same exact same result, because of the .gitattributes export-ignore attribute iirc.

What should we do instead then?

Make the choice of endpoint configurable and/or use the other endpoint as a fallback.

SuperSandro2000 commented 2 years ago

You can't count on that fetcher giving the same exact same result, because of the .gitattributes export-ignore attribute iirc.

Isn't github also honoring that in archive downloads? Couldn't find documentation about that on the fast.

Make the choice of endpoint configurable and/or use the other endpoint as a fallback.

The none api endpoint should always be used unless opted into because otherwise you use up the unauthenticated github api rate limit for no good reason and older nix versions still would use it.

bew commented 1 year ago

I'm hit by this problem again, trying to tell my collegues to play with Nix by following the new Zero to Nix guides..

I think it can be a deal breaker as it makes it impossible to use flakes done by some people using the new cli or combine it in my own setup, since most flakes have github: dependencies...


I saw there was an attempt with #7029 (closed) and superseeded by #7039 (merged, and should be available in Nix 2.12.0). The 1st one was markes to close this issue, but not the second (mistake?). :point_right: However I tested with Nix 2.12.0 and I still have the same problem, it's trying to use api.github.com and I don't really know how to debug that..

Kha commented 1 year ago

As commented on that PR, api. is still being used to translate a ref (branch, tag, short SHA) to a rev (full SHA). I don't actually know how to expand a short SHA without using api., ls-remote does not seem to work for that.

roberth commented 1 year ago

If we change the github: fetcher to fetch a tree instead of a commit, it seems that we get the raw data instead of the "export archive", which means that it would become interchangeable with the actual git fetcher, especially if implemented with libgit2 (and no features like git-crypt, lfs or submodules enabled, in which case you'd want to go with the real git fetcher anyway). When they're interchangeable, the git+https fetcher can serve as a fallback for github: sources.

nomeata commented 1 year ago

This just hit me using nix run nixpkgs#coreutils in a script that runs on some public CI infrastructure (netlify). Maybe the nix run nixpkgs#foo idiom wasn't the best idea here, but would still be better if it somehow didn’t rely on API endpoints that are not meant for heavy use.

SuperSandro2000 commented 1 year ago

This should be fixed since 2.12 with #7039

IMO we can close this.

simonzkl commented 1 year ago

This should be fixed since 2.12 with #7039

IMO we can close this.

Nope, I can confirm it's still using api.github.com in 2.17 without access-tokens set.

error:
       … while fetching the input 'github:NixOS/nixpkgs/nixpkgs-unstable'
       error: unable to download 'https://api.github.com/repos/NixOS/nixpkgs/commits/nixpkgs-unstable': HTTP error 403
SuperSandro2000 commented 1 year ago

This is probably happening here when resolving the rev https://github.com/NixOS/nix/blob/57eb62d2307adfd6ce881b41c861eb873153517c/src/libfetchers/github.cc#L258-L276

simonzkl commented 1 year ago

Actually this was from nix profile install in CI so it makes sense it queries the Github API for the latest revision. I assume with locked inputs it should be able to download archives directly.

Atry commented 1 year ago

use the https+git scheme

You can't count on that fetcher giving the same exact same result, because of the .gitattributes export-ignore attribute iirc.

I understand it is difficult to match the tarball's checksum via git+https protocol, but why do we ever have github protocol? Why not deprecate github protocol and switch to git+https?

Atry commented 1 year ago

It would be nice if we could provide an option to replace github protocol to git+https protocol in transitive dependencies when generating flake.lock.