On 2024-04-19 at 09:51:03, Patrick Steinhardt wrote: > Hi, > > when starting up, Git will initialize `the_repository` by calling > `initialize_the_repository()`. Part of this is also that we set the hash > algo of `the_repository` to SHA1, which implicitly sets `the_hash_algo` > because it is a macro expanding to `the_repository->hash_algo`. > > Usually, we eventually set up the correct hash algorithm here once we > have properly set up `the_repository` via `setup_git_directory()`. But > in some commands we actually don't require a Git repository, and thus we > will leave the SHA1 hash algorithm in place. > > This has led to some subtle bugs when the context really asks for a > SHA256 repository, which this patch series corrects for most of the > part. Some commands need further work, like for example git-diff(1), > where the user might want to have the ability to pick a hash function > when run outside of a repository. > > Ultimately, the last patch then drops the setup of the fallback hash > algorithm completely. This will cause `the_hash_algo` to be a `NULL` > pointer unless explicitly configured, and thus we now start to crash > when it gets accessed without doing so beforehand. This is a rather > risky thing to do, but it does catch bugs where we might otherwise > accidentally do the wrong thing. And even though I think it is the right > thing to do even conceptually, I'd be okay to drop it if folks think > that the risk is not worth it. I've taken a look, and other than the minor typo I noted, this seems fine. I'm in favour of getting rid of the SHA-1 default, even though my gut tells me we might find a bug or two along the way where things aren't initialized properly. I still think that'll be okay and it's worth doing, since it'll help us prepare for the case in the future where we want to switch the default and also for libification, where we won't want to make those kinds of assumptions. -- brian m. carlson (they/them or he/him) Toronto, Ontario, CA
Attachment:
signature.asc
Description: PGP signature
