From: Junio C Hamano <junio@pobox.com>
Date: Wed, 14 May 2008 04:18:53 GMT
Subject: Re: [PATCH 4/4] filter-branch: fix variable export logic
Message-ID: <7v7idxr00i.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <20080513084638.GD23799@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> 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
+}