Re: ab/fsck-unexpected-type (and "cat-file replace handling and optimization")

[Jeff King <peff@peff.net>]

On Thu, Oct 07, 2021 at 02:37:02PM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > I don't mind rebasing on top if that's easier for Junio, but in that
> > case it may make sense to float the test cleanup to the front of the
> > series.
> 
> Yeah, I would rather not to see such a rebase (and I hate the kind
> of churns we seem to be allowing more and more in our codebase that
> causes needless textual conflicts.  We should really slow down and
> make fewer pointless "clean-up" that disrupts real work).

In this case I think neither is churn. I needed to clean up the earlier
part of the test script so that my later tests didn't break, and Ævar
touched the same area to cover more cases in those tests.

> > - test_expect_success 'cat-file -t and -s on corrupt loose object' '
> > - 	git init --bare corrupt-loose.git &&
> > - 	(
> > - 		cd corrupt-loose.git &&
> > ...
> > - 		test_must_fail git fsck 2>err.fsck &&
> > - 		grep "^error: inflate: data stream error (" err.fsck
> > - 	)
> 
> ... what's this lossage of the whole new test?

Whoops, indeed. I was little too anxious in my editor and then read the
combined diff backwards. That test should obviously stay (it is new in
Ævar's series). It still appears in total in the combined diff, but of
course as a "+" on his side.

One other possible thing to do during the resolution is combine the
cleanup of both objects in a single test (since in Ævar's series, both
objects are created together).

I.e., throw away my side from both conflicts, and do this on top:

diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh
index 0f52ca9cc8..508a70d877 100755
--- a/t/t1006-cat-file.sh
+++ b/t/t1006-cat-file.sh
@@ -487,6 +487,11 @@ test_expect_success "Size of large broken object is correct when type is large"
 	test_cmp expect actual
 '
 
+test_expect_success 'clean up broken objects' '
+	rm .git/objects/$(test_oid_to_path $bogus_short_sha1) \
+	   .git/objects/$(test_oid_to_path $bogus_long_sha1)
+'
+
 test_expect_success 'cat-file -t and -s on corrupt loose object' '
 	git init --bare corrupt-loose.git &&
 	(

That makes the merge more evil, but the end result is perhaps a little
nicer for people reading it later.

Thanks for reading my proposed resolution so carefully. :)

-Peff