threads / patch / 29745

patch, 2 partsgit-p4: remove test bash-isms

Subject: [PATCH 0/2] git-p4: remove test bash-isms

## tl;dr

5 messages between Feb 26, 2012 and Feb 26, 2012. Diffs are folded; open one to read it.

replies: 4people: 2as markdown or json

Pete Wyckoff· Feb 26, 2012, 15:37 UTC · lore

Two fixes to make the t98* tests run in dash.

Pete Wyckoff (2):
  git-p4: remove bash-ism in t9809
  git-p4: remove bash-ism in t9800

 t/t9800-git-p4-basic.sh       |   25 ++++++++++++++++---------
 t/t9809-git-p4-client-view.sh |    2 +-
 2 files changed, 17 insertions(+), 10 deletions(-)

-- 
1.7.9.2.288.g74b75

Pete Wyckoff· Feb 26, 2012, 15:37 UTC · re: Pete Wyckoff · lore

[PATCH 1/2] git-p4: remove bash-ism in t9809

Plain old $# works to count the number of arguments in either bash or dash, even if the arguments have spaces.

Based-on-patch-by: Vitor Antunes <vitor.hda@gmail.com>
Signed-off-by: Pete Wyckoff <pw@padd.com>
---
 t/t9809-git-p4-client-view.sh |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Show changes to t/t9809-git-p4-client-view.sh +1 −1

diff --git a/t/t9809-git-p4-client-view.sh b/t/t9809-git-p4-client-view.sh
index ae9145e..18d93e4 100755
--- a/t/t9809-git-p4-client-view.sh
+++ b/t/t9809-git-p4-client-view.sh
@@ -31,7 +31,7 @@ client_view() {
 #
 check_files_exist() {
 	ok=0 &&
-	num=${#@} &&
+	num=$# &&
 	for arg ; do
 		test_path_is_file "$arg" &&
 		ok=$(($ok + 1))

-- 
1.7.9.2.288.g74b75

Pete Wyckoff· Feb 26, 2012, 15:37 UTC · re: Pete Wyckoff · lore

[PATCH 2/2] git-p4: remove bash-ism in t9800

This works in both bash and dash:

    arf$ bash -c 'VAR=1 env' | grep VAR
    VAR=1
    arf$ dash -c 'VAR=1 env' | grep VAR
    VAR=1

But the variables are not propagated through a function in dash:

    arf$ bash -c 'f() { "$@"
    }; VAR=1 f "env"' | grep VAR
    VAR=1
    arf$ dash -c 'f() { "$@"
    }; VAR=1 f "env"' | grep VAR

Fix constructs like this, in particular, setting variables through test_must_fail.

Based-on-patch-by: Vitor Antunes <vitor.hda@gmail.com>
Signed-off-by: Pete Wyckoff <pw@padd.com>
---
 t/t9800-git-p4-basic.sh |   25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

Show changes to t/t9800-git-p4-basic.sh +16 −9

diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh
index 04ee20e..9f17cdb 100755
--- a/t/t9800-git-p4-basic.sh
+++ b/t/t9800-git-p4-basic.sh
@@ -234,8 +234,11 @@ test_expect_success 'refuse to preserve users without perms' '
 		git config git-p4.skipSubmitEditCheck true &&
 		echo "username-noperms: a change by alice" >>file1 &&
 		git commit --author "Alice <alice@localhost>" -m "perms: a change by alice" file1 &&
-		P4EDITOR=touch P4USER=bob P4PASSWD=secret test_must_fail "$GITP4" commit --preserve-user &&
-		test_must_fail git diff --exit-code HEAD..p4/master
+		# dashism: test_must_fail does not propagate variables
+		P4EDITOR=touch P4USER=bob P4PASSWD=secret &&
+		export P4EDITOR P4USER P4PASSWD &&
+		test_must_fail "$GITP4" commit --preserve-user &&
+		! git diff --exit-code HEAD..p4/master
 	)
 '
 
@@ -250,13 +253,15 @@ test_expect_success 'preserve user where author is unknown to p4' '
 		git commit --author "Bob <bob@localhost>" -m "preserve: a change by bob" file1 &&
 		echo "username-unknown: a change by charlie" >>file1 &&
 		git commit --author "Charlie <charlie@localhost>" -m "preserve: a change by charlie" file1 &&
-		P4EDITOR=touch P4USER=alice P4PASSWD=secret test_must_fail "$GITP4" commit --preserve-user &&
-		test_must_fail git diff --exit-code HEAD..p4/master &&
+		P4EDITOR=touch P4USER=alice P4PASSWD=secret &&
+		export P4EDITOR P4USER P4PASSWD &&
+		test_must_fail "$GITP4" commit --preserve-user &&
+		! git diff --exit-code HEAD..p4/master &&
 
 		echo "$0: repeat with allowMissingP4Users enabled" &&
 		git config git-p4.allowMissingP4Users true &&
 		git config git-p4.preserveUser true &&
-		P4EDITOR=touch P4USER=alice P4PASSWD=secret "$GITP4" commit &&
+		"$GITP4" commit &&
 		git diff --exit-code HEAD..p4/master &&
 		p4_check_commit_author file1 alice
 	)
@@ -275,20 +280,22 @@ test_expect_success 'not preserving user with mixed authorship' '
 		p4_add_user derek Derek &&
 
 		make_change_by_user usernamefile3 Derek derek@localhost &&
-		P4EDITOR=cat P4USER=alice P4PASSWD=secret "$GITP4" commit |\
+		P4EDITOR=cat P4USER=alice P4PASSWD=secret &&
+		export P4EDITOR P4USER P4PASSWD &&
+		"$GITP4" commit |\
 		grep "git author derek@localhost does not match" &&
 
 		make_change_by_user usernamefile3 Charlie charlie@localhost &&
-		P4EDITOR=cat P4USER=alice P4PASSWD=secret "$GITP4" commit |\
+		"$GITP4" commit |\
 		grep "git author charlie@localhost does not match" &&
 
 		make_change_by_user usernamefile3 alice alice@localhost &&
-		P4EDITOR=cat P4USER=alice P4PASSWD=secret "$GITP4" |\
+		"$GITP4" commit |\
 		test_must_fail grep "git author.*does not match" &&
 
 		git config git-p4.skipUserNameCheck true &&
 		make_change_by_user usernamefile3 Charlie charlie@localhost &&
-		P4EDITOR=cat P4USER=alice P4PASSWD=secret "$GITP4" commit |\
+		"$GITP4" commit |\
 		test_must_fail grep "git author.*does not match" &&
 
 		p4_check_commit_author usernamefile3 alice

-- 
1.7.9.2.288.g74b75

Johannes Sixt· Feb 26, 2012, 17:48 UTC · re: Pete Wyckoff · lore

Re: [PATCH 2/2] git-p4: remove bash-ism in t9800

Am 26.02.2012 16:37, schrieb Pete Wyckoff:

Show 7 quoted lines

> -		P4EDITOR=touch P4USER=bob P4PASSWD=secret test_must_fail "$GITP4" commit --preserve-user &&
> -		test_must_fail git diff --exit-code HEAD..p4/master
> +		# dashism: test_must_fail does not propagate variables
> +		P4EDITOR=touch P4USER=bob P4PASSWD=secret &&
> +		export P4EDITOR P4USER P4PASSWD &&
> +		test_must_fail "$GITP4" commit --preserve-user &&
> +		! git diff --exit-code HEAD..p4/master

It is a bashism that variables assigned in front of a shell function are exported. But it is not a dashism that they are not exported; that (surprising?) behavior is actually conforming to POSIX.

With the new code, be aware that the variables remain exported, which might affect subsequent tests in general, though not this one, because the assignments are in a sub-shell:

>  	)
>  '

-- Hannes

Pete Wyckoff· Feb 26, 2012, 18:16 UTC · re: Johannes Sixt · lore

Re: [PATCH 2/2] git-p4: remove bash-ism in t9800

j6t@kdbg.org wrote on Sun, 26 Feb 2012 18:48 +0100:

Show 19 quoted lines

> Am 26.02.2012 16:37, schrieb Pete Wyckoff:
> > -		P4EDITOR=touch P4USER=bob P4PASSWD=secret test_must_fail "$GITP4" commit --preserve-user &&
> > -		test_must_fail git diff --exit-code HEAD..p4/master
> > +		# dashism: test_must_fail does not propagate variables
> > +		P4EDITOR=touch P4USER=bob P4PASSWD=secret &&
> > +		export P4EDITOR P4USER P4PASSWD &&
> > +		test_must_fail "$GITP4" commit --preserve-user &&
> > +		! git diff --exit-code HEAD..p4/master
> 
> It is a bashism that variables assigned in front of a shell function are
> exported. But it is not a dashism that they are not exported; that
> (surprising?) behavior is actually conforming to POSIX.
> 
> With the new code, be aware that the variables remain exported, which
> might affect subsequent tests in general, though not this one, because
> the assignments are in a sub-shell:
> 
> >  	)
> >  '

Interesting, thanks. I thought about the subshell behavior and use, on purpose, the fact that the variables stay exported in the second and third hunks.

		-- Pete