Skip to content

github: use libgit2 transport for ref resolution#15470

Open
domenkozar wants to merge 1 commit intoNixOS:masterfrom
cachix:github-rate-limits
Open

github: use libgit2 transport for ref resolution#15470
domenkozar wants to merge 1 commit intoNixOS:masterfrom
cachix:github-rate-limits

Conversation

@domenkozar
Copy link
Copy Markdown
Member

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.

@domenkozar domenkozar requested a review from edolstra as a code owner March 14, 2026 12:35
@github-actions github-actions Bot added the fetching Networking with the outside (non-Nix) world, input locking label Mar 14, 2026
@domenkozar domenkozar requested a review from Ericson2314 as a code owner March 14, 2026 13:17
domenkozar added a commit to cachix/devenv that referenced this pull request Mar 14, 2026
Upstreamed as NixOS/nix#15470.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@domenkozar domenkozar force-pushed the github-rate-limits branch 2 times, most recently from 0fd340e to 3492d21 Compare March 18, 2026 12:08
Comment thread src/libfetchers/git-utils.cc Outdated
@Eveeifyeve
Copy link
Copy Markdown
Member

Eveeifyeve commented Mar 18, 2026

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

@domenkozar
Copy link
Copy Markdown
Member Author

domenkozar commented Mar 24, 2026

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

Comment thread src/libfetchers/git-utils.cc Outdated
Comment thread src/libfetchers/github.cc Outdated
@domenkozar domenkozar force-pushed the github-rate-limits branch 3 times, most recently from 1ae0dfa to c4e92a7 Compare March 26, 2026 08:35
Comment thread src/libfetchers/git-utils.cc Outdated
Comment thread src/libfetchers/git-utils.cc
Use libgit2's high level remote API (git_remote_create_detached,
git_remote_connect, git_remote_ls) instead of manually downloading
and parsing the smart HTTP pkt-line format. This delegates protocol
handling to libgit2 while keeping auth via custom HTTP headers.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
git_remote_callbacks callbacks = GIT_REMOTE_CALLBACKS_INIT;
callbacks.payload = const_cast<void *>(static_cast<const void *>(&token));
callbacks.credentials =
[](git_credential ** out, const char *, const char *, unsigned int allowed_types, void * payload) -> int {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit:

Suggested change
[](git_credential ** out, const char *, const char *, unsigned int allowed_types, void * payload) -> int {
[](git_credential ** out, const char *, const char *, unsigned int allowedTypes, void * payload) -> int {

auto * tok = static_cast<const std::string *>(payload);
if (tok->empty())
return GIT_PASSTHROUGH;
if (allowed_types & GIT_CREDENTIAL_USERPASS_PLAINTEXT)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (allowed_types & GIT_CREDENTIAL_USERPASS_PLAINTEXT)
if (allowedTypes & GIT_CREDENTIAL_USERPASS_PLAINTEXT)

if (git_remote_connect(remote.get(), GIT_DIRECTION_FETCH, &callbacks, nullptr, nullptr))
throw GitError("connecting to remote '%s'", url);

const git_remote_head ** refs;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit:

Suggested change
const git_remote_head ** refs;
const gitRemoteHead ** refs;

Comment thread src/libfetchers/github.cc
store.requireStoreObjectAccessor(downloadResult.storePath)->readFile(CanonPath::root));
auto owner = getOwner(input);
auto repo = getRepo(input);
auto url = fmt("https://%s/%s/%s.git", host, owner, repo);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Constructing the URL this way is fine, since github doesn't allow anything that would need percent encoding in the repo/owner name. Just a note to readers.

Comment thread src/libfetchers/github.cc

Headers headers = makeHeadersWithAuthTokens(settings, host, input);

auto downloadResult = downloadFile(store, settings, url, "source", headers);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

One thing to note here is that we lose the caching that downloadFile does. Any way to recover it? Is this a concern at all?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is probably the more concerning question I have. Everything else is mostly noise

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

Labels

fetching Networking with the outside (non-Nix) world, input locking

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants