Subject: Re: [PATCH v2 1/2] index-pack: test and document --strict=<msg>

From: Junio C Hamano <[email-masked]>
Date: Fri, 26 Jan 2024 10:03:37 -0800

"John Cai via GitGitGadget" <[email-masked]> writes:

> From: John Cai <[email-masked]>
>
> 5d477a334a (fsck (receive-pack): allow demoting errors to warnings,
> 2015-06-22) allowed a list of fsck msg to downgrade to be passed to
> --strict. However this is a hidden argument that was not documented nor
> tested. Though it is true that most users would not call this option
> directly, (nor use index-pack for that matter) it is still useful to
> document and test this feature.
>
> Signed-off-by: John Cai <[email-masked]>
> ---
>  Documentation/git-index-pack.txt |  9 +++++++--
>  builtin/index-pack.c             |  2 +-
>  t/t5300-pack-object.sh           | 22 ++++++++++++++++++++++
>  3 files changed, 30 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/git-index-pack.txt b/Documentation/git-index-pack.txt
> index 6486620c3d8..f7a98bbf9c8 100644
> --- a/Documentation/git-index-pack.txt
> +++ b/Documentation/git-index-pack.txt
> @@ -79,8 +79,13 @@ OPTIONS
>  	to force the version for the generated pack index, and to force
>  	64-bit index entries on objects located above the given offset.
>  
> ---strict::
> -	Die, if the pack contains broken objects or links.
> +--strict[=<msg-id>=<severity>...]::
> +	Die, if the pack contains broken objects or links. If `<msg-ids>` is passed,
> +	it should be a comma-separated list of `<msg-id>=<severity>` elements where
> +	`<msg-id>` and `<severity>` are used to change the severity of some possible
> +	issues, e.g., `--strict="missingEmail=ignore,badTagName=error"`. See the entry

There no longer is <msg-ids>, so I'll tweak the text perhaps like so:

	An optional value that is a comma-separated list of '<msg-id>=<severity>'
	can be passed to change the severity of some possible issues, ...

while queueing.  Will probably do the same for the --fsck-objects
side in the next patch.

Other than that, thanks for a pleasant read.

> +	for the `fsck.<msg-id>` configuration options in `linkgit:git-fsck[1] for
> +	more information on the possible values of `<msg-id>` and `<severity>`.
>  
>  --progress-title::
>  	For internal use only.
> diff --git a/builtin/index-pack.c b/builtin/index-pack.c
> index 1ea87e01f29..1e53ca23775 100644
> --- a/builtin/index-pack.c
> +++ b/builtin/index-pack.c
> @@ -24,7 +24,7 @@
>  #include "setup.h"
>  
>  static const char index_pack_usage[] =
> -"git index-pack [-v] [-o <index-file>] [--keep | --keep=<msg>] [--[no-]rev-index] [--verify] [--strict] (<pack-file> | --stdin [--fix-thin] [<pack-file>])";
> +"git index-pack [-v] [-o <index-file>] [--keep | --keep=<msg>] [--[no-]rev-index] [--verify] [--strict[=<msg-ids>]] (<pack-file> | --stdin [--fix-thin] [<pack-file>])";
>  
>  struct object_entry {
>  	struct pack_idx_entry idx;
> diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
> index d402ec18b79..496fffa0f8a 100755
> --- a/t/t5300-pack-object.sh
> +++ b/t/t5300-pack-object.sh
> @@ -441,6 +441,28 @@ test_expect_success 'index-pack with --strict' '
>  	)
>  '
>  
> +test_expect_success 'index-pack with --strict downgrading fsck msgs' '
> +	test_when_finished rm -rf strict &&
> +	git init strict &&
> +	(
> +		cd strict &&
> +		test_commit first hello &&
> +		cat >commit <<-EOF &&
> +		tree $(git rev-parse HEAD^{tree})
> +		parent $(git rev-parse HEAD)
> +		author A U Thor
> +		committer A U Thor
> +
> +		commit: this is a commit with bad emails
> +
> +		EOF
> +		git hash-object --literally -t commit -w --stdin <commit >commit_list &&
> +		PACK=$(git pack-objects test <commit_list) &&
> +		test_must_fail git index-pack --strict "test-$PACK.pack" &&
> +		git index-pack --strict="missingEmail=ignore" "test-$PACK.pack"
> +	)
> +'
> +
>  test_expect_success 'honor pack.packSizeLimit' '
>  	git config pack.packSizeLimit 3m &&
>  	packname_10=$(git pack-objects test-10 <obj-list) &&