Re: env+nice, one bug fix, two test corrections

[Eric Blake]

Jim Meyering <jim <at> meyering.net> writes:

> Wasn't that the intent of your original test?  To ensure
> that the "-i" after "--" was interpreted as a program
> name and not as an option?
> 
> > I'll play with this some more and propose a patch later.
> 
> Thanks!

How do these look?


From: Eric Blake <address@hidden>
Date: Mon, 26 Oct 2009 09:26:00 -0600
Subject: [PATCH 1/2] env: document PATH interactions

* doc/coreutils.texi (env invocation): Mention that PATH is
modified prior to exec.
* tests/misc/env: Test this.
---
 doc/coreutils.texi |    5 +++++
 tests/misc/env     |   24 +++++++++++++++++++++---
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index c54ffb8..8a79ba7 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -14408,6 +14408,11 @@ env invocation
 The program should not be a special built-in utility
 (@pxref{Special built-in utilities}).

+Modifications to @env{PATH} take effect prior to searching for
address@hidden  Use caution when reducing @env{PATH}; behavior is
+not portable when @env{PATH} is undefined or omits key directories
+such as @file{/bin}.
+
 @cindex environment, printing

 If no command name is specified following the environment
diff --git a/tests/misc/env b/tests/misc/env
index 07dd9e4..e1e51ed 100755
--- a/tests/misc/env
+++ b/tests/misc/env
@@ -51,9 +51,16 @@ test $? = 126 || fail=1
 env no_such # no such command
 test $? = 127 || fail=1

-# Cygwin requires a minimal environment to launch new processes, so a
-# subsequent env will show more than just what we set.  Hence, it is
-# more portable to grep that our desired changes took place.
+# POSIX is clear that environ may, but need not be, sorted.
+# Environment variable values may contain newlines, which cannot be
+# observed by merely inspecting output from env.
+# Cygwin requires a minimal environment to launch new processes: execve
+# adds missing variables SYSTEMROOT and WINDIR, which show up in a
+# subsequent env.  Cygwin also requires /bin to always be part of PATH,
+# and attempts to unset or reduce PATH may cause execve to fail.
+#
+# For these reasons, it is more portable to grep that our desired changes
+# took place, rather than comparing output of env over an entire environment.
 if env | grep '^ENV_TEST' >/dev/null ; then
   skip_test_ "environment has potential interference from ENV_TEST*"
 fi
@@ -85,6 +92,17 @@ ENV_TEST1=b
 EOF
 compare exp out || fail=1

+# PATH modifications affect exec.
+mkdir unlikely_name || framework_failure
+cat <<EOF > unlikely_name/also_unlikely || framework_failure
+#!/bin/sh
+echo pass
+EOF
+chmod +x unlikely_name/also_unlikely || framework_failure
+env also_unlikely && fail=1
+test x`PATH=$PATH:unlikely_name env also_unlikely` = xpass || fail=1
+test x`env PATH="$PATH":unlikely_name also_unlikely` = xpass || fail=1
+
 # Use -- to end arguments.
 ln -s "$abs_top_builddir/src/echo" ./-i || framework_failure
 case `PATH="$PATH:" env -i echo good` in
-- 
1.6.4.2


>From aa071c1d1b567330bb00947afc9053b5439464e3 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Mon, 26 Oct 2009 13:32:49 -0600
Subject: [PATCH 2/2] tests: clean up tests of env -- handling

* tests/misc/env: Further tweaks, avoiding PATH problems inherent
in testing -i, and testing program name containing =.
* doc/coreutils.texi (env invocation): Mention that intermediate
program is needed to invoke program with name containing =.
* src/env.c: Fix bogus comment.
---
 doc/coreutils.texi |   14 ++++++++++++++
 src/env.c          |   15 ++++++++-------
 tests/misc/env     |   42 +++++++++++++++++++++++++++++-------------
 3 files changed, 51 insertions(+), 20 deletions(-)

diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index 8a79ba7..138cf01 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -14413,6 +14413,20 @@ env invocation
 not portable when @env{PATH} is undefined or omits key directories
 such as @file{/bin}.

+In the rare case that a utility contains a @samp{=} in the name, the
+only way to disambiguate it from a variable assignment is to use an
+intermediate command for @var{command}, and pass the problematic
+program name via @var{args}.  For example, if @file{./prog=} is an
+executable in the current @env{PATH}:
+
address@hidden
+env prog= true # runs 'true', with prog= in environment
+env ./prog= true # runs 'true', with ./prog= in environment
+env -- prog= true # runs 'true', with prog= in environment
+env sh -c '\prog= true' # runs 'prog=' with argument 'true'
+env sh -c 'exec "$@@"' sh prog= true # also runs 'prog='
address@hidden example
+
 @cindex environment, printing

 If no command name is specified following the environment
diff --git a/src/env.c b/src/env.c
index c021e3a..3c437c9 100644
--- a/src/env.c
+++ b/src/env.c
@@ -35,14 +35,14 @@
         zero-length value is different from unsetting it.

    --
-        Indicate that the following argument is the program
-        to invoke.  This is necessary when the program's name
-        begins with "-" or contains a "=".
+        Indicate that the following argument is not an option.
+        This is necessary when the program's name begins with "-",
+        but does not help if the program's name contains a "=".

    The first remaining argument specifies a program to invoke;
    it is searched for according to the specification of the PATH
-   environment variable.  Any arguments following that are
-   passed as arguments to that program.
+   environment variable, after environment modifications.  Any
+   arguments following that are passed as arguments to that program.

    If no command name is specified following the environment
    specifications, the resulting environment is printed.
@@ -72,8 +72,9 @@
         call.

    env -u EDITOR LOGNAME=foo PATH=/energy -- e=mc2 bar baz
-        runs the program "/energy/e=mc2" with environment
-        { LOGNAME=foo PATH=/energy }
+        attempts to run the program "/energy/--" with arguments
+        "e=mc2", "bar" and "baz" in the environment
+        { LOGNAME=foo PATH=/energy }.
 */

 #include <config.h>
diff --git a/tests/misc/env b/tests/misc/env
index e1e51ed..89891fc 100755
--- a/tests/misc/env
+++ b/tests/misc/env
@@ -103,25 +103,41 @@ env also_unlikely && fail=1
 test x`PATH=$PATH:unlikely_name env also_unlikely` = xpass || fail=1
 test x`env PATH="$PATH":unlikely_name also_unlikely` = xpass || fail=1

-# Use -- to end arguments.
-ln -s "$abs_top_builddir/src/echo" ./-i || framework_failure
-case `PATH="$PATH:" env -i echo good` in
+# Use -- to end options (but not variable assignments).
+# On some systems, execve("-i") invokes a shebang script ./-i on PATH as
+# '/bin/sh -i', rather than '/bin/sh -- -i', which doesn't do what we want.
+# Avoid the issue by using an executable rather than a script.
+# Test -u, rather than -i, to avoid PATH problems.
+ln -s "$abs_top_builddir/src/echo" ./-u || framework_failure
+case `PATH=$PATH: env -u echo echo good` in
   good) ;;
   *) fail=1 ;;
 esac
-case `env -i -- PATH=. -i no-echo` in
-  no-echo) ;;
+case `PATH=$PATH: env -- -u pass` in
+  pass) ;;
   *) fail=1 ;;
 esac

-# FIXME - POSIX does not allow this; decide what we want to do
-# cat <<EOF >./c=d || framework_failure
-# #!/bin/sh
-# echo pass
-# EOF
-# chmod +x c=d || framework_failure
-# test "x`env c=d echo fail`" = xfail || fail=1
-# test "x`env -- c=d echo fail`" = xpass || fail=1
+# After options have ended, the first argument not containing = is a program.
+env a=b -- true
+test $? = 127 || fail=1
+ln -s "$abs_top_builddir/src/echo" ./-- || framework_failure
+case `env a=b -- true || echo fail` in
+  true) ;;
+  *) fail=1 ;;
+esac
+
+# No way to directly invoke program name containing =.
+cat <<EOF >./c=d || framework_failure
+#!/bin/sh
+echo pass
+EOF
+chmod +x c=d || framework_failure
+test "x`env c=d echo fail`" = xfail || fail=1
+test "x`env -- c=d echo fail`" = xfail || fail=1
+test "x`env ./c=d echo fail`" = xfail || fail=1
+test "x$(env sh -c '\c=d echo fail')" = xpass || fail=1
+test "x$(env sh -c 'exec "$@"' sh c=d echo fail)" = xpass || fail=1

 # catch unsetenv failure, broken through coreutils 8.0
 env -u a=b true && fail=1
-- 
1.6.4.2