| Message ID | c57e1cca4c7d2a0f52ae8d4e0870e4e0667184fe.1723540604.git.ps@pks.im (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 7298bcc573f8e022854b1811a9a7413281da05ea |
| Headers | show |
| Series | bundle: fix handling of object format | expand |
On Tue, Aug 13, 2024 at 5:18 AM Patrick Steinhardt <ps@pks.im> wrote: > The `git bundle unbundle` subcommand requires a repository to unbundle > the contents into. As thus, the subcommand checks whether we have a > startup repository in the first place, and if not it dies. Perhaps you meant: s/As thus/As such/ > This check happens after we have already opened the bundle though. This > causes a segfault when running outside of a repository starting with > c8aed5e8da (repository: stop setting SHA1 as the default object hash, > 2024-05-07) because we have no hash function set up, but we do try to > parse refs advertised by the bundle's header. > > The next commit will fix that underlying issue by defaulting to the SHA1 > object format for bundles, which will also the described segfault here. Presumably: s/also the/also fix the/ > But as we know that we will die anyway, we can do better than that and > avoid some vain work by moving the check for a repository before we try > to open the bundle. > > Reported-by: ArcticLampyrid <ArcticLampyrid@outlook.com> > Suggested-by: Jeff King <peff@peff.net> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
diff --git a/builtin/bundle.c b/builtin/bundle.c index d5d41a8f67..86d0ed7049 100644 --- a/builtin/bundle.c +++ b/builtin/bundle.c @@ -207,12 +207,13 @@ static int cmd_bundle_unbundle(int argc, const char **argv, const char *prefix) builtin_bundle_unbundle_usage, options, &bundle_file); /* bundle internals use argv[1] as further parameters */ + if (!startup_info->have_repository) + die(_("Need a repository to unbundle.")); + if ((bundle_fd = open_bundle(bundle_file, &header, NULL)) < 0) { ret = 1; goto cleanup; } - if (!startup_info->have_repository) - die(_("Need a repository to unbundle.")); if (progress) strvec_pushl(&extra_index_pack_args, "-v", "--progress-title", _("Unbundling objects"), NULL); diff --git a/t/t6020-bundle-misc.sh b/t/t6020-bundle-misc.sh index fe75a06572..703434b472 100755 --- a/t/t6020-bundle-misc.sh +++ b/t/t6020-bundle-misc.sh @@ -652,4 +652,11 @@ test_expect_success 'send a bundle to standard output' ' test_cmp expect actual ' +test_expect_success 'unbundle outside of a repository' ' + git bundle create some.bundle HEAD && + echo "fatal: Need a repository to unbundle." >expect && + nongit test_must_fail git bundle unbundle "$(pwd)/some.bundle" 2>err && + test_cmp expect err +' + test_done
The `git bundle unbundle` subcommand requires a repository to unbundle the contents into. As thus, the subcommand checks whether we have a startup repository in the first place, and if not it dies. This check happens after we have already opened the bundle though. This causes a segfault when running outside of a repository starting with c8aed5e8da (repository: stop setting SHA1 as the default object hash, 2024-05-07) because we have no hash function set up, but we do try to parse refs advertised by the bundle's header. The next commit will fix that underlying issue by defaulting to the SHA1 object format for bundles, which will also the described segfault here. But as we know that we will die anyway, we can do better than that and avoid some vain work by moving the check for a repository before we try to open the bundle. Reported-by: ArcticLampyrid <ArcticLampyrid@outlook.com> Suggested-by: Jeff King <peff@peff.net> Signed-off-by: Patrick Steinhardt <ps@pks.im> --- builtin/bundle.c | 5 +++-- t/t6020-bundle-misc.sh | 7 +++++++ 2 files changed, 10 insertions(+), 2 deletions(-)