Re: [PATCH v2 1/8] show, log: provide a --remerge-diff capability

On Tue, Dec 28, 2021 at 3:01 PM brian m. carlson
[off-list ref] wrote:

On 2021-12-28 at 22:34:03, Elijah Newren wrote:

quoted

CC'ing brian in case he has comments on the sha256 stuff and whether
he thinks there's a cleaner way to make my tests work with sha256.
(brian: See the very end of the email.)

On Tue, Dec 28, 2021 at 2:56 AM Johannes Altmanninger [off-list ref] wrote:

quoted

On Sat, Dec 25, 2021 at 07:59:12AM +0000, Elijah Newren via GitGitGadget wrote:

quoted

+test_expect_success 'remerge-diff with both a resolved conflict and an unrelated change' '
+     git log -1 --oneline ab_resolution >tmp &&
+     cat <<-EOF >>tmp &&
+     diff --git a/numbers b/numbers
+     index a1fb731..6875544 100644
+     --- a/numbers
+     +++ b/numbers
+     @@ -1,13 +1,9 @@
+      1
+      2
+     -<<<<<<< b0ed5cb (change_a)
+     -three
+     -=======
+     -tres
+     ->>>>>>> 6cd3f82 (change_b)
+     +drei

nice

quoted

+      4
+      5
+      6
+      7
+     -eight
+     +acht
+      9
+     EOF
+     # Hashes above are sha1; rip them out so test works with sha256
+     sed -e "s/[0-9a-f]\{7,\}/HASH/g" tmp >expect &&

Right, sha256 could cause many noisy test changes. I wonder if there is a
more general way to avoid this; maybe default to SHA1 for existing tests?

Not "could", but "does".  And this is not something to be avoided.
The default testsuite we run in CI involves a run of
GIT_TEST_DEFAULT_HASH=sha256 under linux-clang.  Making these tests
SHA1-only just reduces our coverage and makes the transition to SHA256
harder; I think that's the opposite of the direction we want to go.

These changes I've made here are sufficient to make these tests work
under sha256; you can see the test results here:
https://github.com/gitgitgadget/git/runs/4646949283?check_suite_focus=true.
Under "Run ci/run-build-and-tests.sh" note that there are two runs of
tests, and the second has "export GIT_TEST_DEFAULT_HASH=sha256"
preceding it.

There might be a cleaner way to make these tests sha256-compatible,
but this seemed like a pretty simple way to me.

The question here is, do we care very much about testing these specific
hashes?  If so, then we should use test_oid_cache to set up some OIDs
and make sure they're correct for both SHA-1 and SHA-256, and then
replace them in the code with calls to test_oid.

However, my impression is that we probably don't care very much about
what the specific values are, and in that case, this is completely fine.
We do similar things elsewhere in the testsuite.

Thanks for chiming in.  Your impression is right; I don't care about
the specific hashes, just the general form of the diff content, so
I'll keep it as is.