| Message ID | 7e15c3daf95283f62030fe7b5dd0bbf8e467881a.1723540604.git.ps@pks.im (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 96a9a3e42e85874ba5edfcf86d91f7d8c05d5f94 |
| Headers | show |
| Series | bundle: fix handling of object format | expand |
On Tue, Aug 13, 2024 at 11:18:15AM +0200, Patrick Steinhardt wrote: > This is only a symptom of a deeper issue though. Bundles default to the > SHA1 object format unless they advertise an "@object-format=" header. > Consequently, it has been wrong in the first place to use the object > format used by the current repository when parsing bundles. The > consequence is that trying to open a bundle that uses a different object > hash than the current repository will fail: > > $ git bundle list-heads sha1.bundle > error: unrecognized header: ee4b540943284700a32591ad09f7e15bdeb2a10c HEAD (45) That makes sense. And your test below, which covers a mismatch in the hash of the bundle vs the containing repo, would have failed even before the segfault issue. Nice. > Fix the bug by defaulting to the SHA1 object hash. We already handle the > "@object-format=" header as expected, so we don't need to adapt this > part. Yeah, this fix turned out to be delightfully simple as a result. Both this patch and the first one look good to me. Thanks for jumping on this. -Peff
Jeff King <peff@peff.net> writes: > Yeah, this fix turned out to be delightfully simple as a result. > > Both this patch and the first one look good to me. Thanks for jumping on > this. They look good to me too. Thanks, all.
diff --git a/bundle.c b/bundle.c index ce164c37bc..b0a8a925cb 100644 --- a/bundle.c +++ b/bundle.c @@ -89,7 +89,12 @@ int read_bundle_header_fd(int fd, struct bundle_header *header, goto abort; } - header->hash_algo = the_hash_algo; + /* + * The default hash format for bundles is SHA1, unless told otherwise + * by an "object-format=" capability, which is being handled in + * `parse_capability()`. + */ + header->hash_algo = &hash_algos[GIT_HASH_SHA1]; /* The bundle header ends with an empty line */ while (!strbuf_getwholeline_fd(&buf, fd, '\n') && diff --git a/t/t6020-bundle-misc.sh b/t/t6020-bundle-misc.sh index 703434b472..34b5cd62c2 100755 --- a/t/t6020-bundle-misc.sh +++ b/t/t6020-bundle-misc.sh @@ -659,4 +659,29 @@ test_expect_success 'unbundle outside of a repository' ' test_cmp expect err ' +test_expect_success 'list-heads outside of a repository' ' + git bundle create some.bundle HEAD && + cat >expect <<-EOF && + $(git rev-parse HEAD) HEAD + EOF + nongit git bundle list-heads "$(pwd)/some.bundle" >actual && + test_cmp expect actual +' + +for hash in sha1 sha256 +do + test_expect_success "list-heads with bundle using $hash" ' + test_when_finished "rm -rf hash" && + git init --object-format=$hash hash && + test_commit -C hash initial && + git -C hash bundle create hash.bundle HEAD && + + cat >expect <<-EOF && + $(git -C hash rev-parse HEAD) HEAD + EOF + git bundle list-heads hash/hash.bundle >actual && + test_cmp expect actual + ' +done + test_done
We hit a segfault when trying to open a bundle via `git bundle list-heads` when running outside of a repository. This is caused by c8aed5e8da (repository: stop setting SHA1 as the default object hash, 2024-05-07), which stopped setting the default object hash so that `the_hash_algo` is a `NULL` pointer when running outside of any repo. This is only a symptom of a deeper issue though. Bundles default to the SHA1 object format unless they advertise an "@object-format=" header. Consequently, it has been wrong in the first place to use the object format used by the current repository when parsing bundles. The consequence is that trying to open a bundle that uses a different object hash than the current repository will fail: $ git bundle list-heads sha1.bundle error: unrecognized header: ee4b540943284700a32591ad09f7e15bdeb2a10c HEAD (45) Fix the bug by defaulting to the SHA1 object hash. We already handle the "@object-format=" header as expected, so we don't need to adapt this part. Helped-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Patrick Steinhardt <ps@pks.im> --- bundle.c | 7 ++++++- t/t6020-bundle-misc.sh | 25 +++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-)