On Tue, Jun 11, 2024 at 10:54:23AM -0700, Junio C Hamano wrote: > Patrick Steinhardt <ps@xxxxxx> writes: > > > Fix both of these issues by not making it an error anymore when the > > given length exceeds the hash length. Instead, if we have a repository, > > then we truncate the length to the maximum length of `the_hash_algo`. > > Otherwise, we simply leave the abbreviated length intact and store it > > as-is. This is equivalent to the logic in `parse_opt_abbrev_cb()` and is > > handled just fine by `repo_find_unique_abbrev_r()`. In practice, we > > should never even end up using `default_abbrev` without a repository > > anyway given that abbreviating object IDs to unique prefixes requires us > > to have access to an object database. > > Makes sense. > > > diff --git a/config.c b/config.c > > index abce05b774..ab2844d9e1 100644 > > --- a/config.c > > +++ b/config.c > > @@ -1460,11 +1460,14 @@ static int git_default_core_config(const char *var, const char *value, > > if (!strcasecmp(value, "auto")) > > default_abbrev = -1; > > else if (!git_parse_maybe_bool_text(value)) > > - default_abbrev = the_hash_algo->hexsz; > > + default_abbrev = startup_info->have_repository ? > > + the_hash_algo->hexsz : GIT_MAX_HEXSZ; > > We will need to have some code that further adjusts overly long > default_abbrev when we really have to abbreviate (at which time, > hopefully we are already aware of the real hash algorithm used in > the repository, and that may be SHA-1) anyway. > > So do we even need the conditional here? Can't we just set it to > GIT_MAX_HEXSZ here unconditionally? Not really. I was erring on the safe side here to retain the status quo for all relevant cases. As explained in the commit message, the length is only relevant when we have a repository because we otherwise wouldn't be able to abbreviate anything anyway. So essentially, this change is a functional no-op. There's also the question of the second commit, where we only handle `abbrev == the_hash_algo->size`, but not `abbrev > the_hash_algo->size`. It works, but is slower when not truncating the length until the second patch fixes it. So yes, we can set this unconditionally to `GIT_MAX_HEXSZ`, but out of abundance of caution I decided to make this conditional. Patrick
Attachment:
signature.asc
Description: PGP signature
