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

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 15, 2011, 17:20 UTC
Message-ID: <7v4o74b8r5.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20110315101045.GD3923@elie>

Jonathan Nieder <jrnieder@gmail.com> writes:

Show 24 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 all environment variables in the GIT_ namespace in
> test-lib, with a few exceptions:
>
> - GIT_TRACE* are useful for tracking down bugs exhibited by a failing
>   test;
>
> - GIT_DEBUG* are GIT_TRACE variables by another name, practically
>   speaking.  They should probably be tweaked to follow the
>   GIT_TRACE_foo scheme and use trace_printf machinery some time.
>
> - GIT_USE_LOOKUP from v1.5.6-rc0~134^2~1 (sha1-lookup: more memory
>   efficient search in sorted list of SHA-1, 2007-12-29) is about
>   trying an alternate implementation strategy rather than changing
>   semantics and it can be useful to compare performance with and
>   without it set.
>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
> Thanks again.

Sounds sane.

I am perfectly fine with the use of perl but do we know the "perl" binary found on $PATH (as opposed to $PERL_PATH) is good to use for this purpose? The features used in the scriptlet seem so bread-and-butter that I don't think it would make too much of a difference, but we may want to be consistent.

I personally think USE_LOOKUP outlived its usefulness. It was meant to be an easy way to experiment if the sha1_entry_pos() lookup gives better performance while looking up a pack entry, to choose one implementation and discard the other, but I don't think anybody actually did meaningful benchmarks to decide which one to keep.

Perhaps we should discard the codepath USE_LOOKUP turns on, which I suspect hasn't been exercised since v1.5.6 days by anybody. An obvious alternative is for somebody to try using USE_LOOKUP and see if it really gives a better performance with large packs (and if so, always use it and discard the other codepath).

Obviously that is outside the scope of this patch series, but I thought I should mention it before I forget.