[v2,1/5] t3430: drop unnecessary one-shot "VAR=val shell-func" invocation

Commit Message

Eric Sunshine July 26, 2024, 8:15 a.m. UTC

From: Eric Sunshine <[e-mail]>

The behavior of a one-shot environment variable assignment of the form
"VAR=val cmd" is undefined according to POSIX when "cmd" is a shell
function. Indeed the behavior differs between shell implementations and
even different versions of the same shell. One such ill-defined behavior
is that, with some shells, the assignment will outlive the invocation of
the function, thus may potentially impact subsequent commands in the
test, as well as subsequent tests. A common way to work around the
problem is to wrap a subshell around the one-shot assignment, thus
ensuring that the assignment is short-lived.

In this test, the subshell is employed precisely for this purpose; other
side-effects of the subshell, such as losing the effect of `test_tick`
which is invoked by `test_commit`, are immaterial.

These days, we can take advantage of `test_commit --author` to more
clearly convey that the test is interested only in overriding the author
of the commit.

Signed-off-by: Eric Sunshine <[e-mail]>
---
 t/t3430-rebase-merges.sh | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Junio C Hamano July 26, 2024, 6:50 p.m. UTC | #1

Eric Sunshine <[e-mail]> writes:

> From: Eric Sunshine <[e-mail]>
>
> The behavior of a one-shot environment variable assignment of the form
> "VAR=val cmd" is undefined according to POSIX when "cmd" is a shell

Please use the right word to describe what the standard says.

Throughout the topic's discussion, you seem to be repeating
"undefined", but the word POSIX uses for this particular unportable
behaviour is "unspecified".  The differences are subtle, and for
programs that want to be conformant, there is no practical
difference (in other words, we should not rely on the existence or
validity of the value or behaviour if we wanted to be portable).

The former is what results from use of an invalid construct or
feeding an invalid data input.  The implementation can do whatever
it wants to do once you trigger an undefined behaviour.  The latter
is what results from use of a valid construct or valid data input,
but outcome may differ across implementations.  An "unspecified"
behaviour often are still consistent and sensible within a single
conformant implementation.

Eric Sunshine July 26, 2024, 7:32 p.m. UTC | #2

On Fri, Jul 26, 2024 at 2:50 PM Junio C Hamano <[e-mail]> wrote:
> Eric Sunshine <[e-mail]> writes:
> > The behavior of a one-shot environment variable assignment of the form
> > "VAR=val cmd" is undefined according to POSIX when "cmd" is a shell
>
> Please use the right word to describe what the standard says.
>
> Throughout the topic's discussion, you seem to be repeating
> "undefined", but the word POSIX uses for this particular unportable
> behaviour is "unspecified".  The differences are subtle, and for
> programs that want to be conformant, there is no practical
> difference (in other words, we should not rely on the existence or
> validity of the value or behaviour if we wanted to be portable).
>
> The former is what results from use of an invalid construct or
> feeding an invalid data input.  The implementation can do whatever
> it wants to do once you trigger an undefined behaviour.  The latter
> is what results from use of a valid construct or valid data input,
> but outcome may differ across implementations.  An "unspecified"
> behaviour often are still consistent and sensible within a single
> conformant implementation.

Makes sense. Will adjust the commit messages.

Patch

diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh
index 36ca126bcd..2aa8593f77 100755
--- a/t/t3430-rebase-merges.sh
+++ b/t/t3430-rebase-merges.sh
@@ -392,8 +392,7 @@  test_expect_success 'refuse to merge ancestors of HEAD' '
 
 test_expect_success 'root commits' '
 	git checkout --orphan unrelated &&
-	(GIT_AUTHOR_NAME="Parsnip" GIT_AUTHOR_EMAIL="[e-mail]" \
-	 test_commit second-root) &&
+	test_commit --author "Parsnip <[e-mail]>" second-root &&
 	test_commit third-root &&
 	cat >script-from-scratch <<-\EOF &&
 	pick third-root