Re: [PATCH v2 3/4] Integrate hash algorithm support with repo setup

[Junio C Hamano <gitster@pobox.com>]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> On Sun, Oct 29, 2017 at 03:02:20PM -0400, Eric Sunshine wrote:
>> On Sun, Oct 29, 2017 at 1:57 PM, brian m. carlson
>> <sandals@crustytoothpaste.net> wrote:
>> > I can do CURRENT_HASH_ALGO or CURRENT_HASH instead if you think that's
>> > an improvement.  I originally omitted the "algo" portion to keep it
>> > short.
>> 
>> I don't have strong feelings about it aside from worrying about a
>> "current_hash" name clash or a reader misunderstanding what it
>> represents.
>> 
>> Does "current" need to be in the name? What about HASH_ALGO or REPO_HASH_ALGO?
>> 
>> > Alternatively, we could have a current_hash() (or current_hash_algo())
>> > inline function if people like that better.
>> 
>> hash_algo() or repo_hash_algo()?
>
> Those are also fine, and shorter to boot.  I'll wait to see if anyone
> has strong opinions on the direction we should go before making a
> change.

Is the plan to allow running with multiple hash algorithms in
parallel?  I thought what we want to see in the future codebase was
to have the default hash algorithm used for everything except for a
select few codepaths, and assumed that the way we achieve it is to

 - allow very low level helper functions (e.g. read_sha1_file(),
   write_sha1_file_prepare(), etc.) to take a pointer to the hash
   algorithm structure;

 - have higher level helper functions to call these low level
   helpers with a fixed singleton instance of hash algorithm
   structure that represents that default one (SHA2-something?); and

 - a few selected codepaths (e.g. index-pack that reads SHA-1 stream
   and converts it into NewHash pack/index while building the object
   name mapping) use an additional singleton instance of hash
   algorithm structure that represents the SHA-1 hash, and the way
   they use it is *NOT* by replacing "the current" one with SHA-1,
   but by explicitly passing the instance to the low level helpers
   as parameter.

So, "current" does indeed sound quite wrong, as it makes it sound as
if you can swap it anytime and ask "which one is in effect now?".
If we do not want to call the default instance "SHA-2" because we
want to prepare for migrating again by not having the name of a
concrete hash algorithm sprinkled in the codebase all over like we
currently do, why not call the instance "the_hash_algo"?