{"thread":{"id":"33275","subject":"[PATCH 1/3] contrib/subtree: stop explicitly using a bash shell","startedAt":"2013-03-24T19:37:40Z","lastAt":"2013-03-25T17:57:05Z","messageCount":6,"participants":["Paul Campbell","Simon Ruderich","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"212108","messageId":"1364153863-27437-1-git-send-email-pcampbell@kemitix.net","threadId":"33275","inReplyTo":null,"subject":"[PATCH 0/3] Improve POSIX compatibility and general portablity","fromName":"Paul Campbell","fromEmail":"pcampbell@kemitix.net","sentAt":"2013-03-24T19:37:40Z","receivedAt":"2013-03-24T19:37:40Z","isPatch":true,"sender":{"key":"pcampbell@kemitix.net","avatar":"https://gravatar.com/avatar/57584e05501b694929004e43fcd7308f4ad64df2eb0474cd8c7e5f93662bb0f1?d=mp&s=160"},"body":"git-subtree is considered to be a Bash only script and not POSIX\ncompatibile.\n\nI removed the /bin/bash and tested with dash. All the tests \npassed.\n\nI also followed some of the advice found in \nhttps://wiki.ubuntu.com/DashAsBinSh for converting a bash script to be \nmore portable.\n\nPaul Campbell (3):\n  contrib/subtree: stop explicitly using a bash shell\n  contrib/subtree: remove use of -a/-o in [ commands\n  contrib/subtree: replace echo options with printf\n\n contrib/subtree/git-subtree.sh | 27 +++++++++++++++++----------\n 1 file changed, 17 insertions(+), 10 deletions(-)\n\n-- \n1.8.2\n"},{"id":"212105","messageId":"1364153863-27437-2-git-send-email-pcampbell@kemitix.net","threadId":"33275","inReplyTo":"1364153863-27437-1-git-send-email-pcampbell@kemitix.net","subject":"[PATCH 1/3] contrib/subtree: stop explicitly using a bash shell","fromName":"Paul Campbell","fromEmail":"pcampbell@kemitix.net","sentAt":"2013-03-24T19:37:41Z","receivedAt":"2013-03-24T19:37:41Z","isPatch":true,"sender":{"key":"pcampbell@kemitix.net","avatar":"https://gravatar.com/avatar/57584e05501b694929004e43fcd7308f4ad64df2eb0474cd8c7e5f93662bb0f1?d=mp&s=160"},"body":"Don't explicitly use the Bash shell but allow the system to provide a\nhopefully POSIX compatible shell at /bin/sh.\n\nSigned-off-by: Paul Campbell <pcampbell@kemitix.net>\n---\n\nOnly the system's I was able to test this on (Debian squeeze) /bin/sh is\nthe dash shell.\n\n contrib/subtree/git-subtree.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh\nindex 8a23f58..5701376 100755\n--- a/contrib/subtree/git-subtree.sh\n+++ b/contrib/subtree/git-subtree.sh\n@@ -1,4 +1,4 @@\n-#!/bin/bash\n+#!/bin/sh\n #\n # git-subtree.sh: split/join git repositories in subdirectories of this one\n #\n-- \n1.8.2\n"},{"id":"212106","messageId":"1364153863-27437-3-git-send-email-pcampbell@kemitix.net","threadId":"33275","inReplyTo":"1364153863-27437-1-git-send-email-pcampbell@kemitix.net","subject":"[PATCH 2/3] contrib/subtree: remove use of -a/-o in [ commands","fromName":"Paul Campbell","fromEmail":"pcampbell@kemitix.net","sentAt":"2013-03-24T19:37:42Z","receivedAt":"2013-03-24T19:37:42Z","isPatch":true,"sender":{"key":"pcampbell@kemitix.net","avatar":"https://gravatar.com/avatar/57584e05501b694929004e43fcd7308f4ad64df2eb0474cd8c7e5f93662bb0f1?d=mp&s=160"},"body":"Use of -a and -o in the [ command can have confusing semantics.\n\nUse a separate test invocation for each single test, combining them with\n&& and ||, and use ordinary parentheses for grouping.\n\nSigned-off-by: Paul Campbell <pcampbell@kemitix.net>\n---\n contrib/subtree/git-subtree.sh | 14 +++++++-------\n 1 file changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh\nindex 5701376..884cbfb 100755\n--- a/contrib/subtree/git-subtree.sh\n+++ b/contrib/subtree/git-subtree.sh\n@@ -119,7 +119,7 @@ esac\n \n dir=\"$(dirname \"$prefix/.\")\"\n \n-if [ \"$command\" != \"pull\" -a \"$command\" != \"add\" -a \"$command\" != \"push\" ]; then\n+if ( test \"$command\" != \"pull\" ) && ( test \"$command\" != \"add\" ) && ( test \"$command\" != \"push\" ); then\n \trevs=$(git rev-parse $default --revs-only \"$@\") || exit $?\n \tdirs=\"$(git rev-parse --no-revs --no-flags \"$@\")\" || exit $?\n \tif [ -n \"$dirs\" ]; then\n@@ -181,9 +181,9 @@ cache_set()\n {\n \toldrev=\"$1\"\n \tnewrev=\"$2\"\n-\tif [ \"$oldrev\" != \"latest_old\" \\\n-\t     -a \"$oldrev\" != \"latest_new\" \\\n-\t     -a -e \"$cachedir/$oldrev\" ]; then\n+\tif ( test \"$oldrev\" != \"latest_old\" ) \\\n+\t     && ( test \"$oldrev\" != \"latest_new\" ) \\\n+\t     && ( test -e \"$cachedir/$oldrev\" ); then\n \t\tdie \"cache for $oldrev already exists!\"\n \tfi\n \techo \"$newrev\" >\"$cachedir/$oldrev\"\n@@ -273,12 +273,12 @@ find_existing_splits()\n \t\t\tgit-subtree-split:) sub=\"$b\" ;;\n \t\t\tEND)\n \t\t\t\tdebug \"  Main is: '$main'\"\n-\t\t\t\tif [ -z \"$main\" -a -n \"$sub\" ]; then\n+\t\t\t\tif ( test -z \"$main\" ) && ( test -n \"$sub\" ); then\n \t\t\t\t\t# squash commits refer to a subtree\n \t\t\t\t\tdebug \"  Squash: $sq from $sub\"\n \t\t\t\t\tcache_set \"$sq\" \"$sub\"\n \t\t\t\tfi\n-\t\t\t\tif [ -n \"$main\" -a -n \"$sub\" ]; then\n+\t\t\t\tif ( test -n \"$main\" ) && (test -n \"$sub\" ); then\n \t\t\t\t\tdebug \"  Prior: $main -> $sub\"\n \t\t\t\t\tcache_set $main $sub\n \t\t\t\t\tcache_set $sub $sub\n@@ -541,7 +541,7 @@ cmd_add_commit()\n \ttree=$(git write-tree) || exit $?\n \t\n \theadrev=$(git rev-parse HEAD) || exit $?\n-\tif [ -n \"$headrev\" -a \"$headrev\" != \"$rev\" ]; then\n+\tif ( test -n \"$headrev\" ) && ( test \"$headrev\" != \"$rev\" ); then\n \t\theadp=\"-p $headrev\"\n \telse\n \t\theadp=\n-- \n1.8.2\n"},{"id":"212107","messageId":"1364153863-27437-4-git-send-email-pcampbell@kemitix.net","threadId":"33275","inReplyTo":"1364153863-27437-1-git-send-email-pcampbell@kemitix.net","subject":"[PATCH 3/3] contrib/subtree: replace echo options with printf","fromName":"Paul Campbell","fromEmail":"pcampbell@kemitix.net","sentAt":"2013-03-24T19:37:43Z","receivedAt":"2013-03-24T19:37:43Z","isPatch":true,"sender":{"key":"pcampbell@kemitix.net","avatar":"https://gravatar.com/avatar/57584e05501b694929004e43fcd7308f4ad64df2eb0474cd8c7e5f93662bb0f1?d=mp&s=160"},"body":"Options to echo are not portable. In particular, the echo -e option is\nimplemented by some shells, including bash, to expand escape sequences.\n\nUse the printf command instead, which is portable and much more reliable.\n\nOnly instances of echo and say (a wrapper for echo) where they are used\nwith options have been replaced with printf.\n\nsay_progress() is added to mirror the behaviour of say() in respecting\nthe -q/--quiet option.\n\nSigned-off-by: Paul Campbell <pcampbell@kemitix.net>\n---\n\nThis is a better version of the previously submitted patch \n(http://article.gmane.org/gmane.comp.version-control.git/218103) which\nadded another option to echo.\n\n contrib/subtree/git-subtree.sh | 11 +++++++++--\n 1 file changed, 9 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh\nindex 884cbfb..35caf12 100755\n--- a/contrib/subtree/git-subtree.sh\n+++ b/contrib/subtree/git-subtree.sh\n@@ -61,6 +61,13 @@ say()\n \tfi\n }\n \n+say_progress()\n+{\n+\tif [ -z \"$quiet\" ]; then\n+\t\tprintf \"%s\\r\" \"$@\" >&2\n+\tfi\n+}\n+\n assert()\n {\n \tif \"$@\"; then\n@@ -311,7 +318,7 @@ copy_commit()\n \t\t\tGIT_COMMITTER_NAME \\\n \t\t\tGIT_COMMITTER_EMAIL \\\n \t\t\tGIT_COMMITTER_DATE\n-\t\t(echo -n \"$annotate\"; cat ) |\n+\t\t(printf \"$annotate\"; cat ) |\n \t\tgit commit-tree \"$2\" $3  # reads the rest of stdin\n \t) || die \"Can't copy commit $1\"\n }\n@@ -592,7 +599,7 @@ cmd_split()\n \teval \"$grl\" |\n \twhile read rev parents; do\n \t\trevcount=$(($revcount + 1))\n-\t\tsay -n \"$revcount/$revmax ($createcount)\n\"\n+\t\tsay_progress \"$revcount/$revmax ($createcount)\"\n \t\tdebug \"Processing commit: $rev\"\n \t\texists=$(cache_get $rev)\n \t\tif [ -n \"$exists\" ]; then\n-- \n1.8.2\n"},{"id":"212124","messageId":"20130324211713.GA6155@ruderich.org","threadId":"33275","inReplyTo":"1364153863-27437-3-git-send-email-pcampbell@kemitix.net","subject":"Re: [PATCH 2/3] contrib/subtree: remove use of -a/-o in [ commands","fromName":"Simon Ruderich","fromEmail":"simon@ruderich.org","sentAt":"2013-03-24T21:17:13Z","receivedAt":"2013-03-24T21:17:13Z","isPatch":true,"sender":{"key":"simon@ruderich.org","avatar":"https://avatars.githubusercontent.com/u/390994?v=4"},"body":"From: Paul Campbell <pcampbell@kemitix.net>\n\nUse of -a and -o in the [ command can have confusing semantics.\n\nUse a separate test invocation for each single test, combining them with\n&& and ||.\n\nSigned-off-by: Paul Campbell <pcampbell@kemitix.net>\nSigned-off-by: Simon Ruderich <simon@ruderich.org>\n---\n\nOn Sun, Mar 24, 2013 at 07:37:42PM +0000, Paul Campbell wrote:\n> Use a separate test invocation for each single test, combining them with\n> && and ||, and use ordinary parentheses for grouping.\n\nHello Paul,\n\nParentheses are only necessary if both && and || are used to\nenforce precedence; the shell can split the commands without\nneeding the parentheses. In these cases they can all be removed.\n\nRegards\nSimon\n\n contrib/subtree/git-subtree.sh | 14 +++++++-------\n 1 file changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh\nindex 5701376..d02e6c5 100755\n--- a/contrib/subtree/git-subtree.sh\n+++ b/contrib/subtree/git-subtree.sh\n@@ -119,7 +119,7 @@ esac\n \n dir=\"$(dirname \"$prefix/.\")\"\n \n-if [ \"$command\" != \"pull\" -a \"$command\" != \"add\" -a \"$command\" != \"push\" ]; then\n+if test \"$command\" != \"pull\" && test \"$command\" != \"add\" && test \"$command\" != \"push\"; then\n \trevs=$(git rev-parse $default --revs-only \"$@\") || exit $?\n \tdirs=\"$(git rev-parse --no-revs --no-flags \"$@\")\" || exit $?\n \tif [ -n \"$dirs\" ]; then\n@@ -181,9 +181,9 @@ cache_set()\n {\n \toldrev=\"$1\"\n \tnewrev=\"$2\"\n-\tif [ \"$oldrev\" != \"latest_old\" \\\n-\t     -a \"$oldrev\" != \"latest_new\" \\\n-\t     -a -e \"$cachedir/$oldrev\" ]; then\n+\tif test \"$oldrev\" != \"latest_old\" \\\n+\t     && test \"$oldrev\" != \"latest_new\" \\\n+\t     && test -e \"$cachedir/$oldrev\"; then\n \t\tdie \"cache for $oldrev already exists!\"\n \tfi\n \techo \"$newrev\" >\"$cachedir/$oldrev\"\n@@ -273,12 +273,12 @@ find_existing_splits()\n \t\t\tgit-subtree-split:) sub=\"$b\" ;;\n \t\t\tEND)\n \t\t\t\tdebug \"  Main is: '$main'\"\n-\t\t\t\tif [ -z \"$main\" -a -n \"$sub\" ]; then\n+\t\t\t\tif test -z \"$main\" && test -n \"$sub\"; then\n \t\t\t\t\t# squash commits refer to a subtree\n \t\t\t\t\tdebug \"  Squash: $sq from $sub\"\n \t\t\t\t\tcache_set \"$sq\" \"$sub\"\n \t\t\t\tfi\n-\t\t\t\tif [ -n \"$main\" -a -n \"$sub\" ]; then\n+\t\t\t\tif test -n \"$main\" && test -n \"$sub\"; then\n \t\t\t\t\tdebug \"  Prior: $main -> $sub\"\n \t\t\t\t\tcache_set $main $sub\n \t\t\t\t\tcache_set $sub $sub\n@@ -541,7 +541,7 @@ cmd_add_commit()\n \ttree=$(git write-tree) || exit $?\n \t\n \theadrev=$(git rev-parse HEAD) || exit $?\n-\tif [ -n \"$headrev\" -a \"$headrev\" != \"$rev\" ]; then\n+\tif test -n \"$headrev\" && test \"$headrev\" != \"$rev\"; then\n \t\theadp=\"-p $headrev\"\n \telse\n \t\theadp=\n-- \n1.8.2\n\n-- \n+ privacy is necessary\n+ using gnupg http://gnupg.org\n+ public key id: 0x92FEFDB7E44C32F9\n"},{"id":"212184","messageId":"7vy5dbxszy.fsf@alter.siamese.dyndns.org","threadId":"33275","inReplyTo":"1364153863-27437-2-git-send-email-pcampbell@kemitix.net","subject":"Re: [PATCH 1/3] contrib/subtree: stop explicitly using a bash shell","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-25T17:57:05Z","receivedAt":"2013-03-25T17:57:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Campbell <pcampbell@kemitix.net> writes:\n\n> Don't explicitly use the Bash shell but allow the system to provide a\n> hopefully POSIX compatible shell at /bin/sh.\n>\n> Signed-off-by: Paul Campbell <pcampbell@kemitix.net>\n> ---\n>\n> Only the system's I was able to test this on (Debian squeeze) /bin/sh is\n> the dash shell.\n>\n>  contrib/subtree/git-subtree.sh | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh\n> index 8a23f58..5701376 100755\n> --- a/contrib/subtree/git-subtree.sh\n> +++ b/contrib/subtree/git-subtree.sh\n> @@ -1,4 +1,4 @@\n> -#!/bin/bash\n> +#!/bin/sh\n>  #\n>  # git-subtree.sh: split/join git repositories in subdirectories of this one\n>  #\n\nInteresting. I'll leave the final \"yeah, this is safe\" declaration\nto David and Avery, but I've always assumed without checking that\nthis script relied on bash-isms like local variable semantics,\narrays, regexp/substring variable substitutions, etc.\n\nWith a quick scan, however, I do not seem to find anythning\nglaringly unportable.\n"}]}