threads / patch / 12401

patchallow git-am to run in a subdirectory

Subject: [PATCH] allow git-am to run in a subdirectory

## tl;dr

11 messages between Mar 1, 2008 and Mar 3, 2008. Diffs are folded; open one to read it.

replies: 10people: 4as markdown or json

Jeff King· Mar 1, 2008, 06:22 UTC · lore

We just move to the top of the tree and proceed. This shouldn't break any existing callers, since the behavior was previously disallowed.

Signed-off-by: Jeff King <peff@peff.net>
---
I can't imagine anyone would be confused by this behavior. The only
other behavior that would make sense is perhaps trying to apply from the
given prefix rather than moving to the toplevel, but that just seems
crazy and useless to me.

It will fail to find your mbox file if you specified a relative path and it had to change directories. I don't know if that case is worth handling.

My use case, btw, is that I have an "apply this patch" macro in my MUA. For some messages, it's easy to guess "cd /my/git/repo && git am". But for others, the repo is not obvious, so I apply in the current directory. If I am at the root of the repo, it works fine. If I'm not, then it fails annoyingly.

 git-am.sh |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
Show changes to git-am.sh +2 −0
diff --git a/git-am.sh b/git-am.sh
index 2ecebc4..a2c6fea 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -2,6 +2,7 @@
 #
 # Copyright (c) 2005, 2006 Junio C Hamano
 
+SUBDIRECTORY_OK=Yes
 OPTIONS_KEEPDASHDASH=
 OPTIONS_SPEC="\
 git-am [options] <mbox>|<Maildir>...
@@ -25,6 +26,7 @@ skip            skip the current patch"
 . git-sh-setup
 set_reflog_action am
 require_work_tree
+cd_to_toplevel
 
 git var GIT_COMMITTER_IDENT >/dev/null || exit
 
-- 
1.5.4.3.422.g55194
Junio C Hamano· Mar 1, 2008, 07:38 UTC · re: Jeff King · lore

Re: [PATCH] allow git-am to run in a subdirectory

Jeff King <peff@peff.net> writes:
> I can't imagine anyone would be confused by this behavior.

As long as you do not lose sight of the mailbox parameter by chdir'ing around, I am Ok with the patch.

... and after I started writing that, I find...
> ...
> It will fail to find your mbox file if you specified a relative path
> and it had to change directories. I don't know if that case is worth
> handling.
Hmmm ;-).
Jeff King· Mar 1, 2008, 08:12 UTC · re: Junio C Hamano · lore

Re: [PATCH] allow git-am to run in a subdirectory

On Fri, Feb 29, 2008 at 11:38:55PM -0800, Junio C Hamano wrote:
> As long as you do not lose sight of the mailbox parameter by chdir'ing
> around, I am Ok with the patch.
> 
> ... and after I started writing that, I find...

Ugh. Below is a patch that saves the original pwd and prefixes it for relative paths. However:

  - it probably doesn't correctly determine absolute versus relative
    paths on Windows. I don't think we have a solution for fixing this
    within shell scripts.
  - it will eat newlines in parameters names (actually, turning them
    into spaces)

The problem is that I need to turn the original "$@" into a new "$@" that is correctly prefixed, which requires proper quoting. Please, spend some of your shell guru points to show me how to do this correctly and portably.

We could wait on doing the 'cd_to_toplevel' until after the mailsplit, but then your .dotest will end up in a prefixed directory (and of course we have a similar munging problem when we point .dotest to the right spot).

Maybe I should just wait for a git-am rewrite in C. ;)
---
Show changes to git-am.sh +26 −1
diff --git a/git-am.sh b/git-am.sh
index 2ecebc4..a5e0c43 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -2,6 +2,7 @@
 #
 # Copyright (c) 2005, 2006 Junio C Hamano
 
+SUBDIRECTORY_OK=Yes
 OPTIONS_KEEPDASHDASH=
 OPTIONS_SPEC="\
 git-am [options] <mbox>|<Maildir>...
@@ -25,6 +26,13 @@ skip            skip the current patch"
 . git-sh-setup
 set_reflog_action am
 require_work_tree
+orig_pwd=$(pwd)
+cd_to_toplevel
+if test "$(pwd)" = "$orig_pwd"; then
+	orig_pwd=
+else
+	orig_pwd="$orig_pwd/"
+fi
 
 git var GIT_COMMITTER_IDENT >/dev/null || exit
 
