# [PATCH 0/2] git-p4: remove test bash-isms 5 messages from 2012-02-26 to 2012-02-26. Participants: Pete Wyckoff, Johannes Sixt. Thread: https://gitlist.dev/t/29745 ## Pete Wyckoff, 2012-02-26 15:37 Subject: [PATCH 0/2] git-p4: remove test bash-isms Message-ID: <1330270647-8817-1-git-send-email-pw@padd.com> URL: https://gitlist.dev/e/1330270647-8817-1-git-send-email-pw%40padd.com ``` 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, 2012-02-26 15:37 Subject: [PATCH 1/2] git-p4: remove bash-ism in t9809 Message-ID: <1330270647-8817-2-git-send-email-pw@padd.com> URL: https://gitlist.dev/e/1330270647-8817-2-git-send-email-pw%40padd.com In-Reply-To: <1330270647-8817-1-git-send-email-pw@padd.com> ``` 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 Signed-off-by: Pete Wyckoff --- 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 ``` ## Pete Wyckoff, 2012-02-26 15:37 Subject: [PATCH 2/2] git-p4: remove bash-ism in t9800 Message-ID: <1330270647-8817-3-git-send-email-pw@padd.com> URL: https://gitlist.dev/e/1330270647-8817-3-git-send-email-pw%40padd.com In-Reply-To: <1330270647-8817-1-git-send-email-pw@padd.com> ``` 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 Signed-off-by: Pete Wyckoff --- 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 " -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 " -m "preserve: a change by bob" file1 && echo "username-unknown: a change by charlie" >>file1 && git commit --author "Charlie " -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, 2012-02-26 17:48 Subject: Re: [PATCH 2/2] git-p4: remove bash-ism in t9800 Message-ID: <4F4A7057.2020309@kdbg.org> URL: https://gitlist.dev/e/4F4A7057.2020309%40kdbg.org In-Reply-To: <1330270647-8817-3-git-send-email-pw@padd.com> ``` 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 ``` ## Pete Wyckoff, 2012-02-26 18:16 Subject: Re: [PATCH 2/2] git-p4: remove bash-ism in t9800 Message-ID: <20120226181601.GA10130@padd.com> URL: https://gitlist.dev/e/20120226181601.GA10130%40padd.com In-Reply-To: <4F4A7057.2020309@kdbg.org> ``` 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 ```