Re: [PATCH 03/30] object-names: Support input of oids in any supported hash
- From
- Eric W. Biederman <ebiederm@gmail.com>
- Date
- Oct 2, 2023, 01:54 UTC
- Message-ID
- <87il7paiwa.fsf@gmail.froward.int.ebiederm.org>
- In-Reply-To
- <CAPig+cRHBxPZYQ5XYA5Un7LeS21NgqxZGg=Q8D+aQckrw9Ymtg@mail.gmail.com>
Eric Sunshine <sunshine@sunshineco.com> writes:
Show 21 quoted lines
> On Wed, Sep 27, 2023 at 3:56 PM Eric W. Biederman <ebiederm@gmail.com> 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!
Show 15 quoted lines
>> 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@xmission.com>
>> ---
>> 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.
Show 13 quoted lines
>> @@ -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 enoughFair enough.
Show 9 quoted lines
> >> @@ -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