Re: [PATCH 4/4] refs.c: sort the refs by new_sha1 and merge the two update/delete loops into one

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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)?


> +}
> +
>  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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]