[PATCH 00/10] use the $( ... ) construct for command substitution

[PATCH 00/10] use the $( ... ) construct for command substitution

[Elia Pinto]

This patch series continues the changes introduced with the merge
6753d8a85d543253d95184ec2faad6dc197f248:

    Merge branch 'ep/shell-command-substitution'

    Adjust shell scripts to use $(cmd) instead of `cmd`.

The last patch eliminates entirely the backquotes without replacing them 
for optimization.


This is the  seventh series, the other will be sent separately.

Elia Pinto (10):
  t/t7103-reset-bare.sh: use the $( ... ) construct for command
    substitution
  t/t7406-submodule-update.sh: use the $( ... ) construct for command
    substitution
  t/t7408-submodule-reference.sh: use the $( ... ) construct for command
    substitution
  t/t7504-commit-msg-hook.sh: use the $( ... ) construct for command
    substitution
  t/t7505-prepare-commit-msg-hook.sh: use the $( ... ) construct for
    command substitution
  t/t7602-merge-octopus-many.sh: use the $( ... ) construct for command
    substitution
  t/t7700-repack.sh: use the $( ... ) construct for command substitution
  t/t8003-blame-corner-cases.sh: use the $( ... ) construct for command
    substitution
  t/t9001-send-email.sh: use the $( ... ) construct for command
    substitution
  t/t9001-send-email.sh: get rid of unnecessary backquotes

 t/t7103-reset-bare.sh              |  2 +-
 t/t7406-submodule-update.sh        |  4 ++--
 t/t7408-submodule-reference.sh     |  2 +-
 t/t7504-commit-msg-hook.sh         |  2 +-
 t/t7505-prepare-commit-msg-hook.sh | 32 ++++++++++++++++----------------
 t/t7602-merge-octopus-many.sh      |  8 ++++----
 t/t7700-repack.sh                  |  4 ++--
 t/t8003-blame-corner-cases.sh      |  4 ++--
 t/t9001-send-email.sh              | 12 ++++++------
 9 files changed, 35 insertions(+), 35 deletions(-)

-- 
2.3.3.GIT

[PATCH 01/10] t/t7103-reset-bare.sh: use the $( ... ) construct for command substitution

[Elia Pinto]

The Git CodingGuidelines prefer the $(...) construct for command
substitution instead of using the backquotes `...`.

The backquoted form is the traditional method for command
substitution, and is supported by POSIX.  However, all but the
simplest uses become complicated quickly.  In particular, embedded
command substitutions and/or the use of double quotes require
careful escaping with the backslash character.

The patch was generated by:

for _f in $(find . -name "*.sh")
do
	perl -i -pe 'BEGIN{undef $/;} s/`(.+?)`/\$(\1)/smg'  "${_f}"
done

and then carefully proof-read.

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 t/t7103-reset-bare.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t7103-reset-bare.sh b/t/t7103-reset-bare.sh
index 1eef93c..afe36a5 100755
--- a/t/t7103-reset-bare.sh
+++ b/t/t7103-reset-bare.sh
@@ -63,7 +63,7 @@ test_expect_success '"mixed" reset is not allowed in bare' '
 
 test_expect_success '"soft" reset is allowed in bare' '
 	git reset --soft HEAD^ &&
-	test "`git show --pretty=format:%s | head -n 1`" = "one"
+	test "$(git show --pretty=format:%s | head -n 1)" = "one"
 '
 
 test_done
-- 
2.3.3.GIT

[PATCH 02/10] t/t7406-submodule-update.sh: use the $( ... ) construct for command substitution

[Elia Pinto]

The Git CodingGuidelines prefer the $(...) construct for command
substitution instead of using the backquotes `...`.

The backquoted form is the traditional method for command
substitution, and is supported by POSIX.  However, all but the
simplest uses become complicated quickly.  In particular, embedded
command substitutions and/or the use of double quotes require
careful escaping with the backslash character.

The patch was generated by:

for _f in $(find . -name "*.sh")
do
	perl -i -pe 'BEGIN{undef $/;} s/`(.+?)`/\$(\1)/smg'  "${_f}"
done

