Re: [PATCH 1/1] bundle verify: error out if called without an object database

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

[-- Attachment #1: Type: text/plain, Size: 2302 bytes --]

Hi brian,

On Mon, 27 May 2019, brian m. carlson wrote:

> On 2019-05-27 at 19:59:14, Johannes Schindelin via GitGitGadget wrote:
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > The deal with bundles is: they really are thin packs, with very little
>
> Generally a colon can only follow what could be a complete sentence, so
> maybe we want to say something like "The deal with bundles is that they
> really are…" or "The deal with bundles is this: they really are…".

Thanks for educating me.

> > sugar on top. So we really need a repository (or more appropriately, an
> > object database) to work with, when asked to verify a bundle.
> >
> > Let's error out with a useful error message if `git bundle verify` is
> > called without such an object database to work with.
> >
> > Reported by Konstantin Ryabitsev.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> >  bundle.c                | 3 +++
> >  t/t5607-clone-bundle.sh | 6 ++++++
> >  2 files changed, 9 insertions(+)
> >
> > diff --git a/bundle.c b/bundle.c
> > index b45666c49b..b5d21cd80f 100644
> > --- a/bundle.c
> > +++ b/bundle.c
> > @@ -142,6 +142,9 @@ int verify_bundle(struct repository *r,
> >  	int i, ret = 0, req_nr;
> >  	const char *message = _("Repository lacks these prerequisite commits:");
> >
> > +	if (!r || !r->objects || !r->objects->odb)
> > +		return error(_("need a repository to verify a bundle"));
> > +
> >  	repo_init_revisions(r, &revs, NULL);
> >  	for (i = 0; i < p->nr; i++) {
> >  		struct ref_list_entry *e = p->list + i;
> > diff --git a/t/t5607-clone-bundle.sh b/t/t5607-clone-bundle.sh
> > index cf39e9e243..2a0fb15cf1 100755
> > --- a/t/t5607-clone-bundle.sh
> > +++ b/t/t5607-clone-bundle.sh
> > @@ -14,6 +14,12 @@ test_expect_success 'setup' '
> >  	git tag -d third
> >  '
> >
> > +test_expect_success '"verify" needs a worktree' '
>
> Did you want to say "needs a repository"? Or do we really need a
> worktree?

Right, that was a place I meant to change after making up my mind what is
actually required, but forgot.

> Other than that, this looks fine to me. This is a much better experience
> than a BUG and a nice improvement overall.

Thank you! v2 incoming.

Ciao,
Dscho