Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes:
> On Wed, Sep 27, 2023 at 3:56 PM Eric W. Biederman <ebiederm@xxxxxxxxx> wrote:
>> Support short oids encoded in any algorithm, while ensuring enough of
>> the oid is specified to disambiguate between all of the oids in the
>> repository encoded in any algorithm.
>>
>> By default have the code continue to only accept oids specified in the
>> storage hash algorithm of the repository, but when something is
>> ambiguous display all of the possible oids from any oid encoding.
>>
>> A new flag is added GET_OID_HASH_ANY that when supplied causes the
>> code to accept oids specified in any hash algorithm, and to return the
>> oids that were resolved.
>>
>> This implements the functionality that allows both SHA-1 and SHA-256
>> object names, from the "Object names on the command line" section of
>> the hash function transition document.
>>
>> Care is taken in get_short_oid so that when the result is ambiguous
>> the output remains the same of GIT_OID_HASH_ANY was not supplied.
>
> s/of/as if/
Actually s/of/if/
Thank you for catching that. When reviewing this to understand what I
was trying to say I found a bug. The function repo_for_each_abbrev was
passing algo to init_object_disambiguation to properly initialize
ds.bin_pfx.algo, and then was resetting ds.bin_pfx.algo to GIT_HASH_ANY.
Oops!
>> If GET_OID_HASH_ANY was supplied objects of any hash algorithm
>> that match the prefix are displayed.
>>
>> This required updating repo_for_each_abbrev to give it a parameter
>> so that it knows to look at all hash algorithms.
>>
>> Signed-off-by: "Eric W. Biederman" <ebiederm@xxxxxxxxxxxx>
>> ---
>> diff --git a/object-name.c b/object-name.c
>> @@ -49,6 +50,7 @@ struct disambiguate_state {
>> static void update_candidates(struct disambiguate_state *ds, const struct object_id *current)
>> {
>> + /* The hash algorithm of the current has already been filtered */
>
> Is there a word missing after "current"?
No. I forgot to remove "the" before current.
>> @@ -503,8 +516,13 @@ static int sort_ambiguous(const void *a, const void *b, void *ctx)
>> - if (a_type == b_type)
>> - return oidcmp(a, b);
>> + if (a_type == b_type) {
>> + /* Is the hash algorithm the same? */
>> + if (a->algo == b->algo)
>> + return oidcmp(a, b);
>> + else
>> + return a->algo > b->algo ? 1 : -1;
>> + }
>
> Nit: unnecessary comment ("Is the hash algorithm...") is merely
> repeating what the code itself already says clearly enough
Fair enough.
>
>> @@ -553,6 +575,7 @@ static enum get_oid_result get_short_oid(struct repository *r,
>> else
>> ds.fn = default_disambiguate_hint;
>>
>> +
>> find_short_object_filename(&ds);
>
> Nit: unnecessary new blank line
Naughty thing how did that sneak in there ;)
Eric
