{"thread":{"id":"57019","subject":"[PATCH 0/2] worktree: fix incorrectly-ordered messages on Windows","startedAt":"2021-12-03T03:48:39Z","lastAt":"2021-12-11T22:48:05Z","messageCount":14,"participants":["Eric Sunshine","Ævar Arnfjörð Bjarmason","Jeff King","Junio C Hamano","Rafael Silva"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"442928","messageId":"20211203034420.47447-1-sunshine@sunshineco.com","threadId":"57019","inReplyTo":null,"subject":"[PATCH 0/2] worktree: fix incorrectly-ordered messages on Windows","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2021-12-03T03:44:18Z","receivedAt":"2021-12-03T03:48:39Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"This patch series fixes a problem in which a chatty \"Preparing worktree\"\nmessage and a subsequent fatal error message appear in the wrong order\non Microsoft Windows, which may confuse readers into thinking that the\noperation somehow succeeded despite the error.\n\nUnlike the original RFC attempt[*] to fix this problem at a low level in\na generalized fashion, patch [1/2] localizes the fix to git-worktree\nitself by making it conform to common Git practice of issuing chatty\nmessages to stderr rather than to stdout as is currently the case.\n\nPatch [2/2] is just a drive-by fix for a minor documentation problem I\nnoticed along the way.\n\n[*]: https://lore.kernel.org/git/20211130043946.19987-1-sunshine@sunshineco.com/\n\nEric Sunshine (2):\n  worktree: send \"chatty\" messages to stderr\n  git-worktree.txt: add missing `-v` to synopsis for `worktree list`\n\n Documentation/git-worktree.txt |  2 +-\n builtin/worktree.c             | 14 +++++++-------\n t/t2401-worktree-prune.sh      | 14 +++++++-------\n t/t2402-worktree-list.sh       |  2 +-\n t/t2406-worktree-repair.sh     | 30 ++++++++++++------------------\n 5 files changed, 28 insertions(+), 34 deletions(-)\n\n-- \n2.34.1.173.g76aa8bc2d0\n\n"},{"id":"442929","messageId":"20211203034420.47447-2-sunshine@sunshineco.com","threadId":"57019","inReplyTo":"20211203034420.47447-1-sunshine@sunshineco.com","subject":"[PATCH 1/2] worktree: send \"chatty\" messages to stderr","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2021-12-03T03:44:19Z","receivedAt":"2021-12-03T03:48:39Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"The order in which the stdout and stderr streams are flushed is not\nguaranteed to be the same across platforms or `libc` implementations.\nThis lack of determinism can lead to anomalous and potentially confusing\noutput if normal (stdout) output is flushed after error (stderr) output.\nFor instance, the following output which clearly indicates a failure due\nto a fatal error:\n\n    % git worktree add ../foo bar\n    Preparing worktree (checking out 'bar')\n    fatal: 'bar' is already checked out at '.../wherever'\n\nhas been reported[1] on Microsoft Windows to appear as:\n\n    % git worktree add ../foo bar\n    fatal: 'bar' is already checked out at '.../wherever'\n    Preparing worktree (checking out 'bar')\n\nwhich may confuse the reader into thinking that the command somehow\nrecovered and ran to completion despite the error.\n\nThis problem crops up because the \"chatty\" status message \"Preparing\nworktree\" is sent to stdout, whereas the \"fatal\" error message is sent\nto stderr. One way to fix this would be to flush stdout manually before\ngit-worktree reports any errors to stderr.\n\nHowever, common practice in Git is for \"chatty\" messages to be sent to\nstderr. Therefore, a more appropriate fix is to adjust git-worktree to\nconform to that practice by sending its \"chatty\" messages to stderr\nrather than stdout as is currently the case.\n\nThere may be concern that relocating messages from stdout to stderr\ncould break existing tooling, however, these messages are already\ninternationalized, thus are unstable. And, indeed, the \"Preparing\nworktree\" message has already been the subject of somewhat significant\nchanges in 2c27002a0a (worktree: improve message when creating a new\nworktree, 2018-04-24). Moreover, there is existing precedent, such as\n68b939b2f0 (clone: send diagnostic messages to stderr, 2013-09-18) which\nlikewise relocated \"chatty\" messages from stdout to stderr for\ngit-clone.\n\n[1]: https://lore.kernel.org/git/CA+34VNLj6VB1kCkA=MfM7TZR+6HgqNi5-UaziAoCXacSVkch4A@mail.gmail.com/T/\n\nReported-by: Baruch Burstein <bmburstein@gmail.com>\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n builtin/worktree.c         | 14 +++++++-------\n t/t2401-worktree-prune.sh  | 14 +++++++-------\n t/t2402-worktree-list.sh   |  2 +-\n t/t2406-worktree-repair.sh | 30 ++++++++++++------------------\n 4 files changed, 27 insertions(+), 33 deletions(-)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex d22ece93e1..a57fcd0f3c 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -72,7 +72,7 @@ static void delete_worktrees_dir_if_empty(void)\n static void prune_worktree(const char *id, const char *reason)\n {\n \tif (show_only || verbose)\n-\t\tprintf_ln(_(\"Removing %s/%s: %s\"), \"worktrees\", id, reason);\n+\t\tfprintf_ln(stderr, _(\"Removing %s/%s: %s\"), \"worktrees\", id, reason);\n \tif (!show_only)\n \t\tdelete_git_dir(id);\n }\n@@ -418,24 +418,24 @@ static void print_preparing_worktree_line(int detach,\n \tif (force_new_branch) {\n \t\tstruct commit *commit = lookup_commit_reference_by_name(new_branch);\n \t\tif (!commit)\n-\t\t\tprintf_ln(_(\"Preparing worktree (new branch '%s')\"), new_branch);\n+\t\t\tfprintf_ln(stderr, _(\"Preparing worktree (new branch '%s')\"), new_branch);\n \t\telse\n-\t\t\tprintf_ln(_(\"Preparing worktree (resetting branch '%s'; was at %s)\"),\n+\t\t\tfprintf_ln(stderr, _(\"Preparing worktree (resetting branch '%s'; was at %s)\"),\n \t\t\t\t  new_branch,\n \t\t\t\t  find_unique_abbrev(&commit->object.oid, DEFAULT_ABBREV));\n \t} else if (new_branch) {\n-\t\tprintf_ln(_(\"Preparing worktree (new branch '%s')\"), new_branch);\n+\t\tfprintf_ln(stderr, _(\"Preparing worktree (new branch '%s')\"), new_branch);\n \t} else {\n \t\tstruct strbuf s = STRBUF_INIT;\n \t\tif (!detach && !strbuf_check_branch_ref(&s, branch) &&\n \t\t    ref_exists(s.buf))\n-\t\t\tprintf_ln(_(\"Preparing worktree (checking out '%s')\"),\n+\t\t\tfprintf_ln(stderr, _(\"Preparing worktree (checking out '%s')\"),\n \t\t\t\t  branch);\n \t\telse {\n \t\t\tstruct commit *commit = lookup_commit_reference_by_name(branch);\n \t\t\tif (!commit)\n \t\t\t\tdie(_(\"invalid reference: %s\"), branch);\n-\t\t\tprintf_ln(_(\"Preparing worktree (detached HEAD %s)\"),\n+\t\t\tfprintf_ln(stderr, _(\"Preparing worktree (detached HEAD %s)\"),\n \t\t\t\t  find_unique_abbrev(&commit->object.oid, DEFAULT_ABBREV));\n \t\t}\n \t\tstrbuf_release(&s);\n@@ -1006,7 +1006,7 @@ static int remove_worktree(int ac, const char **av, const char *prefix)\n static void report_repair(int iserr, const char *path, const char *msg, void *cb_data)\n {\n \tif (!iserr) {\n-\t\tprintf_ln(_(\"repair: %s: %s\"), msg, path);\n+\t\tfprintf_ln(stderr, _(\"repair: %s: %s\"), msg, path);\n \t} else {\n \t\tint *exit_status = (int *)cb_data;\n \t\tfprintf_ln(stderr, _(\"error: %s: %s\"), msg, path);\ndiff --git a/t/t2401-worktree-prune.sh b/t/t2401-worktree-prune.sh\nindex a615d3b483..3d28c7f06b 100755\n--- a/t/t2401-worktree-prune.sh\n+++ b/t/t2401-worktree-prune.sh\n@@ -19,7 +19,7 @@ test_expect_success 'worktree prune on normal repo' '\n test_expect_success 'prune files inside $GIT_DIR/worktrees' '\n \tmkdir .git/worktrees &&\n \t: >.git/worktrees/abc &&\n-\tgit worktree prune --verbose >actual &&\n+\tgit worktree prune --verbose 2>actual &&\n \tcat >expect <<EOF &&\n Removing worktrees/abc: not a valid directory\n EOF\n@@ -34,7 +34,7 @@ test_expect_success 'prune directories without gitdir' '\n \tcat >expect <<EOF &&\n Removing worktrees/def: gitdir file does not exist\n EOF\n-\tgit worktree prune --verbose >actual &&\n+\tgit worktree prune --verbose 2>actual &&\n \ttest_cmp expect actual &&\n \t! test -d .git/worktrees/def &&\n \t! test -d .git/worktrees\n@@ -45,7 +45,7 @@ test_expect_success SANITY 'prune directories with unreadable gitdir' '\n \t: >.git/worktrees/def/def &&\n \t: >.git/worktrees/def/gitdir &&\n \tchmod u-r .git/worktrees/def/gitdir &&\n-\tgit worktree prune --verbose >actual &&\n+\tgit worktree prune --verbose 2>actual &&\n \ttest_i18ngrep \"Removing worktrees/def: unable to read gitdir file\" actual &&\n \t! test -d .git/worktrees/def &&\n \t! test -d .git/worktrees\n@@ -55,7 +55,7 @@ test_expect_success 'prune directories with invalid gitdir' '\n \tmkdir -p .git/worktrees/def/abc &&\n \t: >.git/worktrees/def/def &&\n \t: >.git/worktrees/def/gitdir &&\n-\tgit worktree prune --verbose >actual &&\n+\tgit worktree prune --verbose 2>actual &&\n \ttest_i18ngrep \"Removing worktrees/def: invalid gitdir file\" actual &&\n \t! test -d .git/worktrees/def &&\n \t! test -d .git/worktrees\n@@ -65,7 +65,7 @@ test_expect_success 'prune directories with gitdir pointing to nowhere' '\n \tmkdir -p .git/worktrees/def/abc &&\n \t: >.git/worktrees/def/def &&\n \techo \"$(pwd)\"/nowhere >.git/worktrees/def/gitdir &&\n-\tgit worktree prune --verbose >actual &&\n+\tgit worktree prune --verbose 2>actual &&\n \ttest_i18ngrep \"Removing worktrees/def: gitdir file points to non-existent location\" actual &&\n \t! test -d .git/worktrees/def &&\n \t! test -d .git/worktrees\n@@ -101,7 +101,7 @@ test_expect_success 'prune duplicate (linked/linked)' '\n \tgit worktree add --detach w2 &&\n \tsed \"s/w2/w1/\" .git/worktrees/w2/gitdir >.git/worktrees/w2/gitdir.new &&\n \tmv .git/worktrees/w2/gitdir.new .git/worktrees/w2/gitdir &&\n-\tgit worktree prune --verbose >actual &&\n+\tgit worktree prune --verbose 2>actual &&\n \ttest_i18ngrep \"duplicate entry\" actual &&\n \ttest -d .git/worktrees/w1 &&\n \t! test -d .git/worktrees/w2\n@@ -114,7 +114,7 @@ test_expect_success 'prune duplicate (main/linked)' '\n \tgit -C repo worktree add --detach ../wt &&\n \trm -fr wt &&\n \tmv repo wt &&\n-\tgit -C wt worktree prune --verbose >actual &&\n+\tgit -C wt worktree prune --verbose 2>actual &&\n \ttest_i18ngrep \"duplicate entry\" actual &&\n \t! test -d .git/worktrees/wt\n '\ndiff --git a/t/t2402-worktree-list.sh b/t/t2402-worktree-list.sh\nindex 4012bd67b0..c8a5a0aac6 100755\n--- a/t/t2402-worktree-list.sh\n+++ b/t/t2402-worktree-list.sh\n@@ -134,7 +134,7 @@ test_expect_success '\"list\" all worktrees with prunable consistent with \"prune\"'\n \tgit worktree list >out &&\n \tgrep \"/prunable  *[0-9a-f].* prunable$\" out &&\n \t! grep \"/unprunable  *[0-9a-f].* unprunable$\" out &&\n-\tgit worktree prune --verbose >out &&\n+\tgit worktree prune --verbose 2>out &&\n \ttest_i18ngrep \"^Removing worktrees/prunable\" out &&\n \ttest_i18ngrep ! \"^Removing worktrees/unprunable\" out\n '\ndiff --git a/t/t2406-worktree-repair.sh b/t/t2406-worktree-repair.sh\nindex f73741886b..5c44453e1c 100755\n--- a/t/t2406-worktree-repair.sh\n+++ b/t/t2406-worktree-repair.sh\n@@ -45,9 +45,8 @@ test_corrupt_gitfile () {\n \tgit worktree add --detach corrupt &&\n \tgit -C corrupt rev-parse --absolute-git-dir >expect &&\n \teval \"$butcher\" &&\n-\tgit -C \"$repairdir\" worktree repair >out 2>err &&\n-\ttest_i18ngrep \"$problem\" out &&\n-\ttest_must_be_empty err &&\n+\tgit -C \"$repairdir\" worktree repair 2>err &&\n+\ttest_i18ngrep \"$problem\" err &&\n \tgit -C corrupt rev-parse --absolute-git-dir >actual &&\n \ttest_cmp expect actual\n }\n@@ -130,10 +129,9 @@ test_expect_success 'repair broken gitdir' '\n \tsed s,orig/\\.git$,moved/.git, .git/worktrees/orig/gitdir >expect &&\n \trm .git/worktrees/orig/gitdir &&\n \tmv orig moved &&\n-\tgit worktree repair moved >out 2>err &&\n+\tgit worktree repair moved 2>err &&\n \ttest_cmp expect .git/worktrees/orig/gitdir &&\n-\ttest_i18ngrep \"gitdir unreadable\" out &&\n-\ttest_must_be_empty err\n+\ttest_i18ngrep \"gitdir unreadable\" err\n '\n \n test_expect_success 'repair incorrect gitdir' '\n@@ -141,10 +139,9 @@ test_expect_success 'repair incorrect gitdir' '\n \tgit worktree add --detach orig &&\n \tsed s,orig/\\.git$,moved/.git, .git/worktrees/orig/gitdir >expect &&\n \tmv orig moved &&\n-\tgit worktree repair moved >out 2>err &&\n+\tgit worktree repair moved 2>err &&\n \ttest_cmp expect .git/worktrees/orig/gitdir &&\n-\ttest_i18ngrep \"gitdir incorrect\" out &&\n-\ttest_must_be_empty err\n+\ttest_i18ngrep \"gitdir incorrect\" err\n '\n \n test_expect_success 'repair gitdir (implicit) from linked worktree' '\n@@ -152,10 +149,9 @@ test_expect_success 'repair gitdir (implicit) from linked worktree' '\n \tgit worktree add --detach orig &&\n \tsed s,orig/\\.git$,moved/.git, .git/worktrees/orig/gitdir >expect &&\n \tmv orig moved &&\n-\tgit -C moved worktree repair >out 2>err &&\n+\tgit -C moved worktree repair 2>err &&\n \ttest_cmp expect .git/worktrees/orig/gitdir &&\n-\ttest_i18ngrep \"gitdir incorrect\" out &&\n-\ttest_must_be_empty err\n+\ttest_i18ngrep \"gitdir incorrect\" err\n '\n \n test_expect_success 'unable to repair gitdir (implicit) from main worktree' '\n@@ -163,9 +159,8 @@ test_expect_success 'unable to repair gitdir (implicit) from main worktree' '\n \tgit worktree add --detach orig &&\n \tcat .git/worktrees/orig/gitdir >expect &&\n \tmv orig moved &&\n-\tgit worktree repair >out 2>err &&\n+\tgit worktree repair 2>err &&\n \ttest_cmp expect .git/worktrees/orig/gitdir &&\n-\ttest_must_be_empty out &&\n \ttest_must_be_empty err\n '\n \n@@ -178,12 +173,11 @@ test_expect_success 'repair multiple gitdir files' '\n \tsed s,orig2/\\.git$,moved2/.git, .git/worktrees/orig2/gitdir >expect2 &&\n \tmv orig1 moved1 &&\n \tmv orig2 moved2 &&\n-\tgit worktree repair moved1 moved2 >out 2>err &&\n+\tgit worktree repair moved1 moved2 2>err &&\n \ttest_cmp expect1 .git/worktrees/orig1/gitdir &&\n \ttest_cmp expect2 .git/worktrees/orig2/gitdir &&\n-\ttest_i18ngrep \"gitdir incorrect:.*orig1/gitdir$\" out &&\n-\ttest_i18ngrep \"gitdir incorrect:.*orig2/gitdir$\" out &&\n-\ttest_must_be_empty err\n+\ttest_i18ngrep \"gitdir incorrect:.*orig1/gitdir$\" err &&\n+\ttest_i18ngrep \"gitdir incorrect:.*orig2/gitdir$\" err\n '\n \n test_expect_success 'repair moved main and linked worktrees' '\n-- \n2.34.1.173.g76aa8bc2d0\n\n"},{"id":"442930","messageId":"20211203034420.47447-3-sunshine@sunshineco.com","threadId":"57019","inReplyTo":"20211203034420.47447-1-sunshine@sunshineco.com","subject":"[PATCH 2/2] git-worktree.txt: add missing `-v` to synopsis for `worktree list`","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2021-12-03T03:44:20Z","receivedAt":"2021-12-03T03:48:42Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"When verbose mode was added to `git worktree list` by 076b444a62\n(worktree: teach `list` verbose mode, 2021-01-27), although the\ndocumentation was updated to reflect the new functionality, the\nsynopsis was overlooked. Correct this minor oversight.\n\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n Documentation/git-worktree.txt | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt\nindex 8a7cbdd19c..9e862fbcf7 100644\n--- a/Documentation/git-worktree.txt\n+++ b/Documentation/git-worktree.txt\n@@ -10,7 +10,7 @@ SYNOPSIS\n --------\n [verse]\n 'git worktree add' [-f] [--detach] [--checkout] [--lock [--reason <string>]] [-b <new-branch>] <path> [<commit-ish>]\n-'git worktree list' [--porcelain]\n+'git worktree list' [-v | --porcelain]\n 'git worktree lock' [--reason <string>] <worktree>\n 'git worktree move' <worktree> <new-path>\n 'git worktree prune' [-n] [-v] [--expire <expire>]\n-- \n2.34.1.173.g76aa8bc2d0\n\n"},{"id":"442950","messageId":"211203.86k0gmt5fl.gmgdl@evledraar.gmail.com","threadId":"57019","inReplyTo":"20211203034420.47447-3-sunshine@sunshineco.com","subject":"Re: [PATCH 2/2] git-worktree.txt: add missing `-v` to synopsis for `worktree list`","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-03T09:13:49Z","receivedAt":"2021-12-03T09:15:47Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Dec 02 2021, Eric Sunshine wrote:\n\n> When verbose mode was added to `git worktree list` by 076b444a62\n> (worktree: teach `list` verbose mode, 2021-01-27), although the\n> documentation was updated to reflect the new functionality, the\n> synopsis was overlooked. Correct this minor oversight.\n>\n> Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>\n> ---\n>  Documentation/git-worktree.txt | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt\n> index 8a7cbdd19c..9e862fbcf7 100644\n> --- a/Documentation/git-worktree.txt\n> +++ b/Documentation/git-worktree.txt\n> @@ -10,7 +10,7 @@ SYNOPSIS\n>  --------\n>  [verse]\n>  'git worktree add' [-f] [--detach] [--checkout] [--lock [--reason <string>]] [-b <new-branch>] <path> [<commit-ish>]\n> -'git worktree list' [--porcelain]\n> +'git worktree list' [-v | --porcelain]\n>  'git worktree lock' [--reason <string>] <worktree>\n>  'git worktree move' <worktree> <new-path>\n>  'git worktree prune' [-n] [-v] [--expire <expire>]\n\nLooks good!\n\nAside: I've been thinking of hacking something up to just change all\nthese \"[verse]\" bits in the *.txt source to:\n\n    [verse]\n    $(git worktree -h)\n\nAnd then have the doc build process pick that up, run 'git $name -h', do\nsome light search/replacement (e.g. \"$cmd\" to \"'$cmd'\") and build the\ndocs like that.\n\nSeems far preferrable to dual-maintaining all of these forever.\n\nBut in the meantime this small fix is obviously correct.\n"},{"id":"442951","messageId":"211203.86fsrat592.gmgdl@evledraar.gmail.com","threadId":"57019","inReplyTo":"20211203034420.47447-2-sunshine@sunshineco.com","subject":"Re: [PATCH 1/2] worktree: send \"chatty\" messages to stderr","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-03T09:17:02Z","receivedAt":"2021-12-03T09:19:42Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Dec 02 2021, Eric Sunshine wrote:\n\n> The order in which the stdout and stderr streams are flushed is not\n> guaranteed to be the same across platforms or `libc` implementations.\n> This lack of determinism can lead to anomalous and potentially confusing\n> output if normal (stdout) output is flushed after error (stderr) output.\n> For instance, the following output which clearly indicates a failure due\n> to a fatal error:\n>\n>     % git worktree add ../foo bar\n>     Preparing worktree (checking out 'bar')\n>     fatal: 'bar' is already checked out at '.../wherever'\n>\n> has been reported[1] on Microsoft Windows to appear as:\n>\n>     % git worktree add ../foo bar\n>     fatal: 'bar' is already checked out at '.../wherever'\n>     Preparing worktree (checking out 'bar')\n\nMakes sense.\n\n>  test_expect_success 'repair incorrect gitdir' '\n> @@ -141,10 +139,9 @@ test_expect_success 'repair incorrect gitdir' '\n>  \tgit worktree add --detach orig &&\n>  \tsed s,orig/\\.git$,moved/.git, .git/worktrees/orig/gitdir >expect &&\n>  \tmv orig moved &&\n> -\tgit worktree repair moved >out 2>err &&\n> +\tgit worktree repair moved 2>err &&\n>  \ttest_cmp expect .git/worktrees/orig/gitdir &&\n> -\ttest_i18ngrep \"gitdir incorrect\" out &&\n> -\ttest_must_be_empty err\n> +\ttest_i18ngrep \"gitdir incorrect\" err\n>  '\n\nThis is just a \"for bonus points\", but maybe we could/should while we're\nat it harden and make the tests more exhaustive by checking the full\noutput of both, e.g.\n\n\tcat >actual.out <<-\\EOF &&\n\tPreparing worktree (checking out 'bar')\n\tEOF\n\tcat >actual.err <<-\\EOF &&\n\tfatal: 'bar' is already checked out at '.../wherever'\n\tEOF\n        <cmd> [...]\n\ttest_cmp expect.out actual.out &&\n\ttest_cmp expect.err actual.err\n\nDoesn't need a re-roll etc., just if you're interested... :)\n"},{"id":"442971","messageId":"CAPig+cR7f1koM7d2GoHMcDhZkQe3=XJD2RVuMajXSpcwZiyGtQ@mail.gmail.com","threadId":"57019","inReplyTo":"211203.86k0gmt5fl.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 2/2] git-worktree.txt: add missing `-v` to synopsis for `worktree list`","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2021-12-03T12:48:35Z","receivedAt":"2021-12-03T12:48:48Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Dec 3, 2021 at 4:15 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:\n> On Thu, Dec 02 2021, Eric Sunshine wrote:\n> > -'git worktree list' [--porcelain]\n> > +'git worktree list' [-v | --porcelain]\n>\n> Aside: I've been thinking of hacking something up to just change all\n> these \"[verse]\" bits in the *.txt source to:\n>\n>     [verse]\n>     $(git worktree -h)\n>\n> And then have the doc build process pick that up, run 'git $name -h', do\n> some light search/replacement (e.g. \"$cmd\" to \"'$cmd'\") and build the\n> docs like that.\n>\n> Seems far preferrable to dual-maintaining all of these forever.\n>\n> But in the meantime this small fix is obviously correct.\n\nOne caution that springs to mind is that there may be external tooling\nwhich processes these documentation files directly, and such a change\nmight break them. (The one which popped to mind immediately was the\ngit-scm.{org,com} website, though I don't know what their tooling\nlooks like.)\n"},{"id":"442972","messageId":"CAPig+cQ1+Hm=2jNKais-X6ahKCkG4Xo9Z4WsqTYryETc9a4mTw@mail.gmail.com","threadId":"57019","inReplyTo":"211203.86fsrat592.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 1/2] worktree: send \"chatty\" messages to stderr","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2021-12-03T13:07:49Z","receivedAt":"2021-12-03T13:08:02Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Dec 3, 2021 at 4:19 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:\n> On Thu, Dec 02 2021, Eric Sunshine wrote:\n> >       git worktree add --detach orig &&\n> >       sed s,orig/\\.git$,moved/.git, .git/worktrees/orig/gitdir >expect &&\n> >       mv orig moved &&\n> > -     git worktree repair moved >out 2>err &&\n> > +     git worktree repair moved 2>err &&\n> >       test_cmp expect .git/worktrees/orig/gitdir &&\n> > -     test_i18ngrep \"gitdir incorrect\" out &&\n> > -     test_must_be_empty err\n> > +     test_i18ngrep \"gitdir incorrect\" err\n> >  '\n>\n> This is just a \"for bonus points\", but maybe we could/should while we're\n> at it harden and make the tests more exhaustive by checking the full\n> output of both, e.g.\n>\n>         cat >actual.out <<-\\EOF &&\n>         Preparing worktree (checking out 'bar')\n>         EOF\n>         cat >actual.err <<-\\EOF &&\n>         fatal: 'bar' is already checked out at '.../wherever'\n>         EOF\n>         <cmd> [...]\n>         test_cmp expect.out actual.out &&\n>         test_cmp expect.err actual.err\n>\n> Doesn't need a re-roll etc., just if you're interested... :)\n\nTo be clear, with the application of the current patch, both of those\nmessages would need to be in the `actual.err` file, and `actual.out`\nwould be empty; not one message in each file as in your snippet.\n\nThat aside, there's still potentially output which is outside the\ncontrol of git-worktree. In the case of this particular negative test,\nyour suggestion should work, but for a positive test, it would be\nharder and uglier (though, of course, not impossible). For instance,\nfor a successful `git worktree add`, the output is:\n\n    Preparing worktree (new branch 'foobar')\n    Updating files: 100% (3993/3993), done.\n    HEAD is now at abe6bb3905 Gobbledygook\n\nThe subsequent lines come from git-reset (which, by the way, is\nsending \"HEAD is now at\" to stdout, though they probably should be on\nstderr, but that's a separate issue).\n\nAnyhow, such a change to the tests should be a separate topic. The\nuser-facing problem addressed by the current patch series need not be\nheld up by a behind-the-scenes change to testing.\n"},{"id":"443001","messageId":"211203.865ys5soqk.gmgdl@evledraar.gmail.com","threadId":"57019","inReplyTo":"CAPig+cR7f1koM7d2GoHMcDhZkQe3=XJD2RVuMajXSpcwZiyGtQ@mail.gmail.com","subject":"Re: [PATCH 2/2] git-worktree.txt: add missing `-v` to synopsis for `worktree list`","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-03T14:57:41Z","receivedAt":"2021-12-03T15:16:24Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Dec 03 2021, Eric Sunshine wrote:\n\n> On Fri, Dec 3, 2021 at 4:15 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:\n>> On Thu, Dec 02 2021, Eric Sunshine wrote:\n>> > -'git worktree list' [--porcelain]\n>> > +'git worktree list' [-v | --porcelain]\n>>\n>> Aside: I've been thinking of hacking something up to just change all\n>> these \"[verse]\" bits in the *.txt source to:\n>>\n>>     [verse]\n>>     $(git worktree -h)\n>>\n>> And then have the doc build process pick that up, run 'git $name -h', do\n>> some light search/replacement (e.g. \"$cmd\" to \"'$cmd'\") and build the\n>> docs like that.\n>>\n>> Seems far preferrable to dual-maintaining all of these forever.\n>>\n>> But in the meantime this small fix is obviously correct.\n>\n> One caution that springs to mind is that there may be external tooling\n> which processes these documentation files directly, and such a change\n> might break them. (The one which popped to mind immediately was the\n> git-scm.{org,com} website, though I don't know what their tooling\n> looks like.)\n\nTrue, I hadn't looked into how that worked before, but behold!\nhttps://github.com/git/git-scm.com/blob/main/lib/tasks/index.rake\n\nIt seems to be a re-implementation of a non-trivial part of the doc\nbuilding process.\n\nIn any case, if we do end up generating more of the documentation\nourselves presumably any such concerns will be brought up then.\n"},{"id":"443004","messageId":"Yao9NwxQ91XO8k1G@coredump.intra.peff.net","threadId":"57019","inReplyTo":"211203.865ys5soqk.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 2/2] git-worktree.txt: add missing `-v` to synopsis for `worktree list`","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-12-03T15:52:23Z","receivedAt":"2021-12-03T15:52:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Dec 03, 2021 at 03:57:41PM +0100, Ævar Arnfjörð Bjarmason wrote:\n\n> > One caution that springs to mind is that there may be external tooling\n> > which processes these documentation files directly, and such a change\n> > might break them. (The one which popped to mind immediately was the\n> > git-scm.{org,com} website, though I don't know what their tooling\n> > looks like.)\n> \n> True, I hadn't looked into how that worked before, but behold!\n> https://github.com/git/git-scm.com/blob/main/lib/tasks/index.rake\n> \n> It seems to be a re-implementation of a non-trivial part of the doc\n> building process.\n\nYeah. It's kind of ugly, but the complication there is that the docs are\nupdated on a running heroku dyno which does not actually have a clone of\nthe new version of Git, let alone an actual build.\n\nIt's also hard for it to just use the output of our \"make\" anyway, since\nthere's some munging that happens to fit the page content inside the\nrest of the site, changing links, etc. We could in theory operate on the\nresult of \"make html\" more directly, but it would definitely require\nsome changes.\n\nIMHO the way the site operates now (with \"live\" updates by extracting\ncontent from git.git and shoving it into a database) is not ideal. It's\nnot like we're importing new Git versions once per minute. It would be\neasier to reason about as a \"static site\" which is built by a process\nwhich actually has a clone of git.git and invokes \"make html\" there,\npost-processes the pages, and saves the whole thing as a Git tree. And\nthen run that build occasionally (at new releases, or changes to the\nsource, but also periodically via GitHub Actions or similar to pick up\nchanged book content). It's just a big enough change (and there are some\ngotcha around things like site search) that I've never gotten around to\nit.\n\n-Peff\n"},{"id":"443101","messageId":"xmqqa6hfmn3u.fsf@gitster.g","threadId":"57019","inReplyTo":"CAPig+cR7f1koM7d2GoHMcDhZkQe3=XJD2RVuMajXSpcwZiyGtQ@mail.gmail.com","subject":"Re: [PATCH 2/2] git-worktree.txt: add missing `-v` to synopsis for `worktree list`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-12-05T09:12:37Z","receivedAt":"2021-12-05T09:12:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n> On Fri, Dec 3, 2021 at 4:15 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:\n>> On Thu, Dec 02 2021, Eric Sunshine wrote:\n>> > -'git worktree list' [--porcelain]\n>> > +'git worktree list' [-v | --porcelain]\n>>\n>> Aside: I've been thinking of hacking something up to just change all\n>> these \"[verse]\" bits in the *.txt source to:\n>>\n>>     [verse]\n>>     $(git worktree -h)\n>>\n>> And then have the doc build process pick that up, run 'git $name -h', do\n>> some light search/replacement (e.g. \"$cmd\" to \"'$cmd'\") and build the\n>> docs like that.\n>>\n>> Seems far preferrable to dual-maintaining all of these forever.\n>>\n>> But in the meantime this small fix is obviously correct.\n>\n> One caution that springs to mind is that there may be external tooling\n> which processes these documentation files directly, and such a change\n> might break them. (The one which popped to mind immediately was the\n> git-scm.{org,com} website, though I don't know what their tooling\n> looks like.)\n\nAlso it would slow us down by making the .txt variant we see in the\nsource tree harder to read (or in this case, impossible to see without\nbuilding the documentation).\n"},{"id":"443149","messageId":"CAPig+cQ82UC3MRSswGtnCcB13wdhTNYzDex=tFSuFwFCjL3ErA@mail.gmail.com","threadId":"57019","inReplyTo":"xmqqa6hfmn3u.fsf@gitster.g","subject":"Re: [PATCH 2/2] git-worktree.txt: add missing `-v` to synopsis for `worktree list`","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2021-12-06T13:34:11Z","receivedAt":"2021-12-06T13:34:25Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Dec 5, 2021 at 4:12 AM Junio C Hamano <gitster@pobox.com> wrote:\n> Eric Sunshine <sunshine@sunshineco.com> writes:\n> > On Fri, Dec 3, 2021 at 4:15 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:\n> >> Aside: I've been thinking of hacking something up to just change all\n> >> these \"[verse]\" bits in the *.txt source to:\n> >>\n> >>     [verse]\n> >>     $(git worktree -h)\n> >>\n> >> And then have the doc build process pick that up, run 'git $name -h', do\n> >> some light search/replacement (e.g. \"$cmd\" to \"'$cmd'\") and build the\n> >> docs like that.\n> >\n> > One caution that springs to mind is that there may be external tooling\n> > which processes these documentation files directly, and such a change\n> > might break them. (The one which popped to mind immediately was the\n> > git-scm.{org,com} website, though I don't know what their tooling\n> > looks like.)\n>\n> Also it would slow us down by making the .txt variant we see in the\n> source tree harder to read (or in this case, impossible to see without\n> building the documentation).\n\nTaking this point into consideration, a middle-ground alternative to\nÆvar's idea would be to add tooling which only compares (by some\ndefinition of \"compare\") the output of `git blah -h` with the synopsis\nin `git-blah.txt` and complains if there are significant differences\n(by some definition \"significant\" and \"difference\"). It doesn't\nautomate-away the work of keeping the synopsis up-to-date, but at\nleast would flag inconsistencies.\n"},{"id":"443154","messageId":"211206.86bl1trbdp.gmgdl@evledraar.gmail.com","threadId":"57019","inReplyTo":"CAPig+cQ82UC3MRSswGtnCcB13wdhTNYzDex=tFSuFwFCjL3ErA@mail.gmail.com","subject":"Re: [PATCH 2/2] git-worktree.txt: add missing `-v` to synopsis for `worktree list`","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-06T15:06:10Z","receivedAt":"2021-12-06T15:53:55Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Dec 06 2021, Eric Sunshine wrote:\n\n> On Sun, Dec 5, 2021 at 4:12 AM Junio C Hamano <gitster@pobox.com> wrote:\n>> Eric Sunshine <sunshine@sunshineco.com> writes:\n>> > On Fri, Dec 3, 2021 at 4:15 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:\n>> >> Aside: I've been thinking of hacking something up to just change all\n>> >> these \"[verse]\" bits in the *.txt source to:\n>> >>\n>> >>     [verse]\n>> >>     $(git worktree -h)\n>> >>\n>> >> And then have the doc build process pick that up, run 'git $name -h', do\n>> >> some light search/replacement (e.g. \"$cmd\" to \"'$cmd'\") and build the\n>> >> docs like that.\n>> >\n>> > One caution that springs to mind is that there may be external tooling\n>> > which processes these documentation files directly, and such a change\n>> > might break them. (The one which popped to mind immediately was the\n>> > git-scm.{org,com} website, though I don't know what their tooling\n>> > looks like.)\n>>\n>> Also it would slow us down by making the .txt variant we see in the\n>> source tree harder to read (or in this case, impossible to see without\n>> building the documentation).\n>\n> Taking this point into consideration, a middle-ground alternative to\n> Ævar's idea would be to add tooling which only compares (by some\n> definition of \"compare\") the output of `git blah -h` with the synopsis\n> in `git-blah.txt` and complains if there are significant differences\n> (by some definition \"significant\" and \"difference\"). It doesn't\n> automate-away the work of keeping the synopsis up-to-date, but at\n> least would flag inconsistencies.\n\nOr we could do the reverse and move the source code version of it to be\ngenerated from the [verse] sections in the documentation.\n\nAnyway, it's not something I was planning to work on any time soon, just\nsomething I'd thought was a good idea for a while, especially given the\ndifferences and divergenge. That can be viewed with:\n\n    parallel -k '\n        git {} -h 2>&1 | grep -e \"^usage\" -e \"^   or\";\n        git help {} 2>&1 | grep -A20 SYNOPSIS | grep -B20 DESCRIPTION\n    ' ::: $(git --list-cmds=builtins) | less\n\nIt's a long-standing UX issue, and we keep re-introducing divergence\nbetween the two.\n\nIf we're going to insist that the version in the *.txt file isn't\ngenerated that categorically closes the door to some logical follow-ups.\n\nE.g. having parse-options automatically generate alternates in cases\nwhere options are mutually exclusive, or adding color output to this\nwhere we color short/long options differently than arguments etc.\n\nThat and e.g. translators needing to do less work for the translated\nmanpages (we already have the translated output in the C code).\n\nWell, I suppose we could have a generating step and then commit the\nequivalent of the compiled file (or section) into git.git every time we\nadd/change an option.\n\nIn general I don't think it's a worthwhile goal to keep the .txt\nversions of the docs as some human-readable 1=1 mapping to what you'd\nget if you generated them. That's already not the case due to includes,\nand e.g. in this case accepting some reasonable amount of\nauto-generation would make them easier to maintain.\n"},{"id":"443184","messageId":"xmqqh7blhb6r.fsf@gitster.g","threadId":"57019","inReplyTo":"211206.86bl1trbdp.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 2/2] git-worktree.txt: add missing `-v` to synopsis for `worktree list`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-12-06T17:53:32Z","receivedAt":"2021-12-06T17:53:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n>> Taking this point into consideration, a middle-ground alternative to\n>> Ævar's idea would be to add tooling which only compares (by some\n>> definition of \"compare\") the output of `git blah -h` with the synopsis\n>> in `git-blah.txt` and complains if there are significant differences\n>> (by some definition \"significant\" and \"difference\"). It doesn't\n>> automate-away the work of keeping the synopsis up-to-date, but at\n>> least would flag inconsistencies.\n>\n> Or we could do the reverse and move the source code version of it to be\n> generated from the [verse] sections in the documentation.\n\nEither would be far more preferrable than committing generated files,\nor committing us to work on unreadable sources.\n\nThanks.\n\n"},{"id":"443896","messageId":"gohp6kilvupxls.fsf@cpm12071.fritz.box","threadId":"57019","inReplyTo":"20211203034420.47447-3-sunshine@sunshineco.com","subject":"Re: [PATCH 2/2] git-worktree.txt: add missing `-v` to synopsis for `worktree list`","fromName":"Rafael Silva","fromEmail":"rafaeloliveira.cs@gmail.com","sentAt":"2021-12-11T22:25:40Z","receivedAt":"2021-12-11T22:48:05Z","isPatch":true,"sender":{"key":"rafaeloliveira.cs@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5935135?v=4"},"body":"\nEric Sunshine <sunshine@sunshineco.com> writes:\n\n> When verbose mode was added to `git worktree list` by 076b444a62\n> (worktree: teach `list` verbose mode, 2021-01-27), although the\n> documentation was updated to reflect the new functionality, the\n> synopsis was overlooked. Correct this minor oversight.\n>\n> Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>\n> ---\n>  Documentation/git-worktree.txt | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt\n> index 8a7cbdd19c..9e862fbcf7 100644\n> --- a/Documentation/git-worktree.txt\n> +++ b/Documentation/git-worktree.txt\n> @@ -10,7 +10,7 @@ SYNOPSIS\n>  --------\n>  [verse]\n>  'git worktree add' [-f] [--detach] [--checkout] [--lock [--reason <string>]] [-b <new-branch>] <path> [<commit-ish>]\n> -'git worktree list' [--porcelain]\n> +'git worktree list' [-v | --porcelain]\n>  'git worktree lock' [--reason <string>] <worktree>\n>  'git worktree move' <worktree> <new-path>\n>  'git worktree prune' [-n] [-v] [--expire <expire>]\n\nOops! Thanks for cleaning up after me.\n\n-- \nThanks\nRafael\n"}]}