Subject: Re: [PATCH 2/2] rev-list: disable commit graph with --verify-objects

From: Junio C Hamano <[email-masked]>
Date: Wed, 07 Sep 2022 09:38:15 -0700

> diff --git a/revision.c b/revision.c
> index ee702e498a..00f9c7943b 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -2426,6 +2426,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg

It is consistent with the description in the proposed log message ...

>  		revs->tree_objects = 1;
>  		revs->blob_objects = 1;
>  		revs->verify_objects = 1;
> +		disable_commit_graph(revs->repo);

... to disable commit graph when "--verify-objects" is requested.

> diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
> index 53c2aa10b7..f9a1bc5de7 100755
> --- a/t/t1450-fsck.sh
> +++ b/t/t1450-fsck.sh
> @@ -507,6 +507,34 @@ test_expect_success 'rev-list --verify-objects with bad sha1' '
>  	test_i18ngrep -q "error: hash mismatch $(dirname $new)$(test_oid ff_2)" out
>  '
>  
> +test_expect_success 'set up repository with commit-graph' '
> +	git init corrupt-graph &&
> +	(
> +		cd corrupt-graph &&
> +		test_commit one &&
> +		test_commit two &&
> +		git commit-graph write --reachable
> +	)
> +'
> +
> +corrupt_graph_obj () {
> +	oid=$(git -C corrupt-graph rev-parse "$1") &&
> +	obj=corrupt-graph/.git/objects/$(test_oid_to_path $oid) &&
> +	test_when_finished 'mv backup $obj' &&

OK.  I missed this when-finished thing in my first read and wondered
who takes care of the 'backup' file.  We obviously cannot use it in
tests that do not follow the "break single loose object and inspect
the command behaviour in the repository" pattern, but that is OK.

> +	mv $obj backup &&
> +	echo garbage >$obj
> +}
> +
> +test_expect_success 'rev-list --verify-objects with commit graph (tip)' '
> +	corrupt_graph_obj HEAD &&
> +	test_must_fail git -C corrupt-graph rev-list --verify-objects HEAD
> +'
> +
> +test_expect_success 'rev-list --verify-objects with commit graph (parent)' '
> +	corrupt_graph_obj HEAD^ &&
> +	test_must_fail git -C corrupt-graph rev-list --verify-objects HEAD
> +'
> +

Looks good, thanks.

>  test_expect_success 'force fsck to ignore double author' '
>  	git cat-file commit HEAD >basis &&
>  	sed "s/^author .*/&,&/" <basis | tr , \\n >multiple-authors &&