[PATCH] git-am: add --directory=<dir> option

Thanks to a200337 (git-am: propagate -C<n>, -p<n> options as well,
2008-12-04) and commits around it, "git am" is equipped to correctly
propagate the command line flags such as -C/-p/-whitespace across a patch
failure and restart.

It is trivial to support --directory option now, resurrecting previous
attempts by Kevin and Simon.

Signed-off-by: Junio C Hamano <redacted>
---

 "What's cooking" has listed kb/am-directory in "Stalled" category for too
 long a time and I dropped it entirely.  This resurrects the feature.

 git-am.sh             |   17 +++++++++++++----
 t/t4252-am-options.sh |    8 ++++++++
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git c/git-am.sh w/git-am.sh
index 4b157fe..7e6329b 100755
--- c/git-am.sh
+++ w/git-am.sh

@@ -16,6 +16,7 @@ s,signoff       add a Signed-off-by line to the commit message
 u,utf8          recode into utf8 (default)
 k,keep          pass -k flag to git-mailinfo
 whitespace=     pass it through git-apply
+directory=      pass it through git-apply
 C=              pass it through git-apply
 p=              pass it through git-apply
 resolvemsg=     override error message when patch failure occurs

@@ -33,6 +34,14 @@ cd_to_toplevel
 git var GIT_COMMITTER_IDENT >/dev/null ||
 	die "You need to set your committer info first"
 
+sq () {
+	for sqarg
+	do
+		printf "%s" "$sqarg" |
+		sed -e 's/'\''/'\''\'\'''\''/g' -e 's/.*/ '\''&'\''/'
+	done
+}
+
 stop_here () {
     echo "$1" >"$dotest/next"
     exit 1

@@ -155,10 +164,10 @@ do
 		;;
 	--resolvemsg)
 		shift; resolvemsg=$1 ;;
-	--whitespace)
-		git_apply_opt="$git_apply_opt $1=$2"; shift ;;
+	--whitespace|--directory)
+		git_apply_opt="$git_apply_opt $(sq "$1=$2")"; shift ;;
 	-C|-p)
-		git_apply_opt="$git_apply_opt $1$2"; shift ;;
+		git_apply_opt="$git_apply_opt $(sq "$1$2")"; shift ;;
 	--)
 		shift; break ;;
 	*)

@@ -459,7 +468,7 @@ do
 
 	case "$resolved" in
 	'')
-		git apply $git_apply_opt --index "$dotest/patch"
+		eval 'git apply '"$git_apply_opt"' --index "$dotest/patch"'
 		apply_status=$?
 		;;
 	t)

diff --git c/t/t4252-am-options.sh w/t/t4252-am-options.sh
index 3ab9e8e..e91a6da 100755
--- c/t/t4252-am-options.sh
+++ w/t/t4252-am-options.sh

@@ -50,4 +50,12 @@ test_expect_success 'interrupted am -C1 -p2' '
 	grep "^Three$" file-2
 '
 
+test_expect_success 'interrupted am --directory="frotz nitfol"' '
+	rm -rf .git/rebase-apply &&
+	git reset --hard initial &&
+	test_must_fail git am --directory="frotz nitfol" "$tm"/am-test-5-? &&
+	git am --skip &&
+	grep One "frotz nitfol/file-5"
+'
+
 test_done

Hi Junio,

Junio C Hamano wrote:

 git-am.sh             |   17 +++++++++++++----
 t/t4252-am-options.sh |    8 ++++++++
 2 files changed, 21 insertions(+), 4 deletions(-)

I think this is missing some

diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
index b9c6fac..64c8178 100644
--- a/Documentation/git-am.txt
+++ b/Documentation/git-am.txt

@@ -66,6 +66,7 @@ default.   You could use `--no-utf8` to override this.
 
 -C<n>::
 -p<n>::
+--directory=<root>::
 	These flags are passed to the 'git-apply' (see linkgit:git-apply[1])
 	program that applies
 	the patch.

