From: Patrick Steinhardt <ps@pks.im>
To: git <git@vger.kernel.org>
Cc: Christian Couder <christian.couder@gmail.com>
Subject: [PATCH 9/9] update-ref: implement interactive transaction handling
Date: Wed, 25 Mar 2020 10:54:29 +0100 [thread overview]
Message-ID: <88c0089bb50bca13efc79a1f8fd42b80286af949.1585129843.git.ps@pks.im> (raw)
In-Reply-To: <cover.1585129842.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 12314 bytes --]
The git-update-ref(1) command can only handle queueing transactions
right now via its "--stdin" parameter, but there is no way for users to
handle the transaction itself in a more explicit way. E.g. in a
replicated scenario, one may imagine a coordinator that spawns
git-update-ref(1) for multiple repositories and only if all agree that
an update is possible will the coordinator send a commit. Such a
transactional session could look like
> start
< start: ok
> update refs/heads/master $OLD $NEW
> prepare
< prepare: ok
# All nodes have returned "ok"
> commit
< commit: ok
or
> start
< start: ok
> create refs/heads/master $OLD $NEW
> prepare
< fatal: cannot lock ref 'refs/heads/master': reference already exists
# On all other nodes:
> abort
< abort: ok
In order to allow for such transactional sessions, this commit
introduces four new commands for git-update-ref(1), which matches those
we have internally already with the exception of "start":
- start: start a new transaction
- prepare: prepare the transaction, that is try to lock all
references and verify their current value matches the
expected one
- commit: explicitly commit a session, that is update references to
match their new expected state
- abort: abort a session and roll back all changes
By design, git-update-ref(1) will commit as soon as standard input is
being closed. While fine in a non-transactional world, it is definitely
unexpected in a transactional world. Because of this, as soon as any of
the new transactional commands is used, the default will change to
aborting without an explicit "commit". To avoid a race between queueing
updates and the first "prepare" that starts a transaction, the "start"
command has been added to start an explicit transaction.
Add some tests to exercise this new functionality.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Documentation/git-update-ref.txt | 26 ++++++
builtin/update-ref.c | 108 ++++++++++++++++++++++---
t/t1400-update-ref.sh | 131 +++++++++++++++++++++++++++++++
3 files changed, 256 insertions(+), 9 deletions(-)
diff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt
index 9bd039ce08..3e737c2360 100644
--- a/Documentation/git-update-ref.txt
+++ b/Documentation/git-update-ref.txt
@@ -66,6 +66,10 @@ performs all modifications together. Specify commands of the form:
delete SP <ref> [SP <oldvalue>] LF
verify SP <ref> [SP <oldvalue>] LF
option SP <opt> LF
+ start LF
+ prepare LF
+ commit LF
+ abort LF
With `--create-reflog`, update-ref will create a reflog for each ref
even if one would not ordinarily be created.
@@ -83,6 +87,10 @@ quoting:
delete SP <ref> NUL [<oldvalue>] NUL
verify SP <ref> NUL [<oldvalue>] NUL
option SP <opt> NUL
+ start NUL
+ prepare NUL
+ commit NUL
+ abort NUL
In this format, use 40 "0" to specify a zero value, and use the empty
string to specify a missing value.
@@ -114,6 +122,24 @@ option::
The only valid option is `no-deref` to avoid dereferencing
a symbolic ref.
+start::
+ Start a transaction. In contrast to a non-transactional session, a
+ transaction will automatically abort if the session ends without an
+ explicit commit.
+
+prepare::
+ Prepare to commit the transaction. This will create lock files for all
+ queued reference updates. If one reference could not be locked, the
+ transaction will be aborted.
+
+commit::
+ Commit all reference updates queued for the transaction, ending the
+ transaction.
+
+abort::
+ Abort the transaction, releasing all locks if the transaction is in
+ prepared state.
+
If all <ref>s can be locked with matching <oldvalue>s
simultaneously, all modifications are performed. Otherwise, no
modifications are performed. Note that while each individual
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index f35e3ca5ae..6b870507e0 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -326,21 +326,80 @@ static void parse_cmd_option(struct ref_transaction *transaction,
die("option unknown: %s", next);
}
+static void parse_cmd_start(struct ref_transaction *transaction,
+ const char *next, const char *end)
+{
+ if (*next != line_termination)
+ die("start: extra input: %s", next);
+ puts("start: ok");
+}
+
+static void parse_cmd_prepare(struct ref_transaction *transaction,
+ const char *next, const char *end)
+{
+ struct strbuf error = STRBUF_INIT;
+ if (*next != line_termination)
+ die("prepare: extra input: %s", next);
+ if (ref_transaction_prepare(transaction, &error))
+ die("prepare: %s", error.buf);
+ puts("prepare: ok");
+}
+
+static void parse_cmd_abort(struct ref_transaction *transaction,
+ const char *next, const char *end)
+{
+ struct strbuf error = STRBUF_INIT;
+ if (*next != line_termination)
+ die("abort: extra input: %s", next);
+ if (ref_transaction_abort(transaction, &error))
+ die("abort: %s", error.buf);
+ puts("abort: ok");
+}
+
+static void parse_cmd_commit(struct ref_transaction *transaction,
+ const char *next, const char *end)
+{
+ struct strbuf error = STRBUF_INIT;
+ if (*next != line_termination)
+ die("commit: extra input: %s", next);
+ if (ref_transaction_commit(transaction, &error))
+ die("commit: %s", error.buf);
+ puts("commit: ok");
+ ref_transaction_free(transaction);
+}
+
+enum update_refs_state {
+ /* Non-transactional state open for updates. */
+ UPDATE_REFS_OPEN,
+ /* A transaction has been started. */
+ UPDATE_REFS_STARTED,
+ /* References are locked and ready for commit */
+ UPDATE_REFS_PREPARED,
+ /* Transaction has been committed or closed. */
+ UPDATE_REFS_CLOSED,
+};
+
static const struct parse_cmd {
const char *prefix;
void (*fn)(struct ref_transaction *, const char *, const char *);
unsigned extra_lines;
+ enum update_refs_state state;
} commands[] = {
- { "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 },
+ { "start", parse_cmd_start, 0, UPDATE_REFS_STARTED },
+ { "prepare", parse_cmd_prepare, 0, UPDATE_REFS_PREPARED },
+ { "abort", parse_cmd_abort, 0, UPDATE_REFS_CLOSED },
+ { "commit", parse_cmd_commit, 0, UPDATE_REFS_CLOSED },
};
static void update_refs_stdin(void)
{
struct strbuf input = STRBUF_INIT, err = STRBUF_INIT;
+ enum update_refs_state state = UPDATE_REFS_OPEN;
struct ref_transaction *transaction;
int i, j;
@@ -371,19 +430,50 @@ 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;
+ switch (state) {
+ case UPDATE_REFS_OPEN:
+ case UPDATE_REFS_STARTED:
+ /* Do not downgrade a transaction to a non-transaction. */
+ if (cmd->state >= state)
+ state = cmd->state;
+ break;
+ case UPDATE_REFS_PREPARED:
+ if (cmd->state != UPDATE_REFS_CLOSED)
+ die("prepared transactions can only be closed");
+ state = cmd->state;
+ break;
+ case UPDATE_REFS_CLOSED:
+ die("transaction is closed");
+ break;
+ }
+
cmd->fn(transaction, input.buf + strlen(cmd->prefix),
input.buf + input.len);
}
- if (ref_transaction_commit(transaction, &err))
- die("%s", err.buf);
+ switch (state) {
+ case UPDATE_REFS_OPEN:
+ /* Commit by default if no transaction was requested. */
+ if (ref_transaction_commit(transaction, &err))
+ die("%s", err.buf);
+ ref_transaction_free(transaction);
+ break;
+ case UPDATE_REFS_STARTED:
+ case UPDATE_REFS_PREPARED:
+ /* If using a transaction, we want to abort it. */
+ if (ref_transaction_abort(transaction, &err))
+ die("%s", err.buf);
+ break;
+ case UPDATE_REFS_CLOSED:
+ /* Otherwise no need to do anything, the transaction was closed already. */
+ break;
+ }
- ref_transaction_free(transaction);
strbuf_release(&err);
strbuf_release(&input);
}
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index a6224ef65f..48d0d42afd 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -1404,4 +1404,135 @@ test_expect_success 'handle per-worktree refs in refs/bisect' '
! test_cmp main-head worktree-head
'
+test_expect_success 'transaction handles empty commit' '
+ cat >stdin <<-EOF &&
+ start
+ prepare
+ commit
+ EOF
+ git update-ref --stdin <stdin >actual &&
+ printf "%s: ok\n" start prepare commit >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success 'transaction handles empty commit with missing prepare' '
+ cat >stdin <<-EOF &&
+ start
+ commit
+ EOF
+ git update-ref --stdin <stdin >actual &&
+ printf "%s: ok\n" start commit >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success 'transaction handles sole commit' '
+ cat >stdin <<-EOF &&
+ commit
+ EOF
+ git update-ref --stdin <stdin >actual &&
+ printf "%s: ok\n" commit >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success 'transaction handles empty abort' '
+ cat >stdin <<-EOF &&
+ start
+ prepare
+ abort
+ EOF
+ git update-ref --stdin <stdin >actual &&
+ printf "%s: ok\n" start prepare abort >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success 'transaction exits on multiple aborts' '
+ cat >stdin <<-EOF &&
+ abort
+ abort
+ EOF
+ test_must_fail git update-ref --stdin <stdin >actual 2>err &&
+ printf "%s: ok\n" abort >expect &&
+ test_cmp expect actual &&
+ grep "fatal: transaction is closed" err
+'
+
+test_expect_success 'transaction exits on start after prepare' '
+ cat >stdin <<-EOF &&
+ prepare
+ start
+ EOF
+ test_must_fail git update-ref --stdin <stdin 2>err >actual &&
+ printf "%s: ok\n" prepare >expect &&
+ test_cmp expect actual &&
+ grep "fatal: prepared transactions can only be closed" err
+'
+
+test_expect_success 'transaction handles empty abort with missing prepare' '
+ cat >stdin <<-EOF &&
+ start
+ abort
+ EOF
+ git update-ref --stdin <stdin >actual &&
+ printf "%s: ok\n" start abort >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success 'transaction handles sole abort' '
+ cat >stdin <<-EOF &&
+ abort
+ EOF
+ git update-ref --stdin <stdin >actual &&
+ printf "%s: ok\n" abort >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success 'transaction can handle commit' '
+ cat >stdin <<-EOF &&
+ start
+ create $a HEAD
+ commit
+ EOF
+ git update-ref --stdin <stdin >actual &&
+ printf "%s: ok\n" start commit >expect &&
+ test_cmp expect actual &&
+ git rev-parse HEAD >expect &&
+ git rev-parse $a >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'transaction can handle abort' '
+ cat >stdin <<-EOF &&
+ start
+ create $b HEAD
+ abort
+ EOF
+ git update-ref --stdin <stdin >actual &&
+ printf "%s: ok\n" start abort >expect &&
+ test_cmp expect actual &&
+ test_path_is_missing .git/$b
+'
+
+test_expect_success 'transaction aborts by default' '
+ cat >stdin <<-EOF &&
+ start
+ create $b HEAD
+ EOF
+ git update-ref --stdin <stdin >actual &&
+ printf "%s: ok\n" start >expect &&
+ test_cmp expect actual &&
+ test_path_is_missing .git/$b
+'
+
+test_expect_success 'transaction with prepare aborts by default' '
+ cat >stdin <<-EOF &&
+ start
+ create $b HEAD
+ prepare
+ EOF
+ git update-ref --stdin <stdin >actual &&
+ printf "%s: ok\n" start prepare >expect &&
+ test_cmp expect actual &&
+ test_path_is_missing .git/$b
+'
+
test_done
--
2.26.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2020-03-25 9:54 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 ` Patrick Steinhardt [this message]
2020-03-27 22:00 ` [PATCH 9/9] update-ref: implement interactive transaction handling 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 ` [PATCH v3 0/9] Support for transactions in `git-update-ref --stdin` Patrick Steinhardt
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=88c0089bb50bca13efc79a1f8fd42b80286af949.1585129843.git.ps@pks.im \
--to=ps@pks.im \
--cc=christian.couder@gmail.com \
--cc=git@vger.kernel.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.
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).