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

Re: [PATCH 4/4] commit: don't lazy-fetch commits

From
JTJonathan Tan <jonathantanmy@google.com>
Date
Dec 1, 2022, 19:11 UTC
Message-ID
<20221201191150.3605771-1-jonathantanmy@google.com>
In-Reply-To
<Y4fFaoRFro2hNDdv@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 34 quoted lines
> OK, so we know we want a commit object because we're in the
> commit-parsing function, so we just ask to disable fetching.
> 
> Two devil's advocate thoughts:
> 
>   1. What if we're wrong that it's a commit? If somebody references a
>      blob in a commit "parent" header, the "right" outcome is for us to
>      say "oops, the type is wrong" when we try to parse it as a commit.
>      But now in a partial clone, we might avoid fetching that supposed
>      commit and say "you don't have this object", even though we could
>      get it.
> 
>      I think I'm OK with that. Either way, the repo is corrupt, and
>      we'll have informed the user of that. The fact that this bizarre
>      and specific sequence of corruptions might not go as far as it can
>      to deduce the root cause is probably fine.
> 
>   2. Are there other places where we'd want to do the same thing? E.g.,
>      in parse_object() we might ask for an object (not knowing its type)
>      only to find out that it is a commit. But we have no idea if we
>      lazy-fetched it or not!
> 
>      I had somehow imagined your series would be hooking in at the level
>      of the lazy-fetch code, and complaining about fetching commits. But
>      that may be tricky to do, because we really don't know the type
>      until after we fetch it, and selectively removing an object from
>      the odb is quite hard. We'd probably have to tell the other side
>      "please, don't send me any commits", which requires a protocol
>      extension, and...yuck.
> 
>      By comparison, your approach is an easy win that may catch problems
>      in practice (and is certainly better than the status quo).
> 
> -Peff

Thanks for taking a look. Let me know if you think that the commit message could be improved to cover these cases. Right now I think that e.g. "When parsing an object believed to be a commit in repo_parse_commit_internal()" instead of "When parsing commits" wouldn't add much value, but I might be missing something.