and then carefully proof-read.

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 t/t7406-submodule-update.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index dda3929..68ea31d 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -14,8 +14,8 @@ submodule and "git submodule update --rebase/--merge" does not detach the HEAD.
 
 compare_head()
 {
-    sha_master=`git rev-list --max-count=1 master`
-    sha_head=`git rev-list --max-count=1 HEAD`
+    sha_master=$(git rev-list --max-count=1 master)
+    sha_head=$(git rev-list --max-count=1 HEAD)
 
     test "$sha_master" = "$sha_head"
 }
-- 
2.3.3.GIT

[PATCH 03/10] t/t7408-submodule-reference.sh: use the $( ... ) construct for command substitution

[Elia Pinto]

The Git CodingGuidelines prefer the $(...) construct for command
substitution instead of using the backquotes `...`.

The backquoted form is the traditional method for command
substitution, and is supported by POSIX.  However, all but the
simplest uses become complicated quickly.  In particular, embedded
command substitutions and/or the use of double quotes require
careful escaping with the backslash character.

The patch was generated by:

for _f in $(find . -name "*.sh")
do
	perl -i -pe 'BEGIN{undef $/;} s/`(.+?)`/\$(\1)/smg'  "${_f}"
done

and then carefully proof-read.

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 t/t7408-submodule-reference.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t7408-submodule-reference.sh b/t/t7408-submodule-reference.sh
index b770b2f..eaea19b 100755
--- a/t/t7408-submodule-reference.sh
+++ b/t/t7408-submodule-reference.sh
@@ -6,7 +6,7 @@
 test_description='test clone --reference'
 . ./test-lib.sh
 
-base_dir=`pwd`
+base_dir=$(pwd)
 
 U=$base_dir/UPLOAD_LOG
 
-- 
2.3.3.GIT

[PATCH 04/10] t/t7504-commit-msg-hook.sh: use the $( ... ) construct for command substitution

[Elia Pinto]

The Git CodingGuidelines prefer the $(...) construct for command
substitution instead of using the backquotes `...`.

The backquoted form is the traditional method for command
substitution, and is supported by POSIX.  However, all but the
simplest uses become complicated quickly.  In particular, embedded
command substitutions and/or the use of double quotes require
careful escaping with the backslash character.

The patch was generated by:

for _f in $(find . -name "*.sh")
do
	perl -i -pe 'BEGIN{undef $/;} s/`(.+?)`/\$(\1)/smg'  "${_f}"
done

and then carefully proof-read.

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 t/t7504-commit-msg-hook.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t7504-commit-msg-hook.sh b/t/t7504-commit-msg-hook.sh
index 1f53ea8..8728db6 100755
--- a/t/t7504-commit-msg-hook.sh
+++ b/t/t7504-commit-msg-hook.sh
@@ -179,7 +179,7 @@ EOF
 chmod +x "$HOOK"
 
 commit_msg_is () {
-	test "`git log --pretty=format:%s%b -1`" = "$1"
+	test "$(git log --pretty=format:%s%b -1)" = "$1"
 }
 
 test_expect_success 'hook edits commit message' '
-- 
2.3.3.GIT

[PATCH 05/10] t/t7505-prepare-commit-msg-hook.sh: use the $( ... ) construct for command substitution

[Elia Pinto]

The Git CodingGuidelines prefer the $(...) construct for command
substitution instead of using the backquotes `...`.

The backquoted form is the traditional method for command
substitution, and is supported by POSIX.  However, all but the
simplest uses become complicated quickly.  In particular, embedded
command substitutions and/or the use of double quotes require
careful escaping with the backslash character.

The patch was generated by:

for _f in $(find . -name "*.sh")
do
	perl -i -pe 'BEGIN{undef $/;} s/`(.+?)`/\$(\1)/smg'  "${_f}"
done

and then carefully proof-read.

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 t/t7505-prepare-commit-msg-hook.sh | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/t/t7505-prepare-commit-msg-hook.sh b/t/t7505-prepare-commit-msg-hook.sh
index 03dce09..b13f729 100755
--- a/t/t7505-prepare-commit-msg-hook.sh
+++ b/t/t7505-prepare-commit-msg-hook.sh
@@ -53,7 +53,7 @@ test_expect_success 'with hook (-m)' '
 	echo "more" >> file &&
 	git add file &&
 	git commit -m "more" &&
-	test "`git log -1 --pretty=format:%s`" = "message (no editor)"
+	test "$(git log -1 --pretty=format:%s)" = "message (no editor)"
 
 '
 
