git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Brad King <brad.king@kitware.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, mhagger@alum.mit.edu
Subject: Re: [PATCH v4 7/8] update-ref: support multiple simultaneous updates
Date: Thu, 05 Sep 2013 19:44:26 -0400	[thread overview]
Message-ID: <5229175A.4070203@kitware.com> (raw)
In-Reply-To: <xmqqk3ivgdxz.fsf@gitster.dls.corp.google.com>

On 9/5/2013 5:23 PM, Junio C Hamano wrote:
> Brad King <brad.king@kitware.com> writes:
>> 	create SP <ref> NUL <newvalue> NUL
>> 	update SP <ref> NUL <newvalue> NUL [<oldvalue>] NUL
> 
> That SP in '-z' format looks strange.  Was there a reason why NUL
> was inappropriate?

The precedent I saw in the -z survey I posted is that NUL is used
to terminate fields that can contain arbitrary text but otherwise
SP or TAB is still used to terminate simple fields.  I recall no
cases that use NUL after fields that only contain simple values.

>> option::
>> 	Specify an option to take effect for following commands.
> 
> This last one is somewhat peculiar, especially because it says
> "following command*s*".
> 
> How would I request to update refs HEAD and next in an all-or-none
> fashion, while applying 'no-deref' only to HEAD but not next?

You're right.  It will be simpler for clients to generate each
sequence of options followed by a <ref> command without worrying
about options possibly generated for preceding <ref>s.  So:

option::
	Modify behavior of the next command naming a <ref>.
	The only valid option is `no-deref` to avoid dereferencing
	a symbolic ref.

> When I said "create or update" in the message you are responding to,
> I did not mean that we should have two separate commands.

The nice thing about "create" is that it implies oldvalue=zero, just
as "delete" implies newvalue=zero.  The symmetry feels clean.  Also,
in -z mode we need to treat an empty string <oldvalue> as missing
rather than zero.  The only way to specify create with the "update"
command is with literal 40 "0" as <oldvalue>.

> The regular command line does create-or-update; if it exists already,
> it is an update, and if it does not, it is a create.

The proposed update command can be used for create, update, delete,
or verify with proper arguments and use of 40 "0".  The other <ref>
commands are for convenience, shorter input, and statement of intent.

> 	create-or-update-no-deref <ref> <newvalue> [<oldvalue>]
>         delete-no-deref <ref> [<oldvalue>]
> 
> Also how would one set the reflog message for the proposed update?

This is why I proposed a separate option command.  It can be used
both for no-deref and for other options we want to add later e.g.

  option SP message SP <reason> LF

and with -z:

  option SP message SP <reason> NUL

For now I think it is sufficient for the -m <reason> command-line
option to set the same message for all updates.  The option command
leaves room to add a per-ref message later.

BTW, the reasons I propose message as an option rather than its own
command are:

* It is simpler to document the reach of the one option command
  (affects next <ref>) in one place rather than separately for
  each option-like command.

* It reduces the number of commands that do not take a <ref>.

-Brad

  reply	other threads:[~2013-09-05 23:44 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
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 [this message]
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=5229175A.4070203@kitware.com \
    --to=brad.king@kitware.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=mhagger@alum.mit.edu \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).