Previous: Jeff KingNext: Jeff King
Message 11 of 85 in “Don't lazy-fetch commits when parsing them”
  1. 0/4 Don't lazy-fetch commits when parsing themJonathan Tan, Nov 30, 2022
  2. 1/4 object-file: reread object with exact same argsJonathan Tan, Nov 30, 2022
  3. 2/4 object-file: refactor corrupt object diagnosisJonathan Tan, Nov 30, 2022
  4. Jeff KingNov 30, 2022
  5. Junio C HamanoNov 30, 2022
  6. Jonathan TanDec 1, 2022
  7. 3/4 object-file: refactor replace object lookupJonathan Tan, Nov 30, 2022
  8. Jeff KingNov 30, 2022
  9. 4/4 commit: don't lazy-fetch commitsJonathan Tan, Nov 30, 2022
  10. Jeff KingNov 30, 2022
  11. Jonathan TanDec 1, 2022
  12. Jeff KingDec 1, 2022
  13. Junio C HamanoNov 30, 2022
  14. Jeff KingNov 30, 2022
  15. 0/4 Don't lazy-fetch commits when parsing themJonathan Tan, Dec 1, 2022
  16. 1/4 object-file: reread object with exact same argsJonathan Tan, Dec 1, 2022
  17. 3/4 object-file: refactor replace object lookupJonathan Tan, Dec 1, 2022
  18. 2/4 object-file: refactor corrupt object diagnosisJonathan Tan, Dec 1, 2022
  19. 4/4 commit: don't lazy-fetch commitsJonathan Tan, Dec 1, 2022
  20. Jeff KingDec 1, 2022
  21. Jonathan TanDec 1, 2022
  22. Jeff KingDec 2, 2022
  23. Jonathan TanDec 6, 2022
  24. Jeff KingDec 6, 2022
  25. Junio C HamanoDec 1, 2022
  26. 0/3 Don't lazy-fetch commits when parsing themJonathan Tan, Dec 7, 2022
  27. 1/3 object-file: don't exit early if skipping looseJonathan Tan, Dec 7, 2022
  28. Junio C HamanoDec 7, 2022
  29. Jeff KingDec 7, 2022
  30. Junio C HamanoDec 7, 2022
  31. Jonathan TanDec 7, 2022
  32. 2/3 object-file: emit corruption errors when detectedJonathan Tan, Dec 7, 2022
  33. Junio C HamanoDec 7, 2022
  34. Ævar Arnfjörð BjarmasonDec 7, 2022
  35. Jeff KingDec 7, 2022
  36. Ævar Arnfjörð BjarmasonDec 7, 2022
  37. Jonathan TanDec 7, 2022
  38. Ævar Arnfjörð BjarmasonDec 7, 2022
  39. Jeff KingDec 8, 2022
  40. Jeff KingDec 7, 2022
  41. 3/3 commit: don't lazy-fetch commitsJonathan Tan, Dec 7, 2022
  42. Junio C HamanoDec 7, 2022
  43. Jeff KingDec 7, 2022
  44. 0/4 Don't lazy-fetch commits when parsing themJonathan Tan, Dec 8, 2022
  45. 1/4 object-file: remove OBJECT_INFO_IGNORE_LOOSEJonathan Tan, Dec 8, 2022
  46. 2/4 object-file: refactor map_loose_object_1()Jonathan Tan, Dec 8, 2022
  47. Jeff KingDec 9, 2022
  48. Jonathan TanDec 9, 2022
  49. Jeff KingDec 9, 2022
  50. Jeff KingDec 9, 2022
  51. 3/4 object-file: emit corruption errors when detectedJonathan Tan, Dec 8, 2022
  52. Jeff KingDec 9, 2022
  53. Jonathan TanDec 9, 2022
  54. Ævar Arnfjörð BjarmasonDec 9, 2022
  55. Jonathan TanDec 9, 2022
  56. 4/4 commit: don't lazy-fetch commitsJonathan Tan, Dec 8, 2022
  57. Ævar Arnfjörð BjarmasonDec 9, 2022
  58. 0/4 Don't lazy-fetch commits when parsing themJonathan Tan, Dec 9, 2022
  59. 1/4 object-file: remove OBJECT_INFO_IGNORE_LOOSEJonathan Tan, Dec 9, 2022
  60. 2/4 object-file: refactor map_loose_object_1()Jonathan Tan, Dec 9, 2022
  61. 3/4 object-file: emit corruption errors when detectedJonathan Tan, Dec 9, 2022
  62. Junio C HamanoDec 10, 2022
  63. Jonathan TanDec 12, 2022
  64. Jeff KingDec 12, 2022
  65. Jonathan TanDec 12, 2022
  66. Jeff KingDec 12, 2022
  67. Jonathan TanDec 12, 2022
  68. Jeff KingDec 12, 2022
  69. Jonathan TanDec 12, 2022
  70. Jeff KingDec 13, 2022
  71. 4/4 commit: don't lazy-fetch commitsJonathan Tan, Dec 9, 2022
  72. 0/4 Don't lazy-fetch commits when parsing themJonathan Tan, Dec 12, 2022
  73. 1/4 object-file: remove OBJECT_INFO_IGNORE_LOOSEJonathan Tan, Dec 12, 2022
  74. 2/4 object-file: refactor map_loose_object_1()Jonathan Tan, Dec 12, 2022
  75. 4/4 commit: don't lazy-fetch commitsJonathan Tan, Dec 12, 2022
  76. 3/4 object-file: emit corruption errors when detectedJonathan Tan, Dec 12, 2022
  77. Junio C HamanoDec 13, 2022
  78. Jeff KingDec 13, 2022
  79. 0/4 Don't lazy-fetch commits when parsing themJonathan Tan, Dec 14, 2022
  80. 1/4 object-file: remove OBJECT_INFO_IGNORE_LOOSEJonathan Tan, Dec 14, 2022
  81. 2/4 object-file: refactor map_loose_object_1()Jonathan Tan, Dec 14, 2022
  82. 3/4 object-file: emit corruption errors when detectedJonathan Tan, Dec 14, 2022
  83. 4/4 commit: don't lazy-fetch commitsJonathan Tan, Dec 14, 2022
  84. Jeff KingDec 14, 2022
  85. Junio C HamanoDec 15, 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.