Re: [PATCH 3/8] lock_ref_sha1_basic(): do not set force_write for missing references

From: Stefan Beller <sbeller@google.com>
Date: Feb 10, 2015, 23:24 UTC
Message-ID: <CAGZ79kbrFpgF6_dLYdgT2D0JjWggu8edjV2sgXER5btpmyjDNw@mail.gmail.com>
In-Reply-To: <1423473164-6011-4-git-send-email-mhagger@alum.mit.edu>

On Mon, Feb 9, 2015 at 1:12 AM, Michael Haggerty <mhagger@alum.mit.edu> wrote:

> If a reference is missing, its SHA-1 will be null_sha1, which can't
> possibly match a new value that ref_transaction_commit() is trying to
> update it to. So there is no need to set force_write in this scenario.
>

This commit reverts half the lines of 5bdd8d4a3062a (2008-11, do not force write of packed refs). And reading both commit messages, they seem to contradict each other. (Both agree on "If a reference is missing, its SHA-1 will be null_sha1 as provided by resolve_ref", but the conclusion seems to be different.)

On the other hand, there is more than 6 years difference, so I guess the meaning and implications of some variables and functions may have slightly changed.

Show 49 quoted lines

> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
> ---
>  refs.c | 15 ++++++---------
>  1 file changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index 651e37e..b083858 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -2259,7 +2259,6 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
>         int type, lflags;
>         int mustexist = (old_sha1 && !is_null_sha1(old_sha1));
>         int resolve_flags = 0;
> -       int missing = 0;
>         int attempts_remaining = 3;
>
>         lock = xcalloc(1, sizeof(struct ref_lock));
> @@ -2298,13 +2297,13 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
>                         orig_refname, strerror(errno));
>                 goto error_return;
>         }
> -       missing = is_null_sha1(lock->old_sha1);
> -       /* When the ref did not exist and we are creating it,
> -        * make sure there is no existing ref that is packed
> -        * whose name begins with our refname, nor a ref whose
> -        * name is a proper prefix of our refname.
> +       /*
> +        * When the ref did not exist and we are creating it, make
> +        * sure there is no existing packed ref whose name begins with
> +        * our refname, nor a packed ref whose name is a proper prefix
> +        * of our refname.
>          */
> -       if (missing &&
> +       if (is_null_sha1(lock->old_sha1) &&
>              !is_refname_available(refname, skip, get_packed_refs(&ref_cache))) {
>                 last_errno = ENOTDIR;
>                 goto error_return;
> @@ -2320,8 +2319,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 (missing)
> -               lock->force_write = 1;
>         if ((flags & REF_NODEREF) && (type & REF_ISSYMREF))
>                 lock->force_write = 1;
>
> --
> 2.1.4
>