threads / patch / 41148

patch, 10 partst/t7103-reset-bare.sh: use the $( ... ) construct for command substitution

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

## tl;dr

17 messages between Jan 8, 2016 and Jan 8, 2016. Diffs are folded; open one to read it.

replies: 16people: 4as markdown or json

Elia Pinto· Jan 8, 2016, 11:06 UTC · lore

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

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

Elia Pinto· Jan 8, 2016, 11:06 UTC · re: Elia Pinto · lore

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(-)

Show changes to t/t7103-reset-bare.sh +1 −1

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

Elia Pinto· Jan 8, 2016, 11:06 UTC · re: Elia Pinto · lore

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

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

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(-)

Show changes to t/t7406-submodule-update.sh +2 −2

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

Elia Pinto· Jan 8, 2016, 11:06 UTC · re: Elia Pinto · lore

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

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

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(-)

Show changes to t/t7408-submodule-reference.sh +1 −1

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

Elia Pinto· Jan 8, 2016, 11:06 UTC · re: Elia Pinto · lore

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

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

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(-)

Show changes to t/t7504-commit-msg-hook.sh +1 −1

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

Elia Pinto· Jan 8, 2016, 11:06 UTC · re: Elia Pinto · lore

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

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

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(-)

Show changes to t/t7505-prepare-commit-msg-hook.sh +16 −16

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

Elia Pinto· Jan 8, 2016, 11:06 UTC · re: Elia Pinto · lore

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

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

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(-)

Show changes to t/t7602-merge-octopus-many.sh +4 −4

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

Elia Pinto· Jan 8, 2016, 11:06 UTC · re: Elia Pinto · lore

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

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

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(-)

Show changes to t/t7700-repack.sh +2 −2

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

Elia Pinto· Jan 8, 2016, 11:06 UTC · re: Elia Pinto · lore

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

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

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(-)

Show changes to t/t8003-blame-corner-cases.sh +2 −2

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

Elia Pinto· Jan 8, 2016, 11:06 UTC · re: Elia Pinto · lore

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

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

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(-)

Show changes to t/t9001-send-email.sh +5 −5

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

Elia Pinto· Jan 8, 2016, 11:06 UTC · re: Elia Pinto · lore

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

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(-)

Show changes to t/t9001-send-email.sh +1 −1

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

Matthieu Moy· Jan 8, 2016, 14:52 UTC · re: Elia Pinto · lore

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

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/

Junio C Hamano· Jan 8, 2016, 18:12 UTC · re: Matthieu Moy · lore

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

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

Show 21 quoted lines

> 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.

Elia Pinto· Jan 8, 2016, 22:29 UTC · re: Matthieu Moy · lore

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

2016-01-08 15:52 GMT+01:00 Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>:

Show 24 quoted lines

> 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/

Junio C Hamano· Jan 8, 2016, 22:37 UTC · re: Elia Pinto · lore

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

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?

Elia Pinto· Jan 8, 2016, 23:19 UTC · re: Junio C Hamano · lore

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

2016-01-08 23:37 GMT+01:00 Junio C Hamano <gitster@pobox.com>:

Show 8 quoted lines

> 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

Johannes Sixt· Jan 8, 2016, 20:07 UTC · re: Elia Pinto · lore

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

Am 08.01.2016 um 12:06 schrieb Elia Pinto:

Show 8 quoted lines

> 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.

Show 10 quoted lines

>
> 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.