@@ -62,7 +62,7 @@ test_expect_success 'with hook (-m editor)' '
 	echo "more" >> file &&
 	git add file &&
 	GIT_EDITOR="\"\$FAKE_EDITOR\"" git commit -e -m "more more" &&
-	test "`git log -1 --pretty=format:%s`" = message
+	test "$(git log -1 --pretty=format:%s)" = message
 
 '
 
@@ -71,7 +71,7 @@ test_expect_success 'with hook (-t)' '
 	echo "more" >> file &&
 	git add file &&
 	git commit -t "$(git rev-parse --git-dir)/template" &&
-	test "`git log -1 --pretty=format:%s`" = template
+	test "$(git log -1 --pretty=format:%s)" = template
 
 '
 
@@ -80,7 +80,7 @@ test_expect_success 'with hook (-F)' '
 	echo "more" >> file &&
 	git add file &&
 	(echo more | git commit -F -) &&
-	test "`git log -1 --pretty=format:%s`" = "message (no editor)"
+	test "$(git log -1 --pretty=format:%s)" = "message (no editor)"
 
 '
 
@@ -89,17 +89,17 @@ test_expect_success 'with hook (-F editor)' '
 	echo "more" >> file &&
 	git add file &&
 	(echo more more | GIT_EDITOR="\"\$FAKE_EDITOR\"" git commit -e -F -) &&
-	test "`git log -1 --pretty=format:%s`" = message
+	test "$(git log -1 --pretty=format:%s)" = message
 
 '
 
 test_expect_success 'with hook (-C)' '
 
-	head=`git rev-parse HEAD` &&
+	head=$(git rev-parse HEAD) &&
 	echo "more" >> file &&
 	git add file &&
 	git commit -C $head &&
-	test "`git log -1 --pretty=format:%s`" = "$head (no editor)"
+	test "$(git log -1 --pretty=format:%s)" = "$head (no editor)"
 
 '
 
@@ -108,27 +108,27 @@ test_expect_success 'with hook (editor)' '
 	echo "more more" >> file &&
 	git add file &&
 	GIT_EDITOR="\"\$FAKE_EDITOR\"" git commit &&
-	test "`git log -1 --pretty=format:%s`" = default
+	test "$(git log -1 --pretty=format:%s)" = default
 
 '
 
 test_expect_success 'with hook (--amend)' '
 
-	head=`git rev-parse HEAD` &&
+	head=$(git rev-parse HEAD) &&
 	echo "more" >> file &&
 	git add file &&
 	GIT_EDITOR="\"\$FAKE_EDITOR\"" git commit --amend &&
-	test "`git log -1 --pretty=format:%s`" = "$head"
+	test "$(git log -1 --pretty=format:%s)" = "$head"
 
 '
 
 test_expect_success 'with hook (-c)' '
 
-	head=`git rev-parse HEAD` &&
+	head=$(git rev-parse HEAD) &&
 	echo "more" >> file &&
 	git add file &&
 	GIT_EDITOR="\"\$FAKE_EDITOR\"" git commit -c $head &&
-	test "`git log -1 --pretty=format:%s`" = "$head"
+	test "$(git log -1 --pretty=format:%s)" = "$head"
 
 '
 
@@ -141,7 +141,7 @@ test_expect_success 'with hook (merge)' '
 	git commit -m other &&
 	git checkout - &&
 	git merge --no-ff other &&
-	test "`git log -1 --pretty=format:%s`" = "merge (no editor)"
+	test "$(git log -1 --pretty=format:%s)" = "merge (no editor)"
 '
 
 test_expect_success 'with hook and editor (merge)' '
@@ -153,7 +153,7 @@ test_expect_success 'with hook and editor (merge)' '
 	git commit -m other &&
 	git checkout - &&
 	env GIT_EDITOR="\"\$FAKE_EDITOR\"" git merge --no-ff -e other &&
-	test "`git log -1 --pretty=format:%s`" = "merge"
+	test "$(git log -1 --pretty=format:%s)" = "merge"
 '
 
 cat > "$HOOK" <<'EOF'
@@ -164,7 +164,7 @@ EOF
 test_expect_success 'with failing hook' '
 
 	test_when_finished "git checkout -f master" &&
-	head=`git rev-parse HEAD` &&
+	head=$(git rev-parse HEAD) &&
 	echo "more" >> file &&
 	git add file &&
 	test_must_fail env GIT_EDITOR="\"\$FAKE_EDITOR\"" git commit -c $head
