Home » Mailing list archives » git » 2011 » January » 4

Re: [PATCH v2] Fix false positives in t3404 due to SHELL=/bin/false

Previous thread: [PATCH] Add support for -p/--patch to git-commit by Conrad Irwin on Sunday, December 26, 2010 - 8:36 am. (3 messages)

Next thread: by COCA COLA on Sunday, December 26, 2010 - 11:07 pm. (1 message)
[view original message]
From: Robin H. Johnson
Subject: [PATCH] Fix false positives in t3404 due to SHELL=/bin/false
Date: Sunday, December 26, 2010 - 7:50 pm

If the user's shell in NSS passwd is /bin/false (eg as found during Gentoo's
package building), the git-rebase exec tests will fail, because they call
$SHELL around the command, and in the existing testcase, $SHELL was not being
cleared sufficently.

This lead to false positive failures of t3404 on systems where the package
build user was locked down as noted above.

Signed-off-by: "Robin H. Johnson" <robbat2@gentoo.org>
X-Gentoo-Bug: 349083
X-Gentoo-Bug-URL: http://bugs.gentoo.org/show_bug.cgi?id=349083

diff -Nuar git-1.7.3.4.orig/t/t3404-rebase-interactive.sh git-1.7.3.4/t/t3404-rebase-interactive.sh
--- git-1.7.3.4.orig/t/t3404-rebase-interactive.sh	2010-12-16 02:52:11.000000000 +0000
+++ git-1.7.3.4/t/t3404-rebase-interactive.sh	2010-12-26 22:30:47.826421313 +0000
@@ -67,8 +67,8 @@
 # "exec" commands are ran with the user shell by default, but this may
 # be non-POSIX. For example, if SHELL=zsh then ">file" doesn't work
 # to create a file. Unseting SHELL avoids such non-portable behavior
-# in tests.
-SHELL=
+# in tests. It must be exported for it to take effect where needed.
+export SHELL=
 
 test_expect_success 'rebase -i with the exec command' '
 	git checkout master &&

-- 
Robin Hugh Johnson
Gentoo Linux: Developer, Trustee & Infrastructure Lead
E-Mail     : robbat2@gentoo.org
GnuPG FP   : 11AC BA4F 4778 E3F6 E4ED  F38E B27B 944E 3488 4E85

[ message continues ]
[view original message]
From: Junio C Hamano
Subject: Re: [PATCH] Fix false positives in t3404 due to SHELL=/bin/false
Date: Sunday, December 26, 2010 - 11:10 pm

Thanks.

This probably is still not portable.

	SHELL=
        export SHELL

would be Ok, though.
--


[ message continues ]
[view original message]
From: Robin H. Johnson
Subject: [PATCH v2] Fix false positives in t3404 due to SHELL=/bin/false
Date: Monday, December 27, 2010 - 1:03 am

If the user's shell in NSS passwd is /bin/false (eg as found during Gentoo's
package building), the git-rebase exec tests will fail, because they call
$SHELL around the command, and in the existing testcase, $SHELL was not being
cleared sufficently.

This lead to false positive failures of t3404 on systems where the package
build user was locked down as noted above.

Signed-off-by: "Robin H. Johnson" <robbat2@gentoo.org>
X-Gentoo-Bug: 349083
X-Gentoo-Bug-URL: http://bugs.gentoo.org/show_bug.cgi?id=349083
---
 t/t3404-rebase-interactive.sh |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index d3a3bd2..7d8147b 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -71,8 +71,9 @@ test_expect_success 'setup' '
 # "exec" commands are ran with the user shell by default, but this may
 # be non-POSIX. For example, if SHELL=zsh then ">file" doesn't work
 # to create a file. Unseting SHELL avoids such non-portable behavior
-# in tests.
+# in tests. It must be exported for it to take effect where needed.
 SHELL=
+export SHELL
 
 test_expect_success 'rebase -i with the exec command' '
 	git checkout master &&

-- 
Robin Hugh Johnson
Gentoo Linux: Developer, Trustee & Infrastructure Lead
E-Mail     : robbat2@gentoo.org
GnuPG FP   : 11AC BA4F 4778 E3F6 E4ED  F38E B27B 944E 3488 4E85

[ message continues ]
[view original message]
From: Junio C Hamano
Subject: Re: [PATCH v2] Fix false positives in t3404 due to SHELL=/bin/false
Date: Tuesday, December 28, 2010 - 12:58 pm

Thanks; will queue this version to 'maint'.

I have this nagging suspicion that we may want to revisit this to assign
$SHELL_PATH to it before exporting, and that this might be better done in
t/test-lib.sh at the beginning.  Note that unlike my earlier "your v1
might be less portable than desired", these two points are only
speculations and RFCs.
--


[ message continues ]
[view original message]
From: Vallon, Justin
Subject: RE: [PATCH v2] Fix false positives in t3404 due to SHELL=/bin/false
Date: Tuesday, January 4, 2011 - 7:43 am

# "exec" commands are ran with the user shell by default, but this may
 # be non-POSIX. For example, if SHELL=zsh then ">file" doesn't work
 # to create a file. Unseting SHELL avoids such non-portable behavior

Perl's exec and system do not use SHELL (as far as perlfunc states).  It uses /bin/sh -c "$cmd", or a platform-dependent equivalent.

$SHELL is typically only used when a program wants to invoke a user-shell (ie: editor shell-escape, xterm, typescript, screen).

How was SHELL=/bin/false causing problems?  Is git using $SHELL?

-- 
-Justin


-----Original Message-----
From: git-owner@vger.kernel.org [mailto:git-owner@vger.kernel.org] On Behalf Of Robin H. Johnson
Sent: Monday, December 27, 2010 3:04 AM
To: Junio C Hamano; git@vger.kernel.org
Subject: [PATCH v2] Fix false positives in t3404 due to SHELL=/bin/false

If the user's shell in NSS passwd is /bin/false (eg as found during Gentoo's
package building), the git-rebase exec tests will fail, because they call
$SHELL around the command, and in the existing testcase, $SHELL was not being
cleared sufficently.

This lead to false positive failures of t3404 on systems where the package
build user was locked down as noted above.

Signed-off-by: "Robin H. Johnson" <robbat2@gentoo.org>
X-Gentoo-Bug: 349083
X-Gentoo-Bug-URL: http://bugs.gentoo.org/show_bug.cgi?id=349083
---
 t/t3404-rebase-interactive.sh |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index d3a3bd2..7d8147b 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -71,8 +71,9 @@ test_expect_success 'setup' '
 # "exec" commands are ran with the user shell by default, but this may
 # be non-POSIX. For example, if SHELL=zsh then ">file" doesn't work
 # to create a file. Unseting SHELL avoids such non-portable behavior
-# in tests.
+# in tests. It must be exported for it to take effect where needed.
 SHELL=
+export SHELL
 
 ...
[ message continues ]
[view original message]
From: Robin H. Johnson
Subject: Re: [PATCH v2] Fix false positives in t3404 due to SHELL=/bin/false
Date: Tuesday, January 4, 2011 - 1:35 pm

git-rebase--interactive.sh:
====
${SHELL:-@SHELL_PATH@} -c "$rest" # Actual execution
status=$?
if test "$status" -ne 0
then
	warn "Execution failed: $rest"
====

This always triggers with SHELL=/bin/false if SHELL is unset or empty,
SHELL_PATH gets substituted, which tends to be the correct /bin/sh.

-- 
Robin Hugh Johnson
Gentoo Linux: Developer, Trustee & Infrastructure Lead
E-Mail     : robbat2@gentoo.org
GnuPG FP   : 11AC BA4F 4778 E3F6 E4ED  F38E B27B 944E 3488 4E85

[ message continues ]
[view original message]
From: Matthieu Moy
Subject: Re: [PATCH v2] Fix false positives in t3404 due to SHELL=/bin/false
Date: Tuesday, January 4, 2011 - 3:28 pm

The explanation is in the comment right above the modification in the

(my bad, I wrote this SHELL= without exporting it. Since bash
re-exports already exported variables when they are assigned, and my
/bin/sh points to bash, I didn't notice)

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--


[ message continues ]
[view original message]
From: Jonathan Nieder
Subject: Re: [PATCH v2] Fix false positives in t3404 due to SHELL=/bin/false
Date: Tuesday, January 4, 2011 - 3:58 pm

Isn't that how export works in all Bourne-style shells?  For example:

	$ env var=outside dash -c '
		var=inside;
		dash -c "echo \$var"
	  '
	inside
	$

Maybe in the failing case SHELL was not exported but just set to
/bin/false in .bashrc or similar?
--


[ message continues ]
Previous thread: [PATCH] Add support for -p/--patch to git-commit by Conrad Irwin on Sunday, December 26, 2010 - 8:36 am. (3 messages)

Next thread: by COCA COLA on Sunday, December 26, 2010 - 11:07 pm. (1 message)