{"thread":{"id":"54571","subject":"[PATCH 0/2] update-ref: Allow creation of multiple transactions","startedAt":"2020-11-04T14:57:24Z","lastAt":"2020-11-26T00:43:16Z","messageCount":41,"participants":["Patrick Steinhardt","Jeff King","Junio C Hamano","SZEDER Gábor"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"409083","messageId":"cover.1604501265.git.ps@pks.im","threadId":"54571","inReplyTo":null,"subject":"[PATCH 0/2] update-ref: Allow creation of multiple transactions","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2020-11-04T14:57:12Z","receivedAt":"2020-11-04T14:57:24Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nA few months back, I've implemented a more interactive protocol for\n`git-update-ref --stdin`, which converted it to read commands linewise\nand allow for more control via a set of new controlling commands. One\nthing that was noticeably missing was the ability to create multiple\ntransactions in a single git-update-ref invocation.\n\nAs Peff rightly pointed out a few months back, we're actually almost\nthere to have that. So I took a look today and finally decided to\nimplement it while I was busy implementing another series. And in fact,\nit really didn't require a lof of changes.\n\nSo with this small series, it's now possible to start new transaction\nafter the current transaction has either been committed or aborted. E.g.\nsessions like the following are now possible:\n\n    > start\n    start: ok\n    > create refs/heads/test $A\n    > commit\n    commit: ok\n    > start\n    start: ok\n    > update refs/heads/test $B $A\n    > commit\n    commit: ok\n    > start\n    start: ok\n    > update refs/heads/test $A $B\n    > abort\n    abort: ok\n    > start\n    start: ok\n    > delete refs/heads/test $B\n    > commit\n    commit: ok\n\nI've also updated the performance test in p1400 to use this new\nmechanism, which is a much more direct way to test what we want to test\nin there compared to using not-atomic pushes.\n\nPatrick\n\nPatrick Steinhardt (2):\n  update-ref: Allow creation of multiple transactions\n  p1400: Use `git-update-ref --stdin` to test multiple transactions\n\n Documentation/git-update-ref.txt |  3 +-\n builtin/update-ref.c             | 13 ++++++++-\n t/perf/p1400-update-ref.sh       | 20 +++++--------\n t/t1400-update-ref.sh            | 50 ++++++++++++++++++++++++++++++++\n 4 files changed, 71 insertions(+), 15 deletions(-)\n\n-- \n2.29.2\n\n"},{"id":"409084","messageId":"eec7c2e8ec3e49b34066190d59fc45276bed637f.1604501265.git.ps@pks.im","threadId":"54571","inReplyTo":"cover.1604501265.git.ps@pks.im","subject":"[PATCH 1/2] update-ref: Allow creation of multiple transactions","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2020-11-04T14:57:17Z","receivedAt":"2020-11-04T14:57:24Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"While git-update-ref has recently grown commands which allow interactive\ncontrol of transactions in e48cf33b61 (update-ref: implement interactive\ntransaction handling, 2020-04-02), it is not yet possible to create\nmultiple transactions in a single session. To do so, one currently still\nneeds to invoke the executable multiple times.\n\nThis commit addresses this shortcoming by allowing the \"start\" command\nto create a new transaction if the current transaction has already been\neither committed or aborted.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n Documentation/git-update-ref.txt |  3 +-\n builtin/update-ref.c             | 13 ++++++++-\n t/t1400-update-ref.sh            | 50 ++++++++++++++++++++++++++++++++\n 3 files changed, 64 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt\nindex d401234b03..48b6683071 100644\n--- a/Documentation/git-update-ref.txt\n+++ b/Documentation/git-update-ref.txt\n@@ -125,7 +125,8 @@ option::\n start::\n \tStart a transaction. In contrast to a non-transactional session, a\n \ttransaction will automatically abort if the session ends without an\n-\texplicit commit.\n+\texplicit commit. This command may create a new empty transaction when\n+\tthe current one has been committed or aborted already.\n \n prepare::\n \tPrepare to commit the transaction. This will create lock files for all\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex 8a2df4459c..bb65129012 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -446,7 +446,18 @@ static void update_refs_stdin(void)\n \t\t\tstate = cmd->state;\n \t\t\tbreak;\n \t\tcase UPDATE_REFS_CLOSED:\n-\t\t\tdie(\"transaction is closed\");\n+\t\t\tif (cmd->state != UPDATE_REFS_STARTED)\n+\t\t\t\tdie(\"transaction is closed\");\n+\n+\t\t\t/*\n+\t\t\t * Open a new transaction if we're currently closed and\n+\t\t\t * get a \"start\".\n+\t\t\t */\n+\t\t\tstate = cmd->state;\n+\t\t\ttransaction = ref_transaction_begin(&err);\n+\t\t\tif (!transaction)\n+\t\t\t\tdie(\"%s\", err.buf);\n+\n \t\t\tbreak;\n \t\t}\n \ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex 4c01e08551..72d995aece 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -1526,4 +1526,54 @@ test_expect_success 'transaction with prepare aborts by default' '\n \ttest_path_is_missing .git/$b\n '\n \n+test_expect_success 'transaction can commit multiple times' '\n+\tcat >stdin <<-EOF &&\n+\tstart\n+\tcreate refs/heads/branch-1 $A\n+\tcommit\n+\tstart\n+\tcreate refs/heads/branch-2 $B\n+\tcommit\n+\tEOF\n+\tgit update-ref --stdin <stdin >actual &&\n+\tprintf \"%s: ok\\n\" start commit start commit >expect &&\n+\ttest_cmp expect actual &&\n+\techo \"$A\" >expect &&\n+\tgit rev-parse refs/heads/branch-1 >actual &&\n+\ttest_cmp expect actual &&\n+\techo \"$B\" >expect &&\n+\tgit rev-parse refs/heads/branch-2 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'transaction can create and delete' '\n+\tcat >stdin <<-EOF &&\n+\tstart\n+\tcreate refs/heads/create-and-delete $A\n+\tcommit\n+\tstart\n+\tdelete refs/heads/create-and-delete $A\n+\tcommit\n+\tEOF\n+\tgit update-ref --stdin <stdin >actual &&\n+\tprintf \"%s: ok\\n\" start commit start commit >expect &&\n+\ttest_path_is_missing .git/refs/heads/create-and-delete\n+'\n+\n+test_expect_success 'transaction can commit after abort' '\n+\tcat >stdin <<-EOF &&\n+\tstart\n+\tcreate refs/heads/abort $A\n+\tabort\n+\tstart\n+\tcreate refs/heads/abort $A\n+\tcommit\n+\tEOF\n+\tgit update-ref --stdin <stdin >actual &&\n+\tprintf \"%s: ok\\n\" start abort start commit >expect &&\n+\techo \"$A\" >expect &&\n+\tgit rev-parse refs/heads/abort >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.29.2\n\n"},{"id":"409085","messageId":"bfaac619112b04aa6545f229ea60433cbf8da73a.1604501265.git.ps@pks.im","threadId":"54571","inReplyTo":"cover.1604501265.git.ps@pks.im","subject":"[PATCH 2/2] p1400: Use `git-update-ref --stdin` to test multiple transactions","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2020-11-04T14:57:22Z","receivedAt":"2020-11-04T14:57:33Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In commit 0a0fbbe3ff (refs: remove lookup cache for\nreference-transaction hook, 2020-08-25), a new benchmark was added to\np1400 which has the intention to exercise creation of multiple\ntransactions in a single process. As git-update-ref wasn't yet able to\ncreate multiple transactions with a single run we instead used git-push.\nAs its non-atomic version creates a transaction per reference update,\nthis was the best approximation we could make at that point in time.\n\nNow that `git-update-ref --stdin` supports creation of multiple\ntransactions, let's convert the benchmark to use that instead. It has\nless overhead and it's also a lot clearer what the actual intention is.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n t/perf/p1400-update-ref.sh | 20 +++++++-------------\n 1 file changed, 7 insertions(+), 13 deletions(-)\n\ndiff --git a/t/perf/p1400-update-ref.sh b/t/perf/p1400-update-ref.sh\nindex ce5ac3ed85..dda8a74866 100755\n--- a/t/perf/p1400-update-ref.sh\n+++ b/t/perf/p1400-update-ref.sh\n@@ -7,13 +7,14 @@ test_description=\"Tests performance of update-ref\"\n test_perf_fresh_repo\n \n test_expect_success \"setup\" '\n-\tgit init --bare target-repo.git &&\n \ttest_commit PRE &&\n \ttest_commit POST &&\n-\tprintf \"create refs/heads/%d PRE\\n\" $(test_seq 1000) >create &&\n-\tprintf \"update refs/heads/%d POST PRE\\n\" $(test_seq 1000) >update &&\n-\tprintf \"delete refs/heads/%d POST\\n\" $(test_seq 1000) >delete &&\n-\tgit update-ref --stdin <create\n+\tfor i in $(test_seq 5000)\n+\tdo\n+\t\tprintf \"start\\ncreate refs/heads/%d PRE\\ncommit\\n\" $i &&\n+\t\tprintf \"start\\nupdate refs/heads/%d POST PRE\\ncommit\\n\" $i &&\n+\t\tprintf \"start\\ndelete refs/heads/%d POST\\ncommit\\n\" $i\n+\tdone >instructions\n '\n \n test_perf \"update-ref\" '\n@@ -26,14 +27,7 @@ test_perf \"update-ref\" '\n '\n \n test_perf \"update-ref --stdin\" '\n-\tgit update-ref --stdin <update &&\n-\tgit update-ref --stdin <delete &&\n-\tgit update-ref --stdin <create\n-'\n-\n-test_perf \"nonatomic push\" '\n-\tgit push ./target-repo.git $(test_seq 1000) &&\n-\tgit push --delete ./target-repo.git $(test_seq 1000)\n+\tgit update-ref --stdin <instructions >/dev/null\n '\n \n test_done\n-- \n2.29.2\n\n"},{"id":"409213","messageId":"20201105192901.GA121650@coredump.intra.peff.net","threadId":"54571","inReplyTo":"eec7c2e8ec3e49b34066190d59fc45276bed637f.1604501265.git.ps@pks.im","subject":"Re: [PATCH 1/2] update-ref: Allow creation of multiple transactions","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-11-05T19:29:01Z","receivedAt":"2020-11-05T19:29:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 04, 2020 at 03:57:17PM +0100, Patrick Steinhardt wrote:\n\n> While git-update-ref has recently grown commands which allow interactive\n> control of transactions in e48cf33b61 (update-ref: implement interactive\n> transaction handling, 2020-04-02), it is not yet possible to create\n> multiple transactions in a single session. To do so, one currently still\n> needs to invoke the executable multiple times.\n> \n> This commit addresses this shortcoming by allowing the \"start\" command\n> to create a new transaction if the current transaction has already been\n> either committed or aborted.\n\nThanks for working on this. The amount of change needed is indeed quite\npleasant.\n\n> diff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt\n> index d401234b03..48b6683071 100644\n> --- a/Documentation/git-update-ref.txt\n> +++ b/Documentation/git-update-ref.txt\n> @@ -125,7 +125,8 @@ option::\n>  start::\n>  \tStart a transaction. In contrast to a non-transactional session, a\n>  \ttransaction will automatically abort if the session ends without an\n> -\texplicit commit.\n> +\texplicit commit. This command may create a new empty transaction when\n> +\tthe current one has been committed or aborted already.\n\nReading this made me wonder what would happen if we send a \"start\" when\nthe current one _hasn't_ been committed or aborted. I.e., what does:\n\n  git update-ref --stdin <<EOF\n  start\n  create refs/heads/foo ...\n  start\n  commit\n  EOF\n\ndo? It turns out that the second start is ignored totally (and the\ncommit does indeed update foo). I wonder if we ought to complain about\nit. But that is completely orthogonal to your patch. The behavior is the\nsame before and after.\n\n> --- a/builtin/update-ref.c\n> +++ b/builtin/update-ref.c\n> @@ -446,7 +446,18 @@ static void update_refs_stdin(void)\n>  \t\t\tstate = cmd->state;\n>  \t\t\tbreak;\n>  \t\tcase UPDATE_REFS_CLOSED:\n> -\t\t\tdie(\"transaction is closed\");\n> +\t\t\tif (cmd->state != UPDATE_REFS_STARTED)\n> +\t\t\t\tdie(\"transaction is closed\");\n> +\n> +\t\t\t/*\n> +\t\t\t * Open a new transaction if we're currently closed and\n> +\t\t\t * get a \"start\".\n> +\t\t\t */\n> +\t\t\tstate = cmd->state;\n> +\t\t\ttransaction = ref_transaction_begin(&err);\n> +\t\t\tif (!transaction)\n> +\t\t\t\tdie(\"%s\", err.buf);\n> +\n\nVery nice. This duplicates the state and transaction setup at the start\nof the function, which made me wonder if we could do this:\n\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex bb65129012..140f0d30e9 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -385,14 +385,10 @@ static const struct parse_cmd {\n static void update_refs_stdin(void)\n {\n \tstruct strbuf input = STRBUF_INIT, err = STRBUF_INIT;\n-\tenum update_refs_state state = UPDATE_REFS_OPEN;\n+\tenum update_refs_state state = UPDATE_REFS_CLOSED;\n \tstruct ref_transaction *transaction;\n \tint i, j;\n \n-\ttransaction = ref_transaction_begin(&err);\n-\tif (!transaction)\n-\t\tdie(\"%s\", err.buf);\n-\n \t/* Read each line dispatch its command */\n \twhile (!strbuf_getwholeline(&input, stdin, line_termination)) {\n \t\tconst struct parse_cmd *cmd = NULL;\n\nand just have it auto-open. But of course that doesn't work because we\nmight not see an \"open\" command at all. Traditional callers will start\nwith create/update/etc, and our \"auto-open\" would complain.\n\n> +test_expect_success 'transaction can create and delete' '\n> +\tcat >stdin <<-EOF &&\n> +\tstart\n> +\tcreate refs/heads/create-and-delete $A\n> +\tcommit\n> +\tstart\n> +\tdelete refs/heads/create-and-delete $A\n> +\tcommit\n> +\tEOF\n> +\tgit update-ref --stdin <stdin >actual &&\n> +\tprintf \"%s: ok\\n\" start commit start commit >expect &&\n> +\ttest_path_is_missing .git/refs/heads/create-and-delete\n> +'\n\nThe tests all look quite reasonable to me. Touching .git/refs like this\nis a bit gross (and something we may have to deal with if we introduce\nreftables, etc). But it's pretty pervasive in this file, so matching\nthe existing style is the best option for now.\n\n-Peff\n"},{"id":"409214","messageId":"20201105193455.GB121650@coredump.intra.peff.net","threadId":"54571","inReplyTo":"bfaac619112b04aa6545f229ea60433cbf8da73a.1604501265.git.ps@pks.im","subject":"Re: [PATCH 2/2] p1400: Use `git-update-ref --stdin` to test multiple transactions","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-11-05T19:34:55Z","receivedAt":"2020-11-05T19:34:58Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 04, 2020 at 03:57:22PM +0100, Patrick Steinhardt wrote:\n\n> In commit 0a0fbbe3ff (refs: remove lookup cache for\n> reference-transaction hook, 2020-08-25), a new benchmark was added to\n> p1400 which has the intention to exercise creation of multiple\n> transactions in a single process. As git-update-ref wasn't yet able to\n> create multiple transactions with a single run we instead used git-push.\n> As its non-atomic version creates a transaction per reference update,\n> this was the best approximation we could make at that point in time.\n> \n> Now that `git-update-ref --stdin` supports creation of multiple\n> transactions, let's convert the benchmark to use that instead. It has\n> less overhead and it's also a lot clearer what the actual intention is.\n\nGood direction. The diff confused me for a moment...\n\n> @@ -26,14 +27,7 @@ test_perf \"update-ref\" '\n>  '\n>  \n>  test_perf \"update-ref --stdin\" '\n> -\tgit update-ref --stdin <update &&\n> -\tgit update-ref --stdin <delete &&\n> -\tgit update-ref --stdin <create\n> -'\n> -\n> -test_perf \"nonatomic push\" '\n> -\tgit push ./target-repo.git $(test_seq 1000) &&\n> -\tgit push --delete ./target-repo.git $(test_seq 1000)\n> +\tgit update-ref --stdin <instructions >/dev/null\n>  '\n\n...because we're dropping _two_ tests here. But I think they were\ntesting the same thing, just with varying degrees of quality.\n\nIt could possibly be useful to have perf results broken down by\noperation type (create vs delete vs update), but the original certainly\ndidn't do that. And it's not clear to me it would actually produce\ninteresting results; certainly not related to the hook, but possibly\nrelated to benchmarking ref updates in general. So I don't think it's\nworth worrying about.\n\n-Peff\n"},{"id":"409234","messageId":"xmqqa6vvij37.fsf@gitster.c.googlers.com","threadId":"54571","inReplyTo":"20201105192901.GA121650@coredump.intra.peff.net","subject":"Re: [PATCH 1/2] update-ref: Allow creation of multiple transactions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-11-05T21:34:20Z","receivedAt":"2020-11-05T21:34:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n>> +test_expect_success 'transaction can create and delete' '\n>> +\tcat >stdin <<-EOF &&\n>> +\tstart\n>> +\tcreate refs/heads/create-and-delete $A\n>> +\tcommit\n>> +\tstart\n>> +\tdelete refs/heads/create-and-delete $A\n>> +\tcommit\n>> +\tEOF\n>> +\tgit update-ref --stdin <stdin >actual &&\n>> +\tprintf \"%s: ok\\n\" start commit start commit >expect &&\n>> +\ttest_path_is_missing .git/refs/heads/create-and-delete\n>> +'\n>\n> The tests all look quite reasonable to me. Touching .git/refs like this\n> is a bit gross (and something we may have to deal with if we introduce\n> reftables, etc). But it's pretty pervasive in this file, so matching\n> the existing style is the best option for now.\n\n\nShouldn't \"git show-ref --verify\" be usable portably across ref backends\nto test if a well-formed ref was created (or was not created)?\n\nOn the ref-creation side, there are cases where we need to directly\nfutz with the filesystem entity.  For example, \"git update-ref\"\ncannot be used to place a non-commit at \"refs/heads/foo\", so\nsomething like\n\n\tgit rev-parse HEAD^{tree} >.git/refs/heads/bad-branch\n\ncannot be avoided (this is a tangent but we probably should add a\nway to force setting _any_ value to any ref, that may not even point\nat an existing object or an object of a wrong type, to help test\nscripts).\n\nBut I do not think this is such a case.\n"},{"id":"409268","messageId":"20201106063653.GA3692@ncase","threadId":"54571","inReplyTo":"20201105192901.GA121650@coredump.intra.peff.net","subject":"Re: [PATCH 1/2] update-ref: Allow creation of multiple transactions","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2020-11-06T06:36:53Z","receivedAt":"2020-11-06T06:37:03Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Nov 05, 2020 at 02:29:01PM -0500, Jeff King wrote:\n> On Wed, Nov 04, 2020 at 03:57:17PM +0100, Patrick Steinhardt wrote:\n> \n> > While git-update-ref has recently grown commands which allow interactive\n> > control of transactions in e48cf33b61 (update-ref: implement interactive\n> > transaction handling, 2020-04-02), it is not yet possible to create\n> > multiple transactions in a single session. To do so, one currently still\n> > needs to invoke the executable multiple times.\n> > \n> > This commit addresses this shortcoming by allowing the \"start\" command\n> > to create a new transaction if the current transaction has already been\n> > either committed or aborted.\n> \n> Thanks for working on this. The amount of change needed is indeed quite\n> pleasant.\n> \n> > diff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt\n> > index d401234b03..48b6683071 100644\n> > --- a/Documentation/git-update-ref.txt\n> > +++ b/Documentation/git-update-ref.txt\n> > @@ -125,7 +125,8 @@ option::\n> >  start::\n> >  \tStart a transaction. In contrast to a non-transactional session, a\n> >  \ttransaction will automatically abort if the session ends without an\n> > -\texplicit commit.\n> > +\texplicit commit. This command may create a new empty transaction when\n> > +\tthe current one has been committed or aborted already.\n> \n> Reading this made me wonder what would happen if we send a \"start\" when\n> the current one _hasn't_ been committed or aborted. I.e., what does:\n> \n>   git update-ref --stdin <<EOF\n>   start\n>   create refs/heads/foo ...\n>   start\n>   commit\n>   EOF\n> \n> do? It turns out that the second start is ignored totally (and the\n> commit does indeed update foo). I wonder if we ought to complain about\n> it. But that is completely orthogonal to your patch. The behavior is the\n> same before and after.\n\nAgreed, that's a case where we should raise an error. Doing nothing\nwithout any indication is a bad way of handling it.\n\nPatrick\n"},{"id":"409279","messageId":"20201106175241.GA182486@coredump.intra.peff.net","threadId":"54571","inReplyTo":"xmqqa6vvij37.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 1/2] update-ref: Allow creation of multiple transactions","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-11-06T17:52:41Z","receivedAt":"2020-11-06T17:52:43Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Nov 05, 2020 at 01:34:20PM -0800, Junio C Hamano wrote:\n\n> > The tests all look quite reasonable to me. Touching .git/refs like this\n> > is a bit gross (and something we may have to deal with if we introduce\n> > reftables, etc). But it's pretty pervasive in this file, so matching\n> > the existing style is the best option for now.\n> \n> \n> Shouldn't \"git show-ref --verify\" be usable portably across ref backends\n> to test if a well-formed ref was created (or was not created)?\n> \n> On the ref-creation side, there are cases where we need to directly\n> futz with the filesystem entity.  For example, \"git update-ref\"\n> cannot be used to place a non-commit at \"refs/heads/foo\", so\n> something like\n> \n> \tgit rev-parse HEAD^{tree} >.git/refs/heads/bad-branch\n> \n> cannot be avoided (this is a tangent but we probably should add a\n> way to force setting _any_ value to any ref, that may not even point\n> at an existing object or an object of a wrong type, to help test\n> scripts).\n> \n> But I do not think this is such a case.\n\nYeah, I agree completely that we could be using rev-parse in this\ninstance. But it's definitely not alone there:\n\n  $ git grep -c test_path_is.*.git/refs t/t1400-update-ref.sh\n  t/t1400-update-ref.sh:13\n\nSo it is a question of \"do an ugly thing that fits in with neighbors\" or\n\"be inconsistent but set a good example\". And I am OK with either. Of\ncourse, \"improve the neighbors on top\" would be better still. :)\n\n-Peff\n\nPS And yeah, I agree in the long run we may need some mechanism to\n   override internal safeties in order to test broken cases with\n   reftable. We have sometimes resorted to manually munging on-disk\n   files in similar tests (e.g., for broken packs, etc), but it gets\n   rather tricky.\n"},{"id":"409289","messageId":"xmqqeel6ffll.fsf@gitster.c.googlers.com","threadId":"54571","inReplyTo":"20201106175241.GA182486@coredump.intra.peff.net","subject":"Re: [PATCH 1/2] update-ref: Allow creation of multiple transactions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-11-06T19:30:14Z","receivedAt":"2020-11-06T19:30:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Thu, Nov 05, 2020 at 01:34:20PM -0800, Junio C Hamano wrote:\n>\n>> > The tests all look quite reasonable to me. Touching .git/refs like this\n>> > is a bit gross (and something we may have to deal with if we introduce\n>> > reftables, etc). But it's pretty pervasive in this file, so matching\n>> > the existing style is the best option for now.\n>>  ...\n> Yeah, I agree completely that we could be using rev-parse in this\n> instance. But it's definitely not alone there:\n> ...\n\nYup, this morning I was reviewing what we said in the previous day's\nexchanges and noticed that you weren't advocating but merely saying\nit is not making things worse, and I agree with the assessment.\n\nPerhaps two #leftoverbits are to \n\n (1) clean up this test to create refs using \"update-ref\", and\n     verify refs using \"show-ref --verify\".\n\n (2) If (1) had to leave some direct filesystem access due to the\n     built-in safety that cannot be circumvented, decide which is\n     more appropirate between a test-update-ref test helper only to\n     be used in tests, or a \"--force\" option usable to corrupt\n     repositories with \"update-ref\", implement it, and use it to\n     finish cleaning up tests.\n\nThanks.\n\n\n\n\n\n"},{"id":"409380","messageId":"cover.1604908834.git.ps@pks.im","threadId":"54571","inReplyTo":"cover.1604501265.git.ps@pks.im","subject":"[PATCH v2 0/4] update-ref: Allow creation of multiple transactions","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2020-11-09T10:06:43Z","receivedAt":"2020-11-09T10:06:52Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nthis is the second version of this patch series implementing support for\ncreation of multiple reference transactions in a single git-update-ref\nprocess.\n\nThere's two changes compared to v1:\n\n    - A small refactoring of t1400, which refactors many tests to not\n      touch references via the filesystem but instead to use\n      git-update-ref and git-show-ref. There's still tests in there\n      which do, but converting them is harder as they create and read\n      broken references.\n\n    - I've added another commit on top which disallows restarting of\n      transactions. E.g. writing \"start\\nstart\\n\" to git-update-ref is\n      now going to fail.\n\nPatrick\n\nPatrick Steinhardt (4):\n  t1400: Avoid touching refs on filesystem\n  update-ref: Allow creation of multiple transactions\n  p1400: Use `git-update-ref --stdin` to test multiple transactions\n  update-ref: Disallow restart of ongoing transactions\n\n Documentation/git-update-ref.txt |   3 +-\n builtin/update-ref.c             |  15 +++-\n t/perf/p1400-update-ref.sh       |  20 ++---\n t/t1400-update-ref.sh            | 124 ++++++++++++++++++++++++-------\n 4 files changed, 119 insertions(+), 43 deletions(-)\n\n-- \n2.29.2\n\n"},{"id":"409381","messageId":"9b49e849eaf6786c63016d767d2ad56112d08d51.1604908834.git.ps@pks.im","threadId":"54571","inReplyTo":"cover.1604908834.git.ps@pks.im","subject":"[PATCH v2 1/4] t1400: Avoid touching refs on filesystem","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2020-11-09T10:06:47Z","receivedAt":"2020-11-09T10:06:54Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The testcase t1400 exercises the git-update-ref(1) utility. To do so,\nmany tests directly read and write references via the filesystem,\nassuming that we always use loose and/or packed references. While this\nis true now, it'll change with the introduction of the reftable backend.\n\nConvert those tests to use git-update-ref(1) and git-show-ref(1) where\npossible. As some tests exercise behaviour with broken references and\nneither of those tools actually allows writing or reading broken\nreferences, this commit doesn't adjust all tests.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n t/t1400-update-ref.sh | 63 ++++++++++++++++++++++++-------------------\n 1 file changed, 35 insertions(+), 28 deletions(-)\n\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex 4c01e08551..957bef272d 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -14,6 +14,12 @@ n=$n_dir/fixes\n outside=refs/foo\n bare=bare-repo\n \n+# Some of the tests delete HEAD, which causes us to not treat the current\n+# working directory as a Git repository anymore. To avoid using any potential\n+# parent repository to be discovered, we need to set up the ceiling directories.\n+GIT_CEILING_DIRECTORIES=\"$PWD/..\"\n+export GIT_CEILING_DIRECTORIES\n+\n create_test_commits ()\n {\n \tprfx=\"$1\"\n@@ -48,17 +54,17 @@ test_expect_success \"fail to delete $m with stale ref\" '\n \ttest $B = \"$(git show-ref -s --verify $m)\"\n '\n test_expect_success \"delete $m\" '\n-\ttest_when_finished \"rm -f .git/$m\" &&\n+\ttest_when_finished \"git update-ref -d $m\" &&\n \tgit update-ref -d $m $B &&\n-\ttest_path_is_missing .git/$m\n+\ttest_must_fail git show-ref --verify -q $m\n '\n \n test_expect_success \"delete $m without oldvalue verification\" '\n-\ttest_when_finished \"rm -f .git/$m\" &&\n+\ttest_when_finished \"git update-ref -d $m\" &&\n \tgit update-ref $m $A &&\n \ttest $A = $(git show-ref -s --verify $m) &&\n \tgit update-ref -d $m &&\n-\ttest_path_is_missing .git/$m\n+\ttest_must_fail git show-ref --verify -q $m\n '\n \n test_expect_success \"fail to create $n\" '\n@@ -80,26 +86,26 @@ test_expect_success \"fail to delete $m (by HEAD) with stale ref\" '\n \ttest $B = $(git show-ref -s --verify $m)\n '\n test_expect_success \"delete $m (by HEAD)\" '\n-\ttest_when_finished \"rm -f .git/$m\" &&\n+\ttest_when_finished \"git update-ref -d $m\" &&\n \tgit update-ref -d HEAD $B &&\n-\ttest_path_is_missing .git/$m\n+\ttest_must_fail git show-ref --verify -q $m\n '\n \n test_expect_success \"deleting current branch adds message to HEAD's log\" '\n-\ttest_when_finished \"rm -f .git/$m\" &&\n+\ttest_when_finished \"git update-ref -d $m\" &&\n \tgit update-ref $m $A &&\n \tgit symbolic-ref HEAD $m &&\n \tgit update-ref -m delete-$m -d $m &&\n-\ttest_path_is_missing .git/$m &&\n+\ttest_must_fail git show-ref --verify -q $m &&\n \tgrep \"delete-$m$\" .git/logs/HEAD\n '\n \n test_expect_success \"deleting by HEAD adds message to HEAD's log\" '\n-\ttest_when_finished \"rm -f .git/$m\" &&\n+\ttest_when_finished \"git update-ref -d $m\" &&\n \tgit update-ref $m $A &&\n \tgit symbolic-ref HEAD $m &&\n \tgit update-ref -m delete-by-head -d HEAD &&\n-\ttest_path_is_missing .git/$m &&\n+\ttest_must_fail git show-ref --verify -q $m &&\n \tgrep \"delete-by-head$\" .git/logs/HEAD\n '\n \n@@ -188,17 +194,17 @@ test_expect_success \"move $m (by HEAD)\" '\n \ttest $B = $(git show-ref -s --verify $m)\n '\n test_expect_success \"delete $m (by HEAD) should remove both packed and loose $m\" '\n-\ttest_when_finished \"rm -f .git/$m\" &&\n+\ttest_when_finished \"rm -f git update-ref -d $m\" &&\n \tgit update-ref -d HEAD $B &&\n \t! grep \"$m\" .git/packed-refs &&\n-\ttest_path_is_missing .git/$m\n+\ttest_must_fail git show-ref --verify -q $m\n '\n \n cp -f .git/HEAD .git/HEAD.orig\n test_expect_success 'delete symref without dereference' '\n \ttest_when_finished \"cp -f .git/HEAD.orig .git/HEAD\" &&\n \tgit update-ref --no-deref -d HEAD &&\n-\ttest_path_is_missing .git/HEAD\n+\ttest_must_fail git show-ref --verify -q HEAD\n '\n \n test_expect_success 'delete symref without dereference when the referred ref is packed' '\n@@ -208,7 +214,7 @@ test_expect_success 'delete symref without dereference when the referred ref is\n \tgit commit -m foo &&\n \tgit pack-refs --all &&\n \tgit update-ref --no-deref -d HEAD &&\n-\ttest_path_is_missing .git/HEAD\n+\ttest_must_fail git show-ref --verify -q HEAD\n '\n \n git update-ref -d $m\n@@ -226,25 +232,25 @@ test_expect_success 'update-ref --no-deref -d can delete self-reference' '\n \ttest_when_finished \"rm -f .git/refs/heads/self\" &&\n \ttest_path_is_file .git/refs/heads/self &&\n \tgit update-ref --no-deref -d refs/heads/self &&\n-\ttest_path_is_missing .git/refs/heads/self\n+\ttest_must_fail git show-ref --verify -q refs/heads/self\n '\n \n test_expect_success 'update-ref --no-deref -d can delete reference to bad ref' '\n \t>.git/refs/heads/bad &&\n \ttest_when_finished \"rm -f .git/refs/heads/bad\" &&\n \tgit symbolic-ref refs/heads/ref-to-bad refs/heads/bad &&\n-\ttest_when_finished \"rm -f .git/refs/heads/ref-to-bad\" &&\n+\ttest_when_finished \"git update-ref -d refs/heads/ref-to-bad\" &&\n \ttest_path_is_file .git/refs/heads/ref-to-bad &&\n \tgit update-ref --no-deref -d refs/heads/ref-to-bad &&\n-\ttest_path_is_missing .git/refs/heads/ref-to-bad\n+\ttest_must_fail git show-ref --verify -q refs/heads/ref-to-bad\n '\n \n test_expect_success '(not) create HEAD with old sha1' '\n \ttest_must_fail git update-ref HEAD $A $B\n '\n test_expect_success \"(not) prior created .git/$m\" '\n-\ttest_when_finished \"rm -f .git/$m\" &&\n-\ttest_path_is_missing .git/$m\n+\ttest_when_finished \"git update-ref -d $m\" &&\n+\ttest_must_fail git show-ref --verify -q $m\n '\n \n test_expect_success 'create HEAD' '\n@@ -254,7 +260,7 @@ test_expect_success '(not) change HEAD with wrong SHA1' '\n \ttest_must_fail git update-ref HEAD $B $Z\n '\n test_expect_success \"(not) changed .git/$m\" '\n-\ttest_when_finished \"rm -f .git/$m\" &&\n+\ttest_when_finished \"git update-ref -d $m\" &&\n \t! test $B = $(git show-ref -s --verify $m)\n '\n \n@@ -284,8 +290,8 @@ test_expect_success 'empty directory removal' '\n \ttest_path_is_file .git/refs/heads/d1/d2/r1 &&\n \ttest_path_is_file .git/logs/refs/heads/d1/d2/r1 &&\n \tgit branch -d d1/d2/r1 &&\n-\ttest_path_is_missing .git/refs/heads/d1/d2 &&\n-\ttest_path_is_missing .git/logs/refs/heads/d1/d2 &&\n+\ttest_must_fail git show-ref --verify -q refs/heads/d1/d2 &&\n+\ttest_must_fail git show-ref --verify -q logs/refs/heads/d1/d2 &&\n \ttest_path_is_file .git/refs/heads/d1/r2 &&\n \ttest_path_is_file .git/logs/refs/heads/d1/r2\n '\n@@ -298,8 +304,8 @@ test_expect_success 'symref empty directory removal' '\n \ttest_path_is_file .git/refs/heads/e1/e2/r1 &&\n \ttest_path_is_file .git/logs/refs/heads/e1/e2/r1 &&\n \tgit update-ref -d HEAD &&\n-\ttest_path_is_missing .git/refs/heads/e1/e2 &&\n-\ttest_path_is_missing .git/logs/refs/heads/e1/e2 &&\n+\ttest_must_fail git show-ref --verify -q refs/heads/e1/e2 &&\n+\ttest_must_fail git show-ref --verify -q logs/refs/heads/e1/e2 &&\n \ttest_path_is_file .git/refs/heads/e1/r2 &&\n \ttest_path_is_file .git/logs/refs/heads/e1/r2 &&\n \ttest_path_is_file .git/logs/HEAD\n@@ -1388,7 +1394,8 @@ test_expect_success 'handle per-worktree refs in refs/bisect' '\n \t\tgit rev-parse refs/bisect/something >../worktree-head &&\n \t\tgit for-each-ref | grep refs/bisect/something\n \t) &&\n-\ttest_path_is_missing .git/refs/bisect &&\n+\tgit show-ref >actual &&\n+\t! grep 'refs/bisect' actual &&\n \ttest_must_fail git rev-parse refs/bisect/something &&\n \tgit update-ref refs/bisect/something HEAD &&\n \tgit rev-parse refs/bisect/something >main-head &&\n@@ -1500,7 +1507,7 @@ test_expect_success 'transaction can handle abort' '\n \tgit update-ref --stdin <stdin >actual &&\n \tprintf \"%s: ok\\n\" start abort >expect &&\n \ttest_cmp expect actual &&\n-\ttest_path_is_missing .git/$b\n+\ttest_must_fail git show-ref --verify -q $b\n '\n \n test_expect_success 'transaction aborts by default' '\n@@ -1511,7 +1518,7 @@ test_expect_success 'transaction aborts by default' '\n \tgit update-ref --stdin <stdin >actual &&\n \tprintf \"%s: ok\\n\" start >expect &&\n \ttest_cmp expect actual &&\n-\ttest_path_is_missing .git/$b\n+\ttest_must_fail git show-ref --verify -q $b\n '\n \n test_expect_success 'transaction with prepare aborts by default' '\n@@ -1523,7 +1530,7 @@ test_expect_success 'transaction with prepare aborts by default' '\n \tgit update-ref --stdin <stdin >actual &&\n \tprintf \"%s: ok\\n\" start prepare >expect &&\n \ttest_cmp expect actual &&\n-\ttest_path_is_missing .git/$b\n+\ttest_must_fail git show-ref --verify -q $b\n '\n \n test_done\n-- \n2.29.2\n\n"},{"id":"409382","messageId":"d2e3ec46cb1f41d8018d7b2e50b3d1a5db5c957a.1604908834.git.ps@pks.im","threadId":"54571","inReplyTo":"cover.1604908834.git.ps@pks.im","subject":"[PATCH v2 2/4] update-ref: Allow creation of multiple transactions","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2020-11-09T10:06:52Z","receivedAt":"2020-11-09T10:06:58Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"While git-update-ref has recently grown commands which allow interactive\ncontrol of transactions in e48cf33b61 (update-ref: implement interactive\ntransaction handling, 2020-04-02), it is not yet possible to create\nmultiple transactions in a single session. To do so, one currently still\nneeds to invoke the executable multiple times.\n\nThis commit addresses this shortcoming by allowing the \"start\" command\nto create a new transaction if the current transaction has already been\neither committed or aborted.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n Documentation/git-update-ref.txt |  3 +-\n builtin/update-ref.c             | 13 ++++++++-\n t/t1400-update-ref.sh            | 50 ++++++++++++++++++++++++++++++++\n 3 files changed, 64 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt\nindex d401234b03..48b6683071 100644\n--- a/Documentation/git-update-ref.txt\n+++ b/Documentation/git-update-ref.txt\n@@ -125,7 +125,8 @@ option::\n start::\n \tStart a transaction. In contrast to a non-transactional session, a\n \ttransaction will automatically abort if the session ends without an\n-\texplicit commit.\n+\texplicit commit. This command may create a new empty transaction when\n+\tthe current one has been committed or aborted already.\n \n prepare::\n \tPrepare to commit the transaction. This will create lock files for all\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex 8a2df4459c..bb65129012 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -446,7 +446,18 @@ static void update_refs_stdin(void)\n \t\t\tstate = cmd->state;\n \t\t\tbreak;\n \t\tcase UPDATE_REFS_CLOSED:\n-\t\t\tdie(\"transaction is closed\");\n+\t\t\tif (cmd->state != UPDATE_REFS_STARTED)\n+\t\t\t\tdie(\"transaction is closed\");\n+\n+\t\t\t/*\n+\t\t\t * Open a new transaction if we're currently closed and\n+\t\t\t * get a \"start\".\n+\t\t\t */\n+\t\t\tstate = cmd->state;\n+\t\t\ttransaction = ref_transaction_begin(&err);\n+\t\t\tif (!transaction)\n+\t\t\t\tdie(\"%s\", err.buf);\n+\n \t\t\tbreak;\n \t\t}\n \ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex 957bef272d..7ed41bb328 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -1533,4 +1533,54 @@ test_expect_success 'transaction with prepare aborts by default' '\n \ttest_must_fail git show-ref --verify -q $b\n '\n \n+test_expect_success 'transaction can commit multiple times' '\n+\tcat >stdin <<-EOF &&\n+\tstart\n+\tcreate refs/heads/branch-1 $A\n+\tcommit\n+\tstart\n+\tcreate refs/heads/branch-2 $B\n+\tcommit\n+\tEOF\n+\tgit update-ref --stdin <stdin >actual &&\n+\tprintf \"%s: ok\\n\" start commit start commit >expect &&\n+\ttest_cmp expect actual &&\n+\techo \"$A\" >expect &&\n+\tgit rev-parse refs/heads/branch-1 >actual &&\n+\ttest_cmp expect actual &&\n+\techo \"$B\" >expect &&\n+\tgit rev-parse refs/heads/branch-2 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'transaction can create and delete' '\n+\tcat >stdin <<-EOF &&\n+\tstart\n+\tcreate refs/heads/create-and-delete $A\n+\tcommit\n+\tstart\n+\tdelete refs/heads/create-and-delete $A\n+\tcommit\n+\tEOF\n+\tgit update-ref --stdin <stdin >actual &&\n+\tprintf \"%s: ok\\n\" start commit start commit >expect &&\n+\ttest_must_fail git show-ref --verify refs/heads/create-and-delete\n+'\n+\n+test_expect_success 'transaction can commit after abort' '\n+\tcat >stdin <<-EOF &&\n+\tstart\n+\tcreate refs/heads/abort $A\n+\tabort\n+\tstart\n+\tcreate refs/heads/abort $A\n+\tcommit\n+\tEOF\n+\tgit update-ref --stdin <stdin >actual &&\n+\tprintf \"%s: ok\\n\" start abort start commit >expect &&\n+\techo \"$A\" >expect &&\n+\tgit rev-parse refs/heads/abort >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.29.2\n\n"},{"id":"409383","messageId":"e3d61612dba01a297ad9d0471cd5a804e0425339.1604908834.git.ps@pks.im","threadId":"54571","inReplyTo":"cover.1604908834.git.ps@pks.im","subject":"[PATCH v2 3/4] p1400: Use `git-update-ref --stdin` to test multiple transactions","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2020-11-09T10:06:57Z","receivedAt":"2020-11-09T10:07:02Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In commit 0a0fbbe3ff (refs: remove lookup cache for\nreference-transaction hook, 2020-08-25), a new benchmark was added to\np1400 which has the intention to exercise creation of multiple\ntransactions in a single process. As git-update-ref wasn't yet able to\ncreate multiple transactions with a single run we instead used git-push.\nAs its non-atomic version creates a transaction per reference update,\nthis was the best approximation we could make at that point in time.\n\nNow that `git-update-ref --stdin` supports creation of multiple\ntransactions, let's convert the benchmark to use that instead. It has\nless overhead and it's also a lot clearer what the actual intention is.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n t/perf/p1400-update-ref.sh | 20 +++++++-------------\n 1 file changed, 7 insertions(+), 13 deletions(-)\n\ndiff --git a/t/perf/p1400-update-ref.sh b/t/perf/p1400-update-ref.sh\nindex ce5ac3ed85..dda8a74866 100755\n--- a/t/perf/p1400-update-ref.sh\n+++ b/t/perf/p1400-update-ref.sh\n@@ -7,13 +7,14 @@ test_description=\"Tests performance of update-ref\"\n test_perf_fresh_repo\n \n test_expect_success \"setup\" '\n-\tgit init --bare target-repo.git &&\n \ttest_commit PRE &&\n \ttest_commit POST &&\n-\tprintf \"create refs/heads/%d PRE\\n\" $(test_seq 1000) >create &&\n-\tprintf \"update refs/heads/%d POST PRE\\n\" $(test_seq 1000) >update &&\n-\tprintf \"delete refs/heads/%d POST\\n\" $(test_seq 1000) >delete &&\n-\tgit update-ref --stdin <create\n+\tfor i in $(test_seq 5000)\n+\tdo\n+\t\tprintf \"start\\ncreate refs/heads/%d PRE\\ncommit\\n\" $i &&\n+\t\tprintf \"start\\nupdate refs/heads/%d POST PRE\\ncommit\\n\" $i &&\n+\t\tprintf \"start\\ndelete refs/heads/%d POST\\ncommit\\n\" $i\n+\tdone >instructions\n '\n \n test_perf \"update-ref\" '\n@@ -26,14 +27,7 @@ test_perf \"update-ref\" '\n '\n \n test_perf \"update-ref --stdin\" '\n-\tgit update-ref --stdin <update &&\n-\tgit update-ref --stdin <delete &&\n-\tgit update-ref --stdin <create\n-'\n-\n-test_perf \"nonatomic push\" '\n-\tgit push ./target-repo.git $(test_seq 1000) &&\n-\tgit push --delete ./target-repo.git $(test_seq 1000)\n+\tgit update-ref --stdin <instructions >/dev/null\n '\n \n test_done\n-- \n2.29.2\n\n"},{"id":"409384","messageId":"d9abffef71466a0752c210878bb6b8fca7a72287.1604908834.git.ps@pks.im","threadId":"54571","inReplyTo":"cover.1604908834.git.ps@pks.im","subject":"[PATCH v2 4/4] update-ref: Disallow restart of ongoing transactions","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2020-11-09T10:07:01Z","receivedAt":"2020-11-09T10:07:06Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"It is currently possible to write multiple \"start\" commands into\ngit-update-ref(1) for a single session, which doesn't make a lot of\nsense to do in the first place. It's also not quite obvious what should\nactually happen. Would this just go on with the current transaction as\nif nothing was written or would it recreate a new session which doesn't\nyet have any references? Silently ignoring this usage isn't helping\neither as it may indicate erroneous use of the interface.\n\nThis commit catches this use and instead raises an error if the user is\ntrying to restart an ongoing transaction.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n builtin/update-ref.c  |  2 ++\n t/t1400-update-ref.sh | 11 +++++++++++\n 2 files changed, 13 insertions(+)\n\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex bb65129012..6029a80544 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -436,6 +436,8 @@ static void update_refs_stdin(void)\n \t\tswitch (state) {\n \t\tcase UPDATE_REFS_OPEN:\n \t\tcase UPDATE_REFS_STARTED:\n+\t\t\tif (state == UPDATE_REFS_STARTED && cmd->state == UPDATE_REFS_STARTED)\n+\t\t\t\tdie(\"cannot restart ongoing transaction\");\n \t\t\t/* Do not downgrade a transaction to a non-transaction. */\n \t\t\tif (cmd->state >= state)\n \t\t\t\tstate = cmd->state;\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex 7ed41bb328..e53d973d04 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -1583,4 +1583,15 @@ test_expect_success 'transaction can commit after abort' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'transaction cannot restart ongoing transaction' '\n+\tcat >stdin <<-EOF &&\n+\tstart\n+\tcreate refs/heads/restart $A\n+\tstart\n+\tcommit\n+\tEOF\n+\ttest_must_fail git update-ref --stdin <stdin >actual &&\n+\ttest_must_fail git show-ref --verify refs/heads/restart\n+'\n+\n test_done\n-- \n2.29.2\n\n"},{"id":"409405","messageId":"xmqqpn4mb9bv.fsf@gitster.c.googlers.com","threadId":"54571","inReplyTo":"9b49e849eaf6786c63016d767d2ad56112d08d51.1604908834.git.ps@pks.im","subject":"Re: [PATCH v2 1/4] t1400: Avoid touching refs on filesystem","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-11-09T19:48:20Z","receivedAt":"2020-11-09T19:48:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> The testcase t1400 exercises the git-update-ref(1) utility. To do so,\n> many tests directly read and write references via the filesystem,\n> assuming that we always use loose and/or packed references. While this\n> is true now, it'll change with the introduction of the reftable backend.\n>\n> Convert those tests to use git-update-ref(1) and git-show-ref(1) where\n> possible. As some tests exercise behaviour with broken references and\n> neither of those tools actually allows writing or reading broken\n> references, this commit doesn't adjust all tests.\n>\n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>  t/t1400-update-ref.sh | 63 ++++++++++++++++++++++++-------------------\n>  1 file changed, 35 insertions(+), 28 deletions(-)\n>\n> diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\n> index 4c01e08551..957bef272d 100755\n> --- a/t/t1400-update-ref.sh\n> +++ b/t/t1400-update-ref.sh\n> @@ -14,6 +14,12 @@ n=$n_dir/fixes\n>  outside=refs/foo\n>  bare=bare-repo\n>  \n> +# Some of the tests delete HEAD, which causes us to not treat the current\n> +# working directory as a Git repository anymore. To avoid using any potential\n> +# parent repository to be discovered, we need to set up the ceiling directories.\n> +GIT_CEILING_DIRECTORIES=\"$PWD/..\"\n> +export GIT_CEILING_DIRECTORIES\n> +\n\nInteresting.  The current tests do not need to do this because the\nHEAD-less broken state is transitory and is corrected without using\n\"git\" at all (e.g. echoing a valid value into .git/HEAD), I presume?\n\n>  create_test_commits ()\n>  {\n>  \tprfx=\"$1\"\n> @@ -48,17 +54,17 @@ test_expect_success \"fail to delete $m with stale ref\" '\n>  \ttest $B = \"$(git show-ref -s --verify $m)\"\n>  '\n>  test_expect_success \"delete $m\" '\n> -\ttest_when_finished \"rm -f .git/$m\" &&\n> +\ttest_when_finished \"git update-ref -d $m\" &&\n>  \tgit update-ref -d $m $B &&\n> -\ttest_path_is_missing .git/$m\n> +\ttest_must_fail git show-ref --verify -q $m\n>  '\n>  \n>  test_expect_success \"delete $m without oldvalue verification\" '\n> -\ttest_when_finished \"rm -f .git/$m\" &&\n> +\ttest_when_finished \"git update-ref -d $m\" &&\n>  \tgit update-ref $m $A &&\n>  \ttest $A = $(git show-ref -s --verify $m) &&\n>  \tgit update-ref -d $m &&\n> -\ttest_path_is_missing .git/$m\n> +\ttest_must_fail git show-ref --verify -q $m\n>  '\n>  \n>  test_expect_success \"fail to create $n\" '\n> @@ -80,26 +86,26 @@ test_expect_success \"fail to delete $m (by HEAD) with stale ref\" '\n>  \ttest $B = $(git show-ref -s --verify $m)\n>  '\n>  test_expect_success \"delete $m (by HEAD)\" '\n> -\ttest_when_finished \"rm -f .git/$m\" &&\n> +\ttest_when_finished \"git update-ref -d $m\" &&\n>  \tgit update-ref -d HEAD $B &&\n> -\ttest_path_is_missing .git/$m\n> +\ttest_must_fail git show-ref --verify -q $m\n>  '\n\nDuring the above test, we are on the branch ${m#refs/heads/}, so\n\"update-ref -d HEAD\" is removing the underlying branch ref, making\nit an unborn branch, without destroying the repository, so this is\nperfectly sensible.\n\nThis is a tangent, but what makes this test doubly interesting is\nthat \"git update-ref -d HEAD\" would have allowed us to make it a\nnon-repository if HEAD were detached, and it seems that we do not\nrequire \"--force\" to do so.  We probably should forbid removing HEAD\nthat id detached without \"--force\", which is such a destructive\noperation.\n\n>  cp -f .git/HEAD .git/HEAD.orig\n>  test_expect_success 'delete symref without dereference' '\n>  \ttest_when_finished \"cp -f .git/HEAD.orig .git/HEAD\" &&\n>  \tgit update-ref --no-deref -d HEAD &&\n> -\ttest_path_is_missing .git/HEAD\n> +\ttest_must_fail git show-ref --verify -q HEAD\n>  '\n\nThis is an example of breaking the repository.  I am not sure if the\ntest_must_fail is a good replacement--it would fail even if you say\n\"git show-ref --verify -q refs/heads/$branch\" where $branch is a\nname of a branch that exists, no?\n\nFor now, I think this is fine, but we'd probably clean it up so that\nwithout --force update-ref would not corrupt the repository like\nthis.  When used with --force, or before adding such a safety\nmeasure, how we test if we successfully corrupted the repository is\nan interesting matter.  I'd say\n\n\tgit update-ref --force --no-deref -d HEAD &&\n\ttest_must_fail git show-ref --verify -q HEAD &&\n\tcp -f .git/HEAD.orig .git/HEAD &&\n\tgit show-ref --verify -q \"$m\"\n\nto ensure that other than removing HEAD and corrupting the\nrepository, it did not cause permanent damage by removing the\nunderlying ref, perhaps.\n\nWe may want to add some NEEDSWORK comment around such tests.\n\n>  test_expect_success 'delete symref without dereference when the referred ref is packed' '\n> @@ -208,7 +214,7 @@ test_expect_success 'delete symref without dereference when the referred ref is\n>  \tgit commit -m foo &&\n>  \tgit pack-refs --all &&\n>  \tgit update-ref --no-deref -d HEAD &&\n> -\ttest_path_is_missing .git/HEAD\n> +\ttest_must_fail git show-ref --verify -q HEAD\n>  '\n\nDoes this share the same issue as the above?\n\nThanks.\n"},{"id":"409407","messageId":"xmqqlffab92r.fsf@gitster.c.googlers.com","threadId":"54571","inReplyTo":"d9abffef71466a0752c210878bb6b8fca7a72287.1604908834.git.ps@pks.im","subject":"Re: [PATCH v2 4/4] update-ref: Disallow restart of ongoing transactions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-11-09T19:53:48Z","receivedAt":"2020-11-09T19:53:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> Subject: Re: [PATCH v2 4/4] update-ref: Disallow restart of ongoing transactions\n\nOur convention (cf. \"git shortlog --no-merges\") is not to Capitalize\nthe first word after the \"<area>:\".  Shared across all four patches.\n\n> It is currently possible to write multiple \"start\" commands into\n> git-update-ref(1) for a single session, which doesn't make a lot of\n> sense to do in the first place. It's also not quite obvious what should\n> actually happen. Would this just go on with the current transaction as\n> if nothing was written or would it recreate a new session which doesn't\n> yet have any references? Silently ignoring this usage isn't helping\n> either as it may indicate erroneous use of the interface.\n>\n> This commit catches this use and instead raises an error if the user is\n> trying to restart an ongoing transaction.\n\nHmph, if you think of it as \"restart\", perhaps it would not make\nsense, but in other contexts where the concept of \"transactions\" are\ncommonly used, it is not so unusual to support \"nested\" transactions\nwhere inside an ongoing transaction, a subtransaction whose effect\ncan be rolled back as a whole, can be opened.  So wouldn't it be\nmore appropriate to explain this change like so?\n\n    Since we do not support nested transactions, make sure a \"start\"\n    command, seen after we saw \"start\" that is not closed by either\n    \"abort\" or \"commit\", is flagged as an error.\n\n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>  builtin/update-ref.c  |  2 ++\n>  t/t1400-update-ref.sh | 11 +++++++++++\n>  2 files changed, 13 insertions(+)\n>\n> diff --git a/builtin/update-ref.c b/builtin/update-ref.c\n> index bb65129012..6029a80544 100644\n> --- a/builtin/update-ref.c\n> +++ b/builtin/update-ref.c\n> @@ -436,6 +436,8 @@ static void update_refs_stdin(void)\n>  \t\tswitch (state) {\n>  \t\tcase UPDATE_REFS_OPEN:\n>  \t\tcase UPDATE_REFS_STARTED:\n> +\t\t\tif (state == UPDATE_REFS_STARTED && cmd->state == UPDATE_REFS_STARTED)\n> +\t\t\t\tdie(\"cannot restart ongoing transaction\");\n>  \t\t\t/* Do not downgrade a transaction to a non-transaction. */\n>  \t\t\tif (cmd->state >= state)\n>  \t\t\t\tstate = cmd->state;\n> diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\n> index 7ed41bb328..e53d973d04 100755\n> --- a/t/t1400-update-ref.sh\n> +++ b/t/t1400-update-ref.sh\n> @@ -1583,4 +1583,15 @@ test_expect_success 'transaction can commit after abort' '\n>  \ttest_cmp expect actual\n>  '\n>  \n> +test_expect_success 'transaction cannot restart ongoing transaction' '\n> +\tcat >stdin <<-EOF &&\n> +\tstart\n> +\tcreate refs/heads/restart $A\n> +\tstart\n> +\tcommit\n> +\tEOF\n> +\ttest_must_fail git update-ref --stdin <stdin >actual &&\n> +\ttest_must_fail git show-ref --verify refs/heads/restart\n> +'\n> +\n>  test_done\n"},{"id":"409425","messageId":"20201109222807.GB670413@coredump.intra.peff.net","threadId":"54571","inReplyTo":"xmqqpn4mb9bv.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v2 1/4] t1400: Avoid touching refs on filesystem","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-11-09T22:28:07Z","receivedAt":"2020-11-09T22:28:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 09, 2020 at 11:48:20AM -0800, Junio C Hamano wrote:\n\n> This is a tangent, but what makes this test doubly interesting is\n> that \"git update-ref -d HEAD\" would have allowed us to make it a\n> non-repository if HEAD were detached, and it seems that we do not\n> require \"--force\" to do so.  We probably should forbid removing HEAD\n> that id detached without \"--force\", which is such a destructive\n> operation.\n\nYeah, I'd agree that is a good direction (but it definitely is a tangent\nthat should come in a separate series).\n\n> >  cp -f .git/HEAD .git/HEAD.orig\n> >  test_expect_success 'delete symref without dereference' '\n> >  \ttest_when_finished \"cp -f .git/HEAD.orig .git/HEAD\" &&\n> >  \tgit update-ref --no-deref -d HEAD &&\n> > -\ttest_path_is_missing .git/HEAD\n> > +\ttest_must_fail git show-ref --verify -q HEAD\n> >  '\n> \n> This is an example of breaking the repository.  I am not sure if the\n> test_must_fail is a good replacement--it would fail even if you say\n> \"git show-ref --verify -q refs/heads/$branch\" where $branch is a\n> name of a branch that exists, no?\n\nPerhaps we could more directly check that the repository is broken.\nCoupled with the ceiling-limit from earlier in the script, then:\n\n  git rev-parse --git-dir\n\nshould fail. Maybe that reveals the intent of what we're expecting a\nlittle more clearly (and more so than your \"show-ref before and after\nrestoring HEAD\" example does, in my opinion).\n\nI do have to wonder if this test even cares about HEAD. Could it equally\nwell work on another symref, like refs/remotes/origin/HEAD?\n\n-Peff\n"},{"id":"409427","messageId":"20201109223330.GC670413@coredump.intra.peff.net","threadId":"54571","inReplyTo":"cover.1604908834.git.ps@pks.im","subject":"Re: [PATCH v2 0/4] update-ref: Allow creation of multiple transactions","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-11-09T22:33:30Z","receivedAt":"2020-11-09T22:33:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 09, 2020 at 11:06:43AM +0100, Patrick Steinhardt wrote:\n\n> this is the second version of this patch series implementing support for\n> creation of multiple reference transactions in a single git-update-ref\n> process.\n> \n> There's two changes compared to v1:\n> \n>     - A small refactoring of t1400, which refactors many tests to not\n>       touch references via the filesystem but instead to use\n>       git-update-ref and git-show-ref. There's still tests in there\n>       which do, but converting them is harder as they create and read\n>       broken references.\n> \n>     - I've added another commit on top which disallows restarting of\n>       transactions. E.g. writing \"start\\nstart\\n\" to git-update-ref is\n>       now going to fail.\n\nThanks. Aside from the issues raised by Junio, this all looks good to me\n(and I agree on the fourth one it is just a matter of the commit\nmessage; what the code is doing is a definite improvement).\n\n-Peff\n"},{"id":"409428","messageId":"xmqqzh3q9mvf.fsf@gitster.c.googlers.com","threadId":"54571","inReplyTo":"20201109223330.GC670413@coredump.intra.peff.net","subject":"Re: [PATCH v2 0/4] update-ref: Allow creation of multiple transactions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-11-09T22:38:44Z","receivedAt":"2020-11-09T22:38:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Mon, Nov 09, 2020 at 11:06:43AM +0100, Patrick Steinhardt wrote:\n>\n>> this is the second version of this patch series implementing support for\n>> creation of multiple reference transactions in a single git-update-ref\n>> process.\n>> \n>> There's two changes compared to v1:\n>> \n>>     - A small refactoring of t1400, which refactors many tests to not\n>>       touch references via the filesystem but instead to use\n>>       git-update-ref and git-show-ref. There's still tests in there\n>>       which do, but converting them is harder as they create and read\n>>       broken references.\n>> \n>>     - I've added another commit on top which disallows restarting of\n>>       transactions. E.g. writing \"start\\nstart\\n\" to git-update-ref is\n>>       now going to fail.\n>\n> Thanks. Aside from the issues raised by Junio, this all looks good to me\n> (and I agree on the fourth one it is just a matter of the commit\n> message; what the code is doing is a definite improvement).\n\nYeah, I agree that it is just terminology.  If we were to explain it\nas lack of nested transaction, however, the error message would need\nto be updated.  Other than that, I think this is quite good.\n\nOh, Patrick, please do not forget that it is our convention not to\nCapitalize the word immediately after <area>: on the title of the\ncommit (cf. \"git shortlog --no-merges -200\").\n\nThanks.\n"},{"id":"409480","messageId":"X6qIidgbnhYa3S0o@ncase","threadId":"54571","inReplyTo":"xmqqpn4mb9bv.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v2 1/4] t1400: Avoid touching refs on filesystem","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2020-11-10T12:34:22Z","receivedAt":"2020-11-10T12:34:38Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Nov 09, 2020 at 11:48:20AM -0800, Junio C Hamano wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> > diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\n> > index 4c01e08551..957bef272d 100755\n> > --- a/t/t1400-update-ref.sh\n> > +++ b/t/t1400-update-ref.sh\n> > @@ -14,6 +14,12 @@ n=$n_dir/fixes\n> >  outside=refs/foo\n> >  bare=bare-repo\n> >  \n> > +# Some of the tests delete HEAD, which causes us to not treat the current\n> > +# working directory as a Git repository anymore. To avoid using any potential\n> > +# parent repository to be discovered, we need to set up the ceiling directories.\n> > +GIT_CEILING_DIRECTORIES=\"$PWD/..\"\n> > +export GIT_CEILING_DIRECTORIES\n> > +\n> \n> Interesting.  The current tests do not need to do this because the\n> HEAD-less broken state is transitory and is corrected without using\n> \"git\" at all (e.g. echoing a valid value into .git/HEAD), I presume?\n\nCorrect.\n\n> > @@ -80,26 +86,26 @@ test_expect_success \"fail to delete $m (by HEAD) with stale ref\" '\n> >  \ttest $B = $(git show-ref -s --verify $m)\n> >  '\n> >  test_expect_success \"delete $m (by HEAD)\" '\n> > -\ttest_when_finished \"rm -f .git/$m\" &&\n> > +\ttest_when_finished \"git update-ref -d $m\" &&\n> >  \tgit update-ref -d HEAD $B &&\n> > -\ttest_path_is_missing .git/$m\n> > +\ttest_must_fail git show-ref --verify -q $m\n> >  '\n> \n> During the above test, we are on the branch ${m#refs/heads/}, so\n> \"update-ref -d HEAD\" is removing the underlying branch ref, making\n> it an unborn branch, without destroying the repository, so this is\n> perfectly sensible.\n> \n> This is a tangent, but what makes this test doubly interesting is\n> that \"git update-ref -d HEAD\" would have allowed us to make it a\n> non-repository if HEAD were detached, and it seems that we do not\n> require \"--force\" to do so.  We probably should forbid removing HEAD\n> that id detached without \"--force\", which is such a destructive\n> operation.\n\nThat'd make sense to me, yes. It also took me quite some time to\nactually figure out why tests were misbehaving when I converted it.\nUntil I finally realized: this is not a Git repo anymore, and refs have\nnow been modified directly in the real git.git repository. Just this\nmorning I still found some invalid refs created by the test in git.git.\n\n> >  cp -f .git/HEAD .git/HEAD.orig\n> >  test_expect_success 'delete symref without dereference' '\n> >  \ttest_when_finished \"cp -f .git/HEAD.orig .git/HEAD\" &&\n> >  \tgit update-ref --no-deref -d HEAD &&\n> > -\ttest_path_is_missing .git/HEAD\n> > +\ttest_must_fail git show-ref --verify -q HEAD\n> >  '\n> \n> This is an example of breaking the repository.  I am not sure if the\n> test_must_fail is a good replacement--it would fail even if you say\n> \"git show-ref --verify -q refs/heads/$branch\" where $branch is a\n> name of a branch that exists, no?\n\nRight, it's not. We're not detecting HEAD deletion by means of checking\nthat it doesn't exist anymore, but only detect it because the repo is\nnot a repo anymore. Which would in fact cause most git commands to fail\nnow.\n\n> For now, I think this is fine, but we'd probably clean it up so that\n> without --force update-ref would not corrupt the repository like\n> this.  When used with --force, or before adding such a safety\n> measure, how we test if we successfully corrupted the repository is\n> an interesting matter.  I'd say\n> \n> \tgit update-ref --force --no-deref -d HEAD &&\n> \ttest_must_fail git show-ref --verify -q HEAD &&\n> \tcp -f .git/HEAD.orig .git/HEAD &&\n> \tgit show-ref --verify -q \"$m\"\n> \n> to ensure that other than removing HEAD and corrupting the\n> repository, it did not cause permanent damage by removing the\n> underlying ref, perhaps.\n> \n> We may want to add some NEEDSWORK comment around such tests.\n\nI think the proper way to do the test would be to create a non-mandatory\nsymref and delete it. That'd also be a nice preparation for the\n`--force` parameter already.\n\n> >  test_expect_success 'delete symref without dereference when the referred ref is packed' '\n> > @@ -208,7 +214,7 @@ test_expect_success 'delete symref without dereference when the referred ref is\n> >  \tgit commit -m foo &&\n> >  \tgit pack-refs --all &&\n> >  \tgit update-ref --no-deref -d HEAD &&\n> > -\ttest_path_is_missing .git/HEAD\n> > +\ttest_must_fail git show-ref --verify -q HEAD\n> >  '\n> \n> Does this share the same issue as the above?\n\nYup, it does.\n\nPatrick\n"},{"id":"409499","messageId":"xmqq7dqt87pa.fsf@gitster.c.googlers.com","threadId":"54571","inReplyTo":"X6qIidgbnhYa3S0o@ncase","subject":"Re: [PATCH v2 1/4] t1400: Avoid touching refs on filesystem","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-11-10T17:04:01Z","receivedAt":"2020-11-10T17:04:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n>> For now, I think this is fine, but we'd probably clean it up so that\n>> without --force update-ref would not corrupt the repository like\n>> this.  When used with --force, or before adding such a safety\n>> measure, how we test if we successfully corrupted the repository is\n>> an interesting matter.  I'd say\n>> \n>> \tgit update-ref --force --no-deref -d HEAD &&\n>> \ttest_must_fail git show-ref --verify -q HEAD &&\n>> \tcp -f .git/HEAD.orig .git/HEAD &&\n>> \tgit show-ref --verify -q \"$m\"\n>> \n>> to ensure that other than removing HEAD and corrupting the\n>> repository, it did not cause permanent damage by removing the\n>> underlying ref, perhaps.\n>> \n>> We may want to add some NEEDSWORK comment around such tests.\n>\n> I think the proper way to do the test would be to create a non-mandatory\n> symref and delete it. That'd also be a nice preparation for the\n> `--force` parameter already.\n\nExcellent.  Yes, the test is about creation and removal of a symref;\nthe symref used for testing does not have to be the HEAD.\n\nThanks.\n"},{"id":"409582","messageId":"cover.1605077740.git.ps@pks.im","threadId":"54571","inReplyTo":"cover.1604501265.git.ps@pks.im","subject":"[PATCH v3 0/4] update-ref: allow creation of multiple transactions","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2020-11-11T06:58:34Z","receivedAt":"2020-11-11T07:05:24Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nthis is the third version of this patch series implementing support for\ncreation of multiple reference transactions in a single git-update-ref\nprocess.\n\nThere's three changes compared to v2:\n\n    - Commit subjects have been changed to start with a lowercase.\n\n    - The commit message in patch 4 has been improved.\n\n    - t1400 has been changed to not delete HEAD anymore.\n\nPatrick\n\nPatrick Steinhardt (4):\n  t1400: avoid touching refs on filesystem\n  update-ref: allow creation of multiple transactions\n  p1400: use `git-update-ref --stdin` to test multiple transactions\n  update-ref: disallow \"start\" for ongoing transactions\n\n Documentation/git-update-ref.txt |   3 +-\n builtin/update-ref.c             |  15 +++-\n t/perf/p1400-update-ref.sh       |  20 ++---\n t/t1400-update-ref.sh            | 133 +++++++++++++++++++++++--------\n 4 files changed, 123 insertions(+), 48 deletions(-)\n\n-- \n2.29.2\n\n"},{"id":"409583","messageId":"e66b1bcc62139f62866dc9f25856eaebfe107056.1605077740.git.ps@pks.im","threadId":"54571","inReplyTo":"cover.1605077740.git.ps@pks.im","subject":"[PATCH v3 1/4] t1400: avoid touching refs on filesystem","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2020-11-11T06:58:38Z","receivedAt":"2020-11-11T07:05:26Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The testcase t1400 exercises the git-update-ref(1) utility. To do so,\nmany tests directly read and write references via the filesystem,\nassuming that we always use loose and/or packed references. While this\nis true now, it'll change with the introduction of the reftable backend.\n\nConvert those tests to use git-update-ref(1) and git-show-ref(1) where\npossible. As some tests exercise behaviour with broken references and\nneither of those tools actually allows writing or reading broken\nreferences, this commit doesn't adjust all tests.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n t/t1400-update-ref.sh | 72 +++++++++++++++++++++++--------------------\n 1 file changed, 39 insertions(+), 33 deletions(-)\n\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex 4c01e08551..d7a57488ed 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -14,6 +14,12 @@ n=$n_dir/fixes\n outside=refs/foo\n bare=bare-repo\n \n+# Some of the tests delete HEAD, which causes us to not treat the current\n+# working directory as a Git repository anymore. To avoid using any potential\n+# parent repository to be discovered, we need to set up the ceiling directories.\n+GIT_CEILING_DIRECTORIES=\"$PWD/..\"\n+export GIT_CEILING_DIRECTORIES\n+\n create_test_commits ()\n {\n \tprfx=\"$1\"\n@@ -48,17 +54,17 @@ test_expect_success \"fail to delete $m with stale ref\" '\n \ttest $B = \"$(git show-ref -s --verify $m)\"\n '\n test_expect_success \"delete $m\" '\n-\ttest_when_finished \"rm -f .git/$m\" &&\n+\ttest_when_finished \"git update-ref -d $m\" &&\n \tgit update-ref -d $m $B &&\n-\ttest_path_is_missing .git/$m\n+\ttest_must_fail git show-ref --verify -q $m\n '\n \n test_expect_success \"delete $m without oldvalue verification\" '\n-\ttest_when_finished \"rm -f .git/$m\" &&\n+\ttest_when_finished \"git update-ref -d $m\" &&\n \tgit update-ref $m $A &&\n \ttest $A = $(git show-ref -s --verify $m) &&\n \tgit update-ref -d $m &&\n-\ttest_path_is_missing .git/$m\n+\ttest_must_fail git show-ref --verify -q $m\n '\n \n test_expect_success \"fail to create $n\" '\n@@ -80,26 +86,26 @@ test_expect_success \"fail to delete $m (by HEAD) with stale ref\" '\n \ttest $B = $(git show-ref -s --verify $m)\n '\n test_expect_success \"delete $m (by HEAD)\" '\n-\ttest_when_finished \"rm -f .git/$m\" &&\n+\ttest_when_finished \"git update-ref -d $m\" &&\n \tgit update-ref -d HEAD $B &&\n-\ttest_path_is_missing .git/$m\n+\ttest_must_fail git show-ref --verify -q $m\n '\n \n test_expect_success \"deleting current branch adds message to HEAD's log\" '\n-\ttest_when_finished \"rm -f .git/$m\" &&\n+\ttest_when_finished \"git update-ref -d $m\" &&\n \tgit update-ref $m $A &&\n \tgit symbolic-ref HEAD $m &&\n \tgit update-ref -m delete-$m -d $m &&\n-\ttest_path_is_missing .git/$m &&\n+\ttest_must_fail git show-ref --verify -q $m &&\n \tgrep \"delete-$m$\" .git/logs/HEAD\n '\n \n test_expect_success \"deleting by HEAD adds message to HEAD's log\" '\n-\ttest_when_finished \"rm -f .git/$m\" &&\n+\ttest_when_finished \"git update-ref -d $m\" &&\n \tgit update-ref $m $A &&\n \tgit symbolic-ref HEAD $m &&\n \tgit update-ref -m delete-by-head -d HEAD &&\n-\ttest_path_is_missing .git/$m &&\n+\ttest_must_fail git show-ref --verify -q $m &&\n \tgrep \"delete-by-head$\" .git/logs/HEAD\n '\n \n@@ -188,27 +194,26 @@ test_expect_success \"move $m (by HEAD)\" '\n \ttest $B = $(git show-ref -s --verify $m)\n '\n test_expect_success \"delete $m (by HEAD) should remove both packed and loose $m\" '\n-\ttest_when_finished \"rm -f .git/$m\" &&\n+\ttest_when_finished \"rm -f git update-ref -d $m\" &&\n \tgit update-ref -d HEAD $B &&\n \t! grep \"$m\" .git/packed-refs &&\n-\ttest_path_is_missing .git/$m\n+\ttest_must_fail git show-ref --verify -q $m\n '\n \n-cp -f .git/HEAD .git/HEAD.orig\n test_expect_success 'delete symref without dereference' '\n-\ttest_when_finished \"cp -f .git/HEAD.orig .git/HEAD\" &&\n-\tgit update-ref --no-deref -d HEAD &&\n-\ttest_path_is_missing .git/HEAD\n+\tgit symbolic-ref SYMREF $m &&\n+\tgit update-ref --no-deref -d SYMREF &&\n+\ttest_must_fail git show-ref --verify -q SYMREF\n '\n \n test_expect_success 'delete symref without dereference when the referred ref is packed' '\n-\ttest_when_finished \"cp -f .git/HEAD.orig .git/HEAD\" &&\n \techo foo >foo.c &&\n \tgit add foo.c &&\n \tgit commit -m foo &&\n+\tgit symbolic-ref SYMREF $m &&\n \tgit pack-refs --all &&\n-\tgit update-ref --no-deref -d HEAD &&\n-\ttest_path_is_missing .git/HEAD\n+\tgit update-ref --no-deref -d SYMREF &&\n+\ttest_must_fail git show-ref --verify -q SYMREF\n '\n \n git update-ref -d $m\n@@ -226,25 +231,25 @@ test_expect_success 'update-ref --no-deref -d can delete self-reference' '\n \ttest_when_finished \"rm -f .git/refs/heads/self\" &&\n \ttest_path_is_file .git/refs/heads/self &&\n \tgit update-ref --no-deref -d refs/heads/self &&\n-\ttest_path_is_missing .git/refs/heads/self\n+\ttest_must_fail git show-ref --verify -q refs/heads/self\n '\n \n test_expect_success 'update-ref --no-deref -d can delete reference to bad ref' '\n \t>.git/refs/heads/bad &&\n \ttest_when_finished \"rm -f .git/refs/heads/bad\" &&\n \tgit symbolic-ref refs/heads/ref-to-bad refs/heads/bad &&\n-\ttest_when_finished \"rm -f .git/refs/heads/ref-to-bad\" &&\n+\ttest_when_finished \"git update-ref -d refs/heads/ref-to-bad\" &&\n \ttest_path_is_file .git/refs/heads/ref-to-bad &&\n \tgit update-ref --no-deref -d refs/heads/ref-to-bad &&\n-\ttest_path_is_missing .git/refs/heads/ref-to-bad\n+\ttest_must_fail git show-ref --verify -q refs/heads/ref-to-bad\n '\n \n test_expect_success '(not) create HEAD with old sha1' '\n \ttest_must_fail git update-ref HEAD $A $B\n '\n test_expect_success \"(not) prior created .git/$m\" '\n-\ttest_when_finished \"rm -f .git/$m\" &&\n-\ttest_path_is_missing .git/$m\n+\ttest_when_finished \"git update-ref -d $m\" &&\n+\ttest_must_fail git show-ref --verify -q $m\n '\n \n test_expect_success 'create HEAD' '\n@@ -254,7 +259,7 @@ test_expect_success '(not) change HEAD with wrong SHA1' '\n \ttest_must_fail git update-ref HEAD $B $Z\n '\n test_expect_success \"(not) changed .git/$m\" '\n-\ttest_when_finished \"rm -f .git/$m\" &&\n+\ttest_when_finished \"git update-ref -d $m\" &&\n \t! test $B = $(git show-ref -s --verify $m)\n '\n \n@@ -284,8 +289,8 @@ test_expect_success 'empty directory removal' '\n \ttest_path_is_file .git/refs/heads/d1/d2/r1 &&\n \ttest_path_is_file .git/logs/refs/heads/d1/d2/r1 &&\n \tgit branch -d d1/d2/r1 &&\n-\ttest_path_is_missing .git/refs/heads/d1/d2 &&\n-\ttest_path_is_missing .git/logs/refs/heads/d1/d2 &&\n+\ttest_must_fail git show-ref --verify -q refs/heads/d1/d2 &&\n+\ttest_must_fail git show-ref --verify -q logs/refs/heads/d1/d2 &&\n \ttest_path_is_file .git/refs/heads/d1/r2 &&\n \ttest_path_is_file .git/logs/refs/heads/d1/r2\n '\n@@ -298,8 +303,8 @@ test_expect_success 'symref empty directory removal' '\n \ttest_path_is_file .git/refs/heads/e1/e2/r1 &&\n \ttest_path_is_file .git/logs/refs/heads/e1/e2/r1 &&\n \tgit update-ref -d HEAD &&\n-\ttest_path_is_missing .git/refs/heads/e1/e2 &&\n-\ttest_path_is_missing .git/logs/refs/heads/e1/e2 &&\n+\ttest_must_fail git show-ref --verify -q refs/heads/e1/e2 &&\n+\ttest_must_fail git show-ref --verify -q logs/refs/heads/e1/e2 &&\n \ttest_path_is_file .git/refs/heads/e1/r2 &&\n \ttest_path_is_file .git/logs/refs/heads/e1/r2 &&\n \ttest_path_is_file .git/logs/HEAD\n@@ -1388,7 +1393,8 @@ test_expect_success 'handle per-worktree refs in refs/bisect' '\n \t\tgit rev-parse refs/bisect/something >../worktree-head &&\n \t\tgit for-each-ref | grep refs/bisect/something\n \t) &&\n-\ttest_path_is_missing .git/refs/bisect &&\n+\tgit show-ref >actual &&\n+\t! grep 'refs/bisect' actual &&\n \ttest_must_fail git rev-parse refs/bisect/something &&\n \tgit update-ref refs/bisect/something HEAD &&\n \tgit rev-parse refs/bisect/something >main-head &&\n@@ -1500,7 +1506,7 @@ test_expect_success 'transaction can handle abort' '\n \tgit update-ref --stdin <stdin >actual &&\n \tprintf \"%s: ok\\n\" start abort >expect &&\n \ttest_cmp expect actual &&\n-\ttest_path_is_missing .git/$b\n+\ttest_must_fail git show-ref --verify -q $b\n '\n \n test_expect_success 'transaction aborts by default' '\n@@ -1511,7 +1517,7 @@ test_expect_success 'transaction aborts by default' '\n \tgit update-ref --stdin <stdin >actual &&\n \tprintf \"%s: ok\\n\" start >expect &&\n \ttest_cmp expect actual &&\n-\ttest_path_is_missing .git/$b\n+\ttest_must_fail git show-ref --verify -q $b\n '\n \n test_expect_success 'transaction with prepare aborts by default' '\n@@ -1523,7 +1529,7 @@ test_expect_success 'transaction with prepare aborts by default' '\n \tgit update-ref --stdin <stdin >actual &&\n \tprintf \"%s: ok\\n\" start prepare >expect &&\n \ttest_cmp expect actual &&\n-\ttest_path_is_missing .git/$b\n+\ttest_must_fail git show-ref --verify -q $b\n '\n \n test_done\n-- \n2.29.2\n\n"},{"id":"409584","messageId":"f19917483362569de132c9d81070f2c3e8e365b3.1605077740.git.ps@pks.im","threadId":"54571","inReplyTo":"cover.1605077740.git.ps@pks.im","subject":"[PATCH v3 2/4] update-ref: allow creation of multiple transactions","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2020-11-11T06:58:43Z","receivedAt":"2020-11-11T07:05:29Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"While git-update-ref has recently grown commands which allow interactive\ncontrol of transactions in e48cf33b61 (update-ref: implement interactive\ntransaction handling, 2020-04-02), it is not yet possible to create\nmultiple transactions in a single session. To do so, one currently still\nneeds to invoke the executable multiple times.\n\nThis commit addresses this shortcoming by allowing the \"start\" command\nto create a new transaction if the current transaction has already been\neither committed or aborted.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n Documentation/git-update-ref.txt |  3 +-\n builtin/update-ref.c             | 13 ++++++++-\n t/t1400-update-ref.sh            | 50 ++++++++++++++++++++++++++++++++\n 3 files changed, 64 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt\nindex d401234b03..48b6683071 100644\n--- a/Documentation/git-update-ref.txt\n+++ b/Documentation/git-update-ref.txt\n@@ -125,7 +125,8 @@ option::\n start::\n \tStart a transaction. In contrast to a non-transactional session, a\n \ttransaction will automatically abort if the session ends without an\n-\texplicit commit.\n+\texplicit commit. This command may create a new empty transaction when\n+\tthe current one has been committed or aborted already.\n \n prepare::\n \tPrepare to commit the transaction. This will create lock files for all\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex 8a2df4459c..bb65129012 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -446,7 +446,18 @@ static void update_refs_stdin(void)\n \t\t\tstate = cmd->state;\n \t\t\tbreak;\n \t\tcase UPDATE_REFS_CLOSED:\n-\t\t\tdie(\"transaction is closed\");\n+\t\t\tif (cmd->state != UPDATE_REFS_STARTED)\n+\t\t\t\tdie(\"transaction is closed\");\n+\n+\t\t\t/*\n+\t\t\t * Open a new transaction if we're currently closed and\n+\t\t\t * get a \"start\".\n+\t\t\t */\n+\t\t\tstate = cmd->state;\n+\t\t\ttransaction = ref_transaction_begin(&err);\n+\t\t\tif (!transaction)\n+\t\t\t\tdie(\"%s\", err.buf);\n+\n \t\t\tbreak;\n \t\t}\n \ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex d7a57488ed..2890504edd 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -1532,4 +1532,54 @@ test_expect_success 'transaction with prepare aborts by default' '\n \ttest_must_fail git show-ref --verify -q $b\n '\n \n+test_expect_success 'transaction can commit multiple times' '\n+\tcat >stdin <<-EOF &&\n+\tstart\n+\tcreate refs/heads/branch-1 $A\n+\tcommit\n+\tstart\n+\tcreate refs/heads/branch-2 $B\n+\tcommit\n+\tEOF\n+\tgit update-ref --stdin <stdin >actual &&\n+\tprintf \"%s: ok\\n\" start commit start commit >expect &&\n+\ttest_cmp expect actual &&\n+\techo \"$A\" >expect &&\n+\tgit rev-parse refs/heads/branch-1 >actual &&\n+\ttest_cmp expect actual &&\n+\techo \"$B\" >expect &&\n+\tgit rev-parse refs/heads/branch-2 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'transaction can create and delete' '\n+\tcat >stdin <<-EOF &&\n+\tstart\n+\tcreate refs/heads/create-and-delete $A\n+\tcommit\n+\tstart\n+\tdelete refs/heads/create-and-delete $A\n+\tcommit\n+\tEOF\n+\tgit update-ref --stdin <stdin >actual &&\n+\tprintf \"%s: ok\\n\" start commit start commit >expect &&\n+\ttest_must_fail git show-ref --verify refs/heads/create-and-delete\n+'\n+\n+test_expect_success 'transaction can commit after abort' '\n+\tcat >stdin <<-EOF &&\n+\tstart\n+\tcreate refs/heads/abort $A\n+\tabort\n+\tstart\n+\tcreate refs/heads/abort $A\n+\tcommit\n+\tEOF\n+\tgit update-ref --stdin <stdin >actual &&\n+\tprintf \"%s: ok\\n\" start abort start commit >expect &&\n+\techo \"$A\" >expect &&\n+\tgit rev-parse refs/heads/abort >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.29.2\n\n"},{"id":"409585","messageId":"e393b5350f5ed0c7353b6301755a35d7c3b1b83c.1605077740.git.ps@pks.im","threadId":"54571","inReplyTo":"cover.1605077740.git.ps@pks.im","subject":"[PATCH v3 3/4] p1400: use `git-update-ref --stdin` to test multiple transactions","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2020-11-11T06:58:47Z","receivedAt":"2020-11-11T07:05:32Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In commit 0a0fbbe3ff (refs: remove lookup cache for\nreference-transaction hook, 2020-08-25), a new benchmark was added to\np1400 which has the intention to exercise creation of multiple\ntransactions in a single process. As git-update-ref wasn't yet able to\ncreate multiple transactions with a single run we instead used git-push.\nAs its non-atomic version creates a transaction per reference update,\nthis was the best approximation we could make at that point in time.\n\nNow that `git-update-ref --stdin` supports creation of multiple\ntransactions, let's convert the benchmark to use that instead. It has\nless overhead and it's also a lot clearer what the actual intention is.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n t/perf/p1400-update-ref.sh | 20 +++++++-------------\n 1 file changed, 7 insertions(+), 13 deletions(-)\n\ndiff --git a/t/perf/p1400-update-ref.sh b/t/perf/p1400-update-ref.sh\nindex ce5ac3ed85..dda8a74866 100755\n--- a/t/perf/p1400-update-ref.sh\n+++ b/t/perf/p1400-update-ref.sh\n@@ -7,13 +7,14 @@ test_description=\"Tests performance of update-ref\"\n test_perf_fresh_repo\n \n test_expect_success \"setup\" '\n-\tgit init --bare target-repo.git &&\n \ttest_commit PRE &&\n \ttest_commit POST &&\n-\tprintf \"create refs/heads/%d PRE\\n\" $(test_seq 1000) >create &&\n-\tprintf \"update refs/heads/%d POST PRE\\n\" $(test_seq 1000) >update &&\n-\tprintf \"delete refs/heads/%d POST\\n\" $(test_seq 1000) >delete &&\n-\tgit update-ref --stdin <create\n+\tfor i in $(test_seq 5000)\n+\tdo\n+\t\tprintf \"start\\ncreate refs/heads/%d PRE\\ncommit\\n\" $i &&\n+\t\tprintf \"start\\nupdate refs/heads/%d POST PRE\\ncommit\\n\" $i &&\n+\t\tprintf \"start\\ndelete refs/heads/%d POST\\ncommit\\n\" $i\n+\tdone >instructions\n '\n \n test_perf \"update-ref\" '\n@@ -26,14 +27,7 @@ test_perf \"update-ref\" '\n '\n \n test_perf \"update-ref --stdin\" '\n-\tgit update-ref --stdin <update &&\n-\tgit update-ref --stdin <delete &&\n-\tgit update-ref --stdin <create\n-'\n-\n-test_perf \"nonatomic push\" '\n-\tgit push ./target-repo.git $(test_seq 1000) &&\n-\tgit push --delete ./target-repo.git $(test_seq 1000)\n+\tgit update-ref --stdin <instructions >/dev/null\n '\n \n test_done\n-- \n2.29.2\n\n"},{"id":"409586","messageId":"bacb8960b01b1de53b0060f2582da2b63aeda350.1605077740.git.ps@pks.im","threadId":"54571","inReplyTo":"cover.1605077740.git.ps@pks.im","subject":"[PATCH v3 4/4] update-ref: disallow \"start\" for ongoing transactions","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2020-11-11T06:58:53Z","receivedAt":"2020-11-11T07:05:40Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"It is currently possible to write multiple \"start\" commands into\ngit-update-ref(1) for a single session, but none of them except for the\nfirst one actually have any effect.\n\nUsing such nested \"start\"s may eventually have a sensible effect. One\nmay imagine that it restarts the current transaction, effectively\nemptying it and creating a new one. It may also allow for creation of\nnested transactions. But currently, none of these are implemented.\n\nSilently ignoring this misuse is making it hard to iterate in the future\nif \"start\" is ever going to have meaningful semantics in such a context.\nThis commit thus makes sure to error out in case we see such use.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n builtin/update-ref.c  |  2 ++\n t/t1400-update-ref.sh | 11 +++++++++++\n 2 files changed, 13 insertions(+)\n\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex bb65129012..6029a80544 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -436,6 +436,8 @@ static void update_refs_stdin(void)\n \t\tswitch (state) {\n \t\tcase UPDATE_REFS_OPEN:\n \t\tcase UPDATE_REFS_STARTED:\n+\t\t\tif (state == UPDATE_REFS_STARTED && cmd->state == UPDATE_REFS_STARTED)\n+\t\t\t\tdie(\"cannot restart ongoing transaction\");\n \t\t\t/* Do not downgrade a transaction to a non-transaction. */\n \t\t\tif (cmd->state >= state)\n \t\t\t\tstate = cmd->state;\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex 2890504edd..e109097970 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -1582,4 +1582,15 @@ test_expect_success 'transaction can commit after abort' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'transaction cannot restart ongoing transaction' '\n+\tcat >stdin <<-EOF &&\n+\tstart\n+\tcreate refs/heads/restart $A\n+\tstart\n+\tcommit\n+\tEOF\n+\ttest_must_fail git update-ref --stdin <stdin >actual &&\n+\ttest_must_fail git show-ref --verify refs/heads/restart\n+'\n+\n test_done\n-- \n2.29.2\n\n"},{"id":"409593","messageId":"20201111090454.GA4270@szeder.dev","threadId":"54571","inReplyTo":"e66b1bcc62139f62866dc9f25856eaebfe107056.1605077740.git.ps@pks.im","subject":"Re: [PATCH v3 1/4] t1400: avoid touching refs on filesystem","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2020-11-11T09:04:54Z","receivedAt":"2020-11-11T09:05:09Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Wed, Nov 11, 2020 at 07:58:38AM +0100, Patrick Steinhardt wrote:\n> The testcase t1400 exercises the git-update-ref(1) utility. To do so,\n> many tests directly read and write references via the filesystem,\n> assuming that we always use loose and/or packed references. While this\n> is true now, it'll change with the introduction of the reftable backend.\n> \n> Convert those tests to use git-update-ref(1) and git-show-ref(1) where\n> possible. As some tests exercise behaviour with broken references and\n> neither of those tools actually allows writing or reading broken\n> references, this commit doesn't adjust all tests.\n> \n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>  t/t1400-update-ref.sh | 72 +++++++++++++++++++++++--------------------\n>  1 file changed, 39 insertions(+), 33 deletions(-)\n> \n> diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\n> index 4c01e08551..d7a57488ed 100755\n> --- a/t/t1400-update-ref.sh\n> +++ b/t/t1400-update-ref.sh\n\n> @@ -188,27 +194,26 @@ test_expect_success \"move $m (by HEAD)\" '\n>  \ttest $B = $(git show-ref -s --verify $m)\n>  '\n>  test_expect_success \"delete $m (by HEAD) should remove both packed and loose $m\" '\n> -\ttest_when_finished \"rm -f .git/$m\" &&\n> +\ttest_when_finished \"rm -f git update-ref -d $m\" &&\n\nThere is a leftover 'rm -f' here.\n\n>  \tgit update-ref -d HEAD $B &&\n>  \t! grep \"$m\" .git/packed-refs &&\n> -\ttest_path_is_missing .git/$m\n> +\ttest_must_fail git show-ref --verify -q $m\n>  '\n"},{"id":"409597","messageId":"X6u2Lz9khi+DeF87@ncase","threadId":"54571","inReplyTo":"20201111090454.GA4270@szeder.dev","subject":"Re: [PATCH v3 1/4] t1400: avoid touching refs on filesystem","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2020-11-11T10:00:15Z","receivedAt":"2020-11-11T10:00:25Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Nov 11, 2020 at 10:04:54AM +0100, SZEDER Gábor wrote:\n> On Wed, Nov 11, 2020 at 07:58:38AM +0100, Patrick Steinhardt wrote:\n> > The testcase t1400 exercises the git-update-ref(1) utility. To do so,\n> > many tests directly read and write references via the filesystem,\n> > assuming that we always use loose and/or packed references. While this\n> > is true now, it'll change with the introduction of the reftable backend.\n> > \n> > Convert those tests to use git-update-ref(1) and git-show-ref(1) where\n> > possible. As some tests exercise behaviour with broken references and\n> > neither of those tools actually allows writing or reading broken\n> > references, this commit doesn't adjust all tests.\n> > \n> > Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> > ---\n> >  t/t1400-update-ref.sh | 72 +++++++++++++++++++++++--------------------\n> >  1 file changed, 39 insertions(+), 33 deletions(-)\n> > \n> > diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\n> > index 4c01e08551..d7a57488ed 100755\n> > --- a/t/t1400-update-ref.sh\n> > +++ b/t/t1400-update-ref.sh\n> \n> > @@ -188,27 +194,26 @@ test_expect_success \"move $m (by HEAD)\" '\n> >  \ttest $B = $(git show-ref -s --verify $m)\n> >  '\n> >  test_expect_success \"delete $m (by HEAD) should remove both packed and loose $m\" '\n> > -\ttest_when_finished \"rm -f .git/$m\" &&\n> > +\ttest_when_finished \"rm -f git update-ref -d $m\" &&\n> \n> There is a leftover 'rm -f' here.\n\nOops, thanks a lot for catching. Funny that it still passed.\n\nPatrick\n"},{"id":"409598","messageId":"20201111102412.GB4270@szeder.dev","threadId":"54571","inReplyTo":"X6u2Lz9khi+DeF87@ncase","subject":"Re: [PATCH v3 1/4] t1400: avoid touching refs on filesystem","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2020-11-11T10:24:12Z","receivedAt":"2020-11-11T10:24:18Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Wed, Nov 11, 2020 at 11:00:15AM +0100, Patrick Steinhardt wrote:\n> On Wed, Nov 11, 2020 at 10:04:54AM +0100, SZEDER Gábor wrote:\n> > On Wed, Nov 11, 2020 at 07:58:38AM +0100, Patrick Steinhardt wrote:\n> > > The testcase t1400 exercises the git-update-ref(1) utility. To do so,\n> > > many tests directly read and write references via the filesystem,\n> > > assuming that we always use loose and/or packed references. While this\n> > > is true now, it'll change with the introduction of the reftable backend.\n> > > \n> > > Convert those tests to use git-update-ref(1) and git-show-ref(1) where\n> > > possible. As some tests exercise behaviour with broken references and\n> > > neither of those tools actually allows writing or reading broken\n> > > references, this commit doesn't adjust all tests.\n> > > \n> > > Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> > > ---\n> > >  t/t1400-update-ref.sh | 72 +++++++++++++++++++++++--------------------\n> > >  1 file changed, 39 insertions(+), 33 deletions(-)\n> > > \n> > > diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\n> > > index 4c01e08551..d7a57488ed 100755\n> > > --- a/t/t1400-update-ref.sh\n> > > +++ b/t/t1400-update-ref.sh\n> > \n> > > @@ -188,27 +194,26 @@ test_expect_success \"move $m (by HEAD)\" '\n> > >  \ttest $B = $(git show-ref -s --verify $m)\n> > >  '\n> > >  test_expect_success \"delete $m (by HEAD) should remove both packed and loose $m\" '\n> > > -\ttest_when_finished \"rm -f .git/$m\" &&\n> > > +\ttest_when_finished \"rm -f git update-ref -d $m\" &&\n> > \n> > There is a leftover 'rm -f' here.\n> \n> Oops, thanks a lot for catching. Funny that it still passed.\n\nThe '-f' option makes it ignore missing files like 'git', 'update-ref'\nand whatever $m happens to expand to in this test, and '/bin/rm' from\ncoreutils (and apparently on macOS) understands the '-d' option\n(\"remove empty directories\"), so there was no error to fail this test\non common setups.  (I didn't investigate whether and how the not\ndeleted $m ref affects subsequent tests...)  But nowadays we have a CI\njob running the tests with 'busybox sh' whose builtin 'rm' doesn't\nhave a '-d' option, and it errors out with:\n\n  + rm -f git update-ref -d refs/heads/master\n  rm: unrecognized option: d\n  BusyBox v1.31.1 () multi-call binary.\n  Usage: ......\n\n\nhttps://travis-ci.org/github/git/git/jobs/742890453#L2853\n\n"},{"id":"409730","messageId":"20201111230659.GA632312@coredump.intra.peff.net","threadId":"54571","inReplyTo":"e66b1bcc62139f62866dc9f25856eaebfe107056.1605077740.git.ps@pks.im","subject":"Re: [PATCH v3 1/4] t1400: avoid touching refs on filesystem","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-11-11T23:06:59Z","receivedAt":"2020-11-12T01:32:24Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 11, 2020 at 07:58:38AM +0100, Patrick Steinhardt wrote:\n\n> The testcase t1400 exercises the git-update-ref(1) utility. To do so,\n> many tests directly read and write references via the filesystem,\n> assuming that we always use loose and/or packed references. While this\n> is true now, it'll change with the introduction of the reftable backend.\n> \n> Convert those tests to use git-update-ref(1) and git-show-ref(1) where\n> possible. As some tests exercise behaviour with broken references and\n> neither of those tools actually allows writing or reading broken\n> references, this commit doesn't adjust all tests.\n\nDo you want to mention the switch away from using HEAD in some tests\nhere?\n\n> +# Some of the tests delete HEAD, which causes us to not treat the current\n> +# working directory as a Git repository anymore. To avoid using any potential\n> +# parent repository to be discovered, we need to set up the ceiling directories.\n> +GIT_CEILING_DIRECTORIES=\"$PWD/..\"\n> +export GIT_CEILING_DIRECTORIES\n\nDo we still need this, now that we're not deleting HEAD? I think we do\nstill delete a branch via HEAD, but that should leave an unborn branch,\nwhich is still a valid repo.\n\n>  test_expect_success \"deleting current branch adds message to HEAD's log\" '\n> -\ttest_when_finished \"rm -f .git/$m\" &&\n> +\ttest_when_finished \"git update-ref -d $m\" &&\n>  \tgit update-ref $m $A &&\n>  \tgit symbolic-ref HEAD $m &&\n>  \tgit update-ref -m delete-$m -d $m &&\n> -\ttest_path_is_missing .git/$m &&\n> +\ttest_must_fail git show-ref --verify -q $m &&\n>  \tgrep \"delete-$m$\" .git/logs/HEAD\n>  '\n\nE.g., these ones should leave a valid repo (and must remain HEAD,\nbecause it's special for reflogging).\n\n> -cp -f .git/HEAD .git/HEAD.orig\n>  test_expect_success 'delete symref without dereference' '\n> -\ttest_when_finished \"cp -f .git/HEAD.orig .git/HEAD\" &&\n> -\tgit update-ref --no-deref -d HEAD &&\n> -\ttest_path_is_missing .git/HEAD\n> +\tgit symbolic-ref SYMREF $m &&\n> +\tgit update-ref --no-deref -d SYMREF &&\n> +\ttest_must_fail git show-ref --verify -q SYMREF\n>  '\n\nAnd now this one is safe. Good.\n\nI wonder, though...is it still testing the same thing as the original?\nThis is not related to the use of SYMREF vs HEAD, but wouldn't show-ref\nsimilarly fail if we had deleted $m, but left SYMREF in place (i.e., if\n--no-deref didn't actually do anything)?\n\nPerhaps this would be better:\n\n  # confirm that the pointed-to ref is still there\n  git show-ref --verify $m &&\n  # but our symref is not\n  test_must_fail git show-ref --verify SYMREF &&\n  test_must_fail git symbolic-ref SYMREF\n\n>  test_expect_success 'delete symref without dereference when the referred ref is packed' '\n> -\ttest_when_finished \"cp -f .git/HEAD.orig .git/HEAD\" &&\n>  \techo foo >foo.c &&\n>  \tgit add foo.c &&\n>  \tgit commit -m foo &&\n> +\tgit symbolic-ref SYMREF $m &&\n>  \tgit pack-refs --all &&\n> -\tgit update-ref --no-deref -d HEAD &&\n> -\ttest_path_is_missing .git/HEAD\n> +\tgit update-ref --no-deref -d SYMREF &&\n> +\ttest_must_fail git show-ref --verify -q SYMREF\n>  '\n\nLikewise here.\n\n-Peff\n"},{"id":"409855","messageId":"X64/ETgmrandZ8l3@ncase","threadId":"54571","inReplyTo":"20201111230659.GA632312@coredump.intra.peff.net","subject":"Re: [PATCH v3 1/4] t1400: avoid touching refs on filesystem","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2020-11-13T08:08:49Z","receivedAt":"2020-11-13T08:08:56Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Nov 11, 2020 at 06:06:59PM -0500, Jeff King wrote:\n> On Wed, Nov 11, 2020 at 07:58:38AM +0100, Patrick Steinhardt wrote:\n> > +# Some of the tests delete HEAD, which causes us to not treat the current\n> > +# working directory as a Git repository anymore. To avoid using any potential\n> > +# parent repository to be discovered, we need to set up the ceiling directories.\n> > +GIT_CEILING_DIRECTORIES=\"$PWD/..\"\n> > +export GIT_CEILING_DIRECTORIES\n> \n> Do we still need this, now that we're not deleting HEAD? I think we do\n> still delete a branch via HEAD, but that should leave an unborn branch,\n> which is still a valid repo.\n\nGood point, we don't.\n\n> >  test_expect_success \"deleting current branch adds message to HEAD's log\" '\n> > -\ttest_when_finished \"rm -f .git/$m\" &&\n> > +\ttest_when_finished \"git update-ref -d $m\" &&\n> >  \tgit update-ref $m $A &&\n> >  \tgit symbolic-ref HEAD $m &&\n> >  \tgit update-ref -m delete-$m -d $m &&\n> > -\ttest_path_is_missing .git/$m &&\n> > +\ttest_must_fail git show-ref --verify -q $m &&\n> >  \tgrep \"delete-$m$\" .git/logs/HEAD\n> >  '\n> \n> E.g., these ones should leave a valid repo (and must remain HEAD,\n> because it's special for reflogging).\n> \n> > -cp -f .git/HEAD .git/HEAD.orig\n> >  test_expect_success 'delete symref without dereference' '\n> > -\ttest_when_finished \"cp -f .git/HEAD.orig .git/HEAD\" &&\n> > -\tgit update-ref --no-deref -d HEAD &&\n> > -\ttest_path_is_missing .git/HEAD\n> > +\tgit symbolic-ref SYMREF $m &&\n> > +\tgit update-ref --no-deref -d SYMREF &&\n> > +\ttest_must_fail git show-ref --verify -q SYMREF\n> >  '\n> \n> And now this one is safe. Good.\n> \n> I wonder, though...is it still testing the same thing as the original?\n> This is not related to the use of SYMREF vs HEAD, but wouldn't show-ref\n> similarly fail if we had deleted $m, but left SYMREF in place (i.e., if\n> --no-deref didn't actually do anything)?\n> \n> Perhaps this would be better:\n> \n>   # confirm that the pointed-to ref is still there\n>   git show-ref --verify $m &&\n>   # but our symref is not\n>   test_must_fail git show-ref --verify SYMREF &&\n>   test_must_fail git symbolic-ref SYMREF\n\nIt would be, but I bailed at this point because we don't actually have\n\"$m\" at this point. But agreed, i'll also include this into both tests.\n\nPatrick\n\n> >  test_expect_success 'delete symref without dereference when the referred ref is packed' '\n> > -\ttest_when_finished \"cp -f .git/HEAD.orig .git/HEAD\" &&\n> >  \techo foo >foo.c &&\n> >  \tgit add foo.c &&\n> >  \tgit commit -m foo &&\n> > +\tgit symbolic-ref SYMREF $m &&\n> >  \tgit pack-refs --all &&\n> > -\tgit update-ref --no-deref -d HEAD &&\n> > -\ttest_path_is_missing .git/HEAD\n> > +\tgit update-ref --no-deref -d SYMREF &&\n> > +\ttest_must_fail git show-ref --verify -q SYMREF\n> >  '\n> \n> Likewise here.\n> \n> -Peff\n"},{"id":"409856","messageId":"cover.1605254957.git.ps@pks.im","threadId":"54571","inReplyTo":"cover.1604501265.git.ps@pks.im","subject":"[PATCH v4 0/4] update-ref: allow creation of multiple transactions","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2020-11-13T08:12:26Z","receivedAt":"2020-11-13T08:12:33Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nthis is the fourth version of this patch series implementing support for\ncreation of multiple reference transactions in a single git-update-ref\nprocess.\n\nAll changes are only in the first patch updating t1400:\n\n    - A stale 'rm -f' was removed.\n\n    - The workaround around broken repos was removed as we no longer\n      delete HEAD.\n\n    - Tests have been improved to verify that deleting a symref won't\n      delete their target.\n\nPatrick\n\n\nPatrick Steinhardt (4):\n  t1400: avoid touching refs on filesystem\n  update-ref: allow creation of multiple transactions\n  p1400: use `git-update-ref --stdin` to test multiple transactions\n  update-ref: disallow \"start\" for ongoing transactions\n\n Documentation/git-update-ref.txt |   3 +-\n builtin/update-ref.c             |  15 +++-\n t/perf/p1400-update-ref.sh       |  20 ++---\n t/t1400-update-ref.sh            | 138 +++++++++++++++++++++++--------\n 4 files changed, 126 insertions(+), 50 deletions(-)\n\n-- \n2.29.2\n\n"},{"id":"409857","messageId":"617d48b00a13c8ef82749f0b610997625f6cf222.1605254957.git.ps@pks.im","threadId":"54571","inReplyTo":"cover.1605254957.git.ps@pks.im","subject":"[PATCH v4 1/4] t1400: avoid touching refs on filesystem","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2020-11-13T08:12:31Z","receivedAt":"2020-11-13T08:12:38Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The testcase t1400 exercises the git-update-ref(1) utility. To do so,\nmany tests directly read and write references via the filesystem,\nassuming that we always use loose and/or packed references. While this\nis true now, it'll change with the introduction of the reftable backend.\n\nConvert those tests to use git-update-ref(1) and git-show-ref(1) where\npossible. Furthermore, two tests are converted to not delete HEAD\nanymore, as this results in a broken repository. They've instead been\nupdated to create a non-mandatory symybolic reference and delete that\none instead.\n\nSome tests remain which exercise behaviour with broken references, which\ncannot currently be converted to use regular git tooling.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n t/t1400-update-ref.sh | 77 +++++++++++++++++++++++--------------------\n 1 file changed, 42 insertions(+), 35 deletions(-)\n\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex 4c01e08551..b782dafff5 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -48,17 +48,17 @@ test_expect_success \"fail to delete $m with stale ref\" '\n \ttest $B = \"$(git show-ref -s --verify $m)\"\n '\n test_expect_success \"delete $m\" '\n-\ttest_when_finished \"rm -f .git/$m\" &&\n+\ttest_when_finished \"git update-ref -d $m\" &&\n \tgit update-ref -d $m $B &&\n-\ttest_path_is_missing .git/$m\n+\ttest_must_fail git show-ref --verify -q $m\n '\n \n test_expect_success \"delete $m without oldvalue verification\" '\n-\ttest_when_finished \"rm -f .git/$m\" &&\n+\ttest_when_finished \"git update-ref -d $m\" &&\n \tgit update-ref $m $A &&\n \ttest $A = $(git show-ref -s --verify $m) &&\n \tgit update-ref -d $m &&\n-\ttest_path_is_missing .git/$m\n+\ttest_must_fail git show-ref --verify -q $m\n '\n \n test_expect_success \"fail to create $n\" '\n@@ -80,26 +80,26 @@ test_expect_success \"fail to delete $m (by HEAD) with stale ref\" '\n \ttest $B = $(git show-ref -s --verify $m)\n '\n test_expect_success \"delete $m (by HEAD)\" '\n-\ttest_when_finished \"rm -f .git/$m\" &&\n+\ttest_when_finished \"git update-ref -d $m\" &&\n \tgit update-ref -d HEAD $B &&\n-\ttest_path_is_missing .git/$m\n+\ttest_must_fail git show-ref --verify -q $m\n '\n \n test_expect_success \"deleting current branch adds message to HEAD's log\" '\n-\ttest_when_finished \"rm -f .git/$m\" &&\n+\ttest_when_finished \"git update-ref -d $m\" &&\n \tgit update-ref $m $A &&\n \tgit symbolic-ref HEAD $m &&\n \tgit update-ref -m delete-$m -d $m &&\n-\ttest_path_is_missing .git/$m &&\n+\ttest_must_fail git show-ref --verify -q $m &&\n \tgrep \"delete-$m$\" .git/logs/HEAD\n '\n \n test_expect_success \"deleting by HEAD adds message to HEAD's log\" '\n-\ttest_when_finished \"rm -f .git/$m\" &&\n+\ttest_when_finished \"git update-ref -d $m\" &&\n \tgit update-ref $m $A &&\n \tgit symbolic-ref HEAD $m &&\n \tgit update-ref -m delete-by-head -d HEAD &&\n-\ttest_path_is_missing .git/$m &&\n+\ttest_must_fail git show-ref --verify -q $m &&\n \tgrep \"delete-by-head$\" .git/logs/HEAD\n '\n \n@@ -188,31 +188,37 @@ test_expect_success \"move $m (by HEAD)\" '\n \ttest $B = $(git show-ref -s --verify $m)\n '\n test_expect_success \"delete $m (by HEAD) should remove both packed and loose $m\" '\n-\ttest_when_finished \"rm -f .git/$m\" &&\n+\ttest_when_finished \"git update-ref -d $m\" &&\n \tgit update-ref -d HEAD $B &&\n \t! grep \"$m\" .git/packed-refs &&\n-\ttest_path_is_missing .git/$m\n+\ttest_must_fail git show-ref --verify -q $m\n '\n \n-cp -f .git/HEAD .git/HEAD.orig\n test_expect_success 'delete symref without dereference' '\n-\ttest_when_finished \"cp -f .git/HEAD.orig .git/HEAD\" &&\n-\tgit update-ref --no-deref -d HEAD &&\n-\ttest_path_is_missing .git/HEAD\n+\ttest_when_finished \"git update-ref -d $m\" &&\n+\techo foo >foo.c &&\n+\tgit add foo.c &&\n+\tgit commit -m foo &&\n+\tgit symbolic-ref SYMREF $m &&\n+\tgit update-ref --no-deref -d SYMREF &&\n+\tgit show-ref --verify -q $m &&\n+\ttest_must_fail git show-ref --verify -q SYMREF &&\n+\ttest_must_fail git symbolic-ref SYMREF\n '\n \n test_expect_success 'delete symref without dereference when the referred ref is packed' '\n-\ttest_when_finished \"cp -f .git/HEAD.orig .git/HEAD\" &&\n+\ttest_when_finished \"git update-ref -d $m\" &&\n \techo foo >foo.c &&\n \tgit add foo.c &&\n \tgit commit -m foo &&\n+\tgit symbolic-ref SYMREF $m &&\n \tgit pack-refs --all &&\n-\tgit update-ref --no-deref -d HEAD &&\n-\ttest_path_is_missing .git/HEAD\n+\tgit update-ref --no-deref -d SYMREF &&\n+\tgit show-ref --verify -q $m &&\n+\ttest_must_fail git show-ref --verify -q SYMREF &&\n+\ttest_must_fail git symbolic-ref SYMREF\n '\n \n-git update-ref -d $m\n-\n test_expect_success 'update-ref -d is not confused by self-reference' '\n \tgit symbolic-ref refs/heads/self refs/heads/self &&\n \ttest_when_finished \"rm -f .git/refs/heads/self\" &&\n@@ -226,25 +232,25 @@ test_expect_success 'update-ref --no-deref -d can delete self-reference' '\n \ttest_when_finished \"rm -f .git/refs/heads/self\" &&\n \ttest_path_is_file .git/refs/heads/self &&\n \tgit update-ref --no-deref -d refs/heads/self &&\n-\ttest_path_is_missing .git/refs/heads/self\n+\ttest_must_fail git show-ref --verify -q refs/heads/self\n '\n \n test_expect_success 'update-ref --no-deref -d can delete reference to bad ref' '\n \t>.git/refs/heads/bad &&\n \ttest_when_finished \"rm -f .git/refs/heads/bad\" &&\n \tgit symbolic-ref refs/heads/ref-to-bad refs/heads/bad &&\n-\ttest_when_finished \"rm -f .git/refs/heads/ref-to-bad\" &&\n+\ttest_when_finished \"git update-ref -d refs/heads/ref-to-bad\" &&\n \ttest_path_is_file .git/refs/heads/ref-to-bad &&\n \tgit update-ref --no-deref -d refs/heads/ref-to-bad &&\n-\ttest_path_is_missing .git/refs/heads/ref-to-bad\n+\ttest_must_fail git show-ref --verify -q refs/heads/ref-to-bad\n '\n \n test_expect_success '(not) create HEAD with old sha1' '\n \ttest_must_fail git update-ref HEAD $A $B\n '\n test_expect_success \"(not) prior created .git/$m\" '\n-\ttest_when_finished \"rm -f .git/$m\" &&\n-\ttest_path_is_missing .git/$m\n+\ttest_when_finished \"git update-ref -d $m\" &&\n+\ttest_must_fail git show-ref --verify -q $m\n '\n \n test_expect_success 'create HEAD' '\n@@ -254,7 +260,7 @@ test_expect_success '(not) change HEAD with wrong SHA1' '\n \ttest_must_fail git update-ref HEAD $B $Z\n '\n test_expect_success \"(not) changed .git/$m\" '\n-\ttest_when_finished \"rm -f .git/$m\" &&\n+\ttest_when_finished \"git update-ref -d $m\" &&\n \t! test $B = $(git show-ref -s --verify $m)\n '\n \n@@ -284,8 +290,8 @@ test_expect_success 'empty directory removal' '\n \ttest_path_is_file .git/refs/heads/d1/d2/r1 &&\n \ttest_path_is_file .git/logs/refs/heads/d1/d2/r1 &&\n \tgit branch -d d1/d2/r1 &&\n-\ttest_path_is_missing .git/refs/heads/d1/d2 &&\n-\ttest_path_is_missing .git/logs/refs/heads/d1/d2 &&\n+\ttest_must_fail git show-ref --verify -q refs/heads/d1/d2 &&\n+\ttest_must_fail git show-ref --verify -q logs/refs/heads/d1/d2 &&\n \ttest_path_is_file .git/refs/heads/d1/r2 &&\n \ttest_path_is_file .git/logs/refs/heads/d1/r2\n '\n@@ -298,8 +304,8 @@ test_expect_success 'symref empty directory removal' '\n \ttest_path_is_file .git/refs/heads/e1/e2/r1 &&\n \ttest_path_is_file .git/logs/refs/heads/e1/e2/r1 &&\n \tgit update-ref -d HEAD &&\n-\ttest_path_is_missing .git/refs/heads/e1/e2 &&\n-\ttest_path_is_missing .git/logs/refs/heads/e1/e2 &&\n+\ttest_must_fail git show-ref --verify -q refs/heads/e1/e2 &&\n+\ttest_must_fail git show-ref --verify -q logs/refs/heads/e1/e2 &&\n \ttest_path_is_file .git/refs/heads/e1/r2 &&\n \ttest_path_is_file .git/logs/refs/heads/e1/r2 &&\n \ttest_path_is_file .git/logs/HEAD\n@@ -1388,7 +1394,8 @@ test_expect_success 'handle per-worktree refs in refs/bisect' '\n \t\tgit rev-parse refs/bisect/something >../worktree-head &&\n \t\tgit for-each-ref | grep refs/bisect/something\n \t) &&\n-\ttest_path_is_missing .git/refs/bisect &&\n+\tgit show-ref >actual &&\n+\t! grep 'refs/bisect' actual &&\n \ttest_must_fail git rev-parse refs/bisect/something &&\n \tgit update-ref refs/bisect/something HEAD &&\n \tgit rev-parse refs/bisect/something >main-head &&\n@@ -1500,7 +1507,7 @@ test_expect_success 'transaction can handle abort' '\n \tgit update-ref --stdin <stdin >actual &&\n \tprintf \"%s: ok\\n\" start abort >expect &&\n \ttest_cmp expect actual &&\n-\ttest_path_is_missing .git/$b\n+\ttest_must_fail git show-ref --verify -q $b\n '\n \n test_expect_success 'transaction aborts by default' '\n@@ -1511,7 +1518,7 @@ test_expect_success 'transaction aborts by default' '\n \tgit update-ref --stdin <stdin >actual &&\n \tprintf \"%s: ok\\n\" start >expect &&\n \ttest_cmp expect actual &&\n-\ttest_path_is_missing .git/$b\n+\ttest_must_fail git show-ref --verify -q $b\n '\n \n test_expect_success 'transaction with prepare aborts by default' '\n@@ -1523,7 +1530,7 @@ test_expect_success 'transaction with prepare aborts by default' '\n \tgit update-ref --stdin <stdin >actual &&\n \tprintf \"%s: ok\\n\" start prepare >expect &&\n \ttest_cmp expect actual &&\n-\ttest_path_is_missing .git/$b\n+\ttest_must_fail git show-ref --verify -q $b\n '\n \n test_done\n-- \n2.29.2\n\n"},{"id":"409858","messageId":"8a05b48223f2829f536ea1f95f58c017efb8cf3d.1605254957.git.ps@pks.im","threadId":"54571","inReplyTo":"cover.1605254957.git.ps@pks.im","subject":"[PATCH v4 2/4] update-ref: allow creation of multiple transactions","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2020-11-13T08:12:36Z","receivedAt":"2020-11-13T08:12:41Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"While git-update-ref has recently grown commands which allow interactive\ncontrol of transactions in e48cf33b61 (update-ref: implement interactive\ntransaction handling, 2020-04-02), it is not yet possible to create\nmultiple transactions in a single session. To do so, one currently still\nneeds to invoke the executable multiple times.\n\nThis commit addresses this shortcoming by allowing the \"start\" command\nto create a new transaction if the current transaction has already been\neither committed or aborted.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n Documentation/git-update-ref.txt |  3 +-\n builtin/update-ref.c             | 13 ++++++++-\n t/t1400-update-ref.sh            | 50 ++++++++++++++++++++++++++++++++\n 3 files changed, 64 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt\nindex d401234b03..48b6683071 100644\n--- a/Documentation/git-update-ref.txt\n+++ b/Documentation/git-update-ref.txt\n@@ -125,7 +125,8 @@ option::\n start::\n \tStart a transaction. In contrast to a non-transactional session, a\n \ttransaction will automatically abort if the session ends without an\n-\texplicit commit.\n+\texplicit commit. This command may create a new empty transaction when\n+\tthe current one has been committed or aborted already.\n \n prepare::\n \tPrepare to commit the transaction. This will create lock files for all\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex 8a2df4459c..bb65129012 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -446,7 +446,18 @@ static void update_refs_stdin(void)\n \t\t\tstate = cmd->state;\n \t\t\tbreak;\n \t\tcase UPDATE_REFS_CLOSED:\n-\t\t\tdie(\"transaction is closed\");\n+\t\t\tif (cmd->state != UPDATE_REFS_STARTED)\n+\t\t\t\tdie(\"transaction is closed\");\n+\n+\t\t\t/*\n+\t\t\t * Open a new transaction if we're currently closed and\n+\t\t\t * get a \"start\".\n+\t\t\t */\n+\t\t\tstate = cmd->state;\n+\t\t\ttransaction = ref_transaction_begin(&err);\n+\t\t\tif (!transaction)\n+\t\t\t\tdie(\"%s\", err.buf);\n+\n \t\t\tbreak;\n \t\t}\n \ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex b782dafff5..3144e98b31 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -1533,4 +1533,54 @@ test_expect_success 'transaction with prepare aborts by default' '\n \ttest_must_fail git show-ref --verify -q $b\n '\n \n+test_expect_success 'transaction can commit multiple times' '\n+\tcat >stdin <<-EOF &&\n+\tstart\n+\tcreate refs/heads/branch-1 $A\n+\tcommit\n+\tstart\n+\tcreate refs/heads/branch-2 $B\n+\tcommit\n+\tEOF\n+\tgit update-ref --stdin <stdin >actual &&\n+\tprintf \"%s: ok\\n\" start commit start commit >expect &&\n+\ttest_cmp expect actual &&\n+\techo \"$A\" >expect &&\n+\tgit rev-parse refs/heads/branch-1 >actual &&\n+\ttest_cmp expect actual &&\n+\techo \"$B\" >expect &&\n+\tgit rev-parse refs/heads/branch-2 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'transaction can create and delete' '\n+\tcat >stdin <<-EOF &&\n+\tstart\n+\tcreate refs/heads/create-and-delete $A\n+\tcommit\n+\tstart\n+\tdelete refs/heads/create-and-delete $A\n+\tcommit\n+\tEOF\n+\tgit update-ref --stdin <stdin >actual &&\n+\tprintf \"%s: ok\\n\" start commit start commit >expect &&\n+\ttest_must_fail git show-ref --verify refs/heads/create-and-delete\n+'\n+\n+test_expect_success 'transaction can commit after abort' '\n+\tcat >stdin <<-EOF &&\n+\tstart\n+\tcreate refs/heads/abort $A\n+\tabort\n+\tstart\n+\tcreate refs/heads/abort $A\n+\tcommit\n+\tEOF\n+\tgit update-ref --stdin <stdin >actual &&\n+\tprintf \"%s: ok\\n\" start abort start commit >expect &&\n+\techo \"$A\" >expect &&\n+\tgit rev-parse refs/heads/abort >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.29.2\n\n"},{"id":"409859","messageId":"4778a60832185971a92b708d533cd03ac2af4973.1605254957.git.ps@pks.im","threadId":"54571","inReplyTo":"cover.1605254957.git.ps@pks.im","subject":"[PATCH v4 3/4] p1400: use `git-update-ref --stdin` to test multiple transactions","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2020-11-13T08:12:41Z","receivedAt":"2020-11-13T08:12:45Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In commit 0a0fbbe3ff (refs: remove lookup cache for\nreference-transaction hook, 2020-08-25), a new benchmark was added to\np1400 which has the intention to exercise creation of multiple\ntransactions in a single process. As git-update-ref wasn't yet able to\ncreate multiple transactions with a single run we instead used git-push.\nAs its non-atomic version creates a transaction per reference update,\nthis was the best approximation we could make at that point in time.\n\nNow that `git-update-ref --stdin` supports creation of multiple\ntransactions, let's convert the benchmark to use that instead. It has\nless overhead and it's also a lot clearer what the actual intention is.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n t/perf/p1400-update-ref.sh | 20 +++++++-------------\n 1 file changed, 7 insertions(+), 13 deletions(-)\n\ndiff --git a/t/perf/p1400-update-ref.sh b/t/perf/p1400-update-ref.sh\nindex ce5ac3ed85..dda8a74866 100755\n--- a/t/perf/p1400-update-ref.sh\n+++ b/t/perf/p1400-update-ref.sh\n@@ -7,13 +7,14 @@ test_description=\"Tests performance of update-ref\"\n test_perf_fresh_repo\n \n test_expect_success \"setup\" '\n-\tgit init --bare target-repo.git &&\n \ttest_commit PRE &&\n \ttest_commit POST &&\n-\tprintf \"create refs/heads/%d PRE\\n\" $(test_seq 1000) >create &&\n-\tprintf \"update refs/heads/%d POST PRE\\n\" $(test_seq 1000) >update &&\n-\tprintf \"delete refs/heads/%d POST\\n\" $(test_seq 1000) >delete &&\n-\tgit update-ref --stdin <create\n+\tfor i in $(test_seq 5000)\n+\tdo\n+\t\tprintf \"start\\ncreate refs/heads/%d PRE\\ncommit\\n\" $i &&\n+\t\tprintf \"start\\nupdate refs/heads/%d POST PRE\\ncommit\\n\" $i &&\n+\t\tprintf \"start\\ndelete refs/heads/%d POST\\ncommit\\n\" $i\n+\tdone >instructions\n '\n \n test_perf \"update-ref\" '\n@@ -26,14 +27,7 @@ test_perf \"update-ref\" '\n '\n \n test_perf \"update-ref --stdin\" '\n-\tgit update-ref --stdin <update &&\n-\tgit update-ref --stdin <delete &&\n-\tgit update-ref --stdin <create\n-'\n-\n-test_perf \"nonatomic push\" '\n-\tgit push ./target-repo.git $(test_seq 1000) &&\n-\tgit push --delete ./target-repo.git $(test_seq 1000)\n+\tgit update-ref --stdin <instructions >/dev/null\n '\n \n test_done\n-- \n2.29.2\n\n"},{"id":"409860","messageId":"d50088345979abab14b7ab755866b78ef63f6d56.1605254957.git.ps@pks.im","threadId":"54571","inReplyTo":"cover.1605254957.git.ps@pks.im","subject":"[PATCH v4 4/4] update-ref: disallow \"start\" for ongoing transactions","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2020-11-13T08:12:45Z","receivedAt":"2020-11-13T08:12:49Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"It is currently possible to write multiple \"start\" commands into\ngit-update-ref(1) for a single session, but none of them except for the\nfirst one actually have any effect.\n\nUsing such nested \"start\"s may eventually have a sensible effect. One\nmay imagine that it restarts the current transaction, effectively\nemptying it and creating a new one. It may also allow for creation of\nnested transactions. But currently, none of these are implemented.\n\nSilently ignoring this misuse is making it hard to iterate in the future\nif \"start\" is ever going to have meaningful semantics in such a context.\nThis commit thus makes sure to error out in case we see such use.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n builtin/update-ref.c  |  2 ++\n t/t1400-update-ref.sh | 11 +++++++++++\n 2 files changed, 13 insertions(+)\n\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex bb65129012..6029a80544 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -436,6 +436,8 @@ static void update_refs_stdin(void)\n \t\tswitch (state) {\n \t\tcase UPDATE_REFS_OPEN:\n \t\tcase UPDATE_REFS_STARTED:\n+\t\t\tif (state == UPDATE_REFS_STARTED && cmd->state == UPDATE_REFS_STARTED)\n+\t\t\t\tdie(\"cannot restart ongoing transaction\");\n \t\t\t/* Do not downgrade a transaction to a non-transaction. */\n \t\t\tif (cmd->state >= state)\n \t\t\t\tstate = cmd->state;\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex 3144e98b31..31b64be521 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -1583,4 +1583,15 @@ test_expect_success 'transaction can commit after abort' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'transaction cannot restart ongoing transaction' '\n+\tcat >stdin <<-EOF &&\n+\tstart\n+\tcreate refs/heads/restart $A\n+\tstart\n+\tcommit\n+\tEOF\n+\ttest_must_fail git update-ref --stdin <stdin >actual &&\n+\ttest_must_fail git show-ref --verify refs/heads/restart\n+'\n+\n test_done\n-- \n2.29.2\n\n"},{"id":"409916","messageId":"20201113204031.GA780435@coredump.intra.peff.net","threadId":"54571","inReplyTo":"617d48b00a13c8ef82749f0b610997625f6cf222.1605254957.git.ps@pks.im","subject":"Re: [PATCH v4 1/4] t1400: avoid touching refs on filesystem","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-11-13T20:40:31Z","receivedAt":"2020-11-13T20:40:33Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Nov 13, 2020 at 09:12:31AM +0100, Patrick Steinhardt wrote:\n\n> The testcase t1400 exercises the git-update-ref(1) utility. To do so,\n> many tests directly read and write references via the filesystem,\n> assuming that we always use loose and/or packed references. While this\n> is true now, it'll change with the introduction of the reftable backend.\n> \n> Convert those tests to use git-update-ref(1) and git-show-ref(1) where\n> possible. Furthermore, two tests are converted to not delete HEAD\n> anymore, as this results in a broken repository. They've instead been\n> updated to create a non-mandatory symybolic reference and delete that\n> one instead.\n\ns/symybolic/symbolic/\n\nOther than, this whole series looks good to me. Thanks for taking the\ntime to do the extra cleanup (which ended up being way more complicated\nthan the original goal :) ).\n\n-Peff\n"},{"id":"410205","messageId":"X7TDxPFjrG4vruHx@ncase","threadId":"54571","inReplyTo":"20201113204031.GA780435@coredump.intra.peff.net","subject":"Re: [PATCH v4 1/4] t1400: avoid touching refs on filesystem","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2020-11-18T06:48:36Z","receivedAt":"2020-11-18T06:49:06Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Nov 13, 2020 at 03:40:31PM -0500, Jeff King wrote:\n> On Fri, Nov 13, 2020 at 09:12:31AM +0100, Patrick Steinhardt wrote:\n> \n> > The testcase t1400 exercises the git-update-ref(1) utility. To do so,\n> > many tests directly read and write references via the filesystem,\n> > assuming that we always use loose and/or packed references. While this\n> > is true now, it'll change with the introduction of the reftable backend.\n> > \n> > Convert those tests to use git-update-ref(1) and git-show-ref(1) where\n> > possible. Furthermore, two tests are converted to not delete HEAD\n> > anymore, as this results in a broken repository. They've instead been\n> > updated to create a non-mandatory symybolic reference and delete that\n> > one instead.\n> \n> s/symybolic/symbolic/\n> \n> Other than, this whole series looks good to me. Thanks for taking the\n> time to do the extra cleanup (which ended up being way more complicated\n> than the original goal :) ).\n\nThanks!\n\nJunio, shall I fix this typo with another version or will you fix this\nup locally?\n\nPatrick\n"},{"id":"410207","messageId":"xmqq5z63406p.fsf@gitster.c.googlers.com","threadId":"54571","inReplyTo":"X7TDxPFjrG4vruHx@ncase","subject":"Re: [PATCH v4 1/4] t1400: avoid touching refs on filesystem","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-11-18T07:02:38Z","receivedAt":"2020-11-18T07:02:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> Junio, shall I fix this typo with another version or will you fix this\n> up locally?\n\nPlease make it a habit to check what is on 'seen' before asking.\n\nGiven we have timezone differences (and this late at local night I\nam not in an environment in which I can check it myself easily),\ndoing so would be much faster.\n\nThanks.\n"},{"id":"410821","messageId":"xmqqh7pdcbb1.fsf@gitster.c.googlers.com","threadId":"54571","inReplyTo":"cover.1605254957.git.ps@pks.im","subject":"Re: [PATCH v4 0/4] update-ref: allow creation of multiple transactions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-11-25T22:37:54Z","receivedAt":"2020-11-25T22:38:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> this is the fourth version of this patch series implementing support for\n> creation of multiple reference transactions in a single git-update-ref\n> process.\n\nIt is my impression that the series is now in good enough shape that\nwe didn't see much discussion on this round.\n\nSo I'll mark this to be merged to 'next', but I ask reviewers to\nplease holler to stop me otherwise.\n\nThanks.\n"},{"id":"410842","messageId":"X776EmARaq3QXRS3@coredump.intra.peff.net","threadId":"54571","inReplyTo":"xmqqh7pdcbb1.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v4 0/4] update-ref: allow creation of multiple transactions","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-11-26T00:42:58Z","receivedAt":"2020-11-26T00:43:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 25, 2020 at 02:37:54PM -0800, Junio C Hamano wrote:\n\n> Patrick Steinhardt <ps@pks.im> writes:\n> \n> > this is the fourth version of this patch series implementing support for\n> > creation of multiple reference transactions in a single git-update-ref\n> > process.\n> \n> It is my impression that the series is now in good enough shape that\n> we didn't see much discussion on this round.\n> \n> So I'll mark this to be merged to 'next', but I ask reviewers to\n> please holler to stop me otherwise.\n\nYeah, this looks good to me for advancing. Thanks.\n\n-Peff\n"}]}