Michael Haggerty <[email protected]> writes:

> Instead, compute the value when it is needed.

> @@ -2318,8 +2317,6 @@ static struct ref_lock *lock_ref_sha1_basic(const char 
> *refname,
>       lock->ref_name = xstrdup(refname);
>       lock->orig_ref_name = xstrdup(orig_refname);
>       ref_file = git_path("%s", refname);
> -     if ((flags & REF_NODEREF) && (type & REF_ISSYMREF))
> -             lock->force_write = 1;
>  
>   retry:
>       switch (safe_create_leading_directories(ref_file)) {
> @@ -3787,8 +3784,13 @@ int ref_transaction_commit(struct ref_transaction 
> *transaction,
>               struct ref_update *update = updates[i];
>  
>               if (!is_null_sha1(update->new_sha1)) {
> -                     if (!update->lock->force_write &&
> -                         !hashcmp(update->lock->old_sha1, update->new_sha1)) 
> {
> +                     if (!((update->type & REF_ISSYMREF)
> +                           && (update->flags & REF_NODEREF))
> +                         && !hashcmp(update->lock->old_sha1, 
> update->new_sha1)) {
> +                             /*
> +                              * The reference already has the desired
> +                              * value, so we don't need to write it.
> +                              */
>                               unlock_ref(update->lock);
>                               update->lock = NULL;
>                       } else if (write_ref_sha1(update->lock, 
> update->new_sha1,

The code before and after the change are equivalent.

It shouldn't be the case, but somehow I find the original slightly
easier to understand.  The before and after says the same thing,
i.e. the code used to be:

 - We say "do the write-out without questioning" when we are
   updating a symbolic ref without dereferencing.

 - Do nothing and unlock if we are not told to "do the write-out
   without questioning" and the update will be a no-op anyway.

while the code after the change says:

 + Do nothing and unlock if we are not handling "update a symbolic
   ref without dereferencing" and the update will be a no-op anyway.

Perhaps the former has the same effect as "avoid a single complex
sentence and use two short sentences instead".

The negation in the condition does not help, either.

 * If we are updating a symbolic ref without dereferencing, or if we
   are updating with a different object name, we definitely have to
   write.

would be easier to understand, perhaps?  I.e.

        if (hashcmp(update->lock->old_sha1, update->lock->new_sha1) ||
            ((update->type & REF_ISSYMREF) && (update->flags & REF_NO_DEREF))) {
                /* do the write-out thing */
        } else {
                /* the request to update from the same to the same is a no-op */
                unlock_ref(update->lock);
                update->lock = NULL;
        }

I dunno.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to