{"thread":{"id":"49929","subject":"[PATCH] Do not fail test if '.' is part of $PATH","startedAt":"2018-12-01T17:08:13Z","lastAt":"2018-12-03T01:01:00Z","messageCount":4,"participants":["H.Merijn Brand","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"364417","messageId":"20181201180757.0b2d3c89@pc09.procura.nl","threadId":"49929","inReplyTo":null,"subject":"[PATCH] Do not fail test if '.' is part of $PATH","fromName":"H.Merijn Brand","fromEmail":"h.m.brand@xs4all.nl","sentAt":"2018-12-01T17:07:57Z","receivedAt":"2018-12-01T17:08:13Z","isPatch":true,"sender":{"key":"h.m.brand@xs4all.nl","avatar":"https://gravatar.com/avatar/5b8f83ee35c427a646cbea3b104346e00ab3663b99bbf435cddeb75cd4b3857b?d=mp&s=160"},"body":"When $PATH contains the current directory as .:PATH, PATH:., PATH:.:PATH,\nor (maybe worse) as :PATH, PATH:, or PATH::PATH - as an empty entry is\nidentical to having dot in $PATH - this test used to fail\n\nThis patch was tested with PATH=$PATH, PATH=.:$PATH, PATH=$PATH:.,\nPATH=$PATH:.:/bin, PATH=:$PATH, PATH=$PATH:, and PATH=$PATH::/bin\n\nSigned-off-by: Johannes Schindelin \nTested-by: H.Merijn Brand - Tux \n\ndiff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh\nindex cf932c851..557f87442 100755\n--- a/t/t0061-run-command.sh\n+++ b/t/t0061-run-command.sh\n@@ -29,7 +29,14 @@ test_expect_success 'run_command can run a command' '\n test_must_be_empty err\n '\n\n-test_expect_success 'run_command is restricted to PATH' '\n+test_lazy_prereq DOT_IN_PATH '\n+ case \":$PATH:\" in\n+ *:.:*|*::*) true ;;\n+ *) false ;;\n+ esac\n+'\n+\n+test_expect_success !DOT_IN_PATH 'run_command is restricted to PATH' '\n write_script should-not-run <<-\\EOF &&\n echo yikes\n EOF\n\n-- \nH.Merijn Brand http://tux.nl Perl Monger http://amsterdam.pm.org/\nusing perl5.00307 .. 5.29 porting perl5 on HP-UX, AIX, and openSUSE\nhttp://mirrors.develooper.com/hpux/ http://www.test-smoke.org/\nhttp://qa.perl.org http://www.goldmark.org/jeff/stupid-disclaimers/\n"},{"id":"364419","messageId":"20181201193822.GA28918@sigill.intra.peff.net","threadId":"49929","inReplyTo":"20181201180757.0b2d3c89@pc09.procura.nl","subject":"Re: [PATCH] Do not fail test if '.' is part of $PATH","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-12-01T19:38:22Z","receivedAt":"2018-12-01T19:38:25Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Dec 01, 2018 at 06:07:57PM +0100, H.Merijn Brand wrote:\n\n> When $PATH contains the current directory as .:PATH, PATH:., PATH:.:PATH,\n> or (maybe worse) as :PATH, PATH:, or PATH::PATH - as an empty entry is\n> identical to having dot in $PATH - this test used to fail\n\nGood catch. The test cares about Git not accidentally adding \".\" to the\nPATH, but we can't check that if it is already there.\n\n> This patch was tested with PATH=$PATH, PATH=.:$PATH, PATH=$PATH:.,\n> PATH=$PATH:.:/bin, PATH=:$PATH, PATH=$PATH:, and PATH=$PATH::/bin\n> [...]\n> +test_lazy_prereq DOT_IN_PATH '\n> + case \":$PATH:\" in\n> + *:.:*|*::*) true ;;\n> + *) false ;;\n> + esac\n> +'\n\nSince the test is ultimately checking \"can we run should-not-run from\nthe current directory\", might it be simpler to actually try that as the\nprecondition? I.e., something like:\n\n test_expect_success 'create program in current directory' '\n\twrite_script should-not-run <<-\\EOF &&\n\techo yikes\n\tEOF\n '\n\n test_lazy_prereq DOT_IN_PATH '\n\tshould-not-run\n '\n\n test_expect_success !DOT_IN_PATH 'run_command is restricted to PATH' '\n\ttest_must_fail test-tool run-command run-command should-not-run\n '\n\n?\n\nThat's more lines, but we don't have to peek into the details of how\n$PATH works.\n\n-Peff\n"},{"id":"364459","messageId":"xmqq4lbviha2.fsf@gitster-ct.c.googlers.com","threadId":"49929","inReplyTo":"20181201193822.GA28918@sigill.intra.peff.net","subject":"Re: [PATCH] Do not fail test if '.' is part of $PATH","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-12-03T00:29:57Z","receivedAt":"2018-12-03T00:30:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Since the test is ultimately checking \"can we run should-not-run from\n> the current directory\", might it be simpler to actually try that as the\n> precondition? I.e., something like:\n> ...\n\nA nice egg of columbus. It also would save us from mischievous\nusers who have should-not-run somewhere no the $PATH that outputs\nthe string we expect (no, I do not think it is a common thing to do;\nI am just saying that the solution covers such an extremely stupid\ncase without special casing).\n\n\n\n"},{"id":"364460","messageId":"xmqqr2ezh1a5.fsf@gitster-ct.c.googlers.com","threadId":"49929","inReplyTo":"20181201180757.0b2d3c89@pc09.procura.nl","subject":"Re: [PATCH] Do not fail test if '.' is part of $PATH","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-12-03T01:00:50Z","receivedAt":"2018-12-03T01:01:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"H.Merijn Brand\" writes:\n\n> When $PATH contains the current directory as .:PATH, PATH:., PATH:.:PATH,\n> or (maybe worse) as :PATH, PATH:, or PATH::PATH - as an empty entry is\n> identical to having dot in $PATH - this test used to fail\n\nIt is totally unclear what \"this test\" refers to. Let's retitle it\nto\n\n> Subject: [PATCH] t0061: do not fail test if '.' is part of $PATH\n\nand do something like this:\n\n t0061 created a script named with an unlikely name in the\n current directory to ensure that it is not found via the\n run_command() API, expecting that $PATH does not contain an\n element that names the current directory (i.e. '.' or '') in a\n sane environment. This obviously would not work if the $PATH\n does contain such an element.\n\n Introduce a DOT_IN_PATH lazy prerequisite to catch such a case\n and skip the test when the environment is not so sane.\n\n> +test_lazy_prereq DOT_IN_PATH '\n> + case \":$PATH:\" in\n> + *:.:*|*::*) true ;;\n> + *) false ;;\n> + esac\n> +'\n> +\n> +test_expect_success !DOT_IN_PATH 'run_command is restricted to PATH' '\n> write_script should-not-run <<-\\EOF &&\n> echo yikes\n> EOF\n\nI also like Peff's more straight-forward approach that avoids\nlooking into PATH but instead ask the shell what we care about\n(i.e. would we end up running 'should-not-run' if we asked the\nsystem to run it without giving an explicit path to it?). The last\nparagraph of the above would need to change if we were to go in that\ndirection to something like\n\n Check if the running shell picks up the script without an\n explicit path to it and skip the test when it does.\n\nperhaps. The code to do so got a bit more compact than what Peff\nwrote but I think it still retains its main beauty, which is how\nstraight-forward it is.\n\n t/t0061-run-command.sh | 10 +++++++++-\n 1 file changed, 9 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh\nindex cf932c8514..17b560370e 100755\n--- a/t/t0061-run-command.sh\n+++ b/t/t0061-run-command.sh\n@@ -29,7 +29,15 @@ 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+\n+test_lazy_prereq RUNS_COMMANDS_FROM_PWD '\n+\twrite_script runs-commands-from-pwd <<-\\EOF &&\n+\ttrue\n+\tEOF\n+\truns-commands-from-pwd >/dev/null 2>&1\n+'\n+\n+test_expect_success !RUNS_COMMANDS_FROM_PWD 'run_command is restricted to PATH' '\n \twrite_script should-not-run <<-\\EOF &&\n \techo yikes\n \tEOF\n"}]}