@@ -121,6 +129,21 @@ reread_subject () {
 	git stripspace <"$1" | sed -e 1q
 }
 
+shellquote() {
+	printf \'
+	printf "$1" | sed "s/'/\\'/g"
+	echo \'
+}
+
+handle_file_args() {
+	for i in "$@"; do
+		case "$i" in
+		/*) shellquote "$i";;
+		 *) shellquote "$orig_pwd$i";;
+		esac
+	done
+}
+
 prec=4
 dotest=.dotest sign= utf8=t keep= skip= interactive= resolved= binary=
 resolvemsg= resume=
@@ -148,7 +171,7 @@ do
 	--skip)
 		skip=t ;;
 	-d|--dotest)
-		shift; dotest=$1;;
+		shift; eval dotest=`handle_file_args "$1"` ;;
 	--resolvemsg)
 		shift; resolvemsg=$1 ;;
 	--whitespace)
@@ -163,6 +186,8 @@ do
 	shift
 done
 
+eval set -- `handle_file_args "$@"`
+
 # If the dotest directory exists, but we have finished applying all the
 # patches in them, clear it out.
 if test -d "$dotest" &&
Jeff King· Mar 1, 2008, 08:20 UTC · re: Junio C Hamano · lore

Re: [PATCH] allow git-am to run in a subdirectory

On Sat, Mar 01, 2008 at 12:15:39AM -0800, Junio C Hamano wrote:
> Sorry for making you do an extra work.  I hoped ;-) was explicit enough
> but apparently it was not...

Heh. I was a little uncomfortable submitting a patch that didn't totally work anyway. Of course, I'm not sure the result is that much better, and it's a lot more complex.

-Peff
Jay Soffian· Mar 3, 2008, 03:26 UTC · re: Jeff King · lore

Re: [PATCH] allow git-am to run in a subdirectory

On Sat, Mar 1, 2008 at 3:12 AM, Jeff King <peff@peff.net> wrote:
>  The problem is that I need to turn the original "$@" into a new "$@"
>  that is correctly prefixed, which requires proper quoting.

Perhaps rev-parse could be taught to optionally convert relative paths to absolute?

j.
Junio C Hamano· Mar 3, 2008, 06:46 UTC · re: Jeff King · lore

Re* [PATCH] allow git-am to run in a subdirectory

Jeff King <peff@peff.net> writes:
> The problem is that I need to turn the original "$@" into a new "$@"
> that is correctly prefixed, which requires proper quoting.
Perhaps like this?
---
 git-am.sh            |   19 +++++++++++++
 t/t4150-am-subdir.sh |   72 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 91 insertions(+), 0 deletions(-)
Show changes to 2 files +91 −0

git-am.sh, t/t4150-am-subdir.sh

diff --git a/git-am.sh b/git-am.sh
index a2c6fea..de34636 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -24,6 +24,7 @@ r,resolved      to be used after a patch failure
 skip            skip the current patch"
 
 . git-sh-setup
+prefix=$(git rev-parse --show-prefix)
 set_reflog_action am
 require_work_tree
 cd_to_toplevel
@@ -206,6 +207,24 @@ else
 	# Start afresh.
 	mkdir -p "$dotest" || exit
 
+	if test -n "$prefix" && test $# != 0
+	then
+		first=t
+		for arg
+		do
+			test -n "$first" && {
+				set x
+				first=
+			}
+			case "$arg" in
+			/*)
+				set "$@" "$arg" ;;
+			*)
+				set "$@" "$prefix$arg" ;;
+			esac
+		done
+		shift
+	fi
 	git mailsplit -d"$prec" -o"$dotest" -b -- "$@" > "$dotest/last" ||  {
 		rm -fr "$dotest"
 		exit 1
diff --git a/t/t4150-am-subdir.sh b/t/t4150-am-subdir.sh
new file mode 100755
index 0000000..929d2cb
--- /dev/null
+++ b/t/t4150-am-subdir.sh
@@ -0,0 +1,72 @@
+#!/bin/sh
+
+test_description='git am running from a subdirectory'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+	echo hello >world &&
+	git add world &&
+	test_tick &&
+	git commit -m initial &&
+	git tag initial &&
+	echo goodbye >world &&
+	git add world &&
+	test_tick &&
+	git commit -m second &&
+	git format-patch --stdout HEAD^ >patchfile &&
+	: >expect
+'
+
+test_expect_success 'am regularly from stdin' '
+	git checkout initial &&
+	git am <patchfile &&
+	git diff master >actual &&
+	diff -u expect actual
+'
+
+test_expect_success 'am regularly from file' '
+	git checkout initial &&
+	git am patchfile &&
+	git diff master >actual &&
+	diff -u expect actual
+'
+
+test_expect_success 'am regularly from stdin in subdirectory' '
+	rm -fr subdir &&
+	git checkout initial &&
+	(
+		mkdir -p subdir &&
+		cd subdir &&
+		git am <../patchfile
+	) &&
+	git diff master>actual &&
+	diff -u expect actual
+'
+
+test_expect_success 'am regularly from file in subdirectory' '
+	rm -fr subdir &&
+	git checkout initial &&
+	(
+		mkdir -p subdir &&
+		cd subdir &&
+		git am ../patchfile
+	) &&
+	git diff master >actual &&
+	diff -u expect actual
+'
+
+test_expect_success 'am regularly from file in subdirectory with full path' '
+	rm -fr subdir &&
+	git checkout initial &&
+	P=$(pwd) &&
+	(
+		mkdir -p subdir &&
+		cd subdir &&
+		git am "$P/patchfile"
+	) &&
+	git diff master >actual &&
+	diff -u expect actual
+'
+
+test_done
Jeff King· Mar 3, 2008, 06:58 UTC · re: Junio C Hamano · lore

Re: Re* [PATCH] allow git-am to run in a subdirectory

On Sun, Mar 02, 2008 at 10:46:31PM -0800, Junio C Hamano wrote:
> > The problem is that I need to turn the original "$@" into a new "$@"
> > that is correctly prefixed, which requires proper quoting.
> 
> Perhaps like this?

Yes. For some reason I didn't think of doing 'set "$@" "$new"' to rebuild the list one by one.

Show 18 quoted lines
> +	if test -n "$prefix" && test $# != 0
> +	then
> +		first=t
> +		for arg
> +		do
> +			test -n "$first" && {
> +				set x
> +				first=
> +			}
> +			case "$arg" in
> +			/*)
> +				set "$@" "$arg" ;;
> +			*)
> +				set "$@" "$prefix$arg" ;;
> +			esac
> +		done
> +		shift
> +	fi

This handles the mailbox parameters. The only other file parameter is $dotest. If I do "cd t && git am --dotest=.foo", then should it be "t/.foo" or ".foo"?

-Peff
Junio C Hamano· Mar 3, 2008, 07:08 UTC · re: Jeff King · lore

Re: Re* [PATCH] allow git-am to run in a subdirectory

Jeff King <peff@peff.net> writes:
> The only other file parameter is
> $dotest. If I do "cd t && git am --dotest=.foo", then should it be
> "t/.foo" or ".foo"?
That brings up an interesting point.
Have people actually used --dotest=<foo> ever?

As far as I know, I do not think it was useful in the real world. Too many things assumed that the spool directory for "am" is .dotest.

And I am very much tempted to say we should remove that option.
Johannes Schindelin· Mar 3, 2008, 11:53 UTC · re: Junio C Hamano · lore

Re: Re* [PATCH] allow git-am to run in a subdirectory

Hi,
On Sun, 2 Mar 2008, Junio C Hamano wrote:
Show 6 quoted lines
> Have people actually used --dotest=<foo> ever?
> 
> As far as I know, I do not think it was useful in the real world.  Too 
> many things assumed that the spool directory for "am" is .dotest.
> 
> And I am very much tempted to say we should remove that option.

Concur. While at it, we could move both .dotest/ and .git/.dotest-merge/ to .git/dotest/, no?

I know, I was opposed to this change, but let's just see how many heads pop up all of a sudden on the mailing list, okay?

Ciao, Dscho

Junio C Hamano· Mar 3, 2008, 17:01 UTC · re: Johannes Schindelin · lore

Re: Re* [PATCH] allow git-am to run in a subdirectory

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Concur.  While at it, we could move both .dotest/ and .git/.dotest-merge/
> to .git/dotest/, no?
We could, yes, while at it, no.

← back to recent threads