Re: [PATCH] t4014: shell portability fix
On Wed, Jun 01, 2016 at 09:57:06AM -0700, Junio C Hamano wrote: > Jeff Kingwrites: > > > Maybe: > > > > We sometimes get around this by using env, like: > > > > test_must_fail env FOO=BAR some-program > > > > But that works for test_must_fail because it further runs its > > arguments via the shell, so we can stick the "env" on the right-hand > > side of the function. It would not work to do: > > > > env FOO=BAR test_must_fail some-program > > > > because env does not know about our shell functions... > > > > is more clear? > > I don't know. What I wanted to say was that "test_must_fail env" > pattern works _only_ when some-program is not a shell function, even > though "test_must_fail some-program" itself without env is OK when > some-program is a shell function. Right, but I think that is taking it to a meta-level. We are already talking about one shell function, test_must_fail versus test_commit. Introducing another one in the test_must_fail to the right of "env" obviously does not work, but that is independent of whether test_must_fail is in use. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t4014: shell portability fix
Jeff Kingwrites: > Maybe: > > We sometimes get around this by using env, like: > > test_must_fail env FOO=BAR some-program > > But that works for test_must_fail because it further runs its > arguments via the shell, so we can stick the "env" on the right-hand > side of the function. It would not work to do: > > env FOO=BAR test_must_fail some-program > > because env does not know about our shell functions... > > is more clear? I don't know. What I wanted to say was that "test_must_fail env" pattern works _only_ when some-program is not a shell function, even though "test_must_fail some-program" itself without env is OK when some-program is a shell function. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t4014: shell portability fix
On Wed, Jun 01, 2016 at 08:07:16AM -0700, Junio C Hamano wrote: > > Here's the patch I wrote up earlier (but was too timid to send out after > > my barrage of emails :) ). > > Looks very sensible. I'll drop all these "Attempt to test with > ksh93 found these breakages" patches and queue this one. Curious based on our previous discussion, I applied your patches and did a "make SHELL_PATH=/bin/ksh93 test". There were only a handful of failures remaining. Some were definitely the "../.git" thing (which Andreas earlier reported as fixed, though the fix still has not made it into the version in Debian), but some were just puzzling. I shrugged and gave up. > > 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: > > We can do "test_must_fail test_commit ...", but "test_must_fail env > FOO=BAR test_commit ..." would not work, right? > > If so s/because/when/, perhaps? Right. What I was trying to say is that it works in this case because test_must_fail further executes its arguments, which gives us an opportunity to put the "env" on the right-hand side of the function call. I'm not sure s/because/when/ makes that any better, though. Maybe: We sometimes get around this by using env, like: test_must_fail env FOO=BAR some-program But that works for test_must_fail because it further runs its arguments via the shell, so we can stick the "env" on the right-hand side of the function. It would not work to do: env FOO=BAR test_must_fail some-program because env does not know about our shell functions... is more clear? -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t4014: shell portability fix
Jeff Kingwrites: >> >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. Ahh, right. > Here's the patch I wrote up earlier (but was too timid to send out after > my barrage of emails :) ). Looks very sensible. I'll drop all these "Attempt to test with ksh93 found these breakages" patches and queue this one. > -- >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: We can do "test_must_fail test_commit ...", but "test_must_fail env FOO=BAR test_commit ..." would not work, right? If so s/because/when/, perhaps? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t4014: shell portability fix
On Tue, May 31, 2016 at 11:49:10PM -0700, Junio C Hamano wrote: > Jeff Kingwrites: > > > 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 --- 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t4014: shell portability fix
Junio C Hamanowrites: >> *) >> "$@" >> exit > > ... or 'exec "$@"' Not really. I am an idiot. The whole point of this exercise is that we would want to have a shell function as $1 at this point in the flow; 'exec' would not be appropriate here. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t4014: shell portability fix
Jeff Kingwrites: > 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? > eval "export ${1%%=*}" > shift > ;; > *) > "$@" > exit ... or 'exec "$@"' > ;; > esac > done > ) > } You can dedent the whole thing and remove the outermost {} pair. Other than that, looks good to me. > > -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t4014: shell portability fix
On Tue, May 31, 2016 at 10:31 PM, Jeff Kingwrote: > On Tue, May 31, 2016 at 08:09:43PM -0400, Eric Sunshine wrote: >> I was under the impression that the project was moving toward 'env' to >> deal[1] with this sort of issue. >> >> [1]: 512477b (tests: use "env" to run commands with temporary env-var >> settings, 2014-03-18) > > We can use it with test_must_fail, because it takes a command name: > > test_must_fail env FOO=BAR whatever-you-would-have-run > > But I don't think it works in the general case, as test_commit does not > run its arguments. [...] You're right, of course. I should have seen this. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t4014: shell portability fix
On Wed, Jun 01, 2016 at 07:48:12AM +0200, Johannes Sixt wrote: > Am 01.06.2016 um 05:31 schrieb Jeff King: > > printf '%s' "$1" | sed "s/'/'''/g" > ... > > export -p | grep "^declare -x $1=" > ... > > "$fake_env_var_=$(shellquote > > "$fake_env_orig_")" > ... > > eval "$fake_env_var_=$(shellquote "$fake_env_val_")" > > An intolerable number of new processes... Please stop this mental exercise. Please read to the end of the thread? -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t4014: shell portability fix
Am 01.06.2016 um 05:31 schrieb Jeff King: printf '%s' "$1" | sed "s/'/'''/g" ... export -p | grep "^declare -x $1=" ... "$fake_env_var_=$(shellquote "$fake_env_orig_")" ... eval "$fake_env_var_=$(shellquote "$fake_env_val_")" An intolerable number of new processes... Please stop this mental exercise. -- Hannes -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t4014: shell portability fix
On Wed, Jun 01, 2016 at 01:33:25AM -0400, Jeff King wrote: > Here is the "final" version of the more complicated scheme I came up > with. That I think should be fairly portable, but the subshell thing is > probably way less gross. 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#*=}" eval "export ${1%%=*}" shift ;; *) "$@" exit ;; esac done ) } -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t4014: shell portability fix
On Tue, May 31, 2016 at 11:44:13PM -0400, Jeff King wrote: > So this is gross, but I think it actually _is_ portable, with the > exception of the "is it exported" check. Hmm. So after thinking on this, I realized we don't have to do the clean-up ourselves at all, if we simply operate in a subshell. That means that any shell functions we call couldn't mutate the state, but that's probably an acceptable compromise, if the goal is to behave like env except for being able to call shell functions. Here is the "final" version of the more complicated scheme I came up with. That I think should be fairly portable, but the subshell thing is probably way less gross. -- >8 -- test_var_is_set () { eval "test -n \"\${$1+set}\"" } test_var_is_exported () { sh -c "test -n \"\${$1+set}\"" } # set var named by $1 to contents of $2 test_set_var () { eval "$1=\$2" } # set var named by $1 to contents of var named by $2 test_copy_var () { eval "test_set_var \$1 \"\$$2\"" } # just a syntactic convenience add_to () { eval "$1=\"\$$1 \$2\"" } test_env () { test_env_restore_= while test $# -gt 0 do case "$1" in *=*) # this whole thing is not safe when the var name has # spaces or other meta-characters, but since the names # all come from our test scripts, that should be OK test_env_var_=${1%%=*} test_env_orig_=test_env_orig_$test_env_var_ test_copy_var $test_env_orig_ $test_env_var_ test_env_val_=${1#*=} shift # unset value and clear export flag... add_to test_env_restore_ "unset $test_env_var_" # ...and then restore what was there, if anything if test_var_is_set "$test_env_var_" then add_to test_env_restore_ \ "test_copy_var $test_env_var_ $test_env_orig_" if test_var_is_exported "$test_env_var_" then add_to test_env_restore_ \ "export $test_env_var_" fi fi # and then clean up our temp variable add_to test_env_restore_ "unset $test_env_orig_" test_set_var $test_env_var_ "$test_env_val_" eval "export $test_env_var_" ;; *) "$@" test_env_ret_=$? eval "$test_env_restore_" return $test_env_ret_ ;; esac done } # simple exercise show () { echo >&2 "$1: here=$myvar ${myvar+(set)} sub=$(sh -c 'echo "$myvar ${myvar+(set)}"')" } doit () { echo "==> $1" show before test_env myvar="temporary $myvar" show during show after } unset myvar doit unset myvar="horrible \"\\' mess" doit with-value export myvar doit exported-with-value myvar= export myvar doit exported-but-blank -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t4014: shell portability fix
On Tue, May 31, 2016 at 11:44:13PM -0400, Jeff King wrote: > # probably not portable; also, possible without sub-program? > is_exported () { > export -p | grep "^declare -x $1=" > } Obviously this should have been "grep -q" (and my test didn't notice because the variable isn't actually exported!). But yeah, this is not very portable. Doing: export =content for i in bash dash mksh ksh93 do printf '%5s ==> ' $i $i -c 'export -p' | head -1 done yields: bash ==> declare -x ="content" dash ==> export ='content' mksh ==> export =content ksh93 ==> export =content which is actually not too hard to pattern-match, but who knows what else exists. I guess another strategy would be to actually spawn a sub-process and see if it has the variable set. I think my code also doesn't handle exported-but-unset variables, though I'm not sure we really need to care about that in practice. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t4014: shell portability fix
On Tue, May 31, 2016 at 11:31:40PM -0400, Jeff King wrote: > I wondered if we could implement our own "env" in the shell, but it's a > little non-trivial, because: > [...] > Here's what I came up with, which does seem to work. It's pretty gnarly, > though. Here's a revised version that drops the shellquoting by using extra layers of "eval" indirection. You may have heard of 3-star C programmers. I think I just became a 2-dollar shell programmer. So this is gross, but I think it actually _is_ portable, with the exception of the "is it exported" check. -- >8 -- # is there a simpler way, even when the contents are "unset"? is_unset () { eval "test -z \"\${$1}\" && test unset = \"\${$1-unset}\"" } # probably not portable; also, possible without sub-program? is_exported () { export -p | grep "^declare -x $1=" } # set var named by $1 to contents of $2 set_var () { eval "$1=\$2" } # set var named by $1 to contents of var named by $2 copy_var () { eval "set_var \$1 \"\$$2\"" } # just a syntactic convenience add_to () { eval "$1=\"\$$1 \$2\"" } fake_env () { fake_env_restore_= while test $# -gt 0 do case "$1" in *=*) # this whole thing is not safe when the var name has # spaces or other meta-characters, but since the names # all come from our test scripts, that should be OK fake_env_var_=${1%%=*} fake_env_orig_=fake_env_orig_$fake_env_var_ copy_var $fake_env_orig_ $fake_env_var_ fake_env_val_=${1#*=} shift # unset value and clear export flag... add_to fake_env_restore_ "unset $fake_env_var_" # ...and then restore what was there, if anything if ! is_unset "$fake_env_var_" then add_to fake_env_restore_ \ "copy_var $fake_env_var_ $fake_env_orig_" if is_exported "$fake_env_var_" then add_to fake_env_restore_ \ "export $fake_env_var_" fi fi # and then clean up our temp variable add_to fake_env_restore_ "unset $fake_env_orig_" set_var $fake_env_var_ "$fake_env_val_" eval "export $fake_env_var_" ;; *) "$@" fake_env_ret_=$? eval "$fake_env_restore_" return $fake_env_ret_ ;; esac done } # simple exercise foo() { echo >&2 "$1: bar=$bar" } bar="horrible \"\\' mess" foo before fake_env bar="temporary $bar" foo during foo after -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t4014: shell portability fix
On Tue, May 31, 2016 at 10:31:59PM -0400, Jeff King wrote: > On Tue, May 31, 2016 at 08:09:43PM -0400, Eric Sunshine wrote: > > > I was under the impression that the project was moving toward 'env' to > > deal[1] with this sort of issue. > > > > [1]: 512477b (tests: use "env" to run commands with temporary env-var > > settings, 2014-03-18) > > We can use it with test_must_fail, because it takes a command name: > > test_must_fail env FOO=BAR whatever-you-would-have-run > > But I don't think it works in the general case, as test_commit does not > run its arguments. So you'd want something like: > > env FOO=BAR test_commit exotic > > but of course that doesn't work because "env" can't find the shell > function. I think with bash you could do: > > export test_commit > env FOO=BAR bash -c test_commit exotic > > but we can't rely on that. I wondered if we could implement our own "env" in the shell, but it's a little non-trivial, because: - our basic tool for setting variable-named variables is "eval", which means we need an extra layer of quoting - we have to restore the variables after. That means telling the difference between unset and empty (possible with "-" versus ":-", I think), but also the difference between unexported and exported (maybe possible by parsing "export -p", but I'd be shocked if that doesn't run into portability problems). Here's what I came up with, which does seem to work. It's pretty gnarly, though. -- >8 -- # possible without a sub-program? # portability issues with no-newline and sed? shellquote () { printf "'" printf '%s' "$1" | sed "s/'/'''/g" printf "'" } # is there a simpler way, even when the contents are "unset"? is_unset () { eval "test -z \"\${$1}\" && test unset = \"\${$1-unset}\"" } # probably not portable; also, possible without sub-program? is_exported () { export -p | grep "^declare -x $1=" } # just a syntactic convenience add_to () { eval "$1=\"\$$1 \$2\"" } fake_env () { fake_env_restore_= while test $# -gt 0 do case "$1" in *=*) # this whole thing is not safe when the var name has # spaces or other meta-characters, but since the names # all come from our test scripts, that should be OK fake_env_var_=${1%%=*} eval "fake_env_orig_=\$$fake_env_var_" fake_env_val_=${1#*=} shift # unset value and clear export flag... add_to fake_env_restore_ "unset $fake_env_var_" # ...and then restore what was there, if anything if ! is_unset "$fake_env_var_" then add_to fake_env_restore_ \ "$fake_env_var_=$(shellquote "$fake_env_orig_")" if is_exported "$fake_env_var_" then add_to fake_env_restore_ \ "export $fake_env_var_" fi fi eval "$fake_env_var_=$(shellquote "$fake_env_val_")" eval "export $fake_env_var_" ;; *) "$@" fake_env_ret_=$? eval "$fake_env_restore_" return $fake_env_ret_ ;; esac done } # simple exercise show() { echo >&2 "$1: myvar=$myvar" } myvar="horrible \"\\' mess" show before fake_env myvar="temporary $myvar" show during show after -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t4014: shell portability fix
On Tue, May 31, 2016 at 08:09:43PM -0400, Eric Sunshine wrote: > >> - GIT_AUTHOR_NAME="éxötìc" test_commit exotic && > >> + (export GIT_AUTHOR_NAME="éxötìc"; test_commit exotic) && > > Isn't "export FOO=val" unportable? Good catch. I was so busy looking for other cases I didn't even see the problem here. > > Thanks. This one is my fault. There's another use of the same name > > elsewhere, but it's to call "git commit" directly, so it's OK. > > I was under the impression that the project was moving toward 'env' to > deal[1] with this sort of issue. > > [1]: 512477b (tests: use "env" to run commands with temporary env-var > settings, 2014-03-18) We can use it with test_must_fail, because it takes a command name: test_must_fail env FOO=BAR whatever-you-would-have-run But I don't think it works in the general case, as test_commit does not run its arguments. So you'd want something like: env FOO=BAR test_commit exotic but of course that doesn't work because "env" can't find the shell function. I think with bash you could do: export test_commit env FOO=BAR bash -c test_commit exotic but we can't rely on that. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t4014: shell portability fix
On Tue, May 31, 2016 at 6:56 PM, Jeff Kingwrote: > On Tue, May 31, 2016 at 03:53:15PM -0700, Junio C Hamano wrote: >> One-shot assignment to an environment variable, i.e. >> >> VAR=VAL cmd >> >> does not work as expected for "cmd" that is a shell function on >> certain shells. test_commit _is_ a helper function and cannot be >> driven that way portably. >> >> Signed-off-by: Junio C Hamano >> --- >> diff --git 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 && >> + (export GIT_AUTHOR_NAME="éxötìc"; test_commit exotic) && Isn't "export FOO=val" unportable? > Thanks. This one is my fault. There's another use of the same name > elsewhere, but it's to call "git commit" directly, so it's OK. I was under the impression that the project was moving toward 'env' to deal[1] with this sort of issue. [1]: 512477b (tests: use "env" to run commands with temporary env-var settings, 2014-03-18) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t4014: shell portability fix
On 31/05/16 23:53, Junio C Hamano wrote: > One-shot assignment to an environment variable, i.e. > > VAR=VAL cmd > > does not work as expected for "cmd" that is a shell function on > certain shells. test_commit _is_ a helper function and cannot be > driven that way portably. > > Signed-off-by: Junio C Hamano> --- > t/t4014-format-patch.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh > index 8049cad..c3aa543 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 && > + (export GIT_AUTHOR_NAME="éxötìc"; test_commit exotic) && Isn't 'export VAR=VAL' non-portable? So, maybe: (GIT_AUTHOR_NAME="éxötìc"; export GIT_AUTHOR_NAME; test_commit exotic) && > test_when_finished "git reset --hard HEAD^" && > git format-patch -1 --stdout --from >patch && > cat >expect <<-\EOF && ATB, Ramsay Jones -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t4014: shell portability fix
On Tue, May 31, 2016 at 03:53:15PM -0700, Junio C Hamano wrote: > One-shot assignment to an environment variable, i.e. > > VAR=VAL cmd > > does not work as expected for "cmd" that is a shell function on > certain shells. test_commit _is_ a helper function and cannot be > driven that way portably. > > Signed-off-by: Junio C Hamano> --- > t/t4014-format-patch.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh > index 8049cad..c3aa543 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 && > + (export GIT_AUTHOR_NAME="éxötìc"; test_commit exotic) && Thanks. This one is my fault. There's another use of the same name elsewhere, but it's to call "git commit" directly, so it's OK. -Peff PS I really hate this particular shell behavior, as it means that your code can break because somebody _else_ decides to replace the command you are calling with a function. But I guess we are stuck with it. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] t4014: shell portability fix
One-shot assignment to an environment variable, i.e. VAR=VAL cmd does not work as expected for "cmd" that is a shell function on certain shells. test_commit _is_ a helper function and cannot be driven that way portably. Signed-off-by: Junio C Hamano--- t/t4014-format-patch.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh index 8049cad..c3aa543 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 && + (export 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 && -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html