{"thread":{"id":"56435","subject":"[PATCH] update-ref: fix streaming of status updates","startedAt":"2021-09-03T09:06:46Z","lastAt":"2021-09-16T04:39:48Z","messageCount":6,"participants":["Patrick Steinhardt","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"434626","messageId":"1e9e62a2c5f74db91aaded83783076c28b757836.1630659922.git.ps@pks.im","threadId":"56435","inReplyTo":null,"subject":"[PATCH] update-ref: fix streaming of status updates","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2021-09-03T09:06:37Z","receivedAt":"2021-09-03T09:06:46Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When executing git-update-ref(1) with the `--stdin` flag, then the user\ncan queue updates and, since e48cf33b61 (update-ref: implement\ninteractive transaction handling, 2020-04-02), interactively drive the\ntransaction's state via a set of transactional verbs. This interactivity\nis somewhat broken though: while the caller can use these verbs to drive\nthe transaction's state, the status messages which confirm that a verb\nhas been processed is not flushed. The caller may thus be left hanging\nwaiting for the acknowledgement.\n\nFix the bug by flushing stdout after writing the status update. Add a\ntest which catches this bug.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n builtin/update-ref.c  | 14 ++++++++++----\n t/t1400-update-ref.sh | 32 ++++++++++++++++++++++++++++++++\n 2 files changed, 42 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex 6029a80544..a84e7b47a2 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -302,6 +302,12 @@ static void parse_cmd_verify(struct ref_transaction *transaction,\n \tstrbuf_release(&err);\n }\n \n+static void report_ok(const char *command)\n+{\n+\tfprintf(stdout, \"%s: ok\\n\", command);\n+\tfflush(stdout);\n+}\n+\n static void parse_cmd_option(struct ref_transaction *transaction,\n \t\t\t     const char *next, const char *end)\n {\n@@ -317,7 +323,7 @@ static void parse_cmd_start(struct ref_transaction *transaction,\n {\n \tif (*next != line_termination)\n \t\tdie(\"start: extra input: %s\", next);\n-\tputs(\"start: ok\");\n+\treport_ok(\"start\");\n }\n \n static void parse_cmd_prepare(struct ref_transaction *transaction,\n@@ -328,7 +334,7 @@ static void parse_cmd_prepare(struct ref_transaction *transaction,\n \t\tdie(\"prepare: extra input: %s\", next);\n \tif (ref_transaction_prepare(transaction, &error))\n \t\tdie(\"prepare: %s\", error.buf);\n-\tputs(\"prepare: ok\");\n+\treport_ok(\"prepare\");\n }\n \n static void parse_cmd_abort(struct ref_transaction *transaction,\n@@ -339,7 +345,7 @@ static void parse_cmd_abort(struct ref_transaction *transaction,\n \t\tdie(\"abort: extra input: %s\", next);\n \tif (ref_transaction_abort(transaction, &error))\n \t\tdie(\"abort: %s\", error.buf);\n-\tputs(\"abort: ok\");\n+\treport_ok(\"abort\");\n }\n \n static void parse_cmd_commit(struct ref_transaction *transaction,\n@@ -350,7 +356,7 @@ static void parse_cmd_commit(struct ref_transaction *transaction,\n \t\tdie(\"commit: extra input: %s\", next);\n \tif (ref_transaction_commit(transaction, &error))\n \t\tdie(\"commit: %s\", error.buf);\n-\tputs(\"commit: ok\");\n+\treport_ok(\"commit\");\n \tref_transaction_free(transaction);\n }\n \ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex 4506cd435b..1e754e258f 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -1598,6 +1598,38 @@ test_expect_success 'transaction cannot restart ongoing transaction' '\n \ttest_must_fail git show-ref --verify refs/heads/restart\n '\n \n+test_expect_success PIPE 'transaction flushes status updates' '\n+\tmkfifo in out &&\n+\t(git update-ref --stdin <in >out &) &&\n+\n+\texec 9>in &&\n+\ttest_when_finished \"exec 9>&-\" &&\n+\n+\techo \"start\" >&9 &&\n+\techo \"start: ok\" >expected &&\n+\tread line <out &&\n+\techo \"$line\" >actual &&\n+\ttest_cmp expected actual &&\n+\n+\techo \"create refs/heads/flush $A\" >&9 &&\n+\n+\techo prepare >&9 &&\n+\techo \"prepare: ok\" >expected &&\n+\tread line <out &&\n+\techo \"$line\" >actual &&\n+\ttest_cmp expected actual &&\n+\n+\t# This must now fail given that we have locked the ref.\n+\ttest_must_fail git update-ref refs/heads/flush $B 2>stderr &&\n+\tgrep \"fatal: update_ref failed for ref ${SQ}refs/heads/flush${SQ}: cannot lock ref\" stderr &&\n+\n+\techo commit >&9 &&\n+\techo \"commit: ok\" >expected &&\n+\tread line <out &&\n+\techo \"$line\" >actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_expect_success 'directory not created deleting packed ref' '\n \tgit branch d1/d2/r1 HEAD &&\n \tgit pack-refs --all &&\n-- \n2.33.0\n\n"},{"id":"434666","messageId":"xmqqfsulfqyi.fsf@gitster.g","threadId":"56435","inReplyTo":"1e9e62a2c5f74db91aaded83783076c28b757836.1630659922.git.ps@pks.im","subject":"Re: [PATCH] update-ref: fix streaming of status updates","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-09-03T18:34:45Z","receivedAt":"2021-09-03T18:34:51Z","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> When executing git-update-ref(1) with the `--stdin` flag, then the user\n> can queue updates and, since e48cf33b61 (update-ref: implement\n> interactive transaction handling, 2020-04-02), interactively drive the\n> transaction's state via a set of transactional verbs. This interactivity\n> is somewhat broken though: while the caller can use these verbs to drive\n> the transaction's state, the status messages which confirm that a verb\n> has been processed is not flushed. The caller may thus be left hanging\n> waiting for the acknowledgement.\n>\n> Fix the bug by flushing stdout after writing the status update. Add a\n> test which catches this bug.\n\nNobody hit by this for more than a year is interesting.\n\nWill queue.  Thanks.\n\n>\n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>  builtin/update-ref.c  | 14 ++++++++++----\n>  t/t1400-update-ref.sh | 32 ++++++++++++++++++++++++++++++++\n>  2 files changed, 42 insertions(+), 4 deletions(-)\n>\n> diff --git a/builtin/update-ref.c b/builtin/update-ref.c\n> index 6029a80544..a84e7b47a2 100644\n> --- a/builtin/update-ref.c\n> +++ b/builtin/update-ref.c\n> @@ -302,6 +302,12 @@ static void parse_cmd_verify(struct ref_transaction *transaction,\n>  \tstrbuf_release(&err);\n>  }\n>  \n> +static void report_ok(const char *command)\n> +{\n> +\tfprintf(stdout, \"%s: ok\\n\", command);\n> +\tfflush(stdout);\n> +}\n> +\n>  static void parse_cmd_option(struct ref_transaction *transaction,\n>  \t\t\t     const char *next, const char *end)\n>  {\n> @@ -317,7 +323,7 @@ static void parse_cmd_start(struct ref_transaction *transaction,\n>  {\n>  \tif (*next != line_termination)\n>  \t\tdie(\"start: extra input: %s\", next);\n> -\tputs(\"start: ok\");\n> +\treport_ok(\"start\");\n>  }\n>  \n>  static void parse_cmd_prepare(struct ref_transaction *transaction,\n> @@ -328,7 +334,7 @@ static void parse_cmd_prepare(struct ref_transaction *transaction,\n>  \t\tdie(\"prepare: extra input: %s\", next);\n>  \tif (ref_transaction_prepare(transaction, &error))\n>  \t\tdie(\"prepare: %s\", error.buf);\n> -\tputs(\"prepare: ok\");\n> +\treport_ok(\"prepare\");\n>  }\n>  \n>  static void parse_cmd_abort(struct ref_transaction *transaction,\n> @@ -339,7 +345,7 @@ static void parse_cmd_abort(struct ref_transaction *transaction,\n>  \t\tdie(\"abort: extra input: %s\", next);\n>  \tif (ref_transaction_abort(transaction, &error))\n>  \t\tdie(\"abort: %s\", error.buf);\n> -\tputs(\"abort: ok\");\n> +\treport_ok(\"abort\");\n>  }\n>  \n>  static void parse_cmd_commit(struct ref_transaction *transaction,\n> @@ -350,7 +356,7 @@ static void parse_cmd_commit(struct ref_transaction *transaction,\n>  \t\tdie(\"commit: extra input: %s\", next);\n>  \tif (ref_transaction_commit(transaction, &error))\n>  \t\tdie(\"commit: %s\", error.buf);\n> -\tputs(\"commit: ok\");\n> +\treport_ok(\"commit\");\n>  \tref_transaction_free(transaction);\n>  }\n>  \n> diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\n> index 4506cd435b..1e754e258f 100755\n> --- a/t/t1400-update-ref.sh\n> +++ b/t/t1400-update-ref.sh\n> @@ -1598,6 +1598,38 @@ test_expect_success 'transaction cannot restart ongoing transaction' '\n>  \ttest_must_fail git show-ref --verify refs/heads/restart\n>  '\n>  \n> +test_expect_success PIPE 'transaction flushes status updates' '\n> +\tmkfifo in out &&\n> +\t(git update-ref --stdin <in >out &) &&\n> +\n> +\texec 9>in &&\n> +\ttest_when_finished \"exec 9>&-\" &&\n> +\n> +\techo \"start\" >&9 &&\n> +\techo \"start: ok\" >expected &&\n> +\tread line <out &&\n> +\techo \"$line\" >actual &&\n> +\ttest_cmp expected actual &&\n> +\n> +\techo \"create refs/heads/flush $A\" >&9 &&\n> +\n> +\techo prepare >&9 &&\n> +\techo \"prepare: ok\" >expected &&\n> +\tread line <out &&\n> +\techo \"$line\" >actual &&\n> +\ttest_cmp expected actual &&\n> +\n> +\t# This must now fail given that we have locked the ref.\n> +\ttest_must_fail git update-ref refs/heads/flush $B 2>stderr &&\n> +\tgrep \"fatal: update_ref failed for ref ${SQ}refs/heads/flush${SQ}: cannot lock ref\" stderr &&\n> +\n> +\techo commit >&9 &&\n> +\techo \"commit: ok\" >expected &&\n> +\tread line <out &&\n> +\techo \"$line\" >actual &&\n> +\ttest_cmp expected actual\n> +'\n> +\n>  test_expect_success 'directory not created deleting packed ref' '\n>  \tgit branch d1/d2/r1 HEAD &&\n>  \tgit pack-refs --all &&\n"},{"id":"436005","messageId":"YUIptJNN5DxrkydV@coredump.intra.peff.net","threadId":"56435","inReplyTo":"1e9e62a2c5f74db91aaded83783076c28b757836.1630659922.git.ps@pks.im","subject":"Re: [PATCH] update-ref: fix streaming of status updates","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-09-15T17:13:24Z","receivedAt":"2021-09-15T17:13:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"> diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\n> index 4506cd435b..1e754e258f 100755\n> --- a/t/t1400-update-ref.sh\n> +++ b/t/t1400-update-ref.sh\n> @@ -1598,6 +1598,38 @@ test_expect_success 'transaction cannot restart ongoing transaction' '\n>  \ttest_must_fail git show-ref --verify refs/heads/restart\n>  '\n>  \n> +test_expect_success PIPE 'transaction flushes status updates' '\n> +\tmkfifo in out &&\n> +\t(git update-ref --stdin <in >out &) &&\n> +\n> +\texec 9>in &&\n> +\ttest_when_finished \"exec 9>&-\" &&\n> +\n> +\techo \"start\" >&9 &&\n> +\techo \"start: ok\" >expected &&\n> +\tread line <out &&\n> +\techo \"$line\" >actual &&\n> +\ttest_cmp expected actual &&\n> +\n> +\techo \"create refs/heads/flush $A\" >&9 &&\n> +\n> +\techo prepare >&9 &&\n> +\techo \"prepare: ok\" >expected &&\n> +\tread line <out &&\n> +\techo \"$line\" >actual &&\n> +\ttest_cmp expected actual &&\n\nI think this test may be racy. I saw a strange failure from it in CI:\n\n  https://github.com/peff/git/runs/3605506649?check_suite_focus=true#step:5:6734\n\nI can't reproduce the problem locally with \"--stress\", but the failure\nthere is on macOS (and likewise, a nearby run failed with a timeout just\nfor macOS, which could be caused by a racy deadlock).\n\nI'm guessing the problem may be the continued opening and closing of\n\"out\" by the read calls from the shell. The update-ref process may get\nSIGPIPE as a result, depending on the write timing.\n\nThe solution is to use open a descriptor for \"out\" like we do for \"in\",\nand then read from that. See 4783e7ea83 (t0008: avoid SIGPIPE race\ncondition on fifo, 2013-07-12) for some prior art.\n\n-Peff\n"},{"id":"436006","messageId":"YUIsZCTqm56KfilP@coredump.intra.peff.net","threadId":"56435","inReplyTo":"YUIptJNN5DxrkydV@coredump.intra.peff.net","subject":"[PATCH] t1400: avoid SIGPIPE race condition on fifo","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-09-15T17:24:52Z","receivedAt":"2021-09-15T17:24:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Sep 15, 2021 at 01:13:25PM -0400, Jeff King wrote:\n\n> I think this test may be racy. I saw a strange failure from it in CI:\n> \n>   https://github.com/peff/git/runs/3605506649?check_suite_focus=true#step:5:6734\n> \n> I can't reproduce the problem locally with \"--stress\", but the failure\n> there is on macOS (and likewise, a nearby run failed with a timeout just\n> for macOS, which could be caused by a racy deadlock).\n\nAh, I just wasn't trying hard enough. Using --run=1,190 lets it run a\nlot more tightly, and I got a failure pretty quickly. Here's the fix (on\ntop of ps/update-ref-batch-flush).\n\n-- >8 --\nSubject: [PATCH] t1400: avoid SIGPIPE race condition on fifo\n\nt1400.190 sometimes fails or even hangs because of the way it uses\nfifos. Our goal is to interactively read and write lines from\nupdate-ref, so we have two fifos, in and out. We open a descriptor\nconnected to \"in\" and redirect output to that, so that update-ref does\nnot see EOF as it would if we opened and closed it for each \"echo\" call.\n\nBut we don't do the same for the output. This leads to a race where our\n\"read response <out\" has not yet opened the fifo, but update-ref tries\nto write to it and gets SIGPIPE. This can result in the test failing, or\nworse, hanging as we wait forever for somebody to write to the pipe.\n\nThis is the same proble we fixed in 4783e7ea83 (t0008: avoid SIGPIPE\nrace condition on fifo, 2013-07-12), and we can fix it the same way, by\nopening a second long-running descriptor.\n\nBefore this patch, running:\n\n  ./t1400-update-ref.sh --run=1,190 --stress\n\nfailed or hung within a few dozen iterations. After it, I ran it for\nseveral hundred without problems.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/t1400-update-ref.sh | 8 +++++---\n 1 file changed, 5 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex 1e754e258f..0d4f73acaa 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -1603,19 +1603,21 @@ test_expect_success PIPE 'transaction flushes status updates' '\n \t(git update-ref --stdin <in >out &) &&\n \n \texec 9>in &&\n+\texec 8<out &&\n \ttest_when_finished \"exec 9>&-\" &&\n+\ttest_when_finished \"exec 8<&-\" &&\n \n \techo \"start\" >&9 &&\n \techo \"start: ok\" >expected &&\n-\tread line <out &&\n+\tread line <&8 &&\n \techo \"$line\" >actual &&\n \ttest_cmp expected actual &&\n \n \techo \"create refs/heads/flush $A\" >&9 &&\n \n \techo prepare >&9 &&\n \techo \"prepare: ok\" >expected &&\n-\tread line <out &&\n+\tread line <&8 &&\n \techo \"$line\" >actual &&\n \ttest_cmp expected actual &&\n \n@@ -1625,7 +1627,7 @@ test_expect_success PIPE 'transaction flushes status updates' '\n \n \techo commit >&9 &&\n \techo \"commit: ok\" >expected &&\n-\tread line <out &&\n+\tread line <&8 &&\n \techo \"$line\" >actual &&\n \ttest_cmp expected actual\n '\n-- \n2.33.0.917.g33ebf6a5f6\n\n"},{"id":"436040","messageId":"xmqqee9pwqig.fsf@gitster.g","threadId":"56435","inReplyTo":"YUIsZCTqm56KfilP@coredump.intra.peff.net","subject":"Re: [PATCH] t1400: avoid SIGPIPE race condition on fifo","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-09-15T20:08:39Z","receivedAt":"2021-09-15T20:08:43Z","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 Wed, Sep 15, 2021 at 01:13:25PM -0400, Jeff King wrote:\n>\n>> I think this test may be racy. I saw a strange failure from it in CI:\n>> \n>>   https://github.com/peff/git/runs/3605506649?check_suite_focus=true#step:5:6734\n>> \n>> I can't reproduce the problem locally with \"--stress\", but the failure\n>> there is on macOS (and likewise, a nearby run failed with a timeout just\n>> for macOS, which could be caused by a racy deadlock).\n>\n> Ah, I just wasn't trying hard enough. Using --run=1,190 lets it run a\n> lot more tightly, and I got a failure pretty quickly. Here's the fix (on\n> top of ps/update-ref-batch-flush).\n>\n> -- >8 --\n> Subject: [PATCH] t1400: avoid SIGPIPE race condition on fifo\n>\n> t1400.190 sometimes fails or even hangs because of the way it uses\n> fifos. Our goal is to interactively read and write lines from\n> update-ref, so we have two fifos, in and out. We open a descriptor\n> connected to \"in\" and redirect output to that, so that update-ref does\n> not see EOF as it would if we opened and closed it for each \"echo\" call.\n\nThanks.  I think I see the same breakage in last night's pushout of\n'seen'.\n\nWill queue.\n"},{"id":"436066","messageId":"YULKiXOpiRmHBNiy@ncase","threadId":"56435","inReplyTo":"YUIsZCTqm56KfilP@coredump.intra.peff.net","subject":"Re: [PATCH] t1400: avoid SIGPIPE race condition on fifo","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2021-09-16T04:39:37Z","receivedAt":"2021-09-16T04:39:48Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Sep 15, 2021 at 01:24:52PM -0400, Jeff King wrote:\n> On Wed, Sep 15, 2021 at 01:13:25PM -0400, Jeff King wrote:\n> \n> > I think this test may be racy. I saw a strange failure from it in CI:\n> > \n> >   https://github.com/peff/git/runs/3605506649?check_suite_focus=true#step:5:6734\n> > \n> > I can't reproduce the problem locally with \"--stress\", but the failure\n> > there is on macOS (and likewise, a nearby run failed with a timeout just\n> > for macOS, which could be caused by a racy deadlock).\n> \n> Ah, I just wasn't trying hard enough. Using --run=1,190 lets it run a\n> lot more tightly, and I got a failure pretty quickly. Here's the fix (on\n> top of ps/update-ref-batch-flush).\n> \n> -- >8 --\n> Subject: [PATCH] t1400: avoid SIGPIPE race condition on fifo\n> \n> t1400.190 sometimes fails or even hangs because of the way it uses\n> fifos. Our goal is to interactively read and write lines from\n> update-ref, so we have two fifos, in and out. We open a descriptor\n> connected to \"in\" and redirect output to that, so that update-ref does\n> not see EOF as it would if we opened and closed it for each \"echo\" call.\n> \n> But we don't do the same for the output. This leads to a race where our\n> \"read response <out\" has not yet opened the fifo, but update-ref tries\n> to write to it and gets SIGPIPE. This can result in the test failing, or\n> worse, hanging as we wait forever for somebody to write to the pipe.\n> \n> This is the same proble we fixed in 4783e7ea83 (t0008: avoid SIGPIPE\n\nType: \"proble\" -> \"problem\".\n\n> race condition on fifo, 2013-07-12), and we can fix it the same way, by\n> opening a second long-running descriptor.\n> \n> Before this patch, running:\n> \n>   ./t1400-update-ref.sh --run=1,190 --stress\n> \n> failed or hung within a few dozen iterations. After it, I ran it for\n> several hundred without problems.\n> \n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n>  t/t1400-update-ref.sh | 8 +++++---\n>  1 file changed, 5 insertions(+), 3 deletions(-)\n> \n> diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\n> index 1e754e258f..0d4f73acaa 100755\n> --- a/t/t1400-update-ref.sh\n> +++ b/t/t1400-update-ref.sh\n> @@ -1603,19 +1603,21 @@ test_expect_success PIPE 'transaction flushes status updates' '\n>  \t(git update-ref --stdin <in >out &) &&\n>  \n>  \texec 9>in &&\n> +\texec 8<out &&\n>  \ttest_when_finished \"exec 9>&-\" &&\n> +\ttest_when_finished \"exec 8<&-\" &&\n>  \n>  \techo \"start\" >&9 &&\n>  \techo \"start: ok\" >expected &&\n> -\tread line <out &&\n> +\tread line <&8 &&\n>  \techo \"$line\" >actual &&\n>  \ttest_cmp expected actual &&\n>  \n>  \techo \"create refs/heads/flush $A\" >&9 &&\n>  \n>  \techo prepare >&9 &&\n>  \techo \"prepare: ok\" >expected &&\n> -\tread line <out &&\n> +\tread line <&8 &&\n>  \techo \"$line\" >actual &&\n>  \ttest_cmp expected actual &&\n>  \n> @@ -1625,7 +1627,7 @@ test_expect_success PIPE 'transaction flushes status updates' '\n>  \n>  \techo commit >&9 &&\n>  \techo \"commit: ok\" >expected &&\n> -\tread line <out &&\n> +\tread line <&8 &&\n>  \techo \"$line\" >actual &&\n>  \ttest_cmp expected actual\n>  '\n> -- \n> 2.33.0.917.g33ebf6a5f6\n> \n\nThanks a lot for digging and fixing my test. The patch looks good to me.\n\nPatrick\n"}]}