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

-----Original Message-----
From: Junio C Hamano [mailto:gitster@pobox.com]
Sent: Tuesday, January 04, 2011 6:39 PM
To: Jonathan Nieder
Cc: Matthieu Moy; Vallon, Justin; Robin H. Johnson; git@vger.kernel.org
Subject: Re: [PATCH v2] Fix false positives in t3404 due to
SHELL=/bin/false

Jonathan Nieder [off-list ref] writes:

quoted

Matthieu Moy wrote:

quoted

(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)

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?

Thanks, you saved me some time responding ;-)

Matthieu's diagnosis is only half correct in that bash is why he didn't
notice the problem, but if in this sequence

var=foo
       export var
       var=bar
       some-command

some-command does not see "bar" as the value of environment variable
"var", your shell is not POSIX (there is no such thing as "re-exporting").

But, when you say "your shell", you are really referring to /bin/sh, because this behavior is being observed in t/t3404-rebase-interactive.sh.  Which leads to...

Robin: have you observed the problem with Gentoo's /bin/sh?

X=1 ; export X ; /bin/sh -c 'X= ; env | grep ^X='

If so, I would qualify the export with a comment about the mis-behavior:

# Reexport in case sh is non-POSIX
export SHELL

(or, just unset SHELL)

Else, I don't think the re-export is needed (something else is causing your trouble).

Because POSIX shells are required to mark variables they inherit from the
environment with the export attribute, your tests will run with SHELL
exported to the environment if your usual shell is bash (i.e. SHELL is
already exported to processes it spawns), even if you use another POSIX
shell to run your git and tests.  That makes the issue doubly harder to
notice.

I don't really follow this.  The #! line is /bin/sh.  The user's $SHELL does not come into play.  Either SHELL is in /bin/sh's environment and it should be cleared in the child, or it isn't and it won't matter.

-- 
-Justin