git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: git <git@vger.kernel.org>
Cc: Christian Couder <christian.couder@gmail.com>,
	Junio C Hamano <gitster@pobox.com>
Subject: [PATCH v3 0/9] Support for transactions in `git-update-ref --stdin`
Date: Thu, 2 Apr 2020 09:09:15 +0200	[thread overview]
Message-ID: <cover.1585811013.git.ps@pks.im> (raw)
In-Reply-To: <cover.1585129842.git.ps@pks.im>

[-- Attachment #1: Type: text/plain, Size: 15162 bytes --]

Hi,

this is the second version of this patch series to introduce transaction
support in `git update-ref --stdin`. The goal is to make commands
available to the user of that command so that he can explicitly control
the given transaction by starting, preparing, and finally committing or
aborting the transaction.

This version improves the argument handling for commands as proposed by
Junio. Instead of the hard-to-understand `extra_lines` field, commands
now have a `args` field that specifies how many arguments a given
command has, including the first one separated by a space. As a result,
the initial space for the first argument is now handled in the generic
part and not moved into each of the command functions anymore. I've also
added a comment that clarifies why we ignore EOF when fetching
additional command lines.

The range-diff against v2 is attached below.

Patrick

Patrick Steinhardt (9):
  refs: fix segfault when aborting empty transaction
  git-update-ref.txt: add missing word
  strbuf: provide function to append whole lines
  update-ref: organize commands in an array
  update-ref: drop unused argument for `parse_refname`
  update-ref: pass end pointer instead of strbuf
  update-ref: move transaction handling into `update_refs_stdin()`
  update-ref: read commands in a line-wise fashion
  update-ref: implement interactive transaction handling

 Documentation/git-update-ref.txt |  28 +++-
 builtin/update-ref.c             | 245 ++++++++++++++++++++++---------
 refs/files-backend.c             |  20 +--
 strbuf.c                         |  10 ++
 strbuf.h                         |   6 +
 t/t1400-update-ref.sh            | 131 +++++++++++++++++
 6 files changed, 364 insertions(+), 76 deletions(-)

Range-diff against v2:
 1:  7a297db4da =  1:  7a297db4da refs: fix segfault when aborting empty transaction
 2:  15857e1b8c =  2:  15857e1b8c git-update-ref.txt: add missing word
 3:  b6546ae44e =  3:  b6546ae44e strbuf: provide function to append whole lines
 4:  bd8c059fbc !  4:  0f881d4936 update-ref: organize commands in an array
    @@ Commit message
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
     
      ## builtin/update-ref.c ##
    -@@ builtin/update-ref.c: static int parse_next_oid(struct strbuf *input, const char **next,
    - /*
    -  * The following five parse_cmd_*() functions parse the corresponding
    -  * command.  In each case, next points at the character following the
    -- * command name and the following space.  They each return a pointer
    -- * to the character terminating the command, and die with an
    -- * explanatory message if there are any parsing problems.  All of
    -- * these functions handle either text or binary format input,
    -- * depending on how line_termination is set.
    -+ * command name.  They each return a pointer to the character
    -+ * terminating the command, and die with an explanatory message if
    -+ * there are any parsing problems.  All of these functions handle
    -+ * either text or binary format input, depending on how
    -+ * line_termination is set.
    -  */
    - 
    - static const char *parse_cmd_update(struct ref_transaction *transaction,
    -@@ builtin/update-ref.c: static const char *parse_cmd_update(struct ref_transaction *transaction,
    - 	struct object_id new_oid, old_oid;
    - 	int have_old;
    - 
    -+	if (!skip_prefix(next, " ", &next))
    -+		die("update: missing space after command");
    -+
    - 	refname = parse_refname(input, &next);
    - 	if (!refname)
    - 		die("update: missing <ref>");
    -@@ builtin/update-ref.c: static const char *parse_cmd_create(struct ref_transaction *transaction,
    - 	char *refname;
    - 	struct object_id new_oid;
    - 
    -+	if (!skip_prefix(next, " ", &next))
    -+		die("create: missing space after command");
    -+
    - 	refname = parse_refname(input, &next);
    - 	if (!refname)
    - 		die("create: missing <ref>");
    -@@ builtin/update-ref.c: static const char *parse_cmd_delete(struct ref_transaction *transaction,
    - 	struct object_id old_oid;
    - 	int have_old;
    - 
    -+	if (!skip_prefix(next, " ", &next))
    -+		die("delete: missing space after command");
    -+
    - 	refname = parse_refname(input, &next);
    - 	if (!refname)
    - 		die("delete: missing <ref>");
    -@@ builtin/update-ref.c: static const char *parse_cmd_verify(struct ref_transaction *transaction,
    - 	char *refname;
    - 	struct object_id old_oid;
    - 
    -+	if (!skip_prefix(next, " ", &next))
    -+		die("verify: missing space after command");
    -+
    - 	refname = parse_refname(input, &next);
    - 	if (!refname)
    - 		die("verify: missing <ref>");
     @@ builtin/update-ref.c: static const char *parse_cmd_verify(struct ref_transaction *transaction,
      	return next;
      }
    @@ builtin/update-ref.c: static const char *parse_cmd_verify(struct ref_transaction
     +				    struct strbuf *input, const char *next)
      {
      	const char *rest;
    -+	if (!skip_prefix(next, " ", &next))
    -+		die("option: missing space after command");
      	if (skip_prefix(next, "no-deref", &rest) && *rest == line_termination)
    - 		update_flags |= REF_NO_DEREF;
    - 	else
     @@ builtin/update-ref.c: static const char *parse_cmd_option(struct strbuf *input, const char *next)
      	return rest;
      }
    @@ builtin/update-ref.c: static const char *parse_cmd_option(struct strbuf *input,
     +		for (i = 0; i < ARRAY_SIZE(command); i++) {
     +			const char *prefix = command[i].prefix;
     +
    -+			if (!skip_prefix(next, prefix, &next))
    -+				continue;
    -+
    -+			/*
    -+			 * Check that the command is terminated by an argument
    -+			 * or line terminator and not only a matching prefix.
    -+			 */
    -+			if (input.buf[strlen(prefix)] != line_termination &&
    -+			    input.buf[strlen(prefix)] != '\0' &&
    -+			    input.buf[strlen(prefix)] != ' ')
    ++			if (!skip_prefix(next, prefix, &next) ||
    ++			    !skip_prefix(next, " ", &next))
     +				continue;
     +
     +			cmd = &command[i];
 5:  49a14d2046 !  5:  c96395a14d update-ref: drop unused argument for `parse_refname`
    @@ builtin/update-ref.c: static const char *parse_arg(const char *next, struct strb
      	struct strbuf ref = STRBUF_INIT;
      
     @@ builtin/update-ref.c: static const char *parse_cmd_update(struct ref_transaction *transaction,
    - 	if (!skip_prefix(next, " ", &next))
    - 		die("update: missing space after command");
    + 	struct object_id new_oid, old_oid;
    + 	int have_old;
      
     -	refname = parse_refname(input, &next);
     +	refname = parse_refname(&next);
    @@ builtin/update-ref.c: static const char *parse_cmd_update(struct ref_transaction
      		die("update: missing <ref>");
      
     @@ builtin/update-ref.c: static const char *parse_cmd_create(struct ref_transaction *transaction,
    - 	if (!skip_prefix(next, " ", &next))
    - 		die("create: missing space after command");
    + 	char *refname;
    + 	struct object_id new_oid;
      
     -	refname = parse_refname(input, &next);
     +	refname = parse_refname(&next);
    @@ builtin/update-ref.c: static const char *parse_cmd_create(struct ref_transaction
      		die("create: missing <ref>");
      
     @@ builtin/update-ref.c: static const char *parse_cmd_delete(struct ref_transaction *transaction,
    - 	if (!skip_prefix(next, " ", &next))
    - 		die("delete: missing space after command");
    + 	struct object_id old_oid;
    + 	int have_old;
      
     -	refname = parse_refname(input, &next);
     +	refname = parse_refname(&next);
    @@ builtin/update-ref.c: static const char *parse_cmd_delete(struct ref_transaction
      		die("delete: missing <ref>");
      
     @@ builtin/update-ref.c: static const char *parse_cmd_verify(struct ref_transaction *transaction,
    - 	if (!skip_prefix(next, " ", &next))
    - 		die("verify: missing space after command");
    + 	char *refname;
    + 	struct object_id old_oid;
      
     -	refname = parse_refname(input, &next);
     +	refname = parse_refname(&next);
 6:  cbe430029d !  6:  192cbf5944 update-ref: pass end pointer instead of strbuf
    @@ builtin/update-ref.c: static const char *parse_cmd_verify(struct ref_transaction
     +				    const char *next, const char *end)
      {
      	const char *rest;
    - 	if (!skip_prefix(next, " ", &next))
    + 	if (skip_prefix(next, "no-deref", &rest) && *rest == line_termination)
     @@ builtin/update-ref.c: static const char *parse_cmd_option(struct ref_transaction *transaction,
      
      static const struct parse_cmd {
 7:  d2f68f59a7 =  7:  be7bcf3dbd update-ref: move transaction handling into `update_refs_stdin()`
 8:  f8786fdeb3 !  8:  02ff6b7337 update-ref: read commands in a line-wise fashion
    @@ Commit message
     
      ## builtin/update-ref.c ##
     @@ builtin/update-ref.c: static int parse_next_oid(const char **next, const char *end,
    -  * line_termination is set.
    +  * depending on how line_termination is set.
       */
      
     -static const char *parse_cmd_update(struct ref_transaction *transaction,
    @@ builtin/update-ref.c: static const char *parse_cmd_verify(struct ref_transaction
     +			     const char *next, const char *end)
      {
      	const char *rest;
    - 	if (!skip_prefix(next, " ", &next))
    -@@ builtin/update-ref.c: static const char *parse_cmd_option(struct ref_transaction *transaction,
    + 	if (skip_prefix(next, "no-deref", &rest) && *rest == line_termination)
      		update_flags |= REF_NO_DEREF;
      	else
      		die("option unknown: %s", next);
    @@ builtin/update-ref.c: static const char *parse_cmd_option(struct ref_transaction
      	const char *prefix;
     -	const char *(*fn)(struct ref_transaction *, const char *, const char *);
     +	void (*fn)(struct ref_transaction *, const char *, const char *);
    -+	/*
    -+	 * If using NUL-terminated format, only the first argument will be
    -+	 * available in the first line. In case a command expects more than one
    -+	 * argument, we thus have to fetch an additional `extra_lines` number
    -+	 * of lines.
    -+	 */
    -+	unsigned extra_lines;
    ++	unsigned args;
      } command[] = {
     -	{ "update", parse_cmd_update },
     -	{ "create", parse_cmd_create },
     -	{ "delete", parse_cmd_delete },
     -	{ "verify", parse_cmd_verify },
     -	{ "option", parse_cmd_option },
    -+	{ "update", parse_cmd_update, 2 },
    -+	{ "create", parse_cmd_create, 1 },
    -+	{ "delete", parse_cmd_delete, 1 },
    -+	{ "verify", parse_cmd_verify, 1 },
    -+	{ "option", parse_cmd_option, 0 },
    ++	{ "update", parse_cmd_update, 3 },
    ++	{ "create", parse_cmd_create, 2 },
    ++	{ "delete", parse_cmd_delete, 2 },
    ++	{ "verify", parse_cmd_verify, 2 },
    ++	{ "option", parse_cmd_option, 1 },
      };
      
      static void update_refs_stdin(void)
    @@ builtin/update-ref.c: static const char *parse_cmd_option(struct ref_transaction
      
      		for (i = 0; i < ARRAY_SIZE(command); i++) {
      			const char *prefix = command[i].prefix;
    ++			char c;
      
    --			if (!skip_prefix(next, prefix, &next))
    +-			if (!skip_prefix(next, prefix, &next) ||
    +-			    !skip_prefix(next, " ", &next))
     +			if (!starts_with(input.buf, prefix))
    ++				continue;
    ++
    ++			/*
    ++			 * If the command has arguments, verify that it's
    ++			 * followed by a space. Otherwise, it shall be followed
    ++			 * by a line terminator.
    ++			 */
    ++			c = command[i].args ? ' ' : line_termination;
    ++			if (input.buf[strlen(prefix)] != c)
      				continue;
      
    - 			/*
    -@@ builtin/update-ref.c: static void update_refs_stdin(void)
    + 			cmd = &command[i];
      			break;
      		}
      		if (!cmd)
    @@ builtin/update-ref.c: static void update_refs_stdin(void)
     -		next = cmd->fn(transaction, next, input.buf + input.len);
     -		next++;
     +		/*
    -+		 * Read extra lines if NUL-terminated. Do not raise an error in
    -+		 * case there is an early EOF to let the command handle missing
    -+		 * arguments with a proper error message.
    ++		 * Read additional arguments if NUL-terminated. Do not raise an
    ++		 * error in case there is an early EOF to let the command
    ++		 * handle missing arguments with a proper error message.
     +		 */
    -+		for (j = 0; line_termination == '\0' && j < cmd->extra_lines; j++)
    ++		for (j = 1; line_termination == '\0' && j < cmd->args; j++)
     +			if (strbuf_appendwholeline(&input, stdin, line_termination))
     +				break;
     +
    -+		cmd->fn(transaction, input.buf + strlen(cmd->prefix),
    ++		cmd->fn(transaction, input.buf + strlen(cmd->prefix) + !!cmd->args,
     +			input.buf + input.len);
      	}
      
 9:  c3fffdf9fa !  9:  5670bea2b1 update-ref: implement interactive transaction handling
    @@ builtin/update-ref.c: static void parse_cmd_option(struct ref_transaction *trans
      static const struct parse_cmd {
      	const char *prefix;
      	void (*fn)(struct ref_transaction *, const char *, const char *);
    -@@ builtin/update-ref.c: static const struct parse_cmd {
    - 	 * of lines.
    - 	 */
    - 	unsigned extra_lines;
    + 	unsigned args;
     +	enum update_refs_state state;
      } command[] = {
    --	{ "update", parse_cmd_update, 2 },
    --	{ "create", parse_cmd_create, 1 },
    --	{ "delete", parse_cmd_delete, 1 },
    --	{ "verify", parse_cmd_verify, 1 },
    --	{ "option", parse_cmd_option, 0 },
    -+	{ "update",  parse_cmd_update,  2, UPDATE_REFS_OPEN },
    -+	{ "create",  parse_cmd_create,  1, UPDATE_REFS_OPEN },
    -+	{ "delete",  parse_cmd_delete,  1, UPDATE_REFS_OPEN },
    -+	{ "verify",  parse_cmd_verify,  1, UPDATE_REFS_OPEN },
    -+	{ "option",  parse_cmd_option,  0, UPDATE_REFS_OPEN },
    +-	{ "update", parse_cmd_update, 3 },
    +-	{ "create", parse_cmd_create, 2 },
    +-	{ "delete", parse_cmd_delete, 2 },
    +-	{ "verify", parse_cmd_verify, 2 },
    +-	{ "option", parse_cmd_option, 1 },
    ++	{ "update",  parse_cmd_update,  3, UPDATE_REFS_OPEN },
    ++	{ "create",  parse_cmd_create,  2, UPDATE_REFS_OPEN },
    ++	{ "delete",  parse_cmd_delete,  2, UPDATE_REFS_OPEN },
    ++	{ "verify",  parse_cmd_verify,  2, UPDATE_REFS_OPEN },
    ++	{ "option",  parse_cmd_option,  1, UPDATE_REFS_OPEN },
     +	{ "start",   parse_cmd_start,   0, UPDATE_REFS_STARTED },
     +	{ "prepare", parse_cmd_prepare, 0, UPDATE_REFS_PREPARED },
     +	{ "abort",   parse_cmd_abort,   0, UPDATE_REFS_CLOSED },
    @@ builtin/update-ref.c: static void update_refs_stdin(void)
     +			break;
     +		}
     +
    - 		cmd->fn(transaction, input.buf + strlen(cmd->prefix),
    + 		cmd->fn(transaction, input.buf + strlen(cmd->prefix) + !!cmd->args,
      			input.buf + input.len);
      	}
      
-- 
2.26.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  parent reply	other threads:[~2020-04-02  7:09 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-25  9:53 [PATCH 0/9] Support for transactions in `git-update-ref --stdin` Patrick Steinhardt
2020-03-25  9:53 ` [PATCH 1/9] refs: fix segfault when aborting empty transaction Patrick Steinhardt
2020-03-27 19:59   ` Junio C Hamano
2020-03-25  9:53 ` [PATCH 2/9] git-update-ref.txt: add missing word Patrick Steinhardt
2020-03-25  9:53 ` [PATCH 3/9] strbuf: provide function to append whole lines Patrick Steinhardt
2020-03-27 21:04   ` Junio C Hamano
2020-03-30 13:25     ` Patrick Steinhardt
2020-03-30 17:12       ` Junio C Hamano
2020-03-25  9:53 ` [PATCH 4/9] update-ref: organize commands in an array Patrick Steinhardt
2020-03-27 21:25   ` Junio C Hamano
2020-03-30  8:05     ` Patrick Steinhardt
2020-03-30 16:55       ` Junio C Hamano
2020-03-30 17:37         ` Patrick Steinhardt
2020-03-25  9:54 ` [PATCH 5/9] update-ref: drop unused argument for `parse_refname` Patrick Steinhardt
2020-03-25  9:54 ` [PATCH 6/9] update-ref: pass end pointer instead of strbuf Patrick Steinhardt
2020-03-25  9:54 ` [PATCH 7/9] update-ref: move transaction handling into `update_refs_stdin()` Patrick Steinhardt
2020-03-27 21:44   ` Junio C Hamano
2020-03-25  9:54 ` [PATCH 8/9] update-ref: read commands in a line-wise fashion Patrick Steinhardt
2020-03-27 21:58   ` Junio C Hamano
2020-03-30  8:11     ` Patrick Steinhardt
2020-03-30 17:39       ` Junio C Hamano
2020-03-25  9:54 ` [PATCH 9/9] update-ref: implement interactive transaction handling Patrick Steinhardt
2020-03-27 22:00   ` Junio C Hamano
2020-03-30 13:46 ` [PATCH v2 0/9] Support for transactions in `git-update-ref --stdin` Patrick Steinhardt
2020-03-30 13:46   ` [PATCH v2 1/9] refs: fix segfault when aborting empty transaction Patrick Steinhardt
2020-03-30 13:46   ` [PATCH v2 2/9] git-update-ref.txt: add missing word Patrick Steinhardt
2020-03-30 13:46   ` [PATCH v2 3/9] strbuf: provide function to append whole lines Patrick Steinhardt
2020-03-30 13:46   ` [PATCH v2 4/9] update-ref: organize commands in an array Patrick Steinhardt
2020-03-30 13:46   ` [PATCH v2 5/9] update-ref: drop unused argument for `parse_refname` Patrick Steinhardt
2020-03-30 13:46   ` [PATCH v2 6/9] update-ref: pass end pointer instead of strbuf Patrick Steinhardt
2020-03-30 13:46   ` [PATCH v2 7/9] update-ref: move transaction handling into `update_refs_stdin()` Patrick Steinhardt
2020-03-30 13:46   ` [PATCH v2 8/9] update-ref: read commands in a line-wise fashion Patrick Steinhardt
2020-03-30 13:47   ` [PATCH v2 9/9] update-ref: implement interactive transaction handling Patrick Steinhardt
2020-04-02  7:09 ` Patrick Steinhardt [this message]
2020-04-02  7:09   ` [PATCH v3 1/9] refs: fix segfault when aborting empty transaction Patrick Steinhardt
2020-04-02  7:09   ` [PATCH v3 2/9] git-update-ref.txt: add missing word Patrick Steinhardt
2020-04-02  7:09   ` [PATCH v3 3/9] strbuf: provide function to append whole lines Patrick Steinhardt
2020-04-02  7:09   ` [PATCH v3 4/9] update-ref: organize commands in an array Patrick Steinhardt
2020-04-02  7:09   ` [PATCH v3 5/9] update-ref: drop unused argument for `parse_refname` Patrick Steinhardt
2020-04-02  7:09   ` [PATCH v3 6/9] update-ref: pass end pointer instead of strbuf Patrick Steinhardt
2020-04-02  7:09   ` [PATCH v3 7/9] update-ref: move transaction handling into `update_refs_stdin()` Patrick Steinhardt
2020-04-02  7:09   ` [PATCH v3 8/9] update-ref: read commands in a line-wise fashion Patrick Steinhardt
2020-04-02  7:10   ` [PATCH v3 9/9] update-ref: implement interactive transaction handling Patrick Steinhardt
2020-04-03 13:40     ` Phillip Wood
2020-04-03 16:51       ` Patrick Steinhardt
2020-04-03 17:33       ` Junio C Hamano
2020-04-03 17:35         ` Junio C Hamano
2020-04-06  7:10         ` Patrick Steinhardt

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=cover.1585811013.git.ps@pks.im \
    --to=ps@pks.im \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).