Re: [PATCH 3/2] git.c: make sure we do not leak GIT_* to alias scripts

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 7, 2015, 18:54 UTC
Message-ID: <xmqqzixmm6na.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <1449329538-18623-1-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

>  Let's hope there will be no third report about this commit..

Hmm, why does this additional test fail only under prove but pass without it?

Show 27 quoted lines

> diff --git a/t/t0001-init.sh b/t/t0001-init.sh
> index f91bbcf..19539fc 100755
> --- a/t/t0001-init.sh
> +++ b/t/t0001-init.sh
> @@ -87,6 +87,33 @@ test_expect_success 'plain nested in bare through aliased command' '
>  	check_config bare-ancestor-aliased.git/plain-nested/.git false unset
>  '
>  
> +test_expect_success 'No extra GIT_* on alias scripts' '
> +	cat <<-\EOF >expected &&
> +	GIT_ATTR_NOSYSTEM
> +	GIT_AUTHOR_EMAIL
> +	GIT_AUTHOR_NAME
> +	GIT_COMMITTER_EMAIL
> +	GIT_COMMITTER_NAME
> +	GIT_CONFIG_NOSYSTEM
> +	GIT_EXEC_PATH
> +	GIT_MERGE_AUTOEDIT
> +	GIT_MERGE_VERBOSITY
> +	GIT_PREFIX
> +	GIT_TEMPLATE_DIR
> +	GIT_TEXTDOMAINDIR
> +	GIT_TRACE_BARE
> +	EOF
> +	cat <<-\EOF >script &&
> +	#!/bin/sh
> +	env | grep GIT_ | sed "s/=.*//" | sort >actual

This is more about coding discipline than style, but piping grep output to sed is wasteful. "sed -ne '/^GIT_/s/=.*//p'" or something like that, perhaps?

I wondered what happens if the user has an unrelated stray variable whose name happens to begin with GIT_ in her environment, but it turns out that we cleanse them in test-lib.sh fairly early, so that would be fine. You need to tighten your "grep" pattern, though.

Show 11 quoted lines

> +	exit 0
> +	EOF
> +	chmod 755 script &&
> +	git config alias.script \!./script &&
> +	( mkdir sub && cd sub && git script ) &&
> +	test_cmp expected actual
> +'
> +
>  test_expect_success 'plain with GIT_WORK_TREE' '
>  	mkdir plain-wt &&
>  	test_must_fail env GIT_WORK_TREE="$(pwd)/plain-wt" git init plain-wt