Re: Assertion failure with git cat-file --batch-command

On Mon, 27 Jul 2026 at 21:26, Pablo Sabater <[email protected]> wrote:
>
> On Mon Jul 27, 2026 at 11:57 AM CEST, Jeff King wrote:
> > On Mon, Jul 27, 2026 at 10:30:43AM +0100, Alan Stokes wrote:
> >
> >> I first observed this in 2.43.0, but it still seems to be present in
> >> 2.54.0.
> >
> > Yeah, I think this has been there since --batch-command was added.
> >
> >> Note that if I ask git cat-file --batch-command to include the
> >> objecttype in the output it is fine (which gives me a workaround). Or
> >> if I use git cat-file --batch.
> >>
> >> IIUC git only fetches the metadata that it needs for each object, and
> >> that is determined from the format. For --batch I guess the type is
> >> always requested, since it is needed to print the object contents. But
> >> for --batch-command that doesn't seem to happen.
> >
> > Yes, exactly. In the normal --batch code path we have this code:
> >
> >         /*
> >          * If we are printing out the object, then always fill in the type,
> >          * since we will want to decide whether or not to stream.
> >          */
> >         if (opt->batch_mode == BATCH_MODE_CONTENTS)
> >                 data.info.typep = &data.type;
> >
> > But for command mode, we don't do the same. This makes your case work:
> >
> > diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> > index 1458dd76d6..78eab9723d 100644
> > --- a/builtin/cat-file.c
> > +++ b/builtin/cat-file.c
> > @@ -690,6 +690,7 @@ static void parse_cmd_contents(struct batch_options *opt,
> >                            struct expand_data *data)
> >  {
> >       opt->batch_mode = BATCH_MODE_CONTENTS;
> > +     data->info.typep = &data->type;
> >       batch_one_object(line, output, opt, data);
> >  }
> >
> >
> > but there's a slight catch. That expand_data is used for every request,
> > not just the current one. In normal --batch mode, every request wants
> > the same data (the user-specified format plus the object contents). But
> > in command mode, some may be "contents" requests and some may just be
> > "info". The code above turns on type-checking for every request, making
> > the "info" ones pay to look up the type.
>
> Yes, for example, both 'info' and the 'remote-object-info' series
> (marked to 'master' in the last "What's cooking") [1] act on
> data->info.typep.
>
> This would make 'info' do a type lookup, and 'remote-object-info'
> request "type" even if it wasn't present on the format.
>
> >
> > A type lookup isn't all that expensive, but it might matter for some
> > formats (e.g., just "%(objectname)" does an existence check and nothing
> > else, so we never even access the object data).
>
> Yes, and only the atoms in the format get expanded, a populated type
> without its atom in the format won't be shown.
> the wasted lookup or a bigger request are the only effect.
>
> >
> > I guess saving and restore data->info.typep would work.
>
> Yes I think that too, I tried this and it worked fine:
>
> static void parse_cmd_contents(struct batch_options *opt,
>                              const char *line,
>                              struct strbuf *output,
>                              struct expand_data *data)
> {
>         enum object_type *saved = data->info.typep;
>
>         opt->batch_mode = BATCH_MODE_CONTENTS;
>         data->info.typep = &data->type;
>         batch_one_object(line, output, opt, data);
>         data->info.typep = saved;
> }

That does look pretty simple and correct.

>
> nit: On the current code the parameters aren't indented correctly.
>
> >
> >> I'm not sure what the correct fix is - always request the type in
> >> --batch-command, or perhaps only if a "contents" command is issued?
> >
> > Yeah, in general if you are asking about "contents" I'd expect you to
> > get the full name/type/size triple. But it's not wrong to ask for less,
> > and certainly we should never hit a BUG(). So I think we'd want a fix
> > along the lines above.
> >
> > Do you want to try your hand at a patch? It would need to do the
> > save/restore, and most importantly add a new test to t1006.

I would be willing to have a go at it. But realistically I probably won't have
time for a month or two. I'm also a complete noob at the whole posting
patches via email process, so it may be slightly chaotic. If anybody else
wanted to deal with it I obviously wouldn't object.

Best wishes,

Alan

> >
> > -Peff
>
> [1]: https://lore.kernel.org/git/[email protected]/
>
> Hope this helps,
> Pablo