Re: [PATCH v2 5/5] fetch: implement fetch.fsck.*
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 30, 2018, 03:47 UTC
- Message-ID
- <xmqqin75ere0.fsf@gitster-ct.c.googlers.com>
- In-Reply-To
- <20180525192811.25680-6-avarab@gmail.com>
Earlier I mumbled "this 4-patch series generally looks good but I need to re-read the implementation step"; I meant this 5-patch series and here is my impression after re-reading the implementation step.
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
Show 6 quoted lines
> diff --git a/Documentation/config.txt b/Documentation/config.txt > index f97f21c022..f69cd31ad0 100644 > --- a/Documentation/config.txt > +++ b/Documentation/config.txt > @@ -1426,6 +1426,16 @@ fetch.fsckObjects:: > ...
Nicely written.
Show 17 quoted lines
> diff --git a/fetch-pack.c b/fetch-pack.c > index 490c38f833..9e4282788e 100644 > --- a/fetch-pack.c > +++ b/fetch-pack.c > @@ -19,6 +19,7 @@ > #include "sha1-array.h" > #include "oidset.h" > #include "packfile.h" > +#include "fsck.h" > > static int transfer_unpack_limit = -1; > static int fetch_unpack_limit = -1; > @@ -33,6 +34,7 @@ static int agent_supported; > static int server_supports_filtering; > static struct lock_file shallow_lock; > static const char *alternate_shallow_file; > +static struct strbuf fsck_msg_types = STRBUF_INIT;
s/msg_types[]/exclude[]/ or something, as this is not just about "we tolerate known and future instances of 0-padded mode in trees", but also "we know this and that object is bad so do not complain" as well.
Other than that, the implementation looks good.
Show 20 quoted lines
> diff --git a/t/t5504-fetch-receive-strict.sh b/t/t5504-fetch-receive-strict.sh > index 49d3621a92..b7f5222c2b 100755 > --- a/t/t5504-fetch-receive-strict.sh > +++ b/t/t5504-fetch-receive-strict.sh > @@ -135,6 +135,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 && > + 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 && > + git --git-dir=dst/.git config fetch.fsck.skipList dst/.git/SKIP && > + echo $commit >dst/.git/SKIP && > + git --git-dir=dst/.git fetch "file://$(pwd)" $refspec > +'
If the test does not change meaning when file://$(pwd) is replaced with "." (or ".." or whatever if other tests below moves cwd around), I'd think it is preferrable. Seeing $(pwd) always makes me nervous about Windows.