Re: [PATCH v3 0/2] index-pack: fsck honor checks
- From
John Cai <johncai86@gmail.com>
- Date
- Jan 27, 2024, 02:31 UTC
- Message-ID
- <BE30DB47-1488-40A3-BD0C-804F97DE0C88@gmail.com>
- In-Reply-To
- <20240126221357.2940676-1-jonathantanmy@google.com>
Hi Jonathan,
On 26 Jan 2024, at 17:13, Jonathan Tan wrote:
Show 26 quoted lines
> Junio C Hamano <gitster@pobox.com> writes: >>> ++--fsck-objects[=<msg-id>=<severity>...]:: >>> ++ Die if the pack contains broken objects. If the pack contains a tree >>> ++ pointing to a .gitmodules blob that does not exist, prints the hash of >>> ++ that blob (for the caller to check) after the hash that goes into the >>> ++ name of the pack/idx file (see "Notes"). >> >> Not a new problem bit I have to wonder what happens if the pack >> contains many trees that point at different blobs for ".gitmodules" >> path and many of these blobs are not included in the packfile? Will >> the caller receive all of these blob object names so that they can >> be verified? The reference to the "Notes" only refer to the fact >> that usually a single hash value that is used in constructing the >> name of the packfile "pack-<Hashvalue>.pack" is emitted to the >> standard output, which is not wrong per se, but does not help >> readers very much wrt to understanding this. >> >> [jc: dragging JTan into the thread, as this comes from his 5476e1ef >> (fetch-pack: print and use dangling .gitmodules, 2021-02-22)]. > > Ah...I can see how that documentation isn't clear. The intention of that > commit is to check every link to a .gitmodules blob. The tests perhaps > should have been written with 2 .gitmodules blobs (in separate commits), > but I think the production code works: I tried changing the test to have > 2 commits each with their own .gitmodules blob, and error messages were > printed for both blobs.
Thanks for clarifying! Would you mind providing a patch to revise the wording here to make it clearer? I would try but I feel like I might get the wording wrong.
> > (If someone changes that test, e.g. to have 2 blobs, the ">h" in the > "configure_exclusion" invocations look superfluous and is perhaps a > copy-and-paste error from other tests that needed the hash later.)
thanks John