* [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
* [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
* [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
* 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
* 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
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).