Skip to content

Add read-only support for local Git bundle files - #7338

Open
benjaminy wants to merge 12 commits into
libgit2:mainfrom
benjaminy:git-bundles-clone-fetch
Open

Add read-only support for local Git bundle files#7338
benjaminy wants to merge 12 commits into
libgit2:mainfrom
benjaminy:git-bundles-clone-fetch

Conversation

@benjaminy

Copy link
Copy Markdown

Summary

Add read-only support for local Git bundle files.

  • Clone self-contained SHA-1 and SHA-256 bundles.
  • Fetch self-contained or incremental bundles when their prerequisites exist locally.
  • Detect bundles by content, regardless of filename extension.
  • Support transfer progress and cancellation.
  • Reject unsupported formats and operations without mutating references.
  • Preserve existing clone behavior when a bundle does not record 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 0678267a5 is intentionally retained as a historical artifact: it reconstructs that proposal’s generic no-HEAD clone 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.

benjaminy and others added 10 commits August 9, 2026 07:26
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.
@benjaminy

Copy link
Copy Markdown
Author

According to our friend Claudexor the current CI failures are from main, not this branch

Comment thread docs/changelog.md Outdated
@@ -1,3 +1,18 @@
Unreleased

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We produce the changelog as part of the release process; please drop this. 🙏

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Dropped.

Comment thread src/libgit2/transports/bundle.c Outdated
return true;
}

int git_transport_bundle__probe(git_bundle_probe_t *out, const char *url)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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...)?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

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.

Comment thread src/libgit2/transport.c Outdated
#endif

static transport_definition local_transport_definition = { "file://", git_transport_local, NULL };
static transport_definition bundle_transport_definition = { "", git_transport_bundle, NULL };

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

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)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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.)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Agreed; no change here per your note.

Comment thread src/libgit2/transports/bundle.c Outdated
* 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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would it? Isn't that just another form of "no"?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

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.

Comment thread src/libgit2/transports/bundle.c Outdated
git_bundle_reader_fromfd(&reader, fd);
error = git_bundle_header_parse(&header, &reader);
git_bundle_reader_dispose(&reader);
git_bundle_header_dispose(&header);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Instead of having a heavy object that is owned by the caller, it seems like this should be part of the bundle parser lifecycle.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

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.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants