* [PATCH] Do not fail test if '.' is part of $PATH
@ 2018-12-01 17:07 H.Merijn Brand
2018-12-01 19:38 ` Jeff King
2018-12-03 1:00 ` Junio C Hamano
0 siblings, 2 replies; 4+ messages in thread
From: H.Merijn Brand @ 2018-12-01 17:07 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 1382 bytes --]
When $PATH contains the current directory as .:PATH, PATH:., PATH:.:PATH,
or (maybe worse) as :PATH, PATH:, or PATH::PATH - as an empty entry is
identical to having dot in $PATH - this test used to fail
This patch was tested with PATH=$PATH, PATH=.:$PATH, PATH=$PATH:.,
PATH=$PATH:.:/bin, PATH=:$PATH, PATH=$PATH:, and PATH=$PATH::/bin
Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Tested-by: H.Merijn Brand - Tux <h.m.brand@xs4all.nl>
diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh
index cf932c851..557f87442 100755
--- a/t/t0061-run-command.sh
+++ b/t/t0061-run-command.sh
@@ -29,7 +29,14 @@ test_expect_success 'run_command can run a command' '
test_must_be_empty err
'
-test_expect_success 'run_command is restricted to PATH' '
+test_lazy_prereq DOT_IN_PATH '
+ case ":$PATH:" in
+ *:.:*|*::*) true ;;
+ *) false ;;
+ esac
+'
+
+test_expect_success !DOT_IN_PATH 'run_command is restricted to PATH' '
write_script should-not-run <<-\EOF &&
echo yikes
EOF
--
H.Merijn Brand http://tux.nl Perl Monger http://amsterdam.pm.org/
using perl5.00307 .. 5.29 porting perl5 on HP-UX, AIX, and openSUSE
http://mirrors.develooper.com/hpux/ http://www.test-smoke.org/
http://qa.perl.org http://www.goldmark.org/jeff/stupid-disclaimers/
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] Do not fail test if '.' is part of $PATH
2018-12-01 17:07 [PATCH] Do not fail test if '.' is part of $PATH H.Merijn Brand
@ 2018-12-01 19:38 ` Jeff King
2018-12-03 0:29 ` Junio C Hamano
2018-12-03 1:00 ` Junio C Hamano
1 sibling, 1 reply; 4+ messages in thread
From: Jeff King @ 2018-12-01 19:38 UTC (permalink / raw)
To: H.Merijn Brand; +Cc: git
On Sat, Dec 01, 2018 at 06:07:57PM +0100, H.Merijn Brand wrote:
> When $PATH contains the current directory as .:PATH, PATH:., PATH:.:PATH,
> or (maybe worse) as :PATH, PATH:, or PATH::PATH - as an empty entry is
> identical to having dot in $PATH - this test used to fail
Good catch. The test cares about Git not accidentally adding "." to the
PATH, but we can't check that if it is already there.
> This patch was tested with PATH=$PATH, PATH=.:$PATH, PATH=$PATH:.,
> PATH=$PATH:.:/bin, PATH=:$PATH, PATH=$PATH:, and PATH=$PATH::/bin
> [...]
> +test_lazy_prereq DOT_IN_PATH '
> + case ":$PATH:" in
> + *:.:*|*::*) true ;;
> + *) false ;;
> + esac
> +'
Since the test is ultimately checking "can we run should-not-run from
the current directory", might it be simpler to actually try that as the
precondition? I.e., something like:
test_expect_success 'create program in current directory' '
write_script should-not-run <<-\EOF &&
echo yikes
EOF
'
test_lazy_prereq DOT_IN_PATH '
should-not-run
'
test_expect_success !DOT_IN_PATH 'run_command is restricted to PATH' '
test_must_fail test-tool run-command run-command should-not-run
'
?
That's more lines, but we don't have to peek into the details of how
$PATH works.
-Peff
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Do not fail test if '.' is part of $PATH
2018-12-01 19:38 ` Jeff King
@ 2018-12-03 0:29 ` Junio C Hamano
0 siblings, 0 replies; 4+ messages in thread
From: Junio C Hamano @ 2018-12-03 0:29 UTC (permalink / raw)
To: Jeff King; +Cc: H.Merijn Brand, git
Jeff King <peff@peff.net> writes:
> Since the test is ultimately checking "can we run should-not-run from
> the current directory", might it be simpler to actually try that as the
> precondition? I.e., something like:
> ...
A nice egg of columbus. It also would save us from mischievous
users who have should-not-run somewhere no the $PATH that outputs
the string we expect (no, I do not think it is a common thing to do;
I am just saying that the solution covers such an extremely stupid
case without special casing).
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Do not fail test if '.' is part of $PATH
2018-12-01 17:07 [PATCH] Do not fail test if '.' is part of $PATH H.Merijn Brand
2018-12-01 19:38 ` Jeff King
@ 2018-12-03 1:00 ` Junio C Hamano
1 sibling, 0 replies; 4+ messages in thread
From: Junio C Hamano @ 2018-12-03 1:00 UTC (permalink / raw)
To: H.Merijn Brand, Jeff King; +Cc: git
"H.Merijn Brand" <h.m.brand@xs4all.nl> writes:
> When $PATH contains the current directory as .:PATH, PATH:., PATH:.:PATH,
> or (maybe worse) as :PATH, PATH:, or PATH::PATH - as an empty entry is
> identical to having dot in $PATH - this test used to fail
It is totally unclear what "this test" refers to. Let's retitle it
to
> Subject: [PATCH] t0061: do not fail test if '.' is part of $PATH
and do something like this:
t0061 created a script named with an unlikely name in the
current directory to ensure that it is not found via the
run_command() API, expecting that $PATH does not contain an
element that names the current directory (i.e. '.' or '') in a
sane environment. This obviously would not work if the $PATH
does contain such an element.
Introduce a DOT_IN_PATH lazy prerequisite to catch such a case
and skip the test when the environment is not so sane.
> +test_lazy_prereq DOT_IN_PATH '
> + case ":$PATH:" in
> + *:.:*|*::*) true ;;
> + *) false ;;
> + esac
> +'
> +
> +test_expect_success !DOT_IN_PATH 'run_command is restricted to PATH' '
> write_script should-not-run <<-\EOF &&
> echo yikes
> EOF
I also like Peff's more straight-forward approach that avoids
looking into PATH but instead ask the shell what we care about
(i.e. would we end up running 'should-not-run' if we asked the
system to run it without giving an explicit path to it?). The last
paragraph of the above would need to change if we were to go in that
direction to something like
Check if the running shell picks up the script without an
explicit path to it and skip the test when it does.
perhaps. The code to do so got a bit more compact than what Peff
wrote but I think it still retains its main beauty, which is how
straight-forward it is.
t/t0061-run-command.sh | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh
index cf932c8514..17b560370e 100755
--- a/t/t0061-run-command.sh
+++ b/t/t0061-run-command.sh
@@ -29,7 +29,15 @@ test_expect_success 'run_command can run a command' '
test_must_be_empty err
'
-test_expect_success 'run_command is restricted to PATH' '
+
+test_lazy_prereq RUNS_COMMANDS_FROM_PWD '
+ write_script runs-commands-from-pwd <<-\EOF &&
+ true
+ EOF
+ runs-commands-from-pwd >/dev/null 2>&1
+'
+
+test_expect_success !RUNS_COMMANDS_FROM_PWD 'run_command is restricted to PATH' '
write_script should-not-run <<-\EOF &&
echo yikes
EOF
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-12-03 1:01 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-01 17:07 [PATCH] Do not fail test if '.' is part of $PATH H.Merijn Brand
2018-12-01 19:38 ` Jeff King
2018-12-03 0:29 ` Junio C Hamano
2018-12-03 1:00 ` Junio C Hamano
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).