All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Brad King <brad.king@kitware.com>
Cc: Martin Fick <mfick@codeaurora.org>, git@vger.kernel.org
Subject: Re: [PATCH/RFC 0/7] Multiple simultaneously locked ref updates
Date: Thu, 29 Aug 2013 11:07:38 -0700	[thread overview]
Message-ID: <xmqqd2ow5q1h.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <521F8033.6070302@kitware.com> (Brad King's message of "Thu, 29 Aug 2013 13:09:07 -0400")

Brad King <brad.king@kitware.com> writes:

> On 08/29/2013 12:21 PM, Junio C Hamano wrote:
>> Brad King <brad.king@kitware.com> writes:
>>> needs to reject duplicate ref names from the stdin lines before
>>> trying to lock the ref twice to avoid this message.
>> 
>> How about trying not to feed duplicates?
>
> Sure, perhaps it is simplest to push the responsibility on the user
> to avoid duplicates.

I didn't mean to force the caller of new "update-ref --stdin"; the
new code you wrote for it is what feeds the input to update_refs()
function, and that is one place you can make sure you do not lock
yourself out.

Besides, if you get two updates to the same ref from --stdin, you
would need to verify these are identical (i.e. updating to the same
new object name from the same old object name; otherwise the requests
are conflicting and do not make sense), so the code to collect the
requests from stdin needs to match potential duplicates anyway.

One clean way to do so may be to put an update request (old and new
sha1) in a structure, and use a string_list to hold list of refs,
with the util field pointing at the update request data.

> The lock file may exist because:
> - another running git process already has the lock, or
> - this process already has the lock because it was asked to
>   update the same file multiple times simultaneously, or
> - a stale lock is left from a git process that crashed earlier.
> In the last case, make sure no other git process is running and
> remove the file manually to continue.

The second case is like left hand not knowing what right hand is
doing, no?  It should not happen if we code it right.

> --------------------------------------------------------------------
>
> IIUC the message cannot say anything about a 'ref' because it is
> used for other file type lock failures too.
>
> Comments?
> -Brad

  reply	other threads:[~2013-08-29 18:07 UTC|newest]

Thread overview: 106+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-29 14:11 [PATCH/RFC 0/7] Multiple simultaneously locked ref updates Brad King
2013-08-29 14:11 ` [PATCH/RFC 1/7] reset: rename update_refs to reset_refs Brad King
2013-08-29 17:17   ` Junio C Hamano
2013-08-29 18:07     ` Brad King
2013-08-29 14:11 ` [PATCH/RFC 2/7] refs: report ref type from lock_any_ref_for_update Brad King
2013-08-29 17:22   ` Junio C Hamano
2013-08-29 18:08     ` Brad King
2013-08-29 14:11 ` [PATCH/RFC 3/7] refs: factor update_ref steps into helpers Brad King
2013-08-29 14:11 ` [PATCH/RFC 4/7] refs: factor delete_ref loose ref step into a helper Brad King
2013-08-29 17:28   ` Junio C Hamano
2013-08-29 18:08     ` Brad King
2013-08-29 14:11 ` [PATCH/RFC 5/7] refs: add function to repack without multiple refs Brad King
2013-08-29 17:34   ` Junio C Hamano
2013-08-29 18:09     ` Brad King
2013-08-29 14:11 ` [PATCH/RFC 6/7] refs: add update_refs for multiple simultaneous updates Brad King
2013-08-29 17:39   ` Junio C Hamano
2013-08-29 18:20     ` Brad King
2013-08-29 18:32       ` Junio C Hamano
2013-08-29 18:38         ` Brad King
2013-08-29 19:30       ` Brad King
2013-08-29 14:11 ` [PATCH/RFC 7/7] update-ref: support " Brad King
2013-08-29 18:34   ` Junio C Hamano
2013-08-29 18:42     ` Brad King
2013-08-29 15:32 ` [PATCH/RFC 0/7] Multiple simultaneously locked ref updates Martin Fick
2013-08-29 15:46   ` Brad King
2013-08-29 16:21     ` Junio C Hamano
2013-08-29 17:09       ` Brad King
2013-08-29 18:07         ` Junio C Hamano [this message]
2013-08-29 18:23           ` Brad King
2013-08-30 18:11 ` [PATCH v2 0/8] " Brad King
2013-08-30 18:11   ` [PATCH v2 1/8] reset: rename update_refs to reset_refs Brad King
2013-08-30 18:12   ` [PATCH v2 2/8] refs: report ref type from lock_any_ref_for_update Brad King
2013-08-30 18:12   ` [PATCH v2 3/8] refs: factor update_ref steps into helpers Brad King
2013-09-01  6:08     ` Junio C Hamano
2013-09-02 17:19       ` Brad King
2013-08-30 18:12   ` [PATCH v2 4/8] refs: factor delete_ref loose ref step into a helper Brad King
2013-08-31 16:30     ` Michael Haggerty
2013-09-02 17:19       ` Brad King
2013-08-30 18:12   ` [PATCH v2 5/8] refs: add function to repack without multiple refs Brad King
2013-08-30 18:12   ` [PATCH v2 6/8] refs: add update_refs for multiple simultaneous updates Brad King
2013-08-31 18:19     ` Michael Haggerty
2013-09-02 17:20       ` Brad King
2013-09-01  6:08     ` Junio C Hamano
2013-09-02 17:20       ` Brad King
2013-09-03  4:43         ` Michael Haggerty
2013-09-03 11:59           ` Brad King
2013-08-30 18:12   ` [PATCH v2 7/8] update-ref: support " Brad King
2013-08-30 22:51     ` Junio C Hamano
2013-09-02 17:23       ` Brad King
2013-08-31 18:42     ` Michael Haggerty
2013-09-02 17:21       ` Brad King
2013-08-30 18:12   ` [PATCH v2 8/8] update-ref: add test cases covering --stdin signature Brad King
2013-09-01  3:41     ` Eric Sunshine
2013-09-02 17:23       ` Brad King
2013-08-31 19:02   ` [PATCH v2 0/8] Multiple simultaneously locked ref updates Michael Haggerty
2013-09-02 17:48   ` [PATCH v3 " Brad King
2013-09-02 17:48     ` [PATCH v3 1/8] reset: rename update_refs to reset_refs Brad King
2013-09-02 17:48     ` [PATCH v3 2/8] refs: report ref type from lock_any_ref_for_update Brad King
2013-09-02 17:48     ` [PATCH v3 3/8] refs: factor update_ref steps into helpers Brad King
2013-09-02 17:48     ` [PATCH v3 4/8] refs: factor delete_ref loose ref step into a helper Brad King
2013-09-02 17:48     ` [PATCH v3 5/8] refs: add function to repack without multiple refs Brad King
2013-09-02 17:48     ` [PATCH v3 6/8] refs: add update_refs for multiple simultaneous updates Brad King
2013-09-02 17:48     ` [PATCH v3 7/8] update-ref: support " Brad King
2013-09-02 18:37       ` Brad King
2013-09-02 17:48     ` [PATCH v3 8/8] update-ref: add test cases covering --stdin signature Brad King
2013-09-03  8:16       ` Eric Sunshine
2013-09-03 12:15         ` Brad King
2013-09-04 15:22     ` [PATCH v4 0/8] Multiple simultaneously locked ref updates Brad King
2013-09-04 15:22       ` [PATCH v4 1/8] reset: rename update_refs to reset_refs Brad King
2013-09-04 15:22       ` [PATCH v4 2/8] refs: report ref type from lock_any_ref_for_update Brad King
2013-09-04 15:22       ` [PATCH v4 3/8] refs: factor update_ref steps into helpers Brad King
2013-09-04 15:22       ` [PATCH v4 4/8] refs: factor delete_ref loose ref step into a helper Brad King
2013-09-04 15:22       ` [PATCH v4 5/8] refs: add function to repack without multiple refs Brad King
2013-09-04 15:22       ` [PATCH v4 6/8] refs: add update_refs for multiple simultaneous updates Brad King
2013-09-04 15:22       ` [PATCH v4 7/8] update-ref: support " Brad King
2013-09-04 18:23         ` Junio C Hamano
2013-09-04 19:59           ` Brad King
2013-09-04 21:27             ` Junio C Hamano
2013-09-05 20:32               ` Brad King
2013-09-05 21:23                 ` Junio C Hamano
2013-09-05 23:44                   ` Brad King
2013-09-04 19:17         ` Junio C Hamano
2013-09-04 19:16           ` Brad King
2013-09-04 15:22       ` [PATCH v4 8/8] update-ref: add test cases covering --stdin signature Brad King
2013-09-09 13:22       ` [PATCH v5 0/8] Multiple simultaneously locked ref updates Brad King
2013-09-09 13:22         ` [PATCH v5 7/8] update-ref: support multiple simultaneous updates Brad King
2013-09-09 13:22         ` [PATCH v5 8/8] update-ref: add test cases covering --stdin signature Brad King
2013-09-10  0:57         ` [PATCH v6 0/8] Multiple simultaneously locked ref updates Brad King
2013-09-10  0:57           ` [PATCH v6 1/8] reset: rename update_refs to reset_refs Brad King
2013-09-10  3:43             ` Ramkumar Ramachandra
2013-09-10  0:57           ` [PATCH v6 2/8] refs: report ref type from lock_any_ref_for_update Brad King
2013-09-10  0:57           ` [PATCH v6 3/8] refs: factor update_ref steps into helpers Brad King
2013-09-10  0:57           ` [PATCH v6 4/8] refs: factor delete_ref loose ref step into a helper Brad King
2013-09-10  0:57           ` [PATCH v6 5/8] refs: add function to repack without multiple refs Brad King
2013-09-10  0:57           ` [PATCH v6 6/8] refs: add update_refs for multiple simultaneous updates Brad King
2013-09-10  0:57           ` [PATCH v6 7/8] update-ref: support " Brad King
2013-09-10 22:51             ` Eric Sunshine
2013-09-11 12:36               ` Brad King
2013-09-11 16:07                 ` Eric Sunshine
2013-09-10  0:57           ` [PATCH v6 8/8] update-ref: add test cases covering --stdin signature Brad King
2013-09-10 22:46             ` Eric Sunshine
2013-09-10 22:54               ` Junio C Hamano
2013-09-11 12:46               ` [PATCH v6.1 " Brad King
2013-09-10 16:30           ` [PATCH v6 0/8] Multiple simultaneously locked ref updates Junio C Hamano
2013-09-10 16:54             ` Brad King
2013-09-10 20:18               ` Junio C Hamano

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=xmqqd2ow5q1h.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=brad.king@kitware.com \
    --cc=git@vger.kernel.org \
    --cc=mfick@codeaurora.org \
    /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.