Re: [PATCH 4/4] filter-branch: fix variable export logic

From: Junio C Hamano <junio@pobox.com>
Date: May 14, 2008, 04:18 UTC
Message-ID: <7v7idxr00i.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <20080513084638.GD23799@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 8 quoted lines

> filter-branch tries to restore "old" copies of some
> environment variables by using the construct:
>
>   unset var
>   test -z "$old_var" || var="$old_var" && export var
>
> However, by the short-circuit logic, we will always run
> 'export var'.

Thanks.

I was confused by this description ("short-circuit logic"), but I do not think there is no short-circuit going on. This is a simple ignorance of shell syntax.

In a shell scriptlet:

	a || b && c

AND list operator (&&) and OR list operator (||) have the same precedence and bind to the left. Because the second part of OR list is always true, we always export.

I have a mild suspecion that this was simply an artifcat of a careless conversion from export var="$val" form we did in the past. I should have caught them back then.

The patch is fine, but I find this easier to read:

+test -z "$ORIG_GIT_DIR" || { + GIT_DIR="$ORIG_GIT_DIR" && export GIT_DIR +} +test -z "$ORIG_GIT_WORK_TREE" || { + GIT_WORK_TREE="$ORIG_GIT_WORK_TREE" && + export GIT_WORK_TREE +} +test -z "$ORIG_GIT_INDEX_FILE" || { + GIT_INDEX_FILE="$ORIG_GIT_INDEX_FILE" && + export GIT_INDEX_FILE +}