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

From: Vallon, Justin <justin.vallon@deshaw.com>
Date: Jan 5, 2011, 15:04 UTC
Message-ID: <982E526FA742C94E9AC26DA766FD07090A33A5@NYCMBX3.winmail.deshaw.com>
In-Reply-To: <7vmxngdys8.fsf@alter.siamese.dyndns.org>

Show 40 quoted lines

>-----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 <jrnieder@gmail.com> writes:
>
>> Matthieu Moy wrote:
>>>
>>> (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).

Show 6 quoted lines

>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