Michael Haggerty <mhagger@xxxxxxxxxxxx> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html