Re: [PATCH] squash! tests: fix overeager scrubbing of environment variables

From: Jeff King <peff@peff.net>
Date: Mar 28, 2011, 13:58 UTC
Message-ID: <20110328135803.GA14763@sigill.intra.peff.net>
In-Reply-To: <20110327221753.GB27445@elie>

On Sun, Mar 27, 2011 at 05:17:53PM -0500, Jonathan Nieder wrote:

Show 26 quoted lines

> How about something like this on top?  Would it still be safe on
> platforms with ancient perl?
> 
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
>  t/test-lib.sh |   10 +++++++++-
>  1 files changed, 9 insertions(+), 1 deletions(-)
> 
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 4a8c443..d28e647 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -46,7 +46,15 @@ unset VISUAL
>  unset EMAIL
>  unset $(perl -e '
>  	my @env = keys %ENV;
> -	my @vars = grep(/^GIT_/ && !/^GIT_(TRACE|DEBUG|USE_LOOKUP|SKIP_TESTS|TEST|PROVE_OPTS)/, @env);
> +	my @vars = grep(/^GIT_/ && !m{^GIT_(
> +			TRACE |
> +			DEBUG |
> +			USE_LOOKUP |
> +			TEST |
> +			.*_TEST |
> +			PROVE |
> +			VALGRIND
> +		)}x, @env);

I think that should be fine. The most exotic thing you use is /x. I don't know when it was introduced, but perl5004delta refers to it as something that was already around before then.

I think it might be a little more readable to get rid of the "|" on each line like this:

    my $ok = join("|", qw(
      TRACE
      DEBUG
      USE_LOOKUP
      TEST
      .*_TEST
      PROVE
      VALGRIND
    ));
    my @vars = grep(/^GIT_/ && !/^GIT_($ok)/o) @env;

which is a little simpler syntax, and makes patches nicer (you don't have to add a trailing "|" on the line before). But that is a minor nit.

-Peff