Re: [PATCH 2/2] index-pack: --fsck-objects to take an optional argument for fsck msgs
From: Junio C Hamano <hidden>
Date: 2024-01-25 23:13:51
"John Cai via GitGitGadget" [off-list ref] writes:
From: John Cai <redacted> git-index-pack has a --strict mode that can take an optional argument to
"mode" -> "option", probably.
provide a list of fsck issues to change their severity. --fsck-objects does not have such a utility, which would be useful if one would like to be more lenient or strict on data integrity in a repository. Like --strict, Allow --fsck-objects to also take a list of fsck msgs to change the severity.
"Allow" -> "allow".
This commit also removes the "For internal use only" note for --fsck-objects, and documents the option. This won't often be used by the normal end user, but it turns out it is useful for Git forges like GitLab.
"This commit also removes", "documents" -> "Remove", "document".
---fsck-objects:: - For internal use only. +--fsck-objects[=<msg-ids>]:: + Instructs index-pack to check for broken objects instead of broken + links. If `<msg-ids>` is passed, it should be a comma-separated list of
Very much pleased to see an additional description that is written to clarify the difference between this option and the other --strict option. The original was totally unclear on this point, and it is very much appreciated. The other option notices and dies upon seeing either a broken object or a dangling link. This one only diagnoses broken objects and does not care if the objects are connected. However, saying "instead of" here tempts readers to mistakenly think that the other one only checks links and this one only checks contents, which is not what we want to say. Perhaps "to check for broken objects, but unlike `--strict`, do not choke on broken links" or something? Same comment on <msg-ids> as the previous step.
+ `<msg-id>=<severity>` where `<msg-id>` and `<severity>` are used to + change the severity of `fsck` errors, eg: `--strict="missingEmail=ignore,badTagName=ignore"`.
Same comment for "eg:" as before.
quoted hunk
@@ -1785,8 +1785,9 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix) } else if (!strcmp(arg, "--check-self-contained-and-connected")) { strict = 1; check_self_contained_and_connected = 1; - } else if (!strcmp(arg, "--fsck-objects")) { + } else if (skip_to_optional_arg(arg, "--fsck-objects", &arg)) { do_fsck_object = 1; + fsck_set_msg_types(&fsck_options, arg); } else if (!strcmp(arg, "--verify")) { verify = 1; } else if (!strcmp(arg, "--verify-stat")) {
The implementation of this part looks quite obvious, once you see how "--strict[=<msgid>=<level>]" is implemented. Looking good. Thanks.