On Fri, Sep 11, 2020 at 11:38:15PM +0000, brian m. carlson wrote: > Neither of those are appealing, so let's tell the repository > initialization code if we're doing a reinit like this, and if so, to > clear the extension if we're using SHA-1. This makes sure we produce a > valid and functional repository and doesn't break any of our other use > cases. Your explanation makes sense, and what you're proposing here seems like a very reasonable path forward. I can't think of anything that would cause it not to work as expected. > -void initialize_repository_version(int hash_algo); > +void initialize_repository_version(int hash_algo, int reinit); I'm not a huge fan of adding a 'reinit' parameter to a function that itself begins with the word 'initialize' (why wouldn't you call 'reinitialize_repository_version()' instead?), but seeing as there are only a couple of callers, maybe it is OK. Alternatively, I certainly wouldn't complain if you did introduce a new function and updated the call-site that passes reinit as 1. > +test_expect_success 'clone with GIT_DEFAULT_HASH' ' > + ( > + sane_unset GIT_DEFAULT_HASH && > + git init test > + ) && > + test_commit -C test foo && > + git clone test test-clone && > + git -C test-clone status > +' > + This test looks very reasonable, and certainly demonstrates the bug and fix. Thanks. With or without my nitpick: Reviewed-by: Taylor Blau <me@xxxxxxxxxxxx> Thanks, Taylor
