diff mbox series

[v4,2/2] index-pack: --fsck-objects to take an optional argument for fsck msgs

Message ID f29ab9136fb4c23c5700a73731a5e220f92b7c30.1706751483.git.gitgitgadget@gmail.com (mailing list archive)
State Accepted
Commit 0f8edf7317c7eb152bf045ed99b46072439088a6
Headers show
Series index-pack: fsck honor checks | expand

Commit Message

John Cai Feb. 1, 2024, 1:38 a.m. UTC 
From: John Cai <johncai86@gmail.com>

git-index-pack has a --strict option that can take an optional argument
to 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.

Remove the "For internal use only" note for --fsck-objects, and document
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.

Reviewed-by: Christian Couder <christian.couder@gmail.com>
Signed-off-by: John Cai <johncai86@gmail.com>
---
 Documentation/git-index-pack.txt | 17 +++++++++++------
 builtin/index-pack.c             |  5 +++--
 t/t5300-pack-object.sh           | 29 ++++++++++++++++++++++++-----
 3 files changed, 38 insertions(+), 13 deletions(-)

Comments

SZEDER Gábor March 8, 2024, 10:24 p.m. UTC | #1 
On Thu, Feb 01, 2024 at 01:38:02AM +0000, John Cai via GitGitGadget wrote:
> diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
> index 496fffa0f8a..a58f91035d1 100755
> --- a/t/t5300-pack-object.sh
> +++ b/t/t5300-pack-object.sh
> @@ -441,8 +441,7 @@ test_expect_success 'index-pack with --strict' '
>  	)
>  '
>  
> -test_expect_success 'index-pack with --strict downgrading fsck msgs' '
> -	test_when_finished rm -rf strict &&
> +test_expect_success 'setup for --strict and --fsck-objects downgrading fsck msgs' '
>  	git init strict &&
>  	(
>  		cd strict &&
> @@ -457,12 +456,32 @@ test_expect_success 'index-pack with --strict downgrading fsck msgs' '
>  
>  		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"
> +		git pack-objects test <commit_list >pack-name
>  	)
>  '
>  
> +test_with_bad_commit () {
> +	must_fail_arg="$1" &&
> +	must_pass_arg="$2" &&
> +	(
> +		cd strict &&
> +		test_expect_fail git index-pack "$must_fail_arg" "test-$(cat pack-name).pack"

There is no such command as 'test_expect_fail', resulting in:

  expecting success of 5300.34 'index-pack with --strict downgrading fsck msgs':
          test_with_bad_commit --strict --strict="missingEmail=ignore"

  + test_with_bad_commit --strict --strict=missingEmail=ignore
  + must_fail_arg=--strict
  + must_pass_arg=--strict=missingEmail=ignore
  + cd strict
  + cat pack-name
  + test_expect_fail git index-pack --strict test-e4e1649155bf444fbd9cd85e376628d6eaf3d3bd.pack
  ./t5300-pack-object.sh: 468: eval: test_expect_fail: not found
  + cat pack-name
  + git index-pack --strict=missingEmail=ignore test-e4e1649155bf444fbd9cd85e376628d6eaf3d3bd.pack
  e4e1649155bf444fbd9cd85e376628d6eaf3d3bd

  ok 34 - index-pack with --strict downgrading fsck msgs

The missing command should fail the test, but it doesn't, because the
&&-chain is broken on this line as well.
John Cai March 9, 2024, 1:55 a.m. UTC | #2 
Hi Szeder,

On 8 Mar 2024, at 17:24, SZEDER Gábor wrote:

> On Thu, Feb 01, 2024 at 01:38:02AM +0000, John Cai via GitGitGadget wrote:
>> diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
>> index 496fffa0f8a..a58f91035d1 100755
>> --- a/t/t5300-pack-object.sh
>> +++ b/t/t5300-pack-object.sh
>> @@ -441,8 +441,7 @@ test_expect_success 'index-pack with --strict' '
>>  	)
>>  '
>>
>> -test_expect_success 'index-pack with --strict downgrading fsck msgs' '
>> -	test_when_finished rm -rf strict &&
>> +test_expect_success 'setup for --strict and --fsck-objects downgrading fsck msgs' '
>>  	git init strict &&
>>  	(
>>  		cd strict &&
>> @@ -457,12 +456,32 @@ test_expect_success 'index-pack with --strict downgrading fsck msgs' '
>>
>>  		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"
>> +		git pack-objects test <commit_list >pack-name
>>  	)
>>  '
>>
>> +test_with_bad_commit () {
>> +	must_fail_arg="$1" &&
>> +	must_pass_arg="$2" &&
>> +	(
>> +		cd strict &&
>> +		test_expect_fail git index-pack "$must_fail_arg" "test-$(cat pack-name).pack"
>
> There is no such command as 'test_expect_fail', resulting in:

Indeed, thanks for catching this.

>
>   expecting success of 5300.34 'index-pack with --strict downgrading fsck msgs':
>           test_with_bad_commit --strict --strict="missingEmail=ignore"
>
>   + test_with_bad_commit --strict --strict=missingEmail=ignore
>   + must_fail_arg=--strict
>   + must_pass_arg=--strict=missingEmail=ignore
>   + cd strict
>   + cat pack-name
>   + test_expect_fail git index-pack --strict test-e4e1649155bf444fbd9cd85e376628d6eaf3d3bd.pack
>   ./t5300-pack-object.sh: 468: eval: test_expect_fail: not found
>   + cat pack-name
>   + git index-pack --strict=missingEmail=ignore test-e4e1649155bf444fbd9cd85e376628d6eaf3d3bd.pack
>   e4e1649155bf444fbd9cd85e376628d6eaf3d3bd
>
>   ok 34 - index-pack with --strict downgrading fsck msgs
>
> The missing command should fail the test, but it doesn't, because the
> &&-chain is broken on this line as well.

yes will need to fix this as well

thanks
John
diff mbox series

Patch

diff --git a/Documentation/git-index-pack.txt b/Documentation/git-index-pack.txt
index 694bb9409bf..5a20deefd5f 100644
--- a/Documentation/git-index-pack.txt
+++ b/Documentation/git-index-pack.txt
@@ -96,13 +96,18 @@  default and "Indexing objects" when `--stdin` is specified.
 --check-self-contained-and-connected::
 	Die if the pack contains broken links. For internal use only.
 
---fsck-objects::
-	For internal use only.
+--fsck-objects[=<msg-id>=<severity>...]::
+	Die if the pack contains broken objects, but unlike `--strict`, don't
+	choke on broken links. 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").
 +
-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").
+An optional comma-separated list of `<msg-id>=<severity>` can be passed to
+change the severity of some possible issues, e.g.,
+`--fsck-objects="missingEmail=ignore,badTagName=ignore"`. 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>`.
 
 --threads=<n>::
 	Specifies the number of threads to spawn when resolving
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 240c7021168..a3a37bd215d 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[=<msg-id>=<severity>...]] (<pack-file> | --stdin [--fix-thin] [<pack-file>])";
+"git index-pack [-v] [-o <index-file>] [--keep | --keep=<msg>] [--[no-]rev-index] [--verify] [--strict[=<msg-id>=<severity>...]] [--fsck-objects[=<msg-id>=<severity>...]] (<pack-file> | --stdin [--fix-thin] [<pack-file>])";
 
 struct object_entry {
 	struct pack_idx_entry idx;
@@ -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")) {
diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
index 496fffa0f8a..a58f91035d1 100755
--- a/t/t5300-pack-object.sh
+++ b/t/t5300-pack-object.sh
@@ -441,8 +441,7 @@  test_expect_success 'index-pack with --strict' '
 	)
 '
 
-test_expect_success 'index-pack with --strict downgrading fsck msgs' '
-	test_when_finished rm -rf strict &&
+test_expect_success 'setup for --strict and --fsck-objects downgrading fsck msgs' '
 	git init strict &&
 	(
 		cd strict &&
@@ -457,12 +456,32 @@  test_expect_success 'index-pack with --strict downgrading fsck msgs' '
 
 		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"
+		git pack-objects test <commit_list >pack-name
 	)
 '
 
+test_with_bad_commit () {
+	must_fail_arg="$1" &&
+	must_pass_arg="$2" &&
+	(
+		cd strict &&
+		test_expect_fail git index-pack "$must_fail_arg" "test-$(cat pack-name).pack"
+		git index-pack "$must_pass_arg" "test-$(cat pack-name).pack"
+	)
+}
+
+test_expect_success 'index-pack with --strict downgrading fsck msgs' '
+	test_with_bad_commit --strict --strict="missingEmail=ignore"
+'
+
+test_expect_success 'index-pack with --fsck-objects downgrading fsck msgs' '
+	test_with_bad_commit --fsck-objects --fsck-objects="missingEmail=ignore"
+'
+
+test_expect_success 'cleanup for --strict and --fsck-objects downgrading fsck msgs' '
+	rm -rf strict
+'
+
 test_expect_success 'honor pack.packSizeLimit' '
 	git config pack.packSizeLimit 3m &&
 	packname_10=$(git pack-objects test-10 <obj-list) &&