Add read-only support for local Git bundle files - #7338
Conversation
Reconstruct the generic no-HEAD clone behavior proposed as part of libgit2 pull request libgit2#7101 by Laurence McGlashan. This is deliberately kept as a separate, independently green baseline commit so that the earlier contribution remains recognizable and the later decision to preserve libgit2's existing behavior can be reviewed as an explicit delta instead of being silently folded into the bundle implementation. When a remote advertises no HEAD, clone leaves the configured initial branch unborn even when that exact branch was just fetched. Select and check out the configured initial branch in that case: look for the configured `refs/heads/<name>` among the advertised references and, when it is present, go through clone's existing explicit-branch path so the local branch, its upstream, and the checkout are all set up in the usual way. When the configured name was not advertised, keep the current behavior and leave it unborn. Do not substitute the only branch, the first branch, or an OID match. Do not create `refs/remotes/origin/HEAD`, because the remote did not advertise one. The policy is generic rather than bundle-specific, so test it through a custom transport that omits HEAD. A remote whose own HEAD is unborn reaches the same path because a reference without an object ID cannot be advertised. This therefore checks out the configured initial branch where libgit2 previously left HEAD unborn. Git distinguishes these cases because upload-pack sends an unborn HEAD's symref target as a capability, but libgit2's transports do not currently carry that information. The later delta in this series backs out this policy change rather than accepting that ambiguity. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017U2Kqd137huTh2hdkWERds
Add an internal parser for v2 and v3 bundle headers: the version, the object format, the prerequisite object ids, the advertised references in header order, and the offset of the first byte of the pack. The parser reads through a small callback abstraction rather than from a descriptor, with a file-descriptor backing and an in-memory backing. That keeps the parser tests free of temporary files and is the precondition for a header-parser fuzz target, which consumes a buffer and would otherwise need one temporary file per iteration. Lines are read incrementally, so neither the header nor the pack is ever held in a single buffer. The result codes are what transport probing will need: `GIT_EINVALID` for input that is not a bundle or whose header is malformed, `GIT_ENOTSUPPORTED` for a well-formed bundle this build cannot use (a filtered bundle, an unknown v3 capability, or an unsupported object format), and any other negative value for an operational failure. An unsupported capability does not stop syntax checking, so a file that is both malformed and unsupported is reported as malformed. Based on the work in libgit2 pull request libgit2#7101 by Laurence McGlashan. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017U2Kqd137huTh2hdkWERds
Add a read-only transport that serves references and objects from a local git bundle, so a bundle path can be used wherever a remote is accepted. Both self-contained and incremental bundles work, in SHA1 and SHA256. There is no URL scheme. A bundle is recognized by its contents, so any path works whatever its extension, matching how the Git command line takes a bundle. Probing runs before the colon-based SSH fallback, which is what lets a Windows drive-rooted path be recognized; the local-path predicate rejects scp-style `host:path` strings without touching the filesystem, so genuine SSH remotes still avoid the probe. Probing classifies the path before opening it: a missing path, a directory, or any other non-regular file falls through silently, as does a regular file that is not a bundle, but allocation, permission, and read failures on an existing regular file are propagated rather than turned into "unsupported URL protocol". A well-formed bundle we cannot use still selects the transport, so connect can report the specific reason. A recorded HEAD is advertised first; a missing one is not invented, matching what `git ls-remote` reports for a bundle. Bundle references carry no symref information, so a recorded HEAD is resolved by its object id through the existing default-branch logic, and clone's no-HEAD fallback handles the rest. Prerequisites are checked for existence in the destination object database before the pack is created, which is what Git 2.50.1 does when it unbundles, although the `git bundle verify` manual uses the stronger "fully linked" wording. Thin-pack resolution against the destination remains the backstop for a delta base that is genuinely missing. When every wanted tip already exists locally the generic fetch path asks the transport for nothing, so unused prerequisites are not checked; that gap is covered by a test rather than papered over. Both callbacks the fetch path invokes unconditionally are implemented. `negotiate_fetch` rejects a requested depth and an already-shallow destination before anything is verified or ingested -- the latter because writing a bundle's empty shallow-root list would delete the destination's `shallow` file -- and compares object formats. `shallow_roots` is a no-op that would otherwise crash every successful fetch. Cancellation is reset when a connection or a negotiation begins, so `git_remote_stop` cancels the operation in flight without poisoning a transport the caller wants to reuse. This is deliberately more than the smart and local transports do; neither ever clears its flag. Based on the work in libgit2 pull request libgit2#7101 by Laurence McGlashan, incorporating the review of pull request libgit2#7221: no invented URL scheme, and HEAD advertised first. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017U2Kqd137huTh2hdkWERds
Record what came out of implementing the plan: the deviations, the one behaviour change the plan did not anticipate, the test-matrix entries that are not covered, and two follow-ups.
Reject over-long bundle header lines instead of reading unbounded input, and stop picking the configured initial branch when a clone's remote advertises no HEAD; leave HEAD at the repository default instead.
Rescan only newly read bytes when looking for a newline, parse oids in place, and propagate odb errors from the prerequisite check. Limit the drive-letter probe to Windows so it runs before the SSH colon check only there, and clear the cancel flag so it cannot leak into a later fetch.
|
According to our friend Claudexor the current CI failures are from main, not this branch |
| @@ -1,3 +1,18 @@ | |||
| Unreleased | |||
There was a problem hiding this comment.
We produce the changelog as part of the release process; please drop this. 🙏
| return true; | ||
| } | ||
|
|
||
| int git_transport_bundle__probe(git_bundle_probe_t *out, const char *url) |
There was a problem hiding this comment.
Callers never seem to care about anything other than whether it's a bundle or not. Having an enum with multiple values seems unnecessary. Should this just be probe(bool *out...)?
There was a problem hiding this comment.
Changed the result to bool. Recognized but unsupported bundles still return true so connect can report the specific unsupported feature, and the int return remains for operational failures.
| #endif | ||
|
|
||
| static transport_definition local_transport_definition = { "file://", git_transport_local, NULL }; | ||
| static transport_definition bundle_transport_definition = { "", git_transport_bundle, NULL }; |
There was a problem hiding this comment.
We should have a protocol definition here, even if it's fake, so that callers can override it meaningfully. If this was bundle:// would that be a problem?
There was a problem hiding this comment.
Changed the fake definition to bundle:// and routed positive content probes through that lookup, so git_transport_register("bundle", ...) can override the built-in transport. It remains an internal dispatch key; this does not add a user-facing bundle:// URL scheme.
| * colon; anything else whose colon precedes the first path separator | ||
| * is scp-style and is left to the ssh transport. | ||
| */ | ||
| static bool bundle_is_local_path(const char *url) |
There was a problem hiding this comment.
Surprised there's not a helper method for this. We should make one. (This is not a thing you need to do, just making a note for myself.)
There was a problem hiding this comment.
Agreed; no change here per your note.
| * Classify the path before opening it. On systems where this | ||
| * probe runs before the directory check, `open` on a directory | ||
| * succeeds and only the read fails; treating that as an | ||
| * operational error would break every ordinary local clone. |
There was a problem hiding this comment.
Would it? Isn't that just another form of "no"?
There was a problem hiding this comment.
You're right. A directory is already a probe miss through the regular-file check. Reworded the comment to explain that the guard avoids opening special files that may block or have side effects.
| git_bundle_reader_fromfd(&reader, fd); | ||
| error = git_bundle_header_parse(&header, &reader); | ||
| git_bundle_reader_dispose(&reader); | ||
| git_bundle_header_dispose(&header); |
There was a problem hiding this comment.
Instead of having a heavy object that is owned by the caller, it seems like this should be part of the bundle parser lifecycle.
There was a problem hiding this comment.
Folded the reader and parsed header into one parser-owned lifecycle. Parsing releases borrowed source state and transient buffers before returning, while parser disposal releases the parsed refs and prerequisites. The transport retains the parser so advertised refs remain available after disconnect.
Summary
Add read-only support for local Git bundle files.
HEAD.The implementation includes an internal, memory-drivable bundle-header parser so a future fuzz target will not require redesigning the parser around temporary files.
This work is inspired by PR #7101 by Laurence McGlashan.
Commit
0678267a5is intentionally retained as a historical artifact: it reconstructs that proposal’s generic no-HEADclone policy so the subsequent decision to preserve existing libgit2 behavior remains visible as an explicit delta.PR #7221 was my first attempt at doing this with Claude Code.
I am now much more experienced with agentic coding tools; hopefully this PR is much stronger.
Bundle creation, pushing, filtered bundles, shallow operations, and Git’s bundle-URI protocol remain out of scope.
Testing
Added focused parser, transport-selection, clone, and fetch coverage, including malformed headers, prerequisite failures, SHA-256 bundles, cancellation, retry behavior, and paths that could otherwise be mistaken for SSH remotes.