On Thu, Apr 10, 2014 at 12:01 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Ronnie Sahlberg <sahlberg@xxxxxxxxxx> writes:
>
>> We want to make sure that we update all refs before we delete any refs
>> so that there is no point in time where the tips may not be reachable
>> from any ref in the system.
>> We currently acheive this by first looping over all updates and applying
>> them before we run a second loop and perform all the deletes.
>>
>> If we sort the new sha1 for all the refs so that a deleted ref,
>> with sha1 0{40} comes at the end of the array, then we can just run
>> a single loop and still guarantee that all updates happen before
>> any deletes.
>>
>> Signed-off-by: Ronnie Sahlberg <sahlberg@xxxxxxxxxx>
>> ---
>> refs.c | 18 +++++++++++-------
>> 1 file changed, 11 insertions(+), 7 deletions(-)
>>
>> diff --git a/refs.c b/refs.c
>> index 1678e12..453318e 100644
>> --- a/refs.c
>> +++ b/refs.c
>> @@ -3309,6 +3309,15 @@ static int ref_update_compare(const void *r1, const void *r2)
>> return strcmp((*u1)->ref_name, (*u2)->ref_name);
>> }
>>
>> +static int ref_delete_compare(const void *r1, const void *r2)
>> +{
>> + const struct ref_update * const *u1 = r1;
>> + const struct ref_update * const *u2 = r2;
>> +
>> + /* -strcmp so that 0{40} sorts to the end */
>> + return -strcmp((*u1)->new_sha1, (*u2)->new_sha1);
>
> Two glitches:
>
> - Didn't you mean hashcmp()?
>
> - Don't you have an explicit ->delete_ref field that is a more
> direct way, than relying on the convention "updating to 0{40}
> is to delte", to express this?
>
> In other words, wouldn't it be more like
>
> return !(*u1)->delete_ref - !(*u2)->delete_ref;
>
> or something (I may be wrong about the sign, tho---I didn't think
> carefully)?
I don't have access to delete_ref from 'struct ref_update'.
But much worse is that the patch is completely bogus.
Sorting and changing the order of the items in the updates[] array
does not affect the ordering of the items in the locks[] array
so this can not work.
I will delete this bogus patch from any new version of the patch series.
regards
ronnie sahlberg
>
>
>> +}
>> +
>> static int ref_update_reject_duplicates(struct ref_update **updates, int n,
>> enum action_on_err onerr)
>> {
>> @@ -3388,13 +3397,8 @@ int update_refs(const char *action, const struct ref_update **updates_orig,
>> unlink_or_warn(git_path("logs/%s", delnames[i]));
>> clear_loose_ref_cache(&ref_cache);
>>
>> - /* Perform updates first so live commits remain referenced */
>> - for (i = 0; i < n; i++)
>> - if (locks[i] && !locks[i]->delete_ref) {
>> - ret |= commit_ref_lock(locks[i]);
>> - locks[i] = NULL;
>> - }
>> - /* And finally perform all deletes */
>> + /* Sort the array so that we perform all updates before any deletes */
>> + qsort(updates, n, sizeof(*updates), ref_delete_compare);
>> for (i = 0; i < n; i++)
>> if (locks[i]) {
>> ret |= commit_ref_lock(locks[i]);
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html