@@ -174,7 +174,7 @@ test_expect_success 'with failing hook' '
 test_expect_success 'with failing hook (--no-verify)' '
 
 	test_when_finished "git checkout -f master" &&
-	head=`git rev-parse HEAD` &&
+	head=$(git rev-parse HEAD) &&
 	echo "more" >> file &&
 	git add file &&
 	test_must_fail env GIT_EDITOR="\"\$FAKE_EDITOR\"" git commit --no-verify -c $head
-- 
2.3.3.GIT

[PATCH 06/10] t/t7602-merge-octopus-many.sh: use the $( ... ) construct for command substitution

[Elia Pinto]

The Git CodingGuidelines prefer the $(...) construct for command
substitution instead of using the backquotes `...`.

The backquoted form is the traditional method for command
substitution, and is supported by POSIX.  However, all but the
simplest uses become complicated quickly.  In particular, embedded
command substitutions and/or the use of double quotes require
careful escaping with the backslash character.

The patch was generated by:

for _f in $(find . -name "*.sh")
do
	perl -i -pe 'BEGIN{undef $/;} s/`(.+?)`/\$(\1)/smg'  "${_f}"
done

and then carefully proof-read.

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 t/t7602-merge-octopus-many.sh | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/t/t7602-merge-octopus-many.sh b/t/t7602-merge-octopus-many.sh
index 955f09f..6abe441 100755
--- a/t/t7602-merge-octopus-many.sh
+++ b/t/t7602-merge-octopus-many.sh
@@ -19,7 +19,7 @@ test_expect_success 'setup' '
 		git add c$i.c &&
 		git commit -m c$i &&
 		git tag c$i &&
-		i=`expr $i + 1` || return 1
+		i=$(expr $i + 1) || return 1
 	done
 '
 
@@ -30,7 +30,7 @@ test_expect_success 'merge c1 with c2, c3, c4, ... c29' '
 	while test $i -le 30
 	do
 		refs="$refs c$i"
-		i=`expr $i + 1`
+		i=$(expr $i + 1)
 	done &&
 	git merge $refs &&
 	test "$(git rev-parse c1)" != "$(git rev-parse HEAD)" &&
@@ -38,14 +38,14 @@ test_expect_success 'merge c1 with c2, c3, c4, ... c29' '
 	while test $i -le 30
 	do
 		test "$(git rev-parse c$i)" = "$(git rev-parse HEAD^$i)" &&
-		i=`expr $i + 1` || return 1
+		i=$(expr $i + 1) || return 1
 	done &&
 	git diff --exit-code &&
 	i=1 &&
 	while test $i -le 30
 	do
 		test -f c$i.c &&
-		i=`expr $i + 1` || return 1
+		i=$(expr $i + 1) || return 1
 	done
 '
 
-- 
2.3.3.GIT

[PATCH 07/10] t/t7700-repack.sh: use the $( ... ) construct for command substitution

[Elia Pinto]

The Git CodingGuidelines prefer the $(...) construct for command
substitution instead of using the backquotes `...`.

The backquoted form is the traditional method for command
substitution, and is supported by POSIX.  However, all but the
simplest uses become complicated quickly.  In particular, embedded
command substitutions and/or the use of double quotes require
careful escaping with the backslash character.

The patch was generated by:

for _f in $(find . -name "*.sh")
do
	perl -i -pe 'BEGIN{undef $/;} s/`(.+?)`/\$(\1)/smg'  "${_f}"
done

and then carefully proof-read.

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 t/t7700-repack.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh
index 021c547..6061a04 100755
--- a/t/t7700-repack.sh
+++ b/t/t7700-repack.sh
@@ -69,7 +69,7 @@ test_expect_success 'writing bitmaps via config can duplicate .keep objects' '
 
 test_expect_success 'loose objects in alternate ODB are not repacked' '
 	mkdir alt_objects &&
-	echo `pwd`/alt_objects > .git/objects/info/alternates &&
+	echo $(pwd)/alt_objects > .git/objects/info/alternates &&
 	echo content3 > file3 &&
 	objsha1=$(GIT_OBJECT_DIRECTORY=alt_objects git hash-object -w file3) &&
 	git add file3 &&
