From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Eric Sunshine <sunshine@sunshineco.com>, Git List <git@vger.kernel.org>
Subject: Re: [PATCH] t4014: shell portability fix
Date: Wed, 1 Jun 2016 03:04:26 -0400 [thread overview]
Message-ID: <20160601070425.GA13648@sigill.intra.peff.net> (raw)
In-Reply-To: <xmqqshwx1k0p.fsf@gitster.mtv.corp.google.com>
On Tue, May 31, 2016 at 11:49:10PM -0700, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > OK, last email tonight, I promise.
> >
> > Here's the subshell version. I'm a little embarrassed not to have
> > thought of it sooner (though the other one was a fun exercise).
> >
> > test_env () {
> > (
> > while test $# -gt 0
> > do
> > case "$1" in
> > *=*)
> > eval "${1%%=*}=\${1#*=}"
>
> Is this an elaborate way to say 'eval "$1"', or is there anything
> more subtle going on?
Notice that the value half isn't expanded until we get inside the eval.
So:
$ good() { eval "${1%%=*}=\${1#*=}"; }
$ bad() { eval "$1"; }
$ good foo="funny variable"; echo $foo
funny variable
$ bad foo="funny variable"
bash: variable: command not found
> > *)
> > "$@"
> > exit
>
> ... or 'exec "$@"'
You can't exec a function, AFAIK (and that was the point of this
exercise).
> > )
> > }
>
> You can dedent the whole thing and remove the outermost {} pair.
True. I didn't know about that until recently. Is it portable
everywhere?
Here's the patch I wrote up earlier (but was too timid to send out after
my barrage of emails :) ).
It doesn't have the dedent, but I don't mind if you want to tweak it.
-- >8 --
Subject: test-lib: add in-shell "env" replacement
The one-shot environment variable syntax:
FOO=BAR some-program
is unportable when some-program is actually a shell
function, like test_must_fail (on some shells FOO remains
set after the function returns, and on others it does not).
We sometimes get around this by using env, like:
test_must_fail env FOO=BAR some-program
But that only works because test_must_fail's arguments are
themselves a command which can be run. You can't run:
env FOO=BAR test_must_fail some-program
because env does not know about our shell functions. So
there is no equivalent for test_commit, for example, and one
must resort to:
(
FOO=BAR
export FOO
test_commit
)
which is a bit verbose. Let's add a version of "env" that
works _inside_ the shell, by creating a subshell, exporting
variables from its argument list, and running the command.
Its use is demonstrated on a currently-unportable case in
t4014.
Signed-off-by: Jeff King <peff@peff.net>
---
t/t4014-format-patch.sh | 2 +-
t/test-lib-functions.sh | 22 ++++++++++++++++++++++
2 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 8049cad..805dc90 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1072,7 +1072,7 @@ test_expect_success '--from omits redundant in-body header' '
'
test_expect_success 'in-body headers trigger content encoding' '
- GIT_AUTHOR_NAME="éxötìc" test_commit exotic &&
+ test_env GIT_AUTHOR_NAME="éxötìc" test_commit exotic &&
test_when_finished "git reset --hard HEAD^" &&
git format-patch -1 --stdout --from >patch &&
cat >expect <<-\EOF &&
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 3978fc0..48884d5 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -939,3 +939,25 @@ mingw_read_file_strip_cr_ () {
eval "$1=\$$1\$line"
done
}
+
+# Like "env FOO=BAR some-program", but run inside a subshell, which means
+# it also works for shell functions (though those functions cannot impact
+# the environment outside of the test_env invocation).
+test_env () {
+ (
+ while test $# -gt 0
+ do
+ case "$1" in
+ *=*)
+ eval "${1%%=*}=\${1#*=}"
+ eval "export ${1%%=*}"
+ shift
+ ;;
+ *)
+ "$@"
+ exit
+ ;;
+ esac
+ done
+ )
+}
--
2.9.0.rc0.174.g479a78d
next prev parent reply other threads:[~2016-06-01 7:04 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-05-31 22:53 [PATCH] t4014: shell portability fix Junio C Hamano
2016-05-31 22:56 ` Jeff King
2016-06-01 0:09 ` Eric Sunshine
2016-06-01 2:31 ` Jeff King
2016-06-01 3:31 ` Jeff King
2016-06-01 3:44 ` Jeff King
2016-06-01 3:54 ` Jeff King
2016-06-01 5:33 ` Jeff King
2016-06-01 5:40 ` Jeff King
2016-06-01 6:49 ` Junio C Hamano
2016-06-01 6:57 ` Junio C Hamano
2016-06-01 7:04 ` Jeff King [this message]
2016-06-01 15:07 ` Junio C Hamano
2016-06-01 16:07 ` Jeff King
2016-06-01 16:57 ` Junio C Hamano
2016-06-01 16:58 ` Jeff King
2016-06-01 5:48 ` Johannes Sixt
2016-06-01 5:48 ` Jeff King
2016-06-01 6:39 ` Eric Sunshine
2016-06-01 0:09 ` Ramsay Jones
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160601070425.GA13648@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=sunshine@sunshineco.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).