Re: [PATCH v3 07/10] fetch: implement fetch.fsck.*
From: Junio C Hamano <hidden>
Date: 2018-07-27 21:08:42
Ævar Arnfjörð Bjarmason [off-list ref] 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.
quoted hunk
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?