git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] [GSOC] cat-file: fix --batch report changed-type bug

From
ZheNing Hu <adlternative@gmail.com>
Date
May 31, 2021, 15:32 UTC
Message-ID
<CAOLTT8T8aJ8X4TFV51JVB-ZAH-5E_E+bAmSMs2UtJ0k3OCON_w@mail.gmail.com>
In-Reply-To
<YLT2UfCZyQIXWIOv@coredump.intra.peff.net>
Jeff King <peff@peff.net> 于2021年5月31日周一 下午10:44写道:
Show 19 quoted lines
>
> > > > +test_expect_success 'cat-file --batch="batman" with --batch-all-objects will work' '
> > > > +     git -C all-two cat-file --batch-all-objects --batch="%(objectname)" | wc -l >expect &&
> > > > +     git -C all-two cat-file --batch-all-objects --batch="batman" | wc -l >actual &&
> > > > +     test_cmp expect actual
> > > > +'
> > >
> > > Is it worth testing both of these? The %(objectname) one will fail in
> > > the same way (because we do not need to run oid_object_info() to get the
> > > oid, which we already have). I'm OK doing both for better coverage, but
> > > it may be worth mentioning either in a comment or in the commit message
> > > that we expect both to fail, and why.
> >
> > Yes, these damages need to be pointed out in the commit message.
>
> I think what confused me here is that you are using "%(objectname)" as
> the "expect" output, but it also exhibits the bug. So I'd expect this
> test to pass even before your patch (though I didn't try it).
>

Yes, %(objectname) should not be used as the output of "expect", it is also a broken part.

Show 8 quoted lines
> Really, the symptom of the bug is that _neither_ of those cat-file
> invocations will exit with a success code. But because they're on the
> left-hand side of a pipe, we wouldn't even notice.
>
> The simplest test is just:
>
>   git -C all-two cat-file --batch-all-objects --batch="%(objectname)" >/dev/null
>

Yes, this is enough. I even think about use something like "!test_must_fail xxx" before... that is not necessary.

Show 6 quoted lines
> which will currently fail. It would be nice to verify that its output is
> sensible, but I'm not sure how to easily do that (it will spew a bunch
> of binary tree data, and it cannot even be parsed reliably since we
> haven't output the sizes).
>
> -Peff

Thanks! -- ZheNing Hu

Previous: Jeff KingNext: Felipe Contreras
Message 5 of 15 in “[GSOC] cat-file: fix --batch report changed-type bug”
  1. [GSOC] cat-file: fix --batch report changed-type bugZheNing Hu via GitGitGadget, May 30, 2021
  2. Jeff KingMay 30, 2021
  3. ZheNing HuMay 31, 2021
  4. Jeff KingMay 31, 2021
  5. ZheNing HuMay 31, 2021
  6. Felipe ContrerasMay 31, 2021
  7. Jeff KingJun 1, 2021
  8. Felipe ContrerasJun 1, 2021
  9. Jeff KingJun 1, 2021
  10. Felipe ContrerasJun 1, 2021
  11. ZheNing HuJun 1, 2021
  12. Junio C HamanoMay 30, 2021
  13. Jeff KingMay 30, 2021
  14. Junio C HamanoJun 1, 2021
  15. ZheNing HuMay 31, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.