@@ -168,7 +168,7 @@ test_expect_success 'packed unreachable obs in alternate ODB are not loosened' '
 '
 
 test_expect_success 'local packed unreachable obs that exist in alternate ODB are not loosened' '
-	echo `pwd`/alt_objects > .git/objects/info/alternates &&
+	echo $(pwd)/alt_objects > .git/objects/info/alternates &&
 	echo "$csha1" | git pack-objects --non-empty --all --reflog pack &&
 	rm -f .git/objects/pack/* &&
 	mv pack-* .git/objects/pack/ &&
-- 
2.3.3.GIT

[PATCH 08/10] t/t8003-blame-corner-cases.sh: use the $( ... ) construct for command substitution

[Elia Pinto]

The Git CodingGuidelines prefer the $(...) construct for command
substitution instead of using the backquotes `...`.

The backquoted form is the traditional method for command
substitution, and is supported by POSIX.  However, all but the
simplest uses become complicated quickly.  In particular, embedded
command substitutions and/or the use of double quotes require
careful escaping with the backslash character.

The patch was generated by:

for _f in $(find . -name "*.sh")
do
	perl -i -pe 'BEGIN{undef $/;} s/`(.+?)`/\$(\1)/smg'  "${_f}"
done

and then carefully proof-read.

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 t/t8003-blame-corner-cases.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t8003-blame-corner-cases.sh b/t/t8003-blame-corner-cases.sh
index 16f1442..6568429 100755
--- a/t/t8003-blame-corner-cases.sh
+++ b/t/t8003-blame-corner-cases.sh
@@ -153,7 +153,7 @@ test_expect_success 'blame path that used to be a directory' '
 '
 
 test_expect_success 'blame to a commit with no author name' '
-  TREE=`git rev-parse HEAD:` &&
+  TREE=$(git rev-parse HEAD:) &&
   cat >badcommit <<EOF &&
 tree $TREE
 author <noname> 1234567890 +0000
@@ -161,7 +161,7 @@ committer David Reiss <dreiss@facebook.com> 1234567890 +0000
 
 some message
 EOF
-  COMMIT=`git hash-object -t commit -w badcommit` &&
+  COMMIT=$(git hash-object -t commit -w badcommit) &&
   git --no-pager blame $COMMIT -- uno >/dev/null
 '
 
-- 
2.3.3.GIT

[PATCH 09/10] t/t9001-send-email.sh: use the $( ... ) construct for command substitution

[Elia Pinto]

The Git CodingGuidelines prefer the $(...) construct for command
substitution instead of using the backquotes `...`.

The backquoted form is the traditional method for command
substitution, and is supported by POSIX.  However, all but the
simplest uses become complicated quickly.  In particular, embedded
command substitutions and/or the use of double quotes require
careful escaping with the backslash character.

The patch was generated by:

for _f in $(find . -name "*.sh")
do
perl -i -pe 'BEGIN{undef $/;} s/`(.+?)`/\$(\1)/smg'  "${_f}"
done

and then carefully proof-read.

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 t/t9001-send-email.sh | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 3c49536..05949a1 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -36,7 +36,7 @@ clean_fake_sendmail () {
 }
 
 test_expect_success $PREREQ 'Extract patches' '
-	patches=`git format-patch -s --cc="One <one@example.com>" --cc=two@example.com -n HEAD^1`
+	patches=$(git format-patch -s --cc="One <one@example.com>" --cc=two@example.com -n HEAD^1)
 '
 
 # Test no confirm early to ensure remaining tests will not hang
@@ -1151,7 +1151,7 @@ test_expect_success $PREREQ '--no-bcc overrides sendemail.bcc' '
 '
 
 test_expect_success $PREREQ 'patches To headers are used by default' '
-	patch=`git format-patch -1 --to="bodies@example.com"` &&
+	patch=$(git format-patch -1 --to="bodies@example.com") &&
 	test_when_finished "rm $patch" &&
 	git send-email \
 		--dry-run \
@@ -1162,7 +1162,7 @@ test_expect_success $PREREQ 'patches To headers are used by default' '
 '
 
 test_expect_success $PREREQ 'patches To headers are appended to' '
-	patch=`git format-patch -1 --to="bodies@example.com"` &&
+	patch=$(git format-patch -1 --to="bodies@example.com") &&
 	test_when_finished "rm $patch" &&
 	git send-email \
 		--dry-run \
@@ -1175,8 +1175,8 @@ test_expect_success $PREREQ 'patches To headers are appended to' '
 '
 
 test_expect_success $PREREQ 'To headers from files reset each patch' '
-	patch1=`git format-patch -1 --to="bodies@example.com"` &&
-	patch2=`git format-patch -1 --to="other@example.com" HEAD~` &&
+	patch1=$(git format-patch -1 --to="bodies@example.com") &&
+	patch2=$(git format-patch -1 --to="other@example.com" HEAD~) &&
 	test_when_finished "rm $patch1 && rm $patch2" &&
 	git send-email \
 		--dry-run \
-- 
2.3.3.GIT

[PATCH 10/10] t/t9001-send-email.sh: get rid of unnecessary backquotes

[Elia Pinto]

Instead of making the shell expand 00* and invoke 'echo' with it,
and then capturing its output as command substitution, just use
the result of expanding 00* directly.

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 t/t9001-send-email.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 05949a1..bcbed38 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1488,7 +1488,7 @@ test_cover_addresses () {
 	clean_fake_sendmail &&
 	rm -fr outdir &&
 	git format-patch --cover-letter -2 -o outdir &&
-	cover=`echo outdir/0000-*.patch` &&
+	cover="outdir/0000-*.patch" &&
 	mv $cover cover-to-edit.patch &&
 	perl -pe "s/^From:/$header: extra\@address.com\nFrom:/" cover-to-edit.patch >"$cover" &&
 	git send-email \
-- 
2.3.3.GIT

Re: [PATCH 10/10] t/t9001-send-email.sh: get rid of unnecessary backquotes

[Matthieu Moy]

Elia Pinto <gitter.spiros@gmail.com> writes:

> Instead of making the shell expand 00* and invoke 'echo' with it,
> and then capturing its output as command substitution, just use
> the result of expanding 00* directly.

This is not actually how it happens. cover=`echo *` expands the * before
the assignment to $cover, while cover="*" assigns a litteral * to
$cover.

Then, when you use $cover, the variable is expanded to * and then it is
expanded to filenames here:

>  	mv $cover cover-to-edit.patch &&

On the other hand, this instance of $cover is quoted, hence the
*-expansion won't happen:

>  	perl -pe "s/^From:/$header: extra\@address.com\nFrom:/" cover-to-edit.patch >"$cover" &&

So, I believe this patch is not correct.

OTOH, patches 1 to 9 look good to me.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH 10/10] t/t9001-send-email.sh: get rid of unnecessary backquotes

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Elia Pinto <gitter.spiros@gmail.com> writes:
>
>> Instead of making the shell expand 00* and invoke 'echo' with it,
>> and then capturing its output as command substitution, just use
>> the result of expanding 00* directly.
>
> This is not actually how it happens. cover=`echo *` expands the * before
> the assignment to $cover, while cover="*" assigns a litteral * to
> $cover.
>
> Then, when you use $cover, the variable is expanded to * and then it is
> expanded to filenames here:
>
>>  	mv $cover cover-to-edit.patch &&
>
> On the other hand, this instance of $cover is quoted, hence the
> *-expansion won't happen:
>
>>  	perl -pe "s/^From:/$header: extra\@address.com\nFrom:/" cover-to-edit.patch >"$cover" &&
>
> So, I believe this patch is not correct.

I think this tried to mimick 6ffd3ec8 but the context covered by
that commit is different from the right hand side of an assignment.

You are right that [10/10] changes where the expansion happens and
is not a faithful conversion.  The result may be the same, though ;-)

If anything, I think

        -	mv $cover cover-to-edit.patch &&
        +	mv "$cover" cover-to-edit.patch &&

is a prudent thing to do, but that is orthogonal to what 10/10
tried to do.

>
> OTOH, patches 1 to 9 look good to me.

Re: [PATCH 10/10] t/t9001-send-email.sh: get rid of unnecessary backquotes

[Elia Pinto]

2016-01-08 15:52 GMT+01:00 Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>:
> Elia Pinto <gitter.spiros@gmail.com> writes:
>
>> Instead of making the shell expand 00* and invoke 'echo' with it,
>> and then capturing its output as command substitution, just use
>> the result of expanding 00* directly.
>
> This is not actually how it happens. cover=`echo *` expands the * before
> the assignment to $cover, while cover="*" assigns a litteral * to
> $cover.
>
> Then, when you use $cover, the variable is expanded to * and then it is
> expanded to filenames here:
>
>>       mv $cover cover-to-edit.patch &&
>
> On the other hand, this instance of $cover is quoted, hence the
> *-expansion won't happen:
>
>>       perl -pe "s/^From:/$header: extra\@address.com\nFrom:/" cover-to-edit.patch >"$cover" &&
>
> So, I believe this patch is not correct.
>
> OTOH, patches 1 to 9 look good to me.
>

Yup. I am really sorry. I was not aware of the particular case,
despite having tested the patch.

Thanks for the review. Junio you kindly remove this patch?

Thank you all very much.

Sorry about the noise

Best regards

> --
> Matthieu Moy
> http://www-verimag.imag.fr/~moy/

Re: [PATCH 10/10] t/t9001-send-email.sh: get rid of unnecessary backquotes

[Junio C Hamano]

Elia Pinto <gitter.spiros@gmail.com> writes:

> Thanks for the review. Junio you kindly remove this patch?
>
> Thank you all very much.

Done, and thanks for working on this.  I am guessing that we are
very close to done, seeing that you now reached the t9xxx series?

Re: [PATCH 10/10] t/t9001-send-email.sh: get rid of unnecessary backquotes

[Elia Pinto]

2016-01-08 23:37 GMT+01:00 Junio C Hamano <gitster@pobox.com>:
> Elia Pinto <gitter.spiros@gmail.com> writes:
>
>> Thanks for the review. Junio you kindly remove this patch?
>>
>> Thank you all very much.
>
> Done, and thanks for working on this.  I am guessing that we are
> very close to done, seeing that you now reached the t9xxx series?
Yes,I've almost finished. Are missing aprox 22  patches,  in addition
to incomplete ( and wrong ! ) patch  we have discussed but I have to
control them.

Best Regards

Re: [PATCH 10/10] t/t9001-send-email.sh: get rid of unnecessary backquotes

[Johannes Sixt]

Am 08.01.2016 um 12:06 schrieb Elia Pinto:
> Instead of making the shell expand 00* and invoke 'echo' with it,
> and then capturing its output as command substitution, just use
> the result of expanding 00* directly.
>
> Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
> ---
>   t/t9001-send-email.sh | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

I notices there are two patches in this series that touch 
t/t9001-send-email.sh. The other one is 9/10, and it claims to be an 
automated conversion. But that cannot be true because it would have 
removed the backquotes that are addressed in this patch.

>
> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
> index 05949a1..bcbed38 100755
> --- a/t/t9001-send-email.sh
> +++ b/t/t9001-send-email.sh
> @@ -1488,7 +1488,7 @@ test_cover_addresses () {
>   	clean_fake_sendmail &&
>   	rm -fr outdir &&
>   	git format-patch --cover-letter -2 -o outdir &&
> -	cover=`echo outdir/0000-*.patch` &&

This expands the pattern and stores the result in $cover, provided there 
exists at least one file that matches the pattern. If such file does not 
exist, the pattern is stored verbatim in $cover.

> +	cover="outdir/0000-*.patch" &&

This does not expand the pattern and stores the pattern verbatim in $cover.

>   	mv $cover cover-to-edit.patch &&

This line succeeds because there pattern is expanded and matches only 
one file.

>   	perl -pe "s/^From:/$header: extra\@address.com\nFrom:/" cover-to-edit.patch >"$cover" &&

In this line, "$cover" is not expanded, and a file named '0000-*.patch' 
will be dropped into subdirectory 'outdir'.

>   	git send-email \

The command that is truncated here looks like this:

         git send-email \
                 --force \
                 --from="Example <nobody@example.com>" \
                 --no-to --no-cc \
                 "$@" \
                 --smtp-server="$(pwd)/fake.sendmail" \
                 outdir/0000-*.patch \
                 outdir/0001-*.patch \
                 outdir/0002-*.patch \
                 2>errors >out &&

Since it uses a pattern that would match a the oddly named file and 
since at this point the original 0000-whatever* file was moved away, the 
pattern still matches only one 0000-* file. The test still succeeds, so 
you did not notices that it has now slightly different behavior.

As much as I would like to remove a sub-process, I think it is better to 
keep the $(echo 0000-*) in this case.