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

["brian m. carlson" <sandals@crustytoothpaste.net>]

[-- Attachment #1: Type: text/plain, Size: 2785 bytes --]

On 2021-12-28 at 22:34:03, Elijah Newren wrote:
> 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 <aclopte@gmail.com> wrote:
> >
> > On Sat, Dec 25, 2021 at 07:59:12AM +0000, Elijah Newren via GitGitGadget wrote:
> > > +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
> >
> > > +      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.
-- 
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]