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

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

[Junio C Hamano]

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 <gitster@pobox.com>
---

 "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

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

[Stephan Beyer]

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.

> diff --git c/git-am.sh w/git-am.sh
> index 4b157fe..7e6329b 100755
> --- c/git-am.sh
> +++ w/git-am.sh
[...]
> @@ -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/.*/ '\''&'\''/'
                                  ^^^

	$ 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

> 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-? &&

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

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

[Junio C Hamano]

Stephan Beyer <s-beyer@gmx.net> 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 <gitster@pobox.com>
---
 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

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

[Stephan Beyer]

Hi,

Junio C Hamano wrote:
> > 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 :-)

> > 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 <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F