Hi Junio, On 26 Jan 2024, at 16:18, Junio C Hamano wrote: > "John Cai via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes: > >> 1: b3b3e8bd0bf ! 1: cdf7fc7fe8a index-pack: test and document --strict=<msg> >> @@ Metadata >> Author: John Cai <johncai86@xxxxxxxxx> >> >> ## 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@xxxxxxxxx> >> Signed-off-by: John Cai <johncai86@xxxxxxxxx> > > 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. Christian was involved on an off-list review of this patch series. You can see it in [1]. 1. https://gitlab.com/gitlab-org/git/-/merge_requests/88 > >> ++ 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)]. sounds good, will wait for some clarification here > >> + >> ++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. good catch > > In any case, I thought your original construction to have this > "unlike" immediately after "die on broken objects" was far easier to > follow. I'll reformulate this to be clearer. From the previous version I realized I didn't take into account the pre-existing "Die if the pack contains broken objects" block so I put it at the beginning. But now I think you're right in that the "Unlike..." comes too late. > > Thanks.
