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: Wed, 04 Sep 2013 15:59:03 -0400 [thread overview]
Message-ID: <52279107.2070205@kitware.com> (raw)
In-Reply-To: <xmqqsixkmonq.fsf@gitster.dls.corp.google.com>
On 09/04/2013 02:23 PM, Junio C Hamano wrote:
> "whitespace-separated" implies that we may allow fields separated with not a single SP, but with double SPs or even HTs between them. I personally do not think we should be so loose
Okay, I will look at making it more strict. See proposed format
below.
>> +Quote arguments containing whitespace as if in C source code.
>
> Probably "as if they were strings in C source code".
Fixed.
>> +terminate each argument by NUL and each line by LF NUL.
>
> This is a somewhat interesting choice of the record terminator. Do we have a precedent to use LF NUL elsewhere? If this is the first such case where we need to express variable number of NUL-separated fields in a record, I think I am fine with LF NUL (but I am sure other people would give us a
> better convention if we ask them politely ;-)), but I just want to make sure we do it the same way as other codepaths, if exist, that have to handle this kind of thing.
Nothing else uses LF NUL. I chose it as a starting point for
this very discussion, which I asked about in $gmane/233653.
In this particular use case we know the last field will never
be LF but that may not be so for future cases. There is no way
to represent sentinel-terminated arbitrary variable-width records
of NUL-terminated fields without some kind of escaping for the
sentinel value, but the whole point of -z is to avoid escaping.
Below is a survey of all mentions of NUL and \0 in documented
formats as of v1.8.4. The summary is that most are fixed-width
records but a few have variable width allowing n or n+1 fields.
In all variable-width cases there is structured information in
the first field that indicates the number of NUL-terminated
fields to expect.
In the motivating case here, we could use a --no-old or
--have-old option to indicate in one field how many more to
expect in the record, but that will be quite verbose.
Side note: I'd like to reserve room for the leading options to
include things like "-m NUL <reason> NUL" so we cannot keep them
all in in a single NUL-terminated, SP-separated field.
Another approach is to introduce a way to represent "not here"
for the <oldvalue> argument that is not an otherwise valid value.
This would make the non-option part of the record have a fixed
width of 3 fields. For example, we could use SP in -z mode:
[-<opt> NUL]... <ref> NUL <new> NUL (<old>|SP) NUL
and the last field can be optional in non-z mode anyway:
[-<opt> SP]... <ref> SP <new> [SP <old>] LF
Or we could use a character like "~" (other ideas?):
[-<opt> NUL]... <ref> NUL <new> NUL (<old>|~) NUL
and make it available in non-z mode too:
[-<opt> SP]... <ref> SP <new> [SP (<old>|~)] LF
Thoughts?
-Brad
Survey of NUL in documented formats:
------------------------------------------------------------------------
* Documentation/diff-format.txt: The -z mode for --numstat prints
NUL-terminated lines but there is exactly one path at the end of
each entry and the earlier fields are separated by TAB because
they are structured.
* Documentation/diff-options.txt: The -z mode for diff-tree output
prints structured SP/TAB-separated fields in a NUL-terminated
field followed by either one or two NUL-terminated paths. This
is variable width but the first field tells us how wide.
* Documentation/git-apply.txt: The -z mode forwards to --numstat
diff options.
* Documentation/git-check-attr.txt: The -z mode for stdin reads
NUL-terminated paths.
* Documentation/git-check-ignore.txt: The -z mode for stdin reads
NUL-terminated paths. The -z mode for output prints a fixed-width
table with every group of four NUL-terminated fields forming a row.
* Documentation/git-checkout-index.txt: The -z mode reads
NUL-terminated paths.
* Documentation/git-commit.txt: The -z mode forwards to git-status.
* Documentation/git-grep.txt: The -z mode separates file names from
the matched line by a NUL. Therefore NUL divides LF-terminated
lines into two pieces.
* Documentation/git-ls-files.txt: The -z mode prints NUL-terminated
lines but there is exactly one path at the end of each entry and the
earlier fields are separated by SP and TAB because they are
structured.
* Documentation/git-ls-tree.txt: The -z mode prints NUL-terminated
lines but there is exactly one path at the end of each entry and the
earlier fields are separated by SP and TAB because they are
structured.
* Documentation/git-mktree.txt: The -z mode reads NUL-terminated lines
as output by ls-tree -z.
* Documentation/git-status.txt: The -z mode of --porcelain separates
a variable number of entries by NUL. The beginning of each entry
allows one to know the number of NUL-terminated fields to expect
(A = 1 total NUL, R = 2 total NULs, etc.).
* Documentation/git-update-index.txt: The -z mode of --stdin separates
paths by NUL. The -z mode of --index-info separates entries by NUL
but there is exactly one path at the end of each entry and the earlier
fields are separated by SP and TAB because they are structured.
* Documentation/rev-list-options.txt: The --header option prints
commits separated by NUL but they are never empty.
------------------------------------------------------------------------
next prev parent reply other threads:[~2013-09-04 20:01 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 [this message]
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=52279107.2070205@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).