Re: [PATCH 3/3] tests: scrub environment of GIT_* variables

From: Jeff King <peff@peff.net>
Date: Mar 15, 2011, 07:37 UTC
Message-ID: <20110315073718.GB11754@sigill.intra.peff.net>
In-Reply-To: <20110315070445.GC29530@elie>

On Tue, Mar 15, 2011 at 02:04:45AM -0500, Jonathan Nieder wrote:

Show 8 quoted lines

> Variables from the inherited environment that are meaningful to git
> can break tests in undesirable ways.  For example,
> 
>  GIT_PAGER=more sh t5400-send-pack.sh -v -i
> 
> hangs.  So unset them in test-lib.
> 
> The variables to unset were found with 'git grep -F -e '"GIT_'.

Thanks. I fixed a few of these in a1231de recently, but was too lazy to do an exhaustive search. Your list and your method look sane to me.

Show 5 quoted lines

> Exception: leave the GIT_USE_LOOKUP variable from v1.5.6-rc0~134^2~1
> (sha1-lookup: more memory efficient search in sorted list of SHA-1,
> 2007-12-29) alone, since it is about trying an alternate
> implementation strategy rather than changing semantics and it can be
> useful to compare performance with and without it set.

Makes sense to me.

Show 5 quoted lines

> Longer term, it would be nice to just unset all GIT_* variables (with
> exceptions like GIT_TRACE and GIT_USE_LOOKUP) with some magic using
> the "env" command.  That is less straightforward than one might hope
> because the values of environment variables can contain embedded
> newlines and equal signs and "env -0" is not portable.

According to posix, just running "set" provides an unambiguous, parseable output. The problem is that it's actual shell, so it's a little tricky to parse (it's single-quoted, and you have to follow values across embedded newlines). So it's probably not worth the headache.

Having a big list of "clear these variables" is probably not the end of the world. If it breaks somebody's test run, they'll probably report it and we'll fix it. We don't add variables like this all that often.

-Peff