Re: [PATCH v3 0/2] index-pack: fsck honor checks

From: Jonathan Tan <jonathantanmy@google.com>
Date: Jan 26, 2024, 22:13 UTC
Message-ID: <20240126221357.2940676-1-jonathantanmy@google.com>
In-Reply-To: <xmqqfryjn686.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:

Show 18 quoted lines

> >      ++--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.

(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.)