github: use libgit2 transport for ref resolution

[domenkozar]

Resolve branch/tag names to commit SHAs via the git HTTP protocol (/info/refs endpoint) instead of the GitHub REST API. This is the same approach already used by SourceHut and avoids API rate limits.

[Eveeifyeve]

I don't see any issues with this, I could be missing something.

[domenkozar]

@xokdvium let's merge this one as well? Looking forward not to hammer github http api

[domenkozar]

Could we merge this one?

[xokdvium]

@domenkozar, could you address the outstanding caching question? That was the only concern I noticed.

[domenkozar]

@domenkozar, could you address the outstanding caching question? That was the only concern I noticed.

See b67be79

[xokdvium]

Thanks, I can go fix the nitpicks myself.

[domenkozar]

@xokdvium fixed some edge cases when fetching refs, good to go.

[domenkozar]

@xokdvium ping

[domenkozar]

I pushed a fix we've caught in cachix/devenv#2842

[xokdvium]

Is github going to hate us for doing this?

For nixpkgs this has to list a whopping 495816 refs currently...

And it takes a whopping 6-10s to return this result - so something is doing a non-trivial amount of work, and it doesn't seem like it's nix, because Percent of CPU this job got: 2%. Is it the roundtrips or are we just hammering github?

Is that reproducible on your end?

[xokdvium]

Tbh I'm a bit wary of how much better this is. At the very least it seems to take significantly more waiting for nix to get the resolved ref. On the other hand it also seems like a loophole, rather than an actually cheaper operation (though I really don't have any visibility into the internals).

The only thing I know is (@emilazy mentioned this to me at some point) that Github really doesn't want us to be hammering the git api (for cloning almost certainly, I don't know about ref resolution).

[Ericson2314]

Insofar that this is basically a loophole, I think we should just go ask a GitHub person first. I rather get karma for being nice, than get anti-karma for exploiting something they forgot got rate-limit.

[xokdvium]

@JamieMagee, do you have more insights into the overheads of doing this?

I'm not crazy and this is much more expensive than what we are currently doing with https://api.%s/repos/%s/%s/commits/%s API and the lack of a ratelimit is not ideal there? Would just like to make sure than Github isn't going to hate us

[JamieMagee]

@xokdvium thanks for the ping. This isn't my area of expertise, but I can make sure it gets raised with the right team.

[JamieMagee]

I raised this with the team that runs GitHub's Git infrastructure.

They'd rather nix didn't switch to ls-remote. Listing all ~495k refs to pick one is more expensive for them than the current per-ref REST call, not less. There's no documented rate limit on it, but they have server-side protection that would throttle it in bulk anyway, and it can slow down clones and fetches for everyone hitting the same repo. So it probably wouldn't fix the rate-limiting reliably.

The current commits/{ref} call is one of their cheapest endpoints. But they can't relax that 60/hr unauthenticated cap for anonymous traffic without opening an abuse vector.

So their recommended fix is use a token (60 to 5k/hr cap) and keep the caching this PR adds, which is worth landing on its own. There's another trick too, which Homebrew leans on: store the ETag from each commits/{ref} response and send it back as If-None-Match, so when the ref hasn't moved (the common case, like everyone resolving nixos-unstable to the same SHA) GitHub returns a 304 Not Modified. Those 304s don't count against the rate limit, so it helps even without a token. It'd also help if nix ecosystem tooling set a user-agent suffix (Determinate Nix, Lix, devenv, nixos-rebuild, setup-nix, etc.) so they can bucket real load.

tl;dr: I'd hold off on the transport swap, split out the cache and SSH-auth fix, push token auth and ETag conditional requests as the near-term mitigation. I'm happy to stay the contact point on GH.

[xokdvium]

So their recommended fix is use a token (60 to 5k/hr cap) and keep the caching this PR adds, which is worth landing on its own. There's another trick too, which Homebrew leans on: store the ETag from each commits/{ref} response and send it back as If-None-Match, so when the ref hasn't moved (the common case, like everyone resolving nixos-unstable to the same SHA) GitHub returns a 304 Not Modified. Those 304s don't count against the rate limit, so it helps even without a token. It'd also help if nix ecosystem tooling set a user-agent suffix (Determinate Nix, Lix, devenv, nixos-rebuild, setup-nix, etc.) so they can bucket real load.

Current approach uses basically all of that AFAIK (ETag caching + we do have a custom user agent: curl: [HTTP/2] [1] [user-agent: curl/8.20.0 Nix/2.35.0...]).

So from what I understand is that there's not much that we can improve in addition to what's already implemented (other than asking users to do authenticated requests - that's already the case).

[xokdvium]

So based off of @JamieMagee's helpful insight, it doesn't seem like there's much that can be done about unauthenticated requests in a way that isn't going to make the situation worse.

We do in fact use caching in downloadFile used for the API call (local caching and If-None-Match for querying when that local TTL expires):

curl: [HTTP/2] [1] OPENED stream for https://api.github.com/repos/nixos/nixpkgs/commits/22.05
...
curl: [HTTP/2] [1] [if-none-match: blahblahblah <- here ]
got header for 'https://api.github.com/repos/nixos/nixpkgs/commits/22.05': HTTP/2 304
...

[Mic92]

What we can improve is making forge login easier. Upstream version of: https://github.com/numtide/nix-auth