{"thread":{"id":"49138","subject":"[PATCH v2] worktree: add --quiet option","startedAt":"2018-08-15T20:56:43Z","lastAt":"2018-08-16T20:42:30Z","messageCount":8,"participants":["Elia Pinto","Thomas Gummerer","Martin Ågren","Eric Sunshine","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"355761","messageId":"20180815205630.32876-1-gitter.spiros@gmail.com","threadId":"49138","inReplyTo":null,"subject":"[PATCH v2] worktree: add --quiet option","fromName":"Elia Pinto","fromEmail":"gitter.spiros@gmail.com","sentAt":"2018-08-15T20:56:30Z","receivedAt":"2018-08-15T20:56:43Z","isPatch":true,"sender":{"key":"gitter.spiros@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158490?v=4"},"body":"Add the '--quiet' option to git worktree,\nas for the other git commands. 'add' is the\nonly command affected by it since all other\ncommands, except 'list', are currently\nsilent by default.\n\nHelped-by: Martin Ågren \nHelped-by: Duy Nguyen \nHelped-by: Eric Sunshine \nSigned-off-by: Elia Pinto \n---\nThis is the second version of the patch.\n\nChanges from the first version\n(https://public-inbox.org/git/CACsJy8A=zp7nFBuWyfeP4UFf3KSsiaor3m0mtgVnhcEYHSw4HA@mail.gmail.com/T/):\n\n- deleted garbage in git-worktree.c and deleted\nsuperfluous blank line in git-worktree.txt.\n- when giving \"--quiet\" to 'add', call git symbolic-ref also with\n\"--quiet\".\n- changed the commit message to be more general, but\nspecifying why the \"--quiet\" option is meaningful only for\nthe 'add' command of git-worktree.\n- in git-worktree.txt the option\n\"--quiet\" is described near the \"--verbose\" option.\n\n Documentation/git-worktree.txt | 4 ++++\n builtin/worktree.c | 16 +++++++++++++---\n t/t2025-worktree-add.sh | 5 +++++\n 3 files changed, 22 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt\nindex 9c26be40f..29a5b7e25 100644\n--- a/Documentation/git-worktree.txt\n+++ b/Documentation/git-worktree.txt\n@@ -173,6 +173,10 @@ This can also be set up as the default behaviour by using the\n \tThis format will remain stable across Git versions and regardless of user\n \tconfiguration. See below for details.\n \n+-q::\n+--quiet::\n+\tWith 'add', suppress feedback messages.\n+\n -v::\n --verbose::\n \tWith `prune`, report all removals.\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex a763dbdcc..41e771439 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -27,6 +27,7 @@ static const char * const worktree_usage[] = {\n struct add_opts {\n \tint force;\n \tint detach;\n+\tint quiet;\n \tint checkout;\n \tint keep_locked;\n };\n@@ -303,9 +304,13 @@ static int add_worktree(const char *path, const char *refname,\n \tif (!is_branch)\n \t\targv_array_pushl(&cp.args, \"update-ref\", \"HEAD\",\n \t\t\t\t oid_to_hex(&commit->object.oid), NULL);\n-\telse\n+\telse {\n \t\targv_array_pushl(&cp.args, \"symbolic-ref\", \"HEAD\",\n \t\t\t\t symref.buf, NULL);\n+\t\tif (opts->quiet)\n+\t\t\targv_array_push(&cp.args, \"--quiet\");\n+\t}\n+\n \tcp.env = child_env.argv;\n \tret = run_command(&cp);\n \tif (ret)\n@@ -315,6 +320,8 @@ static int add_worktree(const char *path, const char *refname,\n \t\tcp.argv = NULL;\n \t\targv_array_clear(&cp.args);\n \t\targv_array_pushl(&cp.args, \"reset\", \"--hard\", NULL);\n+\t\tif (opts->quiet)\n+\t\t\targv_array_push(&cp.args, \"--quiet\");\n \t\tcp.env = child_env.argv;\n \t\tret = run_command(&cp);\n \t\tif (ret)\n@@ -437,6 +444,7 @@ static int add(int ac, const char **av, const char *prefix)\n \t\tOPT_BOOL(0, \"detach\", &opts.detach, N_(\"detach HEAD at named commit\")),\n \t\tOPT_BOOL(0, \"checkout\", &opts.checkout, N_(\"populate the new working tree\")),\n \t\tOPT_BOOL(0, \"lock\", &opts.keep_locked, N_(\"keep the new working tree locked\")),\n+\t\tOPT__QUIET(&opts.quiet, N_(\"suppress progress reporting\")),\n \t\tOPT_PASSTHRU(0, \"track\", &opt_track, NULL,\n \t\t\t N_(\"set up tracking mode (see git-branch(1))\"),\n \t\t\t PARSE_OPT_NOARG | PARSE_OPT_OPTARG),\n@@ -491,8 +499,8 @@ static int add(int ac, const char **av, const char *prefix)\n \t\t\t}\n \t\t}\n \t}\n-\n-\tprint_preparing_worktree_line(opts.detach, branch, new_branch, !!new_branch_force);\n+\tif (!opts.quiet)\n+\t\tprint_preparing_worktree_line(opts.detach, branch, new_branch, !!new_branch_force);\n \n \tif (new_branch) {\n \t\tstruct child_process cp = CHILD_PROCESS_INIT;\n@@ -500,6 +508,8 @@ static int add(int ac, const char **av, const char *prefix)\n \t\targv_array_push(&cp.args, \"branch\");\n \t\tif (new_branch_force)\n \t\t\targv_array_push(&cp.args, \"--force\");\n+\t\tif (opts.quiet)\n+\t\t\targv_array_push(&cp.args, \"--quiet\");\n \t\targv_array_push(&cp.args, new_branch);\n \t\targv_array_push(&cp.args, branch);\n \t\tif (opt_track)\ndiff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh\nindex be6e09314..658647d83 100755\n--- a/t/t2025-worktree-add.sh\n+++ b/t/t2025-worktree-add.sh\n@@ -252,6 +252,11 @@ test_expect_success 'add -B' '\n \ttest_cmp_rev master^ poodle\n '\n \n+test_expect_success 'add --quiet' '\n+\tgit worktree add --quiet ../foo master >expected 2>&1 &&\n+\ttest_must_be_empty expected\n+'\n+\n test_expect_success 'local clone from linked checkout' '\n \tgit clone --local here here-clone &&\n \t( cd here-clone && git fsck )\n-- \n2.18.0.723.g64e6cc43e.dirty\n\n"},{"id":"355776","messageId":"20180815223605.GA11326@hank.intra.tgummerer.com","threadId":"49138","inReplyTo":"20180815205630.32876-1-gitter.spiros@gmail.com","subject":"Re: [PATCH v2] worktree: add --quiet option","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2018-08-15T22:36:05Z","receivedAt":"2018-08-15T22:36:12Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 08/15, Elia Pinto wrote:\n> Add the '--quiet' option to git worktree,\n> as for the other git commands. 'add' is the\n> only command affected by it since all other\n> commands, except 'list', are currently\n> silent by default.\n> \n> Helped-by: Martin Ågren \n> Helped-by: Duy Nguyen \n> Helped-by: Eric Sunshine \n> Signed-off-by: Elia Pinto \n> ---\n> This is the second version of the patch.\n> \n> Changes from the first version\n> (https://public-inbox.org/git/CACsJy8A=zp7nFBuWyfeP4UFf3KSsiaor3m0mtgVnhcEYHSw4HA@mail.gmail.com/T/):\n> \n> - deleted garbage in git-worktree.c and deleted\n> superfluous blank line in git-worktree.txt.\n> - when giving \"--quiet\" to 'add', call git symbolic-ref also with\n> \"--quiet\".\n> - changed the commit message to be more general, but\n> specifying why the \"--quiet\" option is meaningful only for\n> the 'add' command of git-worktree.\n> - in git-worktree.txt the option\n> \"--quiet\" is described near the \"--verbose\" option.\n> \n> Documentation/git-worktree.txt | 4 ++++\n> builtin/worktree.c | 16 +++++++++++++---\n> t/t2025-worktree-add.sh | 5 +++++\n> 3 files changed, 22 insertions(+), 3 deletions(-)\n> \n> diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt\n> index 9c26be40f..29a5b7e25 100644\n> --- a/Documentation/git-worktree.txt\n> +++ b/Documentation/git-worktree.txt\n> @@ -173,6 +173,10 @@ This can also be set up as the default behaviour by using the\n> \tThis format will remain stable across Git versions and regardless of user\n> \tconfiguration. See below for details.\n> \n> +-q::\n> +--quiet::\n> +\tWith 'add', suppress feedback messages.\n\nVery minor nit here, we seem to use backticks everywhere else in this\ndocument, maybe we sould do that here as well? Not sure it's worth\nanother iteration though.\n\nThe rest of the patch looks good to me, thanks!\n\n> -v::\n> --verbose::\n> \tWith `prune`, report all removals.\n> diff --git a/builtin/worktree.c b/builtin/worktree.c\n> index a763dbdcc..41e771439 100644\n> --- a/builtin/worktree.c\n> +++ b/builtin/worktree.c\n> @@ -27,6 +27,7 @@ static const char * const worktree_usage[] = {\n> struct add_opts {\n> \tint force;\n> \tint detach;\n> +\tint quiet;\n> \tint checkout;\n> \tint keep_locked;\n> };\n> @@ -303,9 +304,13 @@ static int add_worktree(const char *path, const char *refname,\n> \tif (!is_branch)\n> \t\targv_array_pushl(&cp.args, \"update-ref\", \"HEAD\",\n> \t\t\t\t oid_to_hex(&commit->object.oid), NULL);\n> -\telse\n> +\telse {\n> \t\targv_array_pushl(&cp.args, \"symbolic-ref\", \"HEAD\",\n> \t\t\t\t symref.buf, NULL);\n> +\t\tif (opts->quiet)\n> +\t\t\targv_array_push(&cp.args, \"--quiet\");\n> +\t}\n> +\n> \tcp.env = child_env.argv;\n> \tret = run_command(&cp);\n> \tif (ret)\n> @@ -315,6 +320,8 @@ static int add_worktree(const char *path, const char *refname,\n> \t\tcp.argv = NULL;\n> \t\targv_array_clear(&cp.args);\n> \t\targv_array_pushl(&cp.args, \"reset\", \"--hard\", NULL);\n> +\t\tif (opts->quiet)\n> +\t\t\targv_array_push(&cp.args, \"--quiet\");\n> \t\tcp.env = child_env.argv;\n> \t\tret = run_command(&cp);\n> \t\tif (ret)\n> @@ -437,6 +444,7 @@ static int add(int ac, const char **av, const char *prefix)\n> \t\tOPT_BOOL(0, \"detach\", &opts.detach, N_(\"detach HEAD at named commit\")),\n> \t\tOPT_BOOL(0, \"checkout\", &opts.checkout, N_(\"populate the new working tree\")),\n> \t\tOPT_BOOL(0, \"lock\", &opts.keep_locked, N_(\"keep the new working tree locked\")),\n> +\t\tOPT__QUIET(&opts.quiet, N_(\"suppress progress reporting\")),\n> \t\tOPT_PASSTHRU(0, \"track\", &opt_track, NULL,\n> \t\t\t N_(\"set up tracking mode (see git-branch(1))\"),\n> \t\t\t PARSE_OPT_NOARG | PARSE_OPT_OPTARG),\n> @@ -491,8 +499,8 @@ static int add(int ac, const char **av, const char *prefix)\n> \t\t\t}\n> \t\t}\n> \t}\n> -\n> -\tprint_preparing_worktree_line(opts.detach, branch, new_branch, !!new_branch_force);\n> +\tif (!opts.quiet)\n> +\t\tprint_preparing_worktree_line(opts.detach, branch, new_branch, !!new_branch_force);\n> \n> \tif (new_branch) {\n> \t\tstruct child_process cp = CHILD_PROCESS_INIT;\n> @@ -500,6 +508,8 @@ static int add(int ac, const char **av, const char *prefix)\n> \t\targv_array_push(&cp.args, \"branch\");\n> \t\tif (new_branch_force)\n> \t\t\targv_array_push(&cp.args, \"--force\");\n> +\t\tif (opts.quiet)\n> +\t\t\targv_array_push(&cp.args, \"--quiet\");\n> \t\targv_array_push(&cp.args, new_branch);\n> \t\targv_array_push(&cp.args, branch);\n> \t\tif (opt_track)\n> diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh\n> index be6e09314..658647d83 100755\n> --- a/t/t2025-worktree-add.sh\n> +++ b/t/t2025-worktree-add.sh\n> @@ -252,6 +252,11 @@ test_expect_success 'add -B' '\n> \ttest_cmp_rev master^ poodle\n> '\n> \n> +test_expect_success 'add --quiet' '\n> +\tgit worktree add --quiet ../foo master >expected 2>&1 &&\n> +\ttest_must_be_empty expected\n> +'\n> +\n> test_expect_success 'local clone from linked checkout' '\n> \tgit clone --local here here-clone &&\n> \t( cd here-clone && git fsck )\n> -- \n> 2.18.0.723.g64e6cc43e.dirty\n> \n"},{"id":"355801","messageId":"CAN0heSrZXjaQ0H1J1Mmqhv9qhiNbRn4fOJ4oO1XrZFEGO4YFug@mail.gmail.com","threadId":"49138","inReplyTo":"20180815205630.32876-1-gitter.spiros@gmail.com","subject":"Re: [PATCH v2] worktree: add --quiet option","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2018-08-16T04:40:51Z","receivedAt":"2018-08-16T04:41:08Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"On Wed, 15 Aug 2018 at 22:56, Elia Pinto wrote:\n> Add the '--quiet' option to git worktree,\n> as for the other git commands. 'add' is the\n> only command affected by it since all other\n> commands, except 'list', are currently\n> silent by default.\n\nThanks for a follow-up.\n\nThe word \"currently\" means I can't shake the feeling that Eric has a\nvery good point in [1]:\n\n It might make sense to say instead that this is adding a --quiet\n option _in general_, rather than doing so specifically for 'add'.\n\nAs an example, if `git worktree move` ever learns to produce some sort\nof output, then Eric's approach would mean that such a hypothetical\n`move` is buggy until it learns to respect `--quiet`. With your\napproach, it would mean that we would get feature requests that `move`\nshould also respect `--quiet` (which we would then need to redefine in\nthe documentation) or that it should learn of a `--quiet-move` (which I\ndo not think is a particularly good UI).\n\nDoing such a patch instead would mean tweaking the commit message\nslightly...\n\n Add the '--quiet' option to git worktree, as for the other git\n commands. Currently, 'add' is the only command affected by it since\n all other commands, except 'list' obviously, are already silent by\n default.\n\n... and the documentation slightly ...\n\n Suppress feedback messages.\n\nIt might make sense adding a comment to the documentation about how this\ncurrently only affects `add`, but such comments do risk going stale. I\nam on the fence about whether the documentation needs to say something\nabout `list` -- who in their right mind would expect `list --quiet` to\nactually suppress the list? And if `list` ever learns to give some\nfeedback messages in addition to the list itself, then we would want\n`--quiet` to suppress *those*, I guess.\n\nOthers might disagree violently with this, but I wonder whether it makes\nsense to add a test to verify that, e.g., `git worktree move --quiet` is\nquiet. Such a test would demonstrate that this is your intention, i.e.,\nanyone digging into a report on \"why does git worktree foo not respect\n--quiet?\" would be 100% convinced that your intention back in 2018 really\nwas to respect it everywhere. It seems wasteful to add such a test for\nall other modes, but maybe you can identify one which you think has a\nhigher risk of learning to output some feedback in the future.\n\nTo be clear, it is fine for you to disagree with the above! :-)\n\n> }\n> -\n> - print_preparing_worktree_line(opts.detach, branch, new_branch, !!new_branch_force);\n> + if (!opts.quiet)\n> + print_preparing_worktree_line(opts.detach, branch, new_branch, !!new_branch_force);\n\nI think that empty line should be kept. Probably not worth a reroll.\n\nGood work!\n\n[1] https://public-inbox.org/git/CAPig+cS-b2yL2FeLRzS+jW-O5fRd1g8Kqak7j1QX5PRP0ojQEQ@mail.gmail.com/\n\nMartin\n"},{"id":"355821","messageId":"CAPig+cR3g8MUt6VAg0RrO3VBgZ4ChsXz1t75xHgT5Q_9_hRzBQ@mail.gmail.com","threadId":"49138","inReplyTo":"CAN0heSrZXjaQ0H1J1Mmqhv9qhiNbRn4fOJ4oO1XrZFEGO4YFug@mail.gmail.com","subject":"Re: [PATCH v2] worktree: add --quiet option","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-08-16T08:25:16Z","receivedAt":"2018-08-16T08:25:30Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Aug 16, 2018 at 12:41 AM Martin Ågren wrote:\n> On Wed, 15 Aug 2018 at 22:56, Elia Pinto wrote:\n> The word \"currently\" means I can't shake the feeling that Eric has a\n> very good point in [1]:\n>\n> It might make sense to say instead that this is adding a --quiet\n> option _in general_, rather than doing so specifically for 'add'.\n>\n> As an example, if `git worktree move` ever learns to produce some sort\n> of output, then Eric's approach would mean that such a hypothetical\n> `move` is buggy until it learns to respect `--quiet`. With your\n> approach, it would mean that we would get feature requests that `move`\n> should also respect `--quiet` [...]\n>\n> Doing such a patch instead would mean tweaking the commit message\n> slightly...\n>\n> Add the '--quiet' option to git worktree, as for the other git\n> commands. Currently, 'add' is the only command affected by it since\n> all other commands, except 'list' obviously, are already silent by\n> default.\n>\n> ... and the documentation slightly ...\n>\n> Suppress feedback messages.\n\nThis is a sensible suggestion for the documentation rather than having\nit call out \"add\" as special (unlike the commit message in which case\nit makes sense to mention that only the behavior of \"add\" is\naffected).\n\n> It might make sense adding a comment to the documentation about how this\n> currently only affects `add`, but such comments do risk going stale.\n\nLet's not mention \"add\" or any other command specially since the\noption is meant to be general.\n\n> I am on the fence about whether the documentation needs to say something\n> about `list` -- who in their right mind would expect `list --quiet` to\n> actually suppress the list?\n\n(/me nudges Martin off the fence onto the side of not bothering to\nmention the obvious)\n\n> Others might disagree violently with this, but I wonder whether it makes\n> sense to add a test to verify that, e.g., `git worktree move --quiet` is\n> quiet. Such a test would demonstrate that this is your intention, i.e.,\n> anyone digging into a report on \"why does git worktree foo not respect\n> --quiet?\" would be 100% convinced that your intention back in 2018 really\n> was to respect it everywhere. It seems wasteful to add such a test for\n> all other modes, but maybe you can identify one which you think has a\n> higher risk of learning to output some feedback in the future.\n\nMy knee-jerk reaction was that such tests seem unnecessary, but I\nthink you convinced me that they would make sense to avoid future\nheadaches since --quiet should indeed mean \"quiet\" generally. (Newly\nadded worktree commands would not be protected by such tests added\ntoday, so it's not foolproof, but still better than nothing.)\n\nHaving said that, though, lack of such tests shouldn't block this\npatch from being accepted. They can always be added later if someone\nwants to do it.\n"},{"id":"355824","messageId":"CAPig+cTSZNtKUXLoR6gqvWT3bD-JgWOF38ZD7i+OocRs4p=GMA@mail.gmail.com","threadId":"49138","inReplyTo":"20180815205630.32876-1-gitter.spiros@gmail.com","subject":"Re: [PATCH v2] worktree: add --quiet option","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-08-16T08:43:44Z","receivedAt":"2018-08-16T08:43:58Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Aug 15, 2018 at 4:56 PM Elia Pinto wrote:\n> Add the '--quiet' option to git worktree,\n> as for the other git commands. 'add' is the\n> only command affected by it since all other\n> commands, except 'list', are currently\n> silent by default.\n\nNit: wrap the commit message at around 70 columns rather than 45 or so.\n\n> Signed-off-by: Elia Pinto \n> ---\n> diff --git a/builtin/worktree.c b/builtin/worktree.c\n> @@ -303,9 +304,13 @@ static int add_worktree(const char *path, const char *refname,\n> + else {\n> argv_array_pushl(&cp.args, \"symbolic-ref\", \"HEAD\",\n> symref.buf, NULL);\n> + if (opts->quiet)\n> + argv_array_push(&cp.args, \"--quiet\");\n> + }\n\nThis constructs the command as \"git symbolic-ref HEAD --quiet\".\nAlthough that's not wrong per-se, it does perhaps set an undesirable\nprecedent. A more standard construction would be:\n\n argv_array_push(..., \"symbolic-ref\");\n if (opts->quiet)\n argv_array_push(..., \"--quiet\");\n argv_array_pushl(..., \"HEAD\", symref.buf, NULL);\n\nI used \"pushl\" on the last one to indicate the semantic relationship\nbetween those two arguments.\n\n> +\n\nNit: the above code is directly related to the code just below, so the\nnew blank line you add here somewhat (undesirably) divorces the pieces\nof code from each other\n\n> cp.env = child_env.argv;\n> ret = run_command(&cp);\n> if (ret)\n> @@ -315,6 +320,8 @@ static int add_worktree(const char *path, const char *refname,\n> argv_array_pushl(&cp.args, \"reset\", \"--hard\", NULL);\n> + if (opts->quiet)\n> + argv_array_push(&cp.args, \"--quiet\");\n\nI could also see this one re-written as:\n\n argv_array_push(...,\"reset\");\n argv_array_push(...,\"--hard\");\n if (opts->quiet)\n argv_array_push(...,\"--quiet\");\n\nnow that the command invocation has added another option.\n\nNot at all worth a re-roll.\n\n> diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh\n> @@ -252,6 +252,11 @@ test_expect_success 'add -B' '\n> +test_expect_success 'add --quiet' '\n> + git worktree add --quiet ../foo master >expected 2>&1 &&\n> + test_must_be_empty expected\n> +'\n\nThis test is going to create the new worktree directly in Git's t/\ndirectory due to \"../foo\". Please don't do that. Use a name without\nthe leading \"../\".\n\nAlso, you should make sure that the worktree doesn't already exist\n(wasn't created by an earlier test), otherwise the \"git worktree add\"\ncommand will fail. So, either choose a distinct worktree name or use\n\"test_might_fail git worktree remove -f \" before the \"git\nworktree add\" command.\n"},{"id":"355827","messageId":"CAN0heSotUQf=q_uOXr5qP64ATQT6wnB_0XAmUPoastJw3asJ=A@mail.gmail.com","threadId":"49138","inReplyTo":"CAPig+cR3g8MUt6VAg0RrO3VBgZ4ChsXz1t75xHgT5Q_9_hRzBQ@mail.gmail.com","subject":"Re: [PATCH v2] worktree: add --quiet option","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2018-08-16T10:11:59Z","receivedAt":"2018-08-16T10:12:21Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"Hi Eric,\n\nOn Thu, 16 Aug 2018 at 10:25, Eric Sunshine wrote:\n> (/me nudges Martin off the fence onto the side of not bothering to\n> mention the obvious)\n\n:-)\n\nThanks for sanity-checking my thoughts. I agree with everything you\nwrote in your reply (and, FWIW, your other findings that you sent\nseparately).\n\nMartin\n"},{"id":"355877","messageId":"xmqqftzedp3p.fsf@gitster-ct.c.googlers.com","threadId":"49138","inReplyTo":"20180815205630.32876-1-gitter.spiros@gmail.com","subject":"Re: [PATCH v2] worktree: add --quiet option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-08-16T20:38:02Z","receivedAt":"2018-08-16T20:38:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elia Pinto writes:\n\n> diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh\n> index be6e09314..658647d83 100755\n> --- a/t/t2025-worktree-add.sh\n> +++ b/t/t2025-worktree-add.sh\n> @@ -252,6 +252,11 @@ test_expect_success 'add -B' '\n> \ttest_cmp_rev master^ poodle\n> '\n> \n> +test_expect_success 'add --quiet' '\n> +\tgit worktree add --quiet ../foo master >expected 2>&1 &&\n> +\ttest_must_be_empty expected\n> +'\n\nThat's misnomer. Unless existing tests in this file are already\nbogus, I'd like to see it called 'actual', which is the name we use\nto store the actual output (to be compared with another file we\ncreate to hold the expected output, typically called 'expect', like\n\"test_cmp expect actual\").\n\nI noticed the breakage after merging this to 'pu'; it seems to die\nwith \"fatal: ../foo already exists\" which comes from die().\n\nOh, more seriously, since when is it OK to muck with stuff _outside_\nthe $TRASH_DIRECTORY, e.g. \"../foo\", which would contaminate t/\ndirectory by creating a direct subdirectly under it?\n\nAhh, and I suspect that it is exactly why I am seeing a failure you\ndid not see---from a previously failed test cycle, \"t/foo\" is left\nbehind because \"make distclean\" would not clean it (of course).\n\nDo not ever touch anywhere outside $TRASH_DIRECTORY. Is this\nsomething we could enforce in our test harness, I wonder...\n\n> test_expect_success 'local clone from linked checkout' '\n> \tgit clone --local here here-clone &&\n> \t( cd here-clone && git fsck )\n"},{"id":"355878","messageId":"xmqqbma2dowe.fsf@gitster-ct.c.googlers.com","threadId":"49138","inReplyTo":"xmqqftzedp3p.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2] worktree: add --quiet option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-08-16T20:42:25Z","receivedAt":"2018-08-16T20:42:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Elia Pinto writes:\n>\n>> diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh\n>> index be6e09314..658647d83 100755\n>> --- a/t/t2025-worktree-add.sh\n>> +++ b/t/t2025-worktree-add.sh\n>> @@ -252,6 +252,11 @@ test_expect_success 'add -B' '\n>> \ttest_cmp_rev master^ poodle\n>> '\n>> \n>> +test_expect_success 'add --quiet' '\n>> +\tgit worktree add --quiet ../foo master >expected 2>&1 &&\n>> +\ttest_must_be_empty expected\n>> +'\n>\n> That's misnomer. Unless existing tests in this file are already\n> bogus, I'd like to see it called 'actual', which is the name we use\n> to store the actual output (to be compared with another file we\n> create to hold the expected output, typically called 'expect', like\n> \"test_cmp expect actual\").\n>\n> I noticed the breakage after merging this to 'pu'; it seems to die\n> with \"fatal: ../foo already exists\" which comes from die().\n>\n> Oh, more seriously, since when is it OK to muck with stuff _outside_\n> the $TRASH_DIRECTORY, e.g. \"../foo\", which would contaminate t/\n> directory by creating a direct subdirectly under it?\n>\n> Ahh, and I suspect that it is exactly why I am seeing a failure you\n> did not see---from a previously failed test cycle, \"t/foo\" is left\n> behind because \"make distclean\" would not clean it (of course).\n>\n> Do not ever touch anywhere outside $TRASH_DIRECTORY. Is this\n> something we could enforce in our test harness, I wonder...\n>\n>> test_expect_success 'local clone from linked checkout' '\n>> \tgit clone --local here here-clone &&\n>> \t( cd here-clone && git fsck )\n\nA quickfix (I wish I had a lot more time to spend to be extra\ncareful, bit I don't) I'll apply for now to get going...\n\n t/t2025-worktree-add.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh\nindex 658647d834..c674697913 100755\n--- a/t/t2025-worktree-add.sh\n+++ b/t/t2025-worktree-add.sh\n@@ -253,8 +253,8 @@ test_expect_success 'add -B' '\n '\n \n test_expect_success 'add --quiet' '\n-\tgit worktree add --quiet ../foo master >expected 2>&1 &&\n-\ttest_must_be_empty expected\n+\tgit worktree add --quiet another-worktree master 2>actual &&\n+\ttest_must_be_empty actual\n '\n \n test_expect_success 'local clone from linked checkout' '\n"}]}