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

[PATCH 0/4] tag: don't misreport type of tagged objects in errors

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Nov 18, 2022, 11:46 UTC
Message-ID
<cover-0.4-00000000000-20221118T113442Z-avarab@gmail.com>
In-Reply-To
<Y3a3qcqNG8W3ueeb@coredump.intra.peff.net>

This series fixes a very long-standing issue where we'll get confused when we parse a tag whose "type" lies about the type of the target object.

It goes on top of Jeff King's just-submitted [1], and the two compliment one another. See [2] for my feedback about what was left over, which this fixes.

Currently we'll parse tags and note what the "type" claims to be. Say a pointer to a "blob" object that claims to be a "commit" in the envelope.

Then when we we'd try to parse that supposed "commit' for real we'd emit a message like:

	error: object <oid> is a blob, not a commit

Which is reversed, i.e. we'd remember the first "blob" we saw, and then get confused about seeing a "commit" when we did the actual parsing.

This is now fixed in almost all cases by having the one caller of parse_tag() which actually knows the type tell it "yes, I'm sure this is a commit".

We'll then be able to see that we have a non-parsed object as scaffolding, but that it's really a commit, and emit the correct:

	error: object <oid> is a commit not a blob

Which goes along with other errors where the tag object itself yells about being unhappy with the object reference.

I submitted a version of these patches back in early 2021[3], this is significantly slimmed down since then.

At the time Jeff King noted that this approach inherently can't cover all possible scenarios. I.e. sometimes our parsing of the envelope isn't followed up by the "real" parse.

Even in those cases we can "get it right as 4/4 here demonstrates.

But there are going to be cases left where we get it wrong, but they're all cases where we get it wrong now. It's probably not worth fixing the long tail of those issues, but now we'll emit a sensible error on the common case of "log" etc.

1. https://lore.kernel.org/git/Y3a3qcqNG8W3ueeb@coredump.intra.peff.net/
2. https://lore.kernel.org/git/221118.86cz9lgjxu.gmgdl@evledraar.gmail.com/
3. https://lore.kernel.org/git/YGTGgFI19fS7Uv6I@coredump.intra.peff.net/
Ævar Arnfjörð Bjarmason (4):
  object-file.c: free the "t.tag" in check_tag()
  object tests: add test for unexpected objects in tags
  tag: don't misreport type of tagged objects in errors
  tag: don't emit potentially incorrect "object is a X, not a Y"
 blob.c                                 |  11 +-
 blob.h                                 |   3 +
 commit.c                               |  11 +-
 commit.h                               |   2 +
 object-file.c                          |   1 +
 object.c                               |  20 +++-
 object.h                               |   2 +
 t/t3800-mktag.sh                       |   1 +
 t/t5302-pack-index.sh                  |   2 +
 t/t6102-rev-list-unexpected-objects.sh | 146 +++++++++++++++++++++++++
 tag.c                                  |  22 +++-
 tag.h                                  |   2 +
 tree.c                                 |  11 +-
 tree.h                                 |   2 +
 14 files changed, 222 insertions(+), 14 deletions(-)
-- 
2.38.0.1511.gcdcff1f1dc2
Previous: Jeff KingNext: Ævar Arnfjörð Bjarmason
Message 9 of 22 in “object.c: use has_object() instead of repo_has_object_file()”
  1. Kousik SanagavarapuNov 16, 2022
  2. Jeff KingNov 16, 2022
  3. Jonathan TanNov 16, 2022
  4. 0/2 fixing parse_object() check for type mismatchJeff King, Nov 17, 2022
  5. 1/2 parse_object(): drop extra "has" check before checking object typeJeff King, Nov 17, 2022
  6. 2/2 parse_object(): check on-disk type of suspected blobJeff King, Nov 17, 2022
  7. Ævar Arnfjörð BjarmasonNov 18, 2022
  8. Jeff KingNov 21, 2022
  9. 0/4 tag: don't misreport type of tagged objects in errorsÆvar Arnfjörð Bjarmason, Nov 18, 2022
  10. 1/4 object-file.c: free the "t.tag" in check_tag()Ævar Arnfjörð Bjarmason, Nov 18, 2022
  11. 2/4 object tests: add test for unexpected objects in tagsÆvar Arnfjörð Bjarmason, Nov 18, 2022
  12. 4/4 tag: don't emit potentially incorrect "object is a X, not a Y"Ævar Arnfjörð Bjarmason, Nov 18, 2022
  13. 3/4 tag: don't misreport type of tagged objects in errorsÆvar Arnfjörð Bjarmason, Nov 18, 2022
  14. 0/3 tag: don't misreport type of tagged objects in errorsÆvar Arnfjörð Bjarmason, Dec 30, 2022
  15. 1/3 object tests: add test for unexpected objects in tagsÆvar Arnfjörð Bjarmason, Dec 30, 2022
  16. Junio C HamanoDec 30, 2022
  17. 3/3 tag: don't emit potentially incorrect "object is a X, not a Y"Ævar Arnfjörð Bjarmason, Dec 30, 2022
  18. 2/3 tag: don't misreport type of tagged objects in errorsÆvar Arnfjörð Bjarmason, Dec 30, 2022
  19. Junio C HamanoDec 30, 2022
  20. Taylor BlauNov 18, 2022
  21. Jeff KingNov 21, 2022
  22. Ævar Arnfjörð BjarmasonNov 22, 2022

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.