[v2,0/9] Support for transactions in `git-update-ref --stdin`

Message

Patrick Steinhardt March 30, 2020, 1:46 p.m. UTC

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.

The second version addresses the feedback of Junio:

    - Renamed the `commands` array to `command`.

    - Adds documentation to the `extra_lines` field to hopefully make
      its intent clearer.

    - Clarify why we ignore EOF when reading `extra_lines`.

    - When reading commands, the code now verifies that a line is not
      only a prefix to the command, but in fact the whole command. This
      allows implementation of multiple commands that have the same
      prefix, e.g. "option" and "options".

Thanks Junio for your review!

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             | 274 +++++++++++++++++++++++--------
 refs/files-backend.c             |  20 ++-
 strbuf.c                         |  10 ++
 strbuf.h                         |   6 +
 t/t1400-update-ref.sh            | 131 +++++++++++++++
 6 files changed, 388 insertions(+), 81 deletions(-)

Range-diff against v1:
 1:  3c7f8c47f3 =  1:  7a297db4da refs: fix segfault when aborting empty transaction
 2:  dd7798abb7 =  2:  15857e1b8c git-update-ref.txt: add missing word
 3:  4b0a963ea5 =  3:  b6546ae44e strbuf: provide function to append whole lines
 4:  50ffc26329 !  4:  bd8c059fbc update-ref: organize commands in an array
    @@ builtin/update-ref.c: static const char *parse_cmd_option(struct strbuf *input,
     +static const struct parse_cmd {
     +	const char *prefix;
     +	const char *(*fn)(struct ref_transaction *, struct strbuf *, const char *);
    -+} commands[] = {
    ++} command[] = {
     +	{ "update", parse_cmd_update },
     +	{ "create", parse_cmd_create },
     +	{ "delete", parse_cmd_delete },
    @@ builtin/update-ref.c: static const char *parse_cmd_option(struct strbuf *input,
     -			next = parse_cmd_option(&input, next);
     -		else
     +
    -+		for (i = 0; i < ARRAY_SIZE(commands); i++) {
    -+			if (!skip_prefix(next, commands[i].prefix , &next))
    ++		for (i = 0; i < ARRAY_SIZE(command); i++) {
    ++			const char *prefix = command[i].prefix;
    ++
    ++			if (!skip_prefix(next, prefix, &next))
     +				continue;
    -+			cmd = &commands[i];
    ++
    ++			/*
    ++			 * 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)] != ' ')
    ++				continue;
    ++
    ++			cmd = &command[i];
     +			break;
     +		}
     +		if (!cmd)
      			die("unknown command: %s", next);
      
    -+		if (input.buf[strlen(cmd->prefix)] != line_termination &&
    -+		    input.buf[strlen(cmd->prefix)] != '\0' &&
    -+		    input.buf[strlen(cmd->prefix)] != ' ')
    -+			die("%s: no separator after command", cmd->prefix);
    -+
     +		next = cmd->fn(transaction, &input, next);
      		next++;
      	}
 5:  a78043b188 =  5:  49a14d2046 update-ref: drop unused argument for `parse_refname`
 6:  51ebb2f849 !  6:  cbe430029d update-ref: pass end pointer instead of strbuf
    @@ builtin/update-ref.c: static const char *parse_cmd_option(struct ref_transaction
      	const char *prefix;
     -	const char *(*fn)(struct ref_transaction *, struct strbuf *, const char *);
     +	const char *(*fn)(struct ref_transaction *, const char *, const char *);
    - } commands[] = {
    + } command[] = {
      	{ "update", parse_cmd_update },
      	{ "create", parse_cmd_create },
     @@ builtin/update-ref.c: static void update_refs_stdin(struct ref_transaction *transaction)
    - 		    input.buf[strlen(cmd->prefix)] != ' ')
    - 			die("%s: no separator after command", cmd->prefix);
    + 		if (!cmd)
    + 			die("unknown command: %s", next);
      
     -		next = cmd->fn(transaction, &input, next);
     +		next = cmd->fn(transaction, next, input.buf + input.len);
 7:  bd92a649d7 =  7:  d2f68f59a7 update-ref: move transaction handling into `update_refs_stdin()`
 8:  1db63f97ec !  8:  f8786fdeb3 update-ref: read commands in a line-wise fashion
    @@ 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;
    - } commands[] = {
    + } command[] = {
     -	{ "update", parse_cmd_update },
     -	{ "create", parse_cmd_create },
     -	{ "delete", parse_cmd_delete },
    @@ builtin/update-ref.c: static const char *parse_cmd_option(struct ref_transaction
     +		else if (isspace(*input.buf))
     +			die("whitespace before command: %s", input.buf);
      
    - 		for (i = 0; i < ARRAY_SIZE(commands); i++) {
    --			if (!skip_prefix(next, commands[i].prefix , &next))
    -+			if (!starts_with(input.buf, commands[i].prefix))
    + 		for (i = 0; i < ARRAY_SIZE(command); i++) {
    + 			const char *prefix = command[i].prefix;
    + 
    +-			if (!skip_prefix(next, prefix, &next))
    ++			if (!starts_with(input.buf, prefix))
      				continue;
    - 			cmd = &commands[i];
    + 
    + 			/*
    +@@ builtin/update-ref.c: static void update_refs_stdin(void)
      			break;
      		}
      		if (!cmd)
     -			die("unknown command: %s", next);
     +			die("unknown command: %s", input.buf);
      
    - 		if (input.buf[strlen(cmd->prefix)] != line_termination &&
    - 		    input.buf[strlen(cmd->prefix)] != '\0' &&
    - 		    input.buf[strlen(cmd->prefix)] != ' ')
    - 			die("%s: no separator after command", cmd->prefix);
    - 
     -		next = cmd->fn(transaction, next, input.buf + input.len);
     -		next++;
    -+		/* Read extra lines if NUL-terminated */
    ++		/*
    ++		 * 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.
    ++		 */
     +		for (j = 0; line_termination == '\0' && j < cmd->extra_lines; j++)
     +			if (strbuf_appendwholeline(&input, stdin, line_termination))
     +				break;
 9:  88c0089bb5 !  9:  c3fffdf9fa 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;
     +	enum update_refs_state state;
    - } commands[] = {
    + } command[] = {
     -	{ "update", parse_cmd_update, 2 },
     -	{ "create", parse_cmd_create, 1 },
     -	{ "delete", parse_cmd_delete, 1 },
    @@ builtin/update-ref.c: static void parse_cmd_option(struct ref_transaction *trans
      	int i, j;
      
     @@ builtin/update-ref.c: static void update_refs_stdin(void)
    - 		    input.buf[strlen(cmd->prefix)] != ' ')
    - 			die("%s: no separator after command", cmd->prefix);
    - 
    --		/* Read extra lines if NUL-terminated */
    -+		/* Read extra lines if NUL-terminated, but let commands handle missing ones. */
    - 		for (j = 0; line_termination == '\0' && j < cmd->extra_lines; j++)
      			if (strbuf_appendwholeline(&input, stdin, line_termination))
      				break;