Re: [PATCH 01/28] t/test-lib: add an SHA1 prerequisite

From: Martin Ågren <martin.agren@gmail.com>
Date: May 8, 2018, 18:26 UTC
Message-ID: <CAN0heSpzBtSBE78zmBx_5ukb4DpHEO==kGgbYAczE+UyBf3HxA@mail.gmail.com>
In-Reply-To: <20180507233000.GI953644@genre.crustytoothpaste.net>

On 8 May 2018 at 01:30, brian m. carlson <sandals@crustytoothpaste.net> wrote:

Show 9 quoted lines

> On Mon, May 07, 2018 at 12:10:39PM +0200, Martin Ågren wrote:
>> On 7 May 2018 at 01:17, brian m. carlson <sandals@crustytoothpaste.net> wrote:
>> > Add an SHA1 prerequisite to annotate both of these types of tests and
>> > disable them when we're using a different hash.  In the future, we can
>> > create versions of these tests which handle both SHA-1 and NewHash.
>>
>> Minor nit: s/can/can and should/
>
> I agree with that sentiment.  I can change that.

To be clear, this was an "if you have independent reasons for rerolling this, then maybe consider possibly doing this".

Show 13 quoted lines

>> So SHA1 means roughly "git hash-object uses SHA-1, so supposedly
>> everything on disk is SHA-1." I could imagine one or two different
>> meanings: "Git was compiled with support for SHA-1 [oids]."
>
> Currently it means that, yes.  It may specialize to mean, "git emits
> SHA-1 output, regardless of the format on disk."  See cases 1 and 2
> below.
>
>> Do we actually need more SHA-1-related prereqs, at least long-term, in
>> which case we would want to find a more specific name for this one now?
>> Is this SHA1_STORAGE, or some much better name than that?
>
> We may.  The transition plan anticipates several states:

"We may" as in, "we may need more SHA1-FOO prereqs later", or "we may want this to be SHA1-BAR"?

Show 9 quoted lines

> 1. Store data in NewHash, but input and output are SHA-1.
> 2. Store data in NewHash; output is SHA-1; input can be either.
> 3. Store data and output in NewHash; input can be either.
> 4. All NewHash.
>
> At this point, I'm working on getting the tests to handle case 4, as
> that's actually the easiest to fix (because if things are wrong, the
> code tends to segfault).  Case 1 will be next, in which case, we may
> need SHA1_STORAGE or such.

Show 7 quoted lines

> I plan to make the SHA1 prerequisite go away or at least be far less
> used in a few releases.  Once we know what NewHash is going to be, it's
> pretty easy to write tooling and translation tables that do the
> conversion for most tests, and a test helper can simply emit the right
> output for whichever algorithm we're using in that situation, whether
> that's the on-disk algorithm, the input algorithm, or the output
> algorithm.

I do not feel entirely relaxed about a reasoning such as "this prereq will soon go away again, so we do not need to think too much about its name and meaning" (heavily paraphrased and possibly a bit pointed, but hopefully not too dishonest).

I guess a counter-argument might be "sure, if only we knew which SHA1-FOOs we will need. Only time and experience will tell." You've certainly spent way more brain-cycles on this than I have, and most likely more than anyone else on this list.

Maybe we want to document the transition-ness of this in the code and/or the commit message. Not only "transition" in the sense of the big transition, but in the sense of "this will probably go away long before the transition is completed."

Show 10 quoted lines

>> I am thinking for example about a repo with NewHash that gets pushed to
>> and fetched from a SHA-1 server, see hash-function-transition.txt, goal
>> 1b. We'd want to always test that SHA-1-related functionality in git.
>> (But only until the day when someone defines a prereq such as "SHA1" to
>> be able to test a git that was compiled without any traces of SHA-1
>> whatsoever.)
>
> I anticipate that by the time we get to that point, the SHA1
> prerequisite will be long gone and can be reused for that purpose,
> should we need it.