"brian m. carlson" <sandals@xxxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxx> 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"?
