Re: [PATCH v3 10/10] fsck: test and document unknown fsck.<msg-id> values

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Fri, Jul 27 2018, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
>> When fsck.<msg-id> is set to an unknown value it'll cause "fsck" to
>> die, but the same is not rue of the "fetch" and "receive"
>> variants. Document this and test for it.
>
> Interesting.  Before documenting and adding a test to cast the
> current behaviour in stone, do we need to see if the discrepancy is
> desired and designed one, or something we may want to fix?

We could change it. This is just something I ran into and figured it
should be tested / documented, and didn't feel any need to change it
myself.

The current behavior is probably more of an organically grown
accident. Maybe we should make all of these warn.

Trying to post-hoc rationalize these, it probably makes sense for
receive.* not to die, since you don't want pushes to fail if you're
tweaking this on a server and typo it, same with fetch (although less
so), whereas "fsck" tends to be more of an offline validation command.

So Yeah, I can change this, or not. What do you/others think?

>
>>
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>> ---
>>  Documentation/config.txt        |  4 ++++
>>  t/t5504-fetch-receive-strict.sh | 14 ++++++++++++++
>>  2 files changed, 18 insertions(+)
>>
>> diff --git a/Documentation/config.txt b/Documentation/config.txt
>> index 57c463c6e2..4cead6119a 100644
>> --- a/Documentation/config.txt
>> +++ b/Documentation/config.txt
>> @@ -1637,6 +1637,10 @@ In general, it is better to enumerate existing objects with problems
>>  with `fsck.skipList`, instead of listing the kind of breakages these
>>  problematic objects share to be ignored, as doing the latter will
>>  allow new instances of the same breakages go unnoticed.
>> ++
>> +Setting an unknown `fsck.<msg-id>` value will cause fsck to die, but
>> +doing the same for `receive.fsck.<msg-id>` and `fetch.fsck.<msg-id>`
>> +will only cause git to warn.
>>
>>  fsck.skipList::
>>  	The path to a sorted list of object names (i.e. one SHA-1 per
>> diff --git a/t/t5504-fetch-receive-strict.sh b/t/t5504-fetch-receive-strict.sh
>> index 7f06b537d3..62f3569891 100755
>> --- a/t/t5504-fetch-receive-strict.sh
>> +++ b/t/t5504-fetch-receive-strict.sh
>> @@ -198,6 +198,10 @@ test_expect_success 'fetch with fetch.fsck.skipList' '
>>  	git --git-dir=dst/.git fetch "file://$(pwd)" $refspec
>>  '
>>
>> +test_expect_success 'fsck.<unknownmsg-id> dies' '
>> +	test_must_fail git -c fsck.whatEver=ignore fsck 2>err &&
>> +	test_i18ngrep "Unhandled message id: whatever" err
>> +'
>>
>>  test_expect_success 'push with receive.fsck.missingEmail=warn' '
>>  	commit="$(git hash-object -t commit -w --stdin <bogus-commit)" &&
>> @@ -211,10 +215,15 @@ test_expect_success 'push with receive.fsck.missingEmail=warn' '
>>  	git --git-dir=dst/.git config fsck.missingEmail warn &&
>>  	test_must_fail git push --porcelain dst bogus &&
>>
>> +	# receive.fsck.<unknownmsg-id> warns
>> +	git --git-dir=dst/.git config \
>> +		receive.fsck.whatEver error &&
>> +
>>  	git --git-dir=dst/.git config \
>>  		receive.fsck.missingEmail warn &&
>>  	git push --porcelain dst bogus >act 2>&1 &&
>>  	grep "missingEmail" act &&
>> +	test_i18ngrep "Skipping unknown msg id.*whatever" act &&
>>  	git --git-dir=dst/.git branch -D bogus &&
>>  	git --git-dir=dst/.git config --add \
>>  		receive.fsck.missingEmail ignore &&
>> @@ -235,10 +244,15 @@ test_expect_success 'fetch with fetch.fsck.missingEmail=warn' '
>>  	git --git-dir=dst/.git config fsck.missingEmail warn &&
>>  	test_must_fail git --git-dir=dst/.git fetch "file://$(pwd)" $refspec &&
>>
>> +	# receive.fsck.<unknownmsg-id> warns
>> +	git --git-dir=dst/.git config \
>> +		fetch.fsck.whatEver error &&
>> +
>>  	git --git-dir=dst/.git config \
>>  		fetch.fsck.missingEmail warn &&
>>  	git --git-dir=dst/.git fetch "file://$(pwd)" $refspec >act 2>&1 &&
>>  	grep "missingEmail" act &&
>> +	test_i18ngrep "Skipping unknown msg id.*whatever" act &&
>>  	rm -rf dst &&
>>  	git init dst &&
>>  	git --git-dir=dst/.git config fetch.fsckobjects true &&