or even

diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
index b9c6fac..327bc3d 100644
--- a/Documentation/git-am.txt
+++ b/Documentation/git-am.txt

@@ -59,13 +59,10 @@ default.   You could use `--no-utf8` to override this.
 	it is supposed to apply to, and we have those blobs
 	available locally.
 
---whitespace=<option>::
-	This flag is passed to the 'git-apply' (see linkgit:git-apply[1])
-	program that applies
-	the patch.
-
 -C<n>::
 -p<n>::
+--directory=<root>::
+--whitespace=<option>::
 	These flags are passed to the 'git-apply' (see linkgit:git-apply[1])
 	program that applies
 	the patch.

[...]

                                  ^^^

	$ echo "/fo'ba" | sed -e 's/'\''/'\''\'\'''\''/g' -e 's/.*/ '\''&'\''/'
	 '/fo'''ba'


Do I have a thinko or should it be this:

+		sed -e 's/'\''/'\''\\\'\'''\''/g' -e 's/.*/ '\''&'\''/'
		                   ^^
			(added for escaping ' outside single quotes)

leading to:

	$ echo "/fo'ba" | sed -e 's/'\''/'\''\\\'\'''\''/g' -e 's/.*/ '\''&'\''/'
	 '/fo'\''ba'


Well, I was unsure, so I've tested...


Without this change:

	$ ./git-am.sh --directory="fo'ba" /tmp/test/*
	Applying: abcdefg
	./git-am.sh: eval: line 471: unexpected EOF while looking for matching
	`''
	./git-am.sh: eval: line 472: syntax error: unexpected end of file
	Patch failed at 0001.
	When you have resolved this problem run "git am --resolved".
	If you would prefer to skip this patch, instead run "git am --skip".
	To restore the original branch and stop patching run "git am --abort".

And with this change:

	$ ./git-am.sh --directory="fo'ba" /tmp/test/*
	Applying: abcdefg
	Applying: asdgasfh

Have you forgotten to add the files prefixed with "am-test-5-" or is this
patch based on another one?

$ git ls-files t/t4252
t/t4252/am-test-1-1
t/t4252/am-test-1-2
t/t4252/am-test-2-1
t/t4252/am-test-2-2
t/t4252/am-test-3-1
t/t4252/am-test-3-2
t/t4252/am-test-4-1
t/t4252/am-test-4-2
t/t4252/file-1-0
t/t4252/file-2-0


Thanks and regards,
  Stephan

--------------- proposed interdiff without am-test-5* ---------------

diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
index b9c6fac..327bc3d 100644
--- a/Documentation/git-am.txt
+++ b/Documentation/git-am.txt

@@ -59,13 +59,10 @@ default.   You could use `--no-utf8` to override this.
 	it is supposed to apply to, and we have those blobs
 	available locally.
 
---whitespace=<option>::
-	This flag is passed to the 'git-apply' (see linkgit:git-apply[1])
-	program that applies
-	the patch.
-
 -C<n>::
 -p<n>::
+--directory=<root>::
+--whitespace=<option>::
 	These flags are passed to the 'git-apply' (see linkgit:git-apply[1])
 	program that applies
 	the patch.

diff --git a/git-am.sh b/git-am.sh
index 7e6329b..ca3dbcd 100755
--- a/git-am.sh
+++ b/git-am.sh

@@ -38,7 +38,7 @@ sq () {
 	for sqarg
 	do
 		printf "%s" "$sqarg" |
-		sed -e 's/'\''/'\''\'\'''\''/g' -e 's/.*/ '\''&'\''/'
+		sed -e 's/'\''/'\''\\\'\'''\''/g' -e 's/.*/ '\''&'\''/'
 	done
 }
 

-- 
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F

Stephan Beyer [off-list ref] writes:

Do I have a thinko or should it be this:

+		sed -e 's/'\''/'\''\\\'\'''\''/g' -e 's/.*/ '\''&'\''/'
		                   ^^
			(added for escaping ' outside single quotes)

Almost.

Certainly my original was bad; shell unquotes to "s/'/'\''/g", but that
backslash is not protected from further interpretation by sed, which
happily turns backslash-single quote into a single quote, which I forgot.

You feed "s/'/'\\\''/g" which correctly protects one backslash from sed by
doubling it, but it has one unnecessary extra backslash.  The extra one
does not hurt because the backslash + single quote is eaten by sed to
produce a single quote, but it is not quite right.

We should be feeding sed with "s/'/'\\''/g", so you need to add one
backslash to mine.

Have you forgotten to add the files prefixed with "am-test-5-" or is this
patch based on another one?

The one I actually queued is b47dfe9 (git-am: add --directory=<dir>
option, 2009-01-11) and it does include these test vectors.  My bad.

This patch is relative to b47dfe9.

-- >8 --
Fix git-am shell quoting

Noticed by Stephan Beyer; the new test is mine.

Signed-off-by: Junio C Hamano <redacted>
---
 git-am.sh             |    2 +-
 t/t4252-am-options.sh |    8 ++++++++
 2 files changed, 9 insertions(+), 1 deletions(-)

diff --git c/git-am.sh w/git-am.sh
index 7e6329b..4beb12d 100755
--- c/git-am.sh
+++ w/git-am.sh

@@ -38,7 +38,7 @@ sq () {
 	for sqarg
 	do
 		printf "%s" "$sqarg" |
-		sed -e 's/'\''/'\''\'\'''\''/g' -e 's/.*/ '\''&'\''/'
+		sed -e 's/'\''/'\''\\'\'''\''/g' -e 's/.*/ '\''&'\''/'
 	done
 }
 

diff --git c/t/t4252-am-options.sh w/t/t4252-am-options.sh
index e91a6da..5fdd188 100755
--- c/t/t4252-am-options.sh
+++ w/t/t4252-am-options.sh

@@ -58,4 +58,12 @@ test_expect_success 'interrupted am --directory="frotz nitfol"' '
 	grep One "frotz nitfol/file-5"
 '
 
+test_expect_success 'apply to a funny path' '
+	with_sq="with'\''sq"
+	rm -fr .git/rebase-apply &&
+	git reset --hard initial &&
+	git am --directory="$with_sq" "$tm"/am-test-5-2 &&
+	test -f "$with_sq/file-5"
+'
+
 test_done

Hi,

Junio C Hamano wrote:

quoted

Do I have a thinko or should it be this:

+		sed -e 's/'\''/'\''\\\'\'''\''/g' -e 's/.*/ '\''&'\''/'
		                   ^^
			(added for escaping ' outside single quotes)

Almost.

Certainly my original was bad; shell unquotes to "s/'/'\''/g", but that
backslash is not protected from further interpretation by sed, which
happily turns backslash-single quote into a single quote, which I forgot.

You feed "s/'/'\\\''/g" which correctly protects one backslash from sed by
doubling it, but it has one unnecessary extra backslash.

My attempt was to escape one backslash and to escape one single quote.

The extra one
does not hurt because the backslash + single quote is eaten by sed to
produce a single quote, but it is not quite right.

Well, this explains why my syntax highlighting has "gone mad" in your
former and in my version.

We should be feeding sed with "s/'/'\\''/g", so you need to add one
backslash to mine.

Ok, works like a charm :-)

quoted

Have you forgotten to add the files prefixed with "am-test-5-" or is this
patch based on another one?

The one I actually queued is b47dfe9 (git-am: add --directory=<dir>
option, 2009-01-11) and it does include these test vectors.  My bad.

Ohh, I did not even notice that you queued it, because I do not track "next".

And in my git-am.txt snippet I even forgot adding the option to the synopsis.
Oh, boy. :-)

I think it's fine now. :-)

Regards,
  Stephan

-- 
Stephan Beyer [off-list ref], PGP 0x6EDDD207FCC5040F