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

[Junio C Hamano <gitster@pobox.com>]

"John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:

>  1:  b3b3e8bd0bf ! 1:  cdf7fc7fe8a index-pack: test and document --strict=<msg>
>      @@ Metadata
>       Author: John Cai <johncai86@gmail.com>
>       
>        ## Commit message ##
>      -    index-pack: test and document --strict=<msg>
>      +    index-pack: test and document --strict=<msg-id>=<severity>...

Ah, I missed this one.  Nice spotting.

>           5d477a334a (fsck (receive-pack): allow demoting errors to warnings,
>           2015-06-22) allowed a list of fsck msg to downgrade to be passed to
>      @@ Commit message
>           directly, (nor use index-pack for that matter) it is still useful to
>           document and test this feature.
>       
>      +    Reviewed-by: Christian Couder <christian.couder@gmail.com>
>           Signed-off-by: John Cai <johncai86@gmail.com>

I haven't seen Christian involved (by getting Cc'ed these patches,
sending out review comments, or giving his Reviewed-by:) during
these three rounds of this topic.  I'll wait until I hear from him
before queuing this, just to be safe.

>      ++	Die, if the pack contains broken objects or links. An optional
>      ++	comma-separated list of `<msg-id>=<severity>` can be passed to change
>      ++	the severity of some possible issues, e.g.,
>      ++	 `--strict="missingEmail=ignore,badTagName=error"`. See the entry for the
>      ++	`fsck.<msg-id>` configuration options in linkgit:git-fsck[1] for more
>      ++	information on the possible values of `<msg-id>` and `<severity>`.

This is much better than the tentative text I tweaked.  Nice.

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

>        +
>      ++Unlike `--strict` however, don't choke on broken links. An optional

You'd need a comma on both sides of "however" used like this, I
think.  

In any case, I thought your original construction to have this
"unlike" immediately after "die on broken objects" was far easier to
follow.

Thanks.