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

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

[Pete Wyckoff]

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

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

[Pete Wyckoff]

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(-)

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

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

[Pete Wyckoff]

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(-)

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

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

[Johannes Sixt]

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:

>  	)
>  '

-- Hannes

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

[Pete Wyckoff]

j6t@kdbg.org wrote on Sun, 26 Feb 2012 18:48 +0100:
> 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