Re: [PATCH v2 4/4] lib-submodule-update: pass OVERWRITING_FAIL

From: Jeff King <peff@peff.net>
To: Denton Liu <liu.denton@gmail.com>
Cc: Git Mailing List <git@vger.kernel.org>,
	Junio C Hamano <gitster@pobox.com>, Taylor Blau <me@ttaylorr.com>,
	Johannes Sixt <j6t@kdbg.org>,
	Eric Sunshine <sunshine@sunshineco.com>
Subject: Re: [PATCH v2 4/4] lib-submodule-update: pass OVERWRITING_FAIL
Date: Thu, 21 May 2020 14:34:59 -0400	[thread overview]
Message-ID: <20200521183459.GB1308647@coredump.intra.peff.net> (raw)
In-Reply-To: <48598e3f9859dc525ec878cd7f3eaadee8bb61b1.1590019226.git.liu.denton@gmail.com>

On Wed, May 20, 2020 at 08:24:18PM -0400, Denton Liu wrote:

>  test_submodule_switch () {
> -	test_submodule_switch_func "git $1"
> +	test_submodule_switch_func "eval \$OVERWRITING_FAIL git $1"
>  }
> [...]
>  test_submodule_forced_switch () {
>  	command="$1"
>  	KNOWN_FAILURE_FORCED_SWITCH_TESTS=1
> -	test_submodule_switch_common "git $command"
> +	test_submodule_switch_common "eval \$OVERWRITING_FAIL git $command"

These both subject $1 to an extra layer of shell eval. That may not
matter since we seem to only pass basic stuff. It could probably be
avoided with an extra variable (though since we're passing along the
eval to be eval'd, I think it gets tricky), so it may not be worth
addressing.

Though IMHO it is nicer still if we can avoid the eval altogether by
doing something like the prep/command split I suggested in a sibling
reply.

-Peff