{"thread":{"id":"51183","subject":"[PATCH 0/1] bundle verify: improve the user experience when called without a .git/ directory","startedAt":"2019-05-27T19:59:17Z","lastAt":"2019-05-28T21:09:11Z","messageCount":11,"participants":["Johannes Schindelin via GitGitGadget","brian m. carlson","Johannes Schindelin","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"376246","messageId":"pull.226.git.gitgitgadget@gmail.com","threadId":"51183","inReplyTo":null,"subject":"[PATCH 0/1] bundle verify: improve the user experience when called without a .git/ directory","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-05-27T19:59:13Z","receivedAt":"2019-05-27T19:59:17Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"The git bundle verify command really needs access to a .git/ \ndirectory. But it did not make sure, instead erroring out with a BUG(),\nmaking for a terrible user experience.\n\nThis patch fixes that.\n\nJohannes Schindelin (1):\n bundle verify: error out if called without an object database\n\n bundle.c | 3 +++\n t/t5607-clone-bundle.sh | 6 ++++++\n 2 files changed, 9 insertions(+)\n\n\nbase-commit: 8104ec994ea3849a968b4667d072fedd1e688642\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-226%2Fdscho%2Ffix-bundle-verify-segfault-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-226/dscho/fix-bundle-verify-segfault-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/226\n-- \ngitgitgadget\n"},{"id":"376247","messageId":"f0545e872344dd25e11db7fe095cde1578b26748.1558987152.git.gitgitgadget@gmail.com","threadId":"51183","inReplyTo":"pull.226.git.gitgitgadget@gmail.com","subject":"[PATCH 1/1] bundle verify: error out if called without an object database","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-05-27T19:59:14Z","receivedAt":"2019-05-27T19:59:18Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThe deal with bundles is: they really are thin packs, with very little\nsugar on top. So we really need a repository (or more appropriately, an\nobject database) to work with, when asked to verify a bundle.\n\nLet's error out with a useful error message if `git bundle verify` is\ncalled without such an object database to work with.\n\nReported by Konstantin Ryabitsev.\n\nSigned-off-by: Johannes Schindelin \n---\n bundle.c | 3 +++\n t/t5607-clone-bundle.sh | 6 ++++++\n 2 files changed, 9 insertions(+)\n\ndiff --git a/bundle.c b/bundle.c\nindex b45666c49b..b5d21cd80f 100644\n--- a/bundle.c\n+++ b/bundle.c\n@@ -142,6 +142,9 @@ int verify_bundle(struct repository *r,\n \tint i, ret = 0, req_nr;\n \tconst char *message = _(\"Repository lacks these prerequisite commits:\");\n \n+\tif (!r || !r->objects || !r->objects->odb)\n+\t\treturn error(_(\"need a repository to verify a bundle\"));\n+\n \trepo_init_revisions(r, &revs, NULL);\n \tfor (i = 0; i < p->nr; i++) {\n \t\tstruct ref_list_entry *e = p->list + i;\ndiff --git a/t/t5607-clone-bundle.sh b/t/t5607-clone-bundle.sh\nindex cf39e9e243..2a0fb15cf1 100755\n--- a/t/t5607-clone-bundle.sh\n+++ b/t/t5607-clone-bundle.sh\n@@ -14,6 +14,12 @@ test_expect_success 'setup' '\n \tgit tag -d third\n '\n \n+test_expect_success '\"verify\" needs a worktree' '\n+\tgit bundle create tip.bundle -1 master &&\n+\ttest_must_fail nongit git bundle verify ../tip.bundle 2>err &&\n+\ttest_i18ngrep \"need a repository\" err\n+'\n+\n test_expect_success 'annotated tags can be excluded by rev-list options' '\n \tgit bundle create bundle --all --since=7.Apr.2005.15:14:00.-0700 &&\n \tgit ls-remote bundle > output &&\n-- \ngitgitgadget\n"},{"id":"376248","messageId":"20190527200838.GF8616@genre.crustytoothpaste.net","threadId":"51183","inReplyTo":"f0545e872344dd25e11db7fe095cde1578b26748.1558987152.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/1] bundle verify: error out if called without an object database","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-05-27T20:08:38Z","receivedAt":"2019-05-27T20:08:50Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2019-05-27 at 19:59:14, Johannes Schindelin via GitGitGadget wrote:\n> From: Johannes Schindelin \n> \n> The deal with bundles is: they really are thin packs, with very little\n\nGenerally a colon can only follow what could be a complete sentence, so\nmaybe we want to say something like \"The deal with bundles is that they\nreally are…\" or \"The deal with bundles is this: they really are…\".\n\n> sugar on top. So we really need a repository (or more appropriately, an\n> object database) to work with, when asked to verify a bundle.\n> \n> Let's error out with a useful error message if `git bundle verify` is\n> called without such an object database to work with.\n> \n> Reported by Konstantin Ryabitsev.\n> \n> Signed-off-by: Johannes Schindelin \n> ---\n> bundle.c | 3 +++\n> t/t5607-clone-bundle.sh | 6 ++++++\n> 2 files changed, 9 insertions(+)\n> \n> diff --git a/bundle.c b/bundle.c\n> index b45666c49b..b5d21cd80f 100644\n> --- a/bundle.c\n> +++ b/bundle.c\n> @@ -142,6 +142,9 @@ int verify_bundle(struct repository *r,\n> \tint i, ret = 0, req_nr;\n> \tconst char *message = _(\"Repository lacks these prerequisite commits:\");\n> \n> +\tif (!r || !r->objects || !r->objects->odb)\n> +\t\treturn error(_(\"need a repository to verify a bundle\"));\n> +\n> \trepo_init_revisions(r, &revs, NULL);\n> \tfor (i = 0; i < p->nr; i++) {\n> \t\tstruct ref_list_entry *e = p->list + i;\n> diff --git a/t/t5607-clone-bundle.sh b/t/t5607-clone-bundle.sh\n> index cf39e9e243..2a0fb15cf1 100755\n> --- a/t/t5607-clone-bundle.sh\n> +++ b/t/t5607-clone-bundle.sh\n> @@ -14,6 +14,12 @@ test_expect_success 'setup' '\n> \tgit tag -d third\n> '\n> \n> +test_expect_success '\"verify\" needs a worktree' '\n\nDid you want to say \"needs a repository\"? Or do we really need a\nworktree?\n\nOther than that, this looks fine to me. This is a much better experience\nthan a BUG and a nice improvement overall.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"376249","messageId":"nycvar.QRO.7.76.6.1905272219000.28524@tvgsbejvaqbjf.bet","threadId":"51183","inReplyTo":"20190527200838.GF8616@genre.crustytoothpaste.net","subject":"Re: [PATCH 1/1] bundle verify: error out if called without an object database","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-05-27T20:20:15Z","receivedAt":"2019-05-27T20:20:29Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi brian,\n\nOn Mon, 27 May 2019, brian m. carlson wrote:\n\n> On 2019-05-27 at 19:59:14, Johannes Schindelin via GitGitGadget wrote:\n> > From: Johannes Schindelin \n> >\n> > The deal with bundles is: they really are thin packs, with very little\n>\n> Generally a colon can only follow what could be a complete sentence, so\n> maybe we want to say something like \"The deal with bundles is that they\n> really are…\" or \"The deal with bundles is this: they really are…\".\n\nThanks for educating me.\n\n> > sugar on top. So we really need a repository (or more appropriately, an\n> > object database) to work with, when asked to verify a bundle.\n> >\n> > Let's error out with a useful error message if `git bundle verify` is\n> > called without such an object database to work with.\n> >\n> > Reported by Konstantin Ryabitsev.\n> >\n> > Signed-off-by: Johannes Schindelin \n> > ---\n> > bundle.c | 3 +++\n> > t/t5607-clone-bundle.sh | 6 ++++++\n> > 2 files changed, 9 insertions(+)\n> >\n> > diff --git a/bundle.c b/bundle.c\n> > index b45666c49b..b5d21cd80f 100644\n> > --- a/bundle.c\n> > +++ b/bundle.c\n> > @@ -142,6 +142,9 @@ int verify_bundle(struct repository *r,\n> > \tint i, ret = 0, req_nr;\n> > \tconst char *message = _(\"Repository lacks these prerequisite commits:\");\n> >\n> > +\tif (!r || !r->objects || !r->objects->odb)\n> > +\t\treturn error(_(\"need a repository to verify a bundle\"));\n> > +\n> > \trepo_init_revisions(r, &revs, NULL);\n> > \tfor (i = 0; i < p->nr; i++) {\n> > \t\tstruct ref_list_entry *e = p->list + i;\n> > diff --git a/t/t5607-clone-bundle.sh b/t/t5607-clone-bundle.sh\n> > index cf39e9e243..2a0fb15cf1 100755\n> > --- a/t/t5607-clone-bundle.sh\n> > +++ b/t/t5607-clone-bundle.sh\n> > @@ -14,6 +14,12 @@ test_expect_success 'setup' '\n> > \tgit tag -d third\n> > '\n> >\n> > +test_expect_success '\"verify\" needs a worktree' '\n>\n> Did you want to say \"needs a repository\"? Or do we really need a\n> worktree?\n\nRight, that was a place I meant to change after making up my mind what is\nactually required, but forgot.\n\n> Other than that, this looks fine to me. This is a much better experience\n> than a BUG and a nice improvement overall.\n\nThank you! v2 incoming.\n\nCiao,\nDscho\n"},{"id":"376251","messageId":"20190527202528.GG8616@genre.crustytoothpaste.net","threadId":"51183","inReplyTo":"8467593c158ffac56897cf02e165173d9c0b5880.1558988458.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 1/1] bundle verify: error out if called without an object database","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-05-27T20:25:28Z","receivedAt":"2019-05-27T20:25:36Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2019-05-27 at 20:21:00, Johannes Schindelin via GitGitGadget wrote:\n> From: Johannes Schindelin \n> \n> The deal with bundles is that they really are thin packs, with very\n> little sugar on top. So we really need a repository (or more\n> appropriately, an object database) to work with, when asked to verify a\n> bundle.\n> \n> Let's error out with a useful error message if `git bundle verify` is\n> called without such an object database to work with.\n> \n> Reported by Konstantin Ryabitsev.\n> \n> Signed-off-by: Johannes Schindelin \n\nI think this looks great.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"376263","messageId":"20190528015133.GA29724@sigill.intra.peff.net","threadId":"51183","inReplyTo":"f0545e872344dd25e11db7fe095cde1578b26748.1558987152.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/1] bundle verify: error out if called without an object database","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-05-28T01:51:33Z","receivedAt":"2019-05-28T01:51:38Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, May 27, 2019 at 12:59:14PM -0700, Johannes Schindelin via GitGitGadget wrote:\n\n> From: Johannes Schindelin \n> \n> The deal with bundles is: they really are thin packs, with very little\n> sugar on top. So we really need a repository (or more appropriately, an\n> object database) to work with, when asked to verify a bundle.\n> \n> Let's error out with a useful error message if `git bundle verify` is\n> called without such an object database to work with.\n\nI think this is going in the right direction, but I think there are a\nfew subtle bits worth thinking about.\n\nAs Gábor noted in the earlier thread, if the bundle doesn't have any\nprerequisites, this _used_ to work before b1ef400eec (setup_git_env:\navoid blind fall-back to \".git\", 2016-10-20). I don't know if anybody\ncares about that case or not, but we could do something like:\n\n if (p->nr)\n\tverify_prerequisites();\n\n /* otherwise, fall through to the printing portions */\n\nand then just check for a repository in verify_prerequisites(), which is\nthe only part that needs to look at the repository object at all.\n\nIf we _are_ OK just forbidding the operation entirely outside of a\nrepository, then should we be doing this check in cmd_bundle() instead?\nWe already have checks there for \"create\" and \"unbundle\".\n\nLikewise:\n\n> diff --git a/bundle.c b/bundle.c\n> index b45666c49b..b5d21cd80f 100644\n> --- a/bundle.c\n> +++ b/bundle.c\n> @@ -142,6 +142,9 @@ int verify_bundle(struct repository *r,\n> \tint i, ret = 0, req_nr;\n> \tconst char *message = _(\"Repository lacks these prerequisite commits:\");\n> \n> +\tif (!r || !r->objects || !r->objects->odb)\n> +\t\treturn error(_(\"need a repository to verify a bundle\"));\n\nThose other checks are done with startup_info->have_repository. I don't\nthink that makes sense here (we were passed in a repository object to\noperate on, so the global might or might not match). Doing it at that\nhigher level means that other callers of verify_bundle() are not\nprotected, but I think may be OK. The top-level commands are generally\nresponsible for setting up the repository and bailing if the requested\noperation does not make sense without one.\n\nIf we do want to leave the check at this level, I'm a little\nuncomfortable with how intimate this check is with the parts of \"struct\nrepository\". For instance, who sets of r->objects and r->objects->odb,\nand is it possible for us to have a working repo struct that has those\nas NULL (i.e., where they could be lazily filled in)? Even if it works\nnow, it seems like a subtle thing that could easily be broken during\nlater refactoring.\n\nInstead, could we have cmd_bundle() pass in NULL when instead of\nthe_repository when there's no repo? That seems like a much easier\npattern in general for low-level code to decide when it has no repo\navailable (though I suspect many code paths would have to be adjusted to\nhandle a NULL repository argument).\n\n-Peff\n"},{"id":"376292","messageId":"nycvar.QRO.7.76.6.1905281309490.44@tvgsbejvaqbjf.bet","threadId":"51183","inReplyTo":"20190528015133.GA29724@sigill.intra.peff.net","subject":"Re: [PATCH 1/1] bundle verify: error out if called without an object database","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-05-28T11:17:41Z","receivedAt":"2019-05-28T11:18:11Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn Mon, 27 May 2019, Jeff King wrote:\n\n> On Mon, May 27, 2019 at 12:59:14PM -0700, Johannes Schindelin via\n> GitGitGadget wrote:\n>\n> > From: Johannes Schindelin \n> >\n> > The deal with bundles is: they really are thin packs, with very little\n> > sugar on top. So we really need a repository (or more appropriately, an\n> > object database) to work with, when asked to verify a bundle.\n> >\n> > Let's error out with a useful error message if `git bundle verify` is\n> > called without such an object database to work with.\n>\n> I think this is going in the right direction, but I think there are a\n> few subtle bits worth thinking about.\n>\n> As Gábor noted in the earlier thread, if the bundle doesn't have any\n> prerequisites, this _used_ to work before b1ef400eec (setup_git_env:\n> avoid blind fall-back to \".git\", 2016-10-20). I don't know if anybody\n> cares about that case or not, but we could do something like:\n>\n> if (p->nr)\n> \tverify_prerequisites();\n>\n> /* otherwise, fall through to the printing portions */\n>\n> and then just check for a repository in verify_prerequisites(), which is\n> the only part that needs to look at the repository object at all.\n\nI am not so sure that I feel comfortable with optimizing for the no-op\ncase. Because that's essentially what this boils down to: if there are no\nprerequisites, there is not a whole lot to do.\n\nAnd I'd rather have the command be consistent about its demands, to abide\nby the Law of Least Surprise.\n\n> If we _are_ OK just forbidding the operation entirely outside of a\n> repository, then should we be doing this check in cmd_bundle() instead?\n> We already have checks there for \"create\" and \"unbundle\".\n\nHmm. If you really want to ;-)\n\n> Likewise:\n>\n> > diff --git a/bundle.c b/bundle.c\n> > index b45666c49b..b5d21cd80f 100644\n> > --- a/bundle.c\n> > +++ b/bundle.c\n> > @@ -142,6 +142,9 @@ int verify_bundle(struct repository *r,\n> > \tint i, ret = 0, req_nr;\n> > \tconst char *message = _(\"Repository lacks these prerequisite commits:\");\n> >\n> > +\tif (!r || !r->objects || !r->objects->odb)\n> > +\t\treturn error(_(\"need a repository to verify a bundle\"));\n>\n> Those other checks are done with startup_info->have_repository. I don't\n> think that makes sense here (we were passed in a repository object to\n> operate on, so the global might or might not match). Doing it at that\n> higher level means that other callers of verify_bundle() are not\n> protected, but I think may be OK. The top-level commands are generally\n> responsible for setting up the repository and bailing if the requested\n> operation does not make sense without one.\n>\n> If we do want to leave the check at this level, I'm a little\n> uncomfortable with how intimate this check is with the parts of \"struct\n> repository\". For instance, who sets of r->objects and r->objects->odb,\n> and is it possible for us to have a working repo struct that has those\n> as NULL (i.e., where they could be lazily filled in)? Even if it works\n> now, it seems like a subtle thing that could easily be broken during\n> later refactoring.\n>\n> Instead, could we have cmd_bundle() pass in NULL when instead of\n> the_repository when there's no repo? That seems like a much easier\n> pattern in general for low-level code to decide when it has no repo\n> available (though I suspect many code paths would have to be adjusted to\n> handle a NULL repository argument).\n\nThat's more complication than I want to introduce. Let's just go with the\n`builtin/bundle.c` patch instead, as you suggested.\n\nThanks,\nDscho\n"},{"id":"376346","messageId":"xmqqftoy1h4h.fsf@gitster-ct.c.googlers.com","threadId":"51183","inReplyTo":"20190528015133.GA29724@sigill.intra.peff.net","subject":"Re: [PATCH 1/1] bundle verify: error out if called without an object database","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-05-28T20:03:26Z","receivedAt":"2019-05-28T20:03:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> As Gábor noted in the earlier thread, if the bundle doesn't have any\n> prerequisites, this _used_ to work before b1ef400eec (setup_git_env:\n> avoid blind fall-back to \".git\", 2016-10-20). I don't know if anybody\n> cares about that case or not, but we could do something like:\n>\n> if (p->nr)\n> \tverify_prerequisites();\n>\n> /* otherwise, fall through to the printing portions */\n>\n> and then just check for a repository in verify_prerequisites(), which is\n> the only part that needs to look at the repository object at all.\n\nIt depends on how important we consider the use of bundles that can\nbe cloned from. If it is important enough, what you suggest is an\nimprovement worth doing over what Dscho did.\n\nA bundle that can be cloned from (i.e. no prerequisite) is meant to\nbe used without having any repository by definition, so it is a\ngrave regression to require object store when verifying such a\nbundle.\n\nOn the other hand, a bundle that cannot be cloned from but only\nusabel for an incremental sneaker-net update needs receiving\nrepository anyway, so it is perfectly fine to require object store.\n"},{"id":"376356","messageId":"20190528205839.GD24650@sigill.intra.peff.net","threadId":"51183","inReplyTo":"nycvar.QRO.7.76.6.1905281309490.44@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 1/1] bundle verify: error out if called without an object database","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-05-28T20:58:39Z","receivedAt":"2019-05-28T20:58:42Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, May 28, 2019 at 01:17:41PM +0200, Johannes Schindelin wrote:\n\n> > As Gábor noted in the earlier thread, if the bundle doesn't have any\n> > prerequisites, this _used_ to work before b1ef400eec (setup_git_env:\n> > avoid blind fall-back to \".git\", 2016-10-20). I don't know if anybody\n> > cares about that case or not, but we could do something like:\n> >\n> > if (p->nr)\n> > \tverify_prerequisites();\n> >\n> > /* otherwise, fall through to the printing portions */\n> >\n> > and then just check for a repository in verify_prerequisites(), which is\n> > the only part that needs to look at the repository object at all.\n> \n> I am not so sure that I feel comfortable with optimizing for the no-op\n> case. Because that's essentially what this boils down to: if there are no\n> prerequisites, there is not a whole lot to do.\n> \n> And I'd rather have the command be consistent about its demands, to abide\n> by the Law of Least Surprise.\n\nI'm OK with that.\n\nSince the breakage was technically attributable to my commit (I say\ntechnically because flushing out these kinds of bugs was exactly the\npoint of it), I feel slightly guilty that I might have broken somebody's\noddball workflow. So I tend to err on the side of restoring the original\nbehavior in that case, even if I can't conceive of it as being\nparticularly useful. :)\n\nBut if nobody is actually in support of it, and given that it has\nalready been broken for 2 years, I certainly do not mind waiting to see\nif anybody screams.\n\n-Peff\n"},{"id":"376358","messageId":"20190528210428.GE24650@sigill.intra.peff.net","threadId":"51183","inReplyTo":"xmqqftoy1h4h.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 1/1] bundle verify: error out if called without an object database","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-05-28T21:04:28Z","receivedAt":"2019-05-28T21:04:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, May 28, 2019 at 01:03:26PM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > As Gábor noted in the earlier thread, if the bundle doesn't have any\n> > prerequisites, this _used_ to work before b1ef400eec (setup_git_env:\n> > avoid blind fall-back to \".git\", 2016-10-20). I don't know if anybody\n> > cares about that case or not, but we could do something like:\n> >\n> > if (p->nr)\n> > \tverify_prerequisites();\n> >\n> > /* otherwise, fall through to the printing portions */\n> >\n> > and then just check for a repository in verify_prerequisites(), which is\n> > the only part that needs to look at the repository object at all.\n> \n> It depends on how important we consider the use of bundles that can\n> be cloned from. If it is important enough, what you suggest is an\n> improvement worth doing over what Dscho did.\n> \n> A bundle that can be cloned from (i.e. no prerequisite) is meant to\n> be used without having any repository by definition, so it is a\n> grave regression to require object store when verifying such a\n> bundle.\n> \n> On the other hand, a bundle that cannot be cloned from but only\n> usabel for an incremental sneaker-net update needs receiving\n> repository anyway, so it is perfectly fine to require object store.\n\nI was thinking this was matters only for \"bundle verify\", which is not\nall that interesting (either you can clone from it, or you cannot).\n\nIt is also used as part of unbundle(), which is called from the\ntransport code. But at that point of a clone, we'd already have created\nthe new repository we're writing into. I.e., \"git clone .../foo.bundle\"\nstill works fine either way.\n\nSo I guess you are asking only about the part that I dismissed above as\n\"not all that interesting\". I.e., do people actually run:\n\n git bundle verify foo.bundle &&\n git clone foo.bundle\n\nThat is not nonsense, per-se, but it is somewhat pointless since the\nclone will verify the bundle itself anyway. But then, I guess I do not\nsee much point in anyone calling \"bundle verify\" in the first place.\n\n-Peff\n"},{"id":"376359","messageId":"xmqqy32qz3pp.fsf@gitster-ct.c.googlers.com","threadId":"51183","inReplyTo":"20190528210428.GE24650@sigill.intra.peff.net","subject":"Re: [PATCH 1/1] bundle verify: error out if called without an object database","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-05-28T21:09:06Z","receivedAt":"2019-05-28T21:09:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> So I guess you are asking only about the part that I dismissed above as\n> \"not all that interesting\". I.e., do people actually run:\n>\n> git bundle verify foo.bundle &&\n> git clone foo.bundle\n>\n> That is not nonsense, per-se, but it is somewhat pointless since the\n> clone will verify the bundle itself anyway. But then, I guess I do not\n> see much point in anyone calling \"bundle verify\" in the first place.\n\nYeah, I tend to agree that 'bundle verify' is pointless before\ntrying to clone from it and seeing it fail (it still would be useful\nif 'bundle verify' gives more info on the breakage than 'clone', but\nit is not all that interesting at that point unless the user is\nworking on git itself).\n"}]}