All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org, Michael Haggerty <mhagger@alum.mit.edu>
Subject: Re: [PATCH v3 2/2] lock_ref_sha1_basic: handle REF_NODEREF with invalid refs
Date: Tue, 12 Jan 2016 11:41:17 -0800	[thread overview]
Message-ID: <xmqqlh7utwki.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20160112135522.GA23255@sigill.intra.peff.net> (Jeff King's message of "Tue, 12 Jan 2016 08:55:22 -0500")

Jeff King <peff@peff.net> writes:

> I also notice that if we are deleting, we _do_ set
> RESOLVE_REF_NO_RECURSE from the very beginning, which means we would
> generally not get a valid lock->old_oid.hash for a symref. But I'm not
> sure what it would mean to delete a symref while asking for its current
> value (it cannot have one!). So I don't think it is a bug.

I started scratching my head after noticing that the NO_RECURSE bit
set in the DELETING codepath before reading the above, and I am
still doing so.

A transaction that attempts to delete an existing symref presumably
wants to make sure that the "old" value it read hasn't changed, but
ensuring the object name (obtained by reading the ref that is
pointed by the symref by dereferencing) are the same is not the
right way to ensure nobody raced with us in the meantime anyway (we
should rather be making sure that the symref is still pointing at
the same ref), so in that sense, in the context of acquiring the
lock, old oid value is meaningless for symrefs.

This patch is a strict improvement as the behaviour for REF_DELETING
case is unchanged by it (an idempotent resolve-ref-unsafe may be
called one more time in some cases), and other cases are better, I
think.

  reply	other threads:[~2016-01-12 19:41 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-11 15:46 [PATCH 0/2] fix corner case with lock_ref_sha1_basic and REF_NODEREF Jeff King
2016-01-11 15:49 ` [PATCH 1/2] checkout,clone: check return value of create_symref Jeff King
2016-01-12  4:09   ` Michael Haggerty
2016-01-12  9:49     ` Jeff King
2016-01-11 15:52 ` [PATCH 2/2] lock_ref_sha1_basic: handle REF_NODEREF with invalid refs Jeff King
2016-01-11 18:28   ` Junio C Hamano
2016-01-12  4:55   ` Michael Haggerty
2016-01-12  9:52     ` Jeff King
2016-01-12 18:11       ` Junio C Hamano
2016-01-12  9:56 ` [PATCH v2 0/2] fix corner case with lock_ref_sha1_basic and REF_NODEREF Jeff King
2016-01-12  9:57   ` [PATCH v2 1/2] checkout,clone: check return value of create_symref Jeff King
2016-01-12  9:58   ` [PATCH v2 2/2] lock_ref_sha1_basic: handle REF_NODEREF with invalid refs Jeff King
2016-01-12 13:26     ` Jeff King
2016-01-12 13:55       ` [PATCH v3 " Jeff King
2016-01-12 19:41         ` Junio C Hamano [this message]
2016-01-12 20:22           ` Jeff King
2016-01-12 20:42             ` Jeff King
2016-01-12 21:43               ` [PATCH v4 0/3] fix corner cases with lock_ref_sha1_basic Jeff King
2016-01-12 21:44                 ` [PATCH 1/3] checkout,clone: check return value of create_symref Jeff King
2016-01-12 21:44                 ` [PATCH 2/3] lock_ref_sha1_basic: always fill old_oid while holding lock Jeff King
2016-01-13  1:25                   ` Eric Sunshine
2016-01-13 11:38                     ` Jeff King
2016-01-12 21:45                 ` [PATCH 3/3] lock_ref_sha1_basic: handle REF_NODEREF with invalid refs Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqlh7utwki.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=mhagger@alum.mit.edu \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.