[PATCH 0/3] Improve POSIX compatibility and general portablity

git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [PATCH 0/3] Improve POSIX compatibility and general portablity
@ 2013-03-24 19:37 Paul Campbell
  2013-03-24 19:37 ` [PATCH 1/3] contrib/subtree: stop explicitly using a bash shell Paul Campbell
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Paul Campbell @ 2013-03-24 19:37 UTC (permalink / raw)
  To: git; +Cc: greened, Paul Campbell

git-subtree is considered to be a Bash only script and not POSIX
compatibile.

I removed the /bin/bash and tested with dash. All the tests 
passed.

I also followed some of the advice found in 
https://wiki.ubuntu.com/DashAsBinSh for converting a bash script to be 
more portable.

Paul Campbell (3):
  contrib/subtree: stop explicitly using a bash shell
  contrib/subtree: remove use of -a/-o in [ commands
  contrib/subtree: replace echo options with printf

 contrib/subtree/git-subtree.sh | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

-- 
1.8.2

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/3] contrib/subtree: stop explicitly using a bash shell
  2013-03-24 19:37 [PATCH 0/3] Improve POSIX compatibility and general portablity Paul Campbell
@ 2013-03-24 19:37 ` Paul Campbell
  2013-03-25 17:57   ` Junio C Hamano
  2013-03-24 19:37 ` [PATCH 2/3] contrib/subtree: remove use of -a/-o in [ commands Paul Campbell
  2013-03-24 19:37 ` [PATCH 3/3] contrib/subtree: replace echo options with printf Paul Campbell
  2 siblings, 1 reply; 6+ messages in thread
From: Paul Campbell @ 2013-03-24 19:37 UTC (permalink / raw)
  To: git; +Cc: greened, Paul Campbell, Avery Pennarun

Don't explicitly use the Bash shell but allow the system to provide a
hopefully POSIX compatible shell at /bin/sh.

Signed-off-by: Paul Campbell <pcampbell@kemitix.net>
---

Only the system's I was able to test this on (Debian squeeze) /bin/sh is
the dash shell.

 contrib/subtree/git-subtree.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 8a23f58..5701376 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -1,4 +1,4 @@
-#!/bin/bash
+#!/bin/sh
 #
 # git-subtree.sh: split/join git repositories in subdirectories of this one
 #
-- 
1.8.2

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/3] contrib/subtree: stop explicitly using a bash shell
  2013-03-24 19:37 ` [PATCH 1/3] contrib/subtree: stop explicitly using a bash shell Paul Campbell
@ 2013-03-25 17:57   ` Junio C Hamano
  0 siblings, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2013-03-25 17:57 UTC (permalink / raw)
  To: Paul Campbell; +Cc: git, greened, Avery Pennarun

Paul Campbell <pcampbell@kemitix.net> writes:

> Don't explicitly use the Bash shell but allow the system to provide a
> hopefully POSIX compatible shell at /bin/sh.
>
> Signed-off-by: Paul Campbell <pcampbell@kemitix.net>
> ---
>
> Only the system's I was able to test this on (Debian squeeze) /bin/sh is
> the dash shell.
>
>  contrib/subtree/git-subtree.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
> index 8a23f58..5701376 100755
> --- a/contrib/subtree/git-subtree.sh
> +++ b/contrib/subtree/git-subtree.sh
> @@ -1,4 +1,4 @@
> -#!/bin/bash
> +#!/bin/sh
>  #
>  # git-subtree.sh: split/join git repositories in subdirectories of this one
>  #

Interesting. I'll leave the final "yeah, this is safe" declaration
to David and Avery, but I've always assumed without checking that
this script relied on bash-isms like local variable semantics,
arrays, regexp/substring variable substitutions, etc.

With a quick scan, however, I do not seem to find anythning
glaringly unportable.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/3] contrib/subtree: remove use of -a/-o in [ commands
  2013-03-24 19:37 [PATCH 0/3] Improve POSIX compatibility and general portablity Paul Campbell
  2013-03-24 19:37 ` [PATCH 1/3] contrib/subtree: stop explicitly using a bash shell Paul Campbell
@ 2013-03-24 19:37 ` Paul Campbell
  2013-03-24 21:17   ` Simon Ruderich
  2013-03-24 19:37 ` [PATCH 3/3] contrib/subtree: replace echo options with printf Paul Campbell
  2 siblings, 1 reply; 6+ messages in thread
From: Paul Campbell @ 2013-03-24 19:37 UTC (permalink / raw)
  To: git
  Cc: greened, Paul Campbell, Avery Pennarun, Pelle Wessman,
	Win Treese, Wayne Walter

Use of -a and -o in the [ command can have confusing semantics.

Use a separate test invocation for each single test, combining them with
&& and ||, and use ordinary parentheses for grouping.

Signed-off-by: Paul Campbell <pcampbell@kemitix.net>
---
 contrib/subtree/git-subtree.sh | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 5701376..884cbfb 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -119,7 +119,7 @@ esac
 
 dir="$(dirname "$prefix/.")"
 
-if [ "$command" != "pull" -a "$command" != "add" -a "$command" != "push" ]; then
+if ( test "$command" != "pull" ) && ( test "$command" != "add" ) && ( test "$command" != "push" ); then
 	revs=$(git rev-parse $default --revs-only "$@") || exit $?
 	dirs="$(git rev-parse --no-revs --no-flags "$@")" || exit $?
 	if [ -n "$dirs" ]; then
@@ -181,9 +181,9 @@ cache_set()
 {
 	oldrev="$1"
 	newrev="$2"
-	if [ "$oldrev" != "latest_old" \
-	     -a "$oldrev" != "latest_new" \
-	     -a -e "$cachedir/$oldrev" ]; then
+	if ( test "$oldrev" != "latest_old" ) \
+	     && ( test "$oldrev" != "latest_new" ) \
+	     && ( test -e "$cachedir/$oldrev" ); then
 		die "cache for $oldrev already exists!"
 	fi
 	echo "$newrev" >"$cachedir/$oldrev"
@@ -273,12 +273,12 @@ find_existing_splits()
 			git-subtree-split:) sub="$b" ;;
 			END)
 				debug "  Main is: '$main'"
-				if [ -z "$main" -a -n "$sub" ]; then
+				if ( test -z "$main" ) && ( test -n "$sub" ); then
 					# squash commits refer to a subtree
 					debug "  Squash: $sq from $sub"
 					cache_set "$sq" "$sub"
 				fi
-				if [ -n "$main" -a -n "$sub" ]; then
+				if ( test -n "$main" ) && (test -n "$sub" ); then
 					debug "  Prior: $main -> $sub"
 					cache_set $main $sub
 					cache_set $sub $sub
@@ -541,7 +541,7 @@ cmd_add_commit()
 	tree=$(git write-tree) || exit $?
 	
 	headrev=$(git rev-parse HEAD) || exit $?
-	if [ -n "$headrev" -a "$headrev" != "$rev" ]; then
+	if ( test -n "$headrev" ) && ( test "$headrev" != "$rev" ); then
 		headp="-p $headrev"
 	else
 		headp=
-- 
1.8.2

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/3] contrib/subtree: remove use of -a/-o in [ commands
  2013-03-24 19:37 ` [PATCH 2/3] contrib/subtree: remove use of -a/-o in [ commands Paul Campbell
@ 2013-03-24 21:17   ` Simon Ruderich
  0 siblings, 0 replies; 6+ messages in thread
From: Simon Ruderich @ 2013-03-24 21:17 UTC (permalink / raw)
  To: Paul Campbell; +Cc: git

From: Paul Campbell <pcampbell@kemitix.net>

Use of -a and -o in the [ command can have confusing semantics.

Use a separate test invocation for each single test, combining them with
&& and ||.

Signed-off-by: Paul Campbell <pcampbell@kemitix.net>
Signed-off-by: Simon Ruderich <simon@ruderich.org>
---

On Sun, Mar 24, 2013 at 07:37:42PM +0000, Paul Campbell wrote:
> Use a separate test invocation for each single test, combining them with
> && and ||, and use ordinary parentheses for grouping.

Hello Paul,

Parentheses are only necessary if both && and || are used to
enforce precedence; the shell can split the commands without
needing the parentheses. In these cases they can all be removed.

Regards
Simon

 contrib/subtree/git-subtree.sh | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 5701376..d02e6c5 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -119,7 +119,7 @@ esac
 
 dir="$(dirname "$prefix/.")"
 
-if [ "$command" != "pull" -a "$command" != "add" -a "$command" != "push" ]; then
+if test "$command" != "pull" && test "$command" != "add" && test "$command" != "push"; then
 	revs=$(git rev-parse $default --revs-only "$@") || exit $?
 	dirs="$(git rev-parse --no-revs --no-flags "$@")" || exit $?
 	if [ -n "$dirs" ]; then
@@ -181,9 +181,9 @@ cache_set()
 {
 	oldrev="$1"
 	newrev="$2"
-	if [ "$oldrev" != "latest_old" \
-	     -a "$oldrev" != "latest_new" \
-	     -a -e "$cachedir/$oldrev" ]; then
+	if test "$oldrev" != "latest_old" \
+	     && test "$oldrev" != "latest_new" \
+	     && test -e "$cachedir/$oldrev"; then
 		die "cache for $oldrev already exists!"
 	fi
 	echo "$newrev" >"$cachedir/$oldrev"
@@ -273,12 +273,12 @@ find_existing_splits()
 			git-subtree-split:) sub="$b" ;;
 			END)
 				debug "  Main is: '$main'"
-				if [ -z "$main" -a -n "$sub" ]; then
+				if test -z "$main" && test -n "$sub"; then
 					# squash commits refer to a subtree
 					debug "  Squash: $sq from $sub"
 					cache_set "$sq" "$sub"
 				fi
-				if [ -n "$main" -a -n "$sub" ]; then
+				if test -n "$main" && test -n "$sub"; then
 					debug "  Prior: $main -> $sub"
 					cache_set $main $sub
 					cache_set $sub $sub
@@ -541,7 +541,7 @@ cmd_add_commit()
 	tree=$(git write-tree) || exit $?
 	
 	headrev=$(git rev-parse HEAD) || exit $?
-	if [ -n "$headrev" -a "$headrev" != "$rev" ]; then
+	if test -n "$headrev" && test "$headrev" != "$rev"; then
 		headp="-p $headrev"
 	else
 		headp=
-- 
1.8.2

-- 
+ privacy is necessary
+ using gnupg http://gnupg.org
+ public key id: 0x92FEFDB7E44C32F9

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/3] contrib/subtree: replace echo options with printf
  2013-03-24 19:37 [PATCH 0/3] Improve POSIX compatibility and general portablity Paul Campbell
  2013-03-24 19:37 ` [PATCH 1/3] contrib/subtree: stop explicitly using a bash shell Paul Campbell
  2013-03-24 19:37 ` [PATCH 2/3] contrib/subtree: remove use of -a/-o in [ commands Paul Campbell
@ 2013-03-24 19:37 ` Paul Campbell
  2 siblings, 0 replies; 6+ messages in thread
From: Paul Campbell @ 2013-03-24 19:37 UTC (permalink / raw)
  To: git; +Cc: greened, Paul Campbell, Avery Pennarun

Options to echo are not portable. In particular, the echo -e option is
implemented by some shells, including bash, to expand escape sequences.

Use the printf command instead, which is portable and much more reliable.

Only instances of echo and say (a wrapper for echo) where they are used
with options have been replaced with printf.

say_progress() is added to mirror the behaviour of say() in respecting
the -q/--quiet option.

Signed-off-by: Paul Campbell <pcampbell@kemitix.net>
---

This is a better version of the previously submitted patch 
(http://article.gmane.org/gmane.comp.version-control.git/218103) which
added another option to echo.

 contrib/subtree/git-subtree.sh | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 884cbfb..35caf12 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -61,6 +61,13 @@ say()
 	fi
 }
 
+say_progress()
+{
+	if [ -z "$quiet" ]; then
+		printf "%s\r" "$@" >&2
+	fi
+}
+
 assert()
 {
 	if "$@"; then
@@ -311,7 +318,7 @@ copy_commit()
 			GIT_COMMITTER_NAME \
 			GIT_COMMITTER_EMAIL \
 			GIT_COMMITTER_DATE
-		(echo -n "$annotate"; cat ) |
+		(printf "$annotate"; cat ) |
 		git commit-tree "$2" $3  # reads the rest of stdin
 	) || die "Can't copy commit $1"
 }
@@ -592,7 +599,7 @@ cmd_split()
 	eval "$grl" |
 	while read rev parents; do
 		revcount=$(($revcount + 1))
-		say -n "$revcount/$revmax ($createcount)
"
+		say_progress "$revcount/$revmax ($createcount)"
 		debug "Processing commit: $rev"
 		exists=$(cache_get $rev)
 		if [ -n "$exists" ]; then
-- 
1.8.2

^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-03-25 17:57 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-03-24 19:37 [PATCH 0/3] Improve POSIX compatibility and general portablity Paul Campbell
2013-03-24 19:37 ` [PATCH 1/3] contrib/subtree: stop explicitly using a bash shell Paul Campbell
2013-03-25 17:57   ` Junio C Hamano
2013-03-24 19:37 ` [PATCH 2/3] contrib/subtree: remove use of -a/-o in [ commands Paul Campbell
2013-03-24 21:17   ` Simon Ruderich
2013-03-24 19:37 ` [PATCH 3/3] contrib/subtree: replace echo options with printf Paul Campbell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).