From e48cf33b61b8fecf1558fb32fcf5ee2987e82890 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 2 Apr 2020 09:10:02 +0200 Subject: update-ref: implement interactive transaction handling 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 Signed-off-by: Junio C Hamano --- Documentation/git-update-ref.txt | 26 ++++++++ builtin/update-ref.c | 106 ++++++++++++++++++++++++++++--- t/t1400-update-ref.sh | 131 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 255 insertions(+), 8 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 [SP ] LF verify SP [SP ] LF option SP 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 NUL [] NUL verify SP NUL [] NUL option SP 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 s can be locked with matching 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 348407b896..b74dd9a69d 100644 --- a/builtin/update-ref.c +++ b/builtin/update-ref.c @@ -312,21 +312,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 args; + enum update_refs_state state; } command[] = { - { "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 }, + { "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; @@ -374,14 +433,45 @@ static void update_refs_stdin(void) 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) + !!cmd->args, 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 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 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 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 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 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 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 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 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 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 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 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 actual && + printf "%s: ok\n" start prepare >expect && + test_cmp expect actual && + test_path_is_missing .git/$b +' + test_done -- cgit v1.2.3