{"thread":{"id":"49660","subject":"[PATCH 0/2] run-command: fix Unix PATH lookup","startedAt":"2018-10-24T07:36:40Z","lastAt":"2018-10-25T01:15:12Z","messageCount":8,"participants":["Jeff King","Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"361376","messageId":"20181024073637.GA31069@sigill.intra.peff.net","threadId":"49660","inReplyTo":null,"subject":"[PATCH 0/2] run-command: fix Unix PATH lookup","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-10-24T07:36:37Z","receivedAt":"2018-10-24T07:36:40Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This series fixes a bug where run-command.c accidentally runs commands\nfrom the current directory when they are not found in the $PATH.\n\nThe second patch is the interesting one. The first one is just a\nnecessary cleanup. Possibly this should be split into two topics: the\nsecond patch slated for maint, and then the first one as an independent\nfixup on tb/filter-alternate-refs (which is in master).\n\n  [1/2]: t5410: use longer path for sample script\n  [2/2]: run-command: mark path lookup errors with ENOENT\n\n run-command.c                      | 20 ++++++++++++++++----\n t/t0061-run-command.sh             | 13 ++++++++++++-\n t/t5410-receive-pack-alternates.sh |  2 +-\n 3 files changed, 29 insertions(+), 6 deletions(-)\n\n-Peff\n"},{"id":"361377","messageId":"20181024073705.GA31202@sigill.intra.peff.net","threadId":"49660","inReplyTo":"20181024073637.GA31069@sigill.intra.peff.net","subject":"[PATCH 1/2] t5410: use longer path for sample script","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-10-24T07:37:06Z","receivedAt":"2018-10-24T07:37:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"t5410 creates a sample script \"alternate-refs\", and sets\ncore.alternateRefsCommand to just \"alternate-refs\". That\nshouldn't work, as \".\" is not in our $PATH, and so we should\nnot find it.\n\nHowever, due to a bug in run-command.c, we sometimes find it\nanyway! Even more confusing, this bug is only in the\nfork-based version of run-command. So the test passes on\nLinux (etc), but fails on Windows.\n\nIn preparation for fixing the run-command bug, let's use a\nmore complete path here.\n\nReported-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/t5410-receive-pack-alternates.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t5410-receive-pack-alternates.sh b/t/t5410-receive-pack-alternates.sh\nindex 457c20c2a5..f00d0da860 100755\n--- a/t/t5410-receive-pack-alternates.sh\n+++ b/t/t5410-receive-pack-alternates.sh\n@@ -23,7 +23,7 @@ test_expect_success 'with core.alternateRefsCommand' '\n \t\t\t--format=\"%(objectname)\" \\\n \t\t\trefs/heads/public/\n \tEOF\n-\ttest_config -C fork core.alternateRefsCommand alternate-refs &&\n+\ttest_config -C fork core.alternateRefsCommand ./alternate-refs &&\n \tgit rev-parse public/branch >expect &&\n \tprintf \"0000\" | git receive-pack fork >actual &&\n \textract_haves <actual >actual.haves &&\n-- \n2.19.1.1094.gd480080bf6\n\n"},{"id":"361379","messageId":"20181024073800.GB31202@sigill.intra.peff.net","threadId":"49660","inReplyTo":"20181024073637.GA31069@sigill.intra.peff.net","subject":"[PATCH 2/2] run-command: mark path lookup errors with ENOENT","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-10-24T07:38:00Z","receivedAt":"2018-10-24T07:38:04Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Since commit e3a434468f (run-command: use the\nasync-signal-safe execv instead of execvp, 2017-04-19),\nprepare_cmd() does its own PATH lookup for any commands we\nrun (on non-Windows platforms).\n\nHowever, its logic does not match the old execvp call when\nwe fail to find a matching entry in the PATH. Instead of\nfeeding the name directly to execv, execvp would consider\nthat an ENOENT error. By continuing and passing the name\ndirectly to execv, we effectively behave as if \".\" was\nincluded at the end of the PATH. This can have confusing and\neven dangerous results.\n\nThe fix itself is pretty straight-forward. There's a new\ntest in t0061 to cover this explicitly, and I've also added\na duplicate of the ENOENT test to ensure that we return the\ncorrect errno for this case.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n run-command.c          | 20 ++++++++++++++++----\n t/t0061-run-command.sh | 13 ++++++++++++-\n 2 files changed, 28 insertions(+), 5 deletions(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex 84b883c213..639ea5ac33 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -380,7 +380,7 @@ static void child_err_spew(struct child_process *cmd, struct child_err *cerr)\n \tset_error_routine(old_errfn);\n }\n \n-static void prepare_cmd(struct argv_array *out, const struct child_process *cmd)\n+static int prepare_cmd(struct argv_array *out, const struct child_process *cmd)\n {\n \tif (!cmd->argv[0])\n \t\tBUG(\"command is empty\");\n@@ -403,16 +403,22 @@ static void prepare_cmd(struct argv_array *out, const struct child_process *cmd)\n \t/*\n \t * If there are no '/' characters in the command then perform a path\n \t * lookup and use the resolved path as the command to exec.  If there\n-\t * are no '/' characters or if the command wasn't found in the path,\n-\t * have exec attempt to invoke the command directly.\n+\t * are '/' characters, we have exec attempt to invoke the command\n+\t * directly.\n \t */\n \tif (!strchr(out->argv[1], '/')) {\n \t\tchar *program = locate_in_PATH(out->argv[1]);\n \t\tif (program) {\n \t\t\tfree((char *)out->argv[1]);\n \t\t\tout->argv[1] = program;\n+\t\t} else {\n+\t\t\targv_array_clear(out);\n+\t\t\terrno = ENOENT;\n+\t\t\treturn -1;\n \t\t}\n \t}\n+\n+\treturn 0;\n }\n \n static char **prep_childenv(const char *const *deltaenv)\n@@ -719,6 +725,12 @@ int start_command(struct child_process *cmd)\n \tstruct child_err cerr;\n \tstruct atfork_state as;\n \n+\tif (prepare_cmd(&argv, cmd) < 0) {\n+\t\tfailed_errno = errno;\n+\t\tcmd->pid = -1;\n+\t\tgoto end_of_spawn;\n+\t}\n+\n \tif (pipe(notify_pipe))\n \t\tnotify_pipe[0] = notify_pipe[1] = -1;\n \n@@ -729,7 +741,6 @@ int start_command(struct child_process *cmd)\n \t\tset_cloexec(null_fd);\n \t}\n \n-\tprepare_cmd(&argv, cmd);\n \tchildenv = prep_childenv(cmd->env);\n \tatfork_prepare(&as);\n \n@@ -910,6 +921,7 @@ int start_command(struct child_process *cmd)\n }\n #endif\n \n+end_of_spawn:\n \tif (cmd->pid < 0) {\n \t\tif (need_in)\n \t\t\tclose_pair(fdin);\ndiff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh\nindex 3e131c5325..cf932c8514 100755\n--- a/t/t0061-run-command.sh\n+++ b/t/t0061-run-command.sh\n@@ -12,10 +12,14 @@ cat >hello-script <<-EOF\n \tcat hello-script\n EOF\n \n-test_expect_success 'start_command reports ENOENT' '\n+test_expect_success 'start_command reports ENOENT (slash)' '\n \ttest-tool run-command start-command-ENOENT ./does-not-exist\n '\n \n+test_expect_success 'start_command reports ENOENT (no slash)' '\n+\ttest-tool run-command start-command-ENOENT does-not-exist\n+'\n+\n test_expect_success 'run_command can run a command' '\n \tcat hello-script >hello.sh &&\n \tchmod +x hello.sh &&\n@@ -25,6 +29,13 @@ test_expect_success 'run_command can run a command' '\n \ttest_must_be_empty err\n '\n \n+test_expect_success 'run_command is restricted to PATH' '\n+\twrite_script should-not-run <<-\\EOF &&\n+\techo yikes\n+\tEOF\n+\ttest_must_fail test-tool run-command run-command should-not-run\n+'\n+\n test_expect_success !MINGW 'run_command can run a script without a #! line' '\n \tcat >hello <<-\\EOF &&\n \tcat hello-script\n-- \n2.19.1.1094.gd480080bf6\n"},{"id":"361391","messageId":"nycvar.QRO.7.76.6.1810241051440.4546@tvgsbejvaqbjf.bet","threadId":"49660","inReplyTo":"20181024073705.GA31202@sigill.intra.peff.net","subject":"Re: [PATCH 1/2] t5410: use longer path for sample script","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-10-24T08:53:31Z","receivedAt":"2018-10-24T08:53:36Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn Wed, 24 Oct 2018, Jeff King wrote:\n\n> t5410 creates a sample script \"alternate-refs\", and sets\n> core.alternateRefsCommand to just \"alternate-refs\". That\n> shouldn't work, as \".\" is not in our $PATH, and so we should\n> not find it.\n> \n> However, due to a bug in run-command.c, we sometimes find it\n> anyway! Even more confusing, this bug is only in the\n> fork-based version of run-command. So the test passes on\n> Linux (etc), but fails on Windows.\n> \n> In preparation for fixing the run-command bug, let's use a\n> more complete path here.\n> \n> Reported-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n\nThank you for the fix! I can confirm that the patch works, and the commit\nmessage is stellar, as per usual for your contributions.\n\nBTW since this breaks every single one of our Continuous Builds on\nWindows, I would be very much in favor of fast-tracking this to `master`.\n\nThanks,\nDscho\n\n>  t/t5410-receive-pack-alternates.sh | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/t/t5410-receive-pack-alternates.sh b/t/t5410-receive-pack-alternates.sh\n> index 457c20c2a5..f00d0da860 100755\n> --- a/t/t5410-receive-pack-alternates.sh\n> +++ b/t/t5410-receive-pack-alternates.sh\n> @@ -23,7 +23,7 @@ test_expect_success 'with core.alternateRefsCommand' '\n>  \t\t\t--format=\"%(objectname)\" \\\n>  \t\t\trefs/heads/public/\n>  \tEOF\n> -\ttest_config -C fork core.alternateRefsCommand alternate-refs &&\n> +\ttest_config -C fork core.alternateRefsCommand ./alternate-refs &&\n>  \tgit rev-parse public/branch >expect &&\n>  \tprintf \"0000\" | git receive-pack fork >actual &&\n>  \textract_haves <actual >actual.haves &&\n> -- \n> 2.19.1.1094.gd480080bf6\n> \n> \n"},{"id":"361393","messageId":"nycvar.QRO.7.76.6.1810241054110.4546@tvgsbejvaqbjf.bet","threadId":"49660","inReplyTo":"20181024073800.GB31202@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] run-command: mark path lookup errors with ENOENT","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-10-24T09:01:54Z","receivedAt":"2018-10-24T09:02:14Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn Wed, 24 Oct 2018, Jeff King wrote:\n\n> Since commit e3a434468f (run-command: use the\n> async-signal-safe execv instead of execvp, 2017-04-19),\n> prepare_cmd() does its own PATH lookup for any commands we\n> run (on non-Windows platforms).\n> \n> However, its logic does not match the old execvp call when\n> we fail to find a matching entry in the PATH. Instead of\n> feeding the name directly to execv, execvp would consider\n> that an ENOENT error. By continuing and passing the name\n> directly to execv, we effectively behave as if \".\" was\n> included at the end of the PATH. This can have confusing and\n> even dangerous results.\n\nFor the record, I tried to come up with an attack vector to exploit this,\nand failed to find one.\n\n> The fix itself is pretty straight-forward. There's a new\n> test in t0061 to cover this explicitly, and I've also added\n> a duplicate of the ENOENT test to ensure that we return the\n> correct errno for this case.\n> \n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n>  run-command.c          | 20 ++++++++++++++++----\n>  t/t0061-run-command.sh | 13 ++++++++++++-\n>  2 files changed, 28 insertions(+), 5 deletions(-)\n> \n> diff --git a/run-command.c b/run-command.c\n> index 84b883c213..639ea5ac33 100644\n> --- a/run-command.c\n> +++ b/run-command.c\n> @@ -380,7 +380,7 @@ static void child_err_spew(struct child_process *cmd, struct child_err *cerr)\n>  \tset_error_routine(old_errfn);\n>  }\n>  \n> -static void prepare_cmd(struct argv_array *out, const struct child_process *cmd)\n> +static int prepare_cmd(struct argv_array *out, const struct child_process *cmd)\n\nI always like when we change functions to return a value that can then\nindicate an error, making the libification effort so much easier.\n\n>  {\n>  \tif (!cmd->argv[0])\n>  \t\tBUG(\"command is empty\");\n> @@ -403,16 +403,22 @@ static void prepare_cmd(struct argv_array *out, const struct child_process *cmd)\n>  \t/*\n>  \t * If there are no '/' characters in the command then perform a path\n>  \t * lookup and use the resolved path as the command to exec.  If there\n> -\t * are no '/' characters or if the command wasn't found in the path,\n> -\t * have exec attempt to invoke the command directly.\n> +\t * are '/' characters, we have exec attempt to invoke the command\n> +\t * directly.\n\nNice. I would have probably forgotten about that comment.\n\n>  \t */\n>  \tif (!strchr(out->argv[1], '/')) {\n>  \t\tchar *program = locate_in_PATH(out->argv[1]);\n>  \t\tif (program) {\n>  \t\t\tfree((char *)out->argv[1]);\n>  \t\t\tout->argv[1] = program;\n> +\t\t} else {\n> +\t\t\targv_array_clear(out);\n> +\t\t\terrno = ENOENT;\n> +\t\t\treturn -1;\n>  \t\t}\n>  \t}\n> +\n> +\treturn 0;\n>  }\n>  \n>  static char **prep_childenv(const char *const *deltaenv)\n> @@ -719,6 +725,12 @@ int start_command(struct child_process *cmd)\n>  \tstruct child_err cerr;\n>  \tstruct atfork_state as;\n>  \n> +\tif (prepare_cmd(&argv, cmd) < 0) {\n> +\t\tfailed_errno = errno;\n> +\t\tcmd->pid = -1;\n> +\t\tgoto end_of_spawn;\n> +\t}\n> +\n>  \tif (pipe(notify_pipe))\n>  \t\tnotify_pipe[0] = notify_pipe[1] = -1;\n>  \n> @@ -729,7 +741,6 @@ int start_command(struct child_process *cmd)\n>  \t\tset_cloexec(null_fd);\n>  \t}\n>  \n> -\tprepare_cmd(&argv, cmd);\n>  \tchildenv = prep_childenv(cmd->env);\n>  \tatfork_prepare(&as);\n>  \n> @@ -910,6 +921,7 @@ int start_command(struct child_process *cmd)\n>  }\n>  #endif\n>  \n> +end_of_spawn:\n\nSadly, this fails to build on Windows:\n\n\trun-command.c: In function 'start_command':\n\trun-command.c:924:1: error: label 'end_of_spawn' defined but not used [-Werror=unused-label]\n\t end_of_spawn:\n\t ^~~~~~~~~~~~\n\nHow about squashing in this diff:\n\n-- snip --\ndiff --git a/run-command.c b/run-command.c\nindex 639ea5ac3366..3f03795a5995 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -918,6 +918,8 @@ int start_command(struct child_process *cmd)\n \t\tclose(fhout);\n \tif (fherr != 2)\n \t\tclose(fherr);\n+\n+\tgoto end_of_spawn;\n }\n #endif\n \n-- snap --\n\nI can confirm that the result compiles and passes t0061.\n\nThanks,\nDscho\n\n>  \tif (cmd->pid < 0) {\n>  \t\tif (need_in)\n>  \t\t\tclose_pair(fdin);\n> diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh\n> index 3e131c5325..cf932c8514 100755\n> --- a/t/t0061-run-command.sh\n> +++ b/t/t0061-run-command.sh\n> @@ -12,10 +12,14 @@ cat >hello-script <<-EOF\n>  \tcat hello-script\n>  EOF\n>  \n> -test_expect_success 'start_command reports ENOENT' '\n> +test_expect_success 'start_command reports ENOENT (slash)' '\n>  \ttest-tool run-command start-command-ENOENT ./does-not-exist\n>  '\n>  \n> +test_expect_success 'start_command reports ENOENT (no slash)' '\n> +\ttest-tool run-command start-command-ENOENT does-not-exist\n> +'\n> +\n>  test_expect_success 'run_command can run a command' '\n>  \tcat hello-script >hello.sh &&\n>  \tchmod +x hello.sh &&\n> @@ -25,6 +29,13 @@ test_expect_success 'run_command can run a command' '\n>  \ttest_must_be_empty err\n>  '\n>  \n> +test_expect_success 'run_command is restricted to PATH' '\n> +\twrite_script should-not-run <<-\\EOF &&\n> +\techo yikes\n> +\tEOF\n> +\ttest_must_fail test-tool run-command run-command should-not-run\n> +'\n> +\n>  test_expect_success !MINGW 'run_command can run a script without a #! line' '\n>  \tcat >hello <<-\\EOF &&\n>  \tcat hello-script\n> -- \n> 2.19.1.1094.gd480080bf6\n> \n"},{"id":"361394","messageId":"20181024091617.GA25118@sigill.intra.peff.net","threadId":"49660","inReplyTo":"nycvar.QRO.7.76.6.1810241054110.4546@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 2/2] run-command: mark path lookup errors with ENOENT","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-10-24T09:16:18Z","receivedAt":"2018-10-24T09:16:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 24, 2018 at 11:01:54AM +0200, Johannes Schindelin wrote:\n\n> > @@ -910,6 +921,7 @@ int start_command(struct child_process *cmd)\n> >  }\n> >  #endif\n> >  \n> > +end_of_spawn:\n> \n> Sadly, this fails to build on Windows:\n> \n> \trun-command.c: In function 'start_command':\n> \trun-command.c:924:1: error: label 'end_of_spawn' defined but not used [-Werror=unused-label]\n> \t end_of_spawn:\n> \t ^~~~~~~~~~~~\n\nDoh. I didn't think of that.\n\n> How about squashing in this diff:\n> \n> -- snip --\n> diff --git a/run-command.c b/run-command.c\n> index 639ea5ac3366..3f03795a5995 100644\n> --- a/run-command.c\n> +++ b/run-command.c\n> @@ -918,6 +918,8 @@ int start_command(struct child_process *cmd)\n>  \t\tclose(fhout);\n>  \tif (fherr != 2)\n>  \t\tclose(fherr);\n> +\n> +\tgoto end_of_spawn;\n>  }\n>  #endif\n>  \n> -- snap --\n> \n> I can confirm that the result compiles and passes t0061.\n\nThat leaves the Windows side of the #else with a funny, useless goto\n(and without even a matching useless one on the Unix side).  Let's put\nit instead inside the half of the #if that actually uses it. Like so\n(actually courtesy of Jonathan Nieder):\n\ndiff --git a/run-command.c b/run-command.c\nindex 639ea5ac33..d679cc267c 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -868,6 +868,8 @@ int start_command(struct child_process *cmd)\n \targv_array_clear(&argv);\n \tfree(childenv);\n }\n+end_of_spawn:\n+\n #else\n {\n \tint fhin = 0, fhout = 1, fherr = 2;\n@@ -921,7 +923,6 @@ int start_command(struct child_process *cmd)\n }\n #endif\n \n-end_of_spawn:\n \tif (cmd->pid < 0) {\n \t\tif (need_in)\n \t\t\tclose_pair(fdin);\n\nThanks for your review!\n\n-Peff\n"},{"id":"361400","messageId":"nycvar.QRO.7.76.6.1810241125360.4546@tvgsbejvaqbjf.bet","threadId":"49660","inReplyTo":"20181024091617.GA25118@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] run-command: mark path lookup errors with ENOENT","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-10-24T09:25:56Z","receivedAt":"2018-10-24T09:26:00Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn Wed, 24 Oct 2018, Jeff King wrote:\n\n> On Wed, Oct 24, 2018 at 11:01:54AM +0200, Johannes Schindelin wrote:\n> \n> > > @@ -910,6 +921,7 @@ int start_command(struct child_process *cmd)\n> > >  }\n> > >  #endif\n> > >  \n> > > +end_of_spawn:\n> > \n> > Sadly, this fails to build on Windows:\n> > \n> > \trun-command.c: In function 'start_command':\n> > \trun-command.c:924:1: error: label 'end_of_spawn' defined but not used [-Werror=unused-label]\n> > \t end_of_spawn:\n> > \t ^~~~~~~~~~~~\n> \n> Doh. I didn't think of that.\n> \n> > How about squashing in this diff:\n> > \n> > -- snip --\n> > diff --git a/run-command.c b/run-command.c\n> > index 639ea5ac3366..3f03795a5995 100644\n> > --- a/run-command.c\n> > +++ b/run-command.c\n> > @@ -918,6 +918,8 @@ int start_command(struct child_process *cmd)\n> >  \t\tclose(fhout);\n> >  \tif (fherr != 2)\n> >  \t\tclose(fherr);\n> > +\n> > +\tgoto end_of_spawn;\n> >  }\n> >  #endif\n> >  \n> > -- snap --\n> > \n> > I can confirm that the result compiles and passes t0061.\n> \n> That leaves the Windows side of the #else with a funny, useless goto\n> (and without even a matching useless one on the Unix side).  Let's put\n> it instead inside the half of the #if that actually uses it. Like so\n> (actually courtesy of Jonathan Nieder):\n> \n> diff --git a/run-command.c b/run-command.c\n> index 639ea5ac33..d679cc267c 100644\n> --- a/run-command.c\n> +++ b/run-command.c\n> @@ -868,6 +868,8 @@ int start_command(struct child_process *cmd)\n>  \targv_array_clear(&argv);\n>  \tfree(childenv);\n>  }\n> +end_of_spawn:\n> +\n>  #else\n>  {\n>  \tint fhin = 0, fhout = 1, fherr = 2;\n> @@ -921,7 +923,6 @@ int start_command(struct child_process *cmd)\n>  }\n>  #endif\n>  \n> -end_of_spawn:\n>  \tif (cmd->pid < 0) {\n>  \t\tif (need_in)\n>  \t\t\tclose_pair(fdin);\n\nThat works for me, too.\n\n> Thanks for your review!\n\nYou're welcome!\n\nThanks,\nDscho\n\n> \n> -Peff\n> \n"},{"id":"361444","messageId":"xmqq5zxqn7xg.fsf@gitster-ct.c.googlers.com","threadId":"49660","inReplyTo":"nycvar.QRO.7.76.6.1810241051440.4546@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 1/2] t5410: use longer path for sample script","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-25T01:15:07Z","receivedAt":"2018-10-25T01:15:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> On Wed, 24 Oct 2018, Jeff King wrote:\n>\n>> t5410 creates a sample script \"alternate-refs\", and sets\n>> core.alternateRefsCommand to just \"alternate-refs\". That\n>> shouldn't work, as \".\" is not in our $PATH, and so we should\n>> not find it.\n>> \n>> However, due to a bug in run-command.c, we sometimes find it\n>> anyway! Even more confusing, this bug is only in the\n>> fork-based version of run-command. So the test passes on\n>> Linux (etc), but fails on Windows.\n>> \n>> In preparation for fixing the run-command bug, let's use a\n>> more complete path here.\n>> \n>> Reported-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>\n>> Signed-off-by: Jeff King <peff@peff.net>\n>> ---\n>\n> Thank you for the fix! I can confirm that the patch works, and the commit\n> message is stellar, as per usual for your contributions.\n>\n> BTW since this breaks every single one of our Continuous Builds on\n> Windows, I would be very much in favor of fast-tracking this to `master`.\n>\n> Thanks,\n> Dscho\n\nI should note to the public that this one, and the companion patch\n2/2, owe greatly to you and Peff's efforts.\n\nThanks, both.\n\n>\n>>  t/t5410-receive-pack-alternates.sh | 2 +-\n>>  1 file changed, 1 insertion(+), 1 deletion(-)\n>> \n>> diff --git a/t/t5410-receive-pack-alternates.sh b/t/t5410-receive-pack-alternates.sh\n>> index 457c20c2a5..f00d0da860 100755\n>> --- a/t/t5410-receive-pack-alternates.sh\n>> +++ b/t/t5410-receive-pack-alternates.sh\n>> @@ -23,7 +23,7 @@ test_expect_success 'with core.alternateRefsCommand' '\n>>  \t\t\t--format=\"%(objectname)\" \\\n>>  \t\t\trefs/heads/public/\n>>  \tEOF\n>> -\ttest_config -C fork core.alternateRefsCommand alternate-refs &&\n>> +\ttest_config -C fork core.alternateRefsCommand ./alternate-refs &&\n>>  \tgit rev-parse public/branch >expect &&\n>>  \tprintf \"0000\" | git receive-pack fork >actual &&\n>>  \textract_haves <actual >actual.haves &&\n>> -- \n>> 2.19.1.1094.gd480080bf6\n>> \n>> \n"}]}