Re: [PATCH v3 01/10] receive.fsck.<msg-id> tests: remove dead code

Ævar Arnfjörð Bjarmason  [off-list ref] writes:

Remove the setting of a receive.fsck.badDate config variable to
"ignore". This was added in efaba7cc77 ("fsck: optionally ignore
specific fsck issues completely", 2015-06-22) but never did anything,
presumably it was part of some work-in-progress code that never made
it into git.git.

None of these tests will emit the "invalid author/committer line - bad
date" warning. The dates on the commit objects we're setting up are
not invalid.

It is a timestamp somewhere mid February of 2009.  Perhaps the code
is playing defensive against the lack of email address on the
deliberately broken author line, i.e.

    author Bugs Bunny 1234567890 +0000
    committer Bugs Bunny [off-list ref] 1234567890 +0000

in case the parser punted and failed to parse that timestamp
correctly?  IOW, the above _could_ be a commit written by "Bugs
Bunny 1234567890" with missing e-mail and missing timestamp.

So I dunno.  It won't break the test with today's system if we
removed this config, but with an updated parser from the next year,
it may start to break.

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

Ævar Arnfjörð Bjarmason  [off-list ref] writes:

quoted

Remove the setting of a receive.fsck.badDate config variable to
"ignore". This was added in efaba7cc77 ("fsck: optionally ignore
specific fsck issues completely", 2015-06-22) but never did anything,
presumably it was part of some work-in-progress code that never made
it into git.git.

None of these tests will emit the "invalid author/committer line - bad
date" warning. The dates on the commit objects we're setting up are
not invalid.

It is a timestamp somewhere mid February of 2009.  Perhaps the code
is playing defensive against the lack of email address on the
deliberately broken author line, i.e.

    author Bugs Bunny 1234567890 +0000
    committer Bugs Bunny [off-list ref] 1234567890 +0000

This is covered by the "missingEmail" part of the test, but there's
nothing wrong with the timestamp itself.

I doubt Johannes remembers why he did this almost a decade ago, but it
looks to me like he was working on some test where the date was also
bad, and never finished it. There's no point in having that "badDate"
now.

in case the parser punted and failed to parse that timestamp
correctly?  IOW, the above _could_ be a commit written by "Bugs
Bunny 1234567890" with missing e-mail and missing timestamp.

So I dunno.  It won't break the test with today's system if we
removed this config, but with an updated parser from the next year,
it may start to break.

I still think it makes sense to remove this particular thing. Let's add
exhaustive tests for all this fsck.* stuff in another series, but no
point in testing for arbitrary fsck errors that aren't going to be
triggered in unrelated tests.

quoted

Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
---
 t/t5504-fetch-receive-strict.sh | 2 --
 1 file changed, 2 deletions(-)

diff --git a/t/t5504-fetch-receive-strict.sh b/t/t5504-fetch-receive-strict.sh
index 49d3621a92..e1f8768094 100755
--- a/t/t5504-fetch-receive-strict.sh
+++ b/t/t5504-fetch-receive-strict.sh

@@ -149,8 +149,6 @@ test_expect_success 'push with receive.fsck.missingEmail=warn' '
 	git --git-dir=dst/.git branch -D bogus &&
 	git --git-dir=dst/.git config --add \
 		receive.fsck.missingEmail ignore &&
-	git --git-dir=dst/.git config --add \
-		receive.fsck.badDate warn &&
 	git push --porcelain dst bogus >act 2>&1 &&
 	! grep "missingEmail" act
 '