Re: [PATCH v3 07/10] fetch: implement fetch.fsck.*

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Ævar Arnfjörð Bjarmason  <avarab@xxxxxxxxx> writes:

> -			argv_array_push(&cmd.args, "--strict");
> +			argv_array_pushf(&cmd.args, "--strict%s",
> +					 fsck_msg_types.buf);
> ...
> +		if (git_config_pathname(&path, var, value))
> +			return 1;
> +		strbuf_addf(&fsck_msg_types, "%cskiplist=%s",
> +			fsck_msg_types.len ? ',' : '=', path);
> ...
> +		if (is_valid_msg_type(var, value))
> +			strbuf_addf(&fsck_msg_types, "%c%s=%s",
> +				fsck_msg_types.len ? ',' : '=', var, value);
> +		else
> +			warning("Skipping unknown msg id '%s'", var);

This follows quite familiar pattern found in receive_pack_config();
looking good.

> diff --git a/t/t5504-fetch-receive-strict.sh b/t/t5504-fetch-receive-strict.sh
> index 57ff78c201..004bfebe98 100755
> --- a/t/t5504-fetch-receive-strict.sh
> +++ b/t/t5504-fetch-receive-strict.sh
> @@ -145,6 +145,20 @@ test_expect_success 'push with receive.fsck.skipList' '
>  	git push --porcelain dst bogus
>  '
>  
> +test_expect_success 'fetch with fetch.fsck.skipList' '
> +	commit="$(git hash-object -t commit -w --stdin <bogus-commit)" &&
> +	refspec=refs/heads/bogus:refs/heads/bogus &&
> +	git push . $commit:refs/heads/bogus &&

I see this used in the previous test for receive.fsck.skipList, but
it is an interesting implementation of "git update-ref" that could
be affected by potential fsck error in push-to-receive-pack transport.
As we are interested in transport into "dst" and we want this creation
of our 'bogus' branch to succeed no matter what, it probably is not
a good idea to use "git push ." like this in the context of this test.

Perhaps leave a 'leftoverbits' comment to force us remember to update
all these uses of local push from the script in the future?

> +	rm -rf dst &&
> +	git init dst &&
> +	git --git-dir=dst/.git config fetch.fsckObjects true &&
> +	test_must_fail git --git-dir=dst/.git fetch "file://$(pwd)" $refspec &&

We see that by default fetch.fsckObjects errors out when it notices
the bogus commit object.

> +	git --git-dir=dst/.git config fetch.fsck.skipList dst/.git/SKIP &&
> +	echo $commit >dst/.git/SKIP &&

And then we set up a skip to see ...

> +	git --git-dir=dst/.git fetch "file://$(pwd)" $refspec

... if that is ignored.  Looks great.

Would this second attempt succeed _without_ the SKIP list, I wonder,
though?  

After the initial attempt that transferred the object, inspected it
and then aborted before pointing a ref to make the object reachable,
wouldn't it be possible for the quickfetch codepath to say "ah, we
locally have that object, so let's see it is a descendant of the tip
of one of our refs *and* all the objects it points at (recursively)
are all available in this repository", as we do not quarantine on
the fetch side?



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux