[PATCH] git-am: indicate where a failed patch is to be found.

[PATCH] git-am: indicate where a failed patch is to be found.

[Paul Gortmaker]

If git am wasn't run with --reject, we assume the end user
knows where to find the patch.  This is normally true for
a single patch, but if the user is processing a mbox with
many patches, they may not have a single broken out patch
handy.  So, provide a helpful hint as to where they can
find the patch to do the manual fixup before eventually
continuing with "git add ... ; git am -r".

Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>

diff --git a/git-am.sh b/git-am.sh
index f8b7a0c..32e6ac0 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -854,7 +854,10 @@ did you forget to use 'git add'?"
 	fi
 	if test $apply_status != 0
 	then
-		eval_gettextln 'Patch failed at $msgnum $FIRSTLINE'
+		eval_gettextln "Patch failed at $msgnum $FIRSTLINE
+You can try running the following command:
+   patch -p1 --dry-run < $dotest/patch
+in order to possibly get more information on why it failed."
 		stop_here_user_resolve $this
 	fi
 
-- 
1.7.9.7

Re: [PATCH] git-am: indicate where a failed patch is to be found.

[Junio C Hamano]

Paul Gortmaker <paul.gortmaker@windriver.com> writes:

> If git am wasn't run with --reject, we assume the end user
> knows where to find the patch.  This is normally true for
> a single patch,

Not at all.  Whether it is a single or broken, the patch is fed to
underlying "apply" from an unadvertised place.

> So, provide a helpful hint as to where they can
> find the patch ...

This is OK, but you may want to give a way to squelch it once the
user learns where it is by following the usual "advice.*" thing.

> ... to do the manual fixup before eventually
> continuing with "git add ... ; git am -r".

This is _NOT_ fine, especially if you suggest "patch" the user may
not have, and more importantly does not have a clue why "git apply"
rejected it ("am" does _not_ use "patch" at all).

Re: [PATCH] git-am: indicate where a failed patch is to be found.

[Paul Gortmaker]

On 12-07-12 01:45 PM, Junio C Hamano wrote:
> Paul Gortmaker <paul.gortmaker@windriver.com> writes:
> 
>> If git am wasn't run with --reject, we assume the end user
>> knows where to find the patch.  This is normally true for
>> a single patch,
> 
> Not at all.  Whether it is a single or broken, the patch is fed to
> underlying "apply" from an unadvertised place.

What I meant by this was the difference between:

	git am 0001-some-standalone-single.patch
vs.
	git am mbox

In the 1st, the standalone patch is 100% clear and easy to access,
because we really don't need/care about the unadvertised place.

Maybe I should have said "knows how to get at the single patch"?

> 
>> So, provide a helpful hint as to where they can
>> find the patch ...
> 
> This is OK, but you may want to give a way to squelch it once the
> user learns where it is by following the usual "advice.*" thing.
> 
>> ... to do the manual fixup before eventually
>> continuing with "git add ... ; git am -r".
> 
> This is _NOT_ fine, especially if you suggest "patch" the user may
> not have, and more importantly does not have a clue why "git apply"
> rejected it ("am" does _not_ use "patch" at all).

I'm not 100% sure I'm following what part here is not OK.  If you
can help me understand that, I'll respin the change accordingly.

Is it the assumption that the user will have the patch
command in /usr/bin not OK, or that the message implies that
git is somehow using /usr/bin/patch is not OK?

In case it helps any, a brief summary of my workflow is this:

git am /tmp/mbox
<some random fail halfway in the queue>
patch -p1 --dry-run < .git/rebase-apply/patch
# gauge status.  Is patch really invalid, or already applied?
# already applied; "git am --skip"
# no, if valid, but with minor issues, apply what we can.
patch -p1 < .git/rebase-apply/patch
# manually deal with rejects (typically with wiggle)
git add any_new_files
git add -u
git am -r

Paul.
--

> 
> 

Re: [PATCH] git-am: indicate where a failed patch is to be found.

[Junio C Hamano]

Paul Gortmaker <paul.gortmaker@windriver.com> writes:

> On 12-07-12 01:45 PM, Junio C Hamano wrote:
>> Paul Gortmaker <paul.gortmaker@windriver.com> writes:
>> 
>>> If git am wasn't run with --reject, we assume the end user
>>> knows where to find the patch.  This is normally true for
>>> a single patch,
>> 
>> Not at all.  Whether it is a single or broken, the patch is fed to
>> underlying "apply" from an unadvertised place.
>
> What I meant by this was the difference between:
>
> 	git am 0001-some-standalone-single.patch
> vs.
> 	git am mbox
>
> In the 1st, the standalone patch is 100% clear and easy to access,
> because we really don't need/care about the unadvertised place.

It does not matter at all that 0001-foo.patch only has a single
patch.  If you are going to fix up the patch after you saw "git am"
failed, you will be fixing .git/rebase-apply/patch with your editor
and re-run "git am" without arguments, at which point "git am" will
not look at your 0001-foo.patch file at all.

>> This is _NOT_ fine, especially if you suggest "patch" the user may
>> not have, and more importantly does not have a clue why "git apply"
>> rejected it ("am" does _not_ use "patch" at all).
>
> I'm not 100% sure I'm following what part here is not OK.  If you
> can help me understand that, I'll respin the change accordingly.

Do not ever mention "patch -p1".  It is not the command that "git
am" uses, and it is not what detected the breakage in the patch.

The command to guide the user to is "git apply".

Re: [PATCH] git-am: indicate where a failed patch is to be found.

[Paul Gortmaker]

On 12-07-12 02:53 PM, Junio C Hamano wrote:
> Paul Gortmaker <paul.gortmaker@windriver.com> writes:
> 
>> On 12-07-12 01:45 PM, Junio C Hamano wrote:
>>> Paul Gortmaker <paul.gortmaker@windriver.com> writes:
>>>
>>>> If git am wasn't run with --reject, we assume the end user
>>>> knows where to find the patch.  This is normally true for
>>>> a single patch,
>>>
>>> Not at all.  Whether it is a single or broken, the patch is fed to
>>> underlying "apply" from an unadvertised place.
>>
>> What I meant by this was the difference between:
>>
>> 	git am 0001-some-standalone-single.patch
>> vs.
>> 	git am mbox
>>
>> In the 1st, the standalone patch is 100% clear and easy to access,
>> because we really don't need/care about the unadvertised place.
> 
> It does not matter at all that 0001-foo.patch only has a single
> patch.  If you are going to fix up the patch after you saw "git am"
> failed, you will be fixing .git/rebase-apply/patch with your editor
> and re-run "git am" without arguments, at which point "git am" will
> not look at your 0001-foo.patch file at all.

I think this is where our two thinking paths diverge.  You are
suggesting I edit and fix the patch.  Yes, occasionally I do
that, if it is a trivial context change.  But hand editing a
patch is not for Joe Average, and gets very complicated in all
but the trivial cases.  So, what happens _way_ more often, is that
I want to apply what can be applied, and deal with the rejects
on a one-by-one basis after that.  (BTW, this is not just me;
this patch came about from discussions with other kernel folks.)

> 
>>> This is _NOT_ fine, especially if you suggest "patch" the user may
>>> not have, and more importantly does not have a clue why "git apply"
>>> rejected it ("am" does _not_ use "patch" at all).
>>
>> I'm not 100% sure I'm following what part here is not OK.  If you
>> can help me understand that, I'll respin the change accordingly.
> 
> Do not ever mention "patch -p1".  It is not the command that "git
> am" uses, and it is not what detected the breakage in the patch.

This may be true, but it _is_ the command that I (and others) have
defaulted to using, if for no other reason than ignorance.

> 
> The command to guide the user to is "git apply".
> 

OK.  But I don't see a "--dry-run" equivalent -- and "git apply --check"
just gives me a repeat of the same fail messages that "git am" did.

With "patch -p1 --dry-run"  I get information that immediately
lets me see whether the patch is viable or not.  Is there a way
to get a similar thing from "git apply" that I've overlooked?

Paul.
---

Re: [PATCH] git-am: indicate where a failed patch is to be found.

[Junio C Hamano]

Paul Gortmaker <paul.gortmaker@windriver.com> writes:

>>>> This is _NOT_ fine, especially if you suggest "patch" the user may
>>>> not have, and more importantly does not have a clue why "git apply"
>>>> rejected it ("am" does _not_ use "patch" at all).
>>>
>>> I'm not 100% sure I'm following what part here is not OK.  If you
>>> can help me understand that, I'll respin the change accordingly.
>> 
>> Do not ever mention "patch -p1".  It is not the command that "git
>> am" uses, and it is not what detected the breakage in the patch.
>
> This may be true, but it _is_ the command that I (and others) have
> defaulted to using, if for no other reason than ignorance.
>> 
>> The command to guide the user to is "git apply".
>
> OK.  But I don't see a "--dry-run" equivalent -- and "git apply --check"
> just gives me a repeat of the same fail messages that "git am" did.
>
> With "patch -p1 --dry-run"  I get information that immediately
> lets me see whether the patch is viable or not.

What do you mean by "viable"?  

Independent from the answer to that question...

Running "git apply -p1" would by definition give you the same
failure without --dry-run (because you know it already failed), no?
Then you could ask for rejects or attempt to apply with reduced
contexts to "git apply" all without having to say --dry-run, as
unapplicable change will not be applied.

Re: [PATCH] git-am: indicate where a failed patch is to be found.

[Paul Gortmaker]

On 12-07-12 04:00 PM, Junio C Hamano wrote:
> Paul Gortmaker <paul.gortmaker@windriver.com> writes:
> 
>>>>> This is _NOT_ fine, especially if you suggest "patch" the user may
>>>>> not have, and more importantly does not have a clue why "git apply"
>>>>> rejected it ("am" does _not_ use "patch" at all).
>>>>
>>>> I'm not 100% sure I'm following what part here is not OK.  If you
>>>> can help me understand that, I'll respin the change accordingly.
>>>
>>> Do not ever mention "patch -p1".  It is not the command that "git
>>> am" uses, and it is not what detected the breakage in the patch.
>>
>> This may be true, but it _is_ the command that I (and others) have
>> defaulted to using, if for no other reason than ignorance.
>>>
>>> The command to guide the user to is "git apply".
>>
>> OK.  But I don't see a "--dry-run" equivalent -- and "git apply --check"
>> just gives me a repeat of the same fail messages that "git am" did.
>>
>> With "patch -p1 --dry-run"  I get information that immediately
>> lets me see whether the patch is viable or not.
> 
> What do you mean by "viable"?  

Sorry, that description was a bit context free.  Two typical cases:

1) applying a series of commits (e.g. preempt RT feature) to a newer
baseline. Some of those commits may have been upstreamed and now
present in mainline.  The "git am" failure doesn't really hint that
"already applied" may be the case -- e.g. consider and compare the
output when we extract and then intentionally try to re-apply something
already in tree, created with:

-------------
$git format-patch 50fb31cf~..50fb31cf
0001-tty-hvc_opal-Fix-debug-function-name.patch
-------------

With "git am":
--------------------------
$git am 0001-tty-hvc_opal-Fix-debug-function-name.patch
Applying: tty/hvc_opal: Fix debug function name
error: patch failed: drivers/tty/hvc/hvc_opal.c:401
error: drivers/tty/hvc/hvc_opal.c: patch does not apply
Patch failed at 0001 tty/hvc_opal: Fix debug function name
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".
---------------------------

...versus 

---------------------------
$patch -p1 --dry-run < 0001-tty-hvc_opal-Fix-debug-function-name.patch 
patching file drivers/tty/hvc/hvc_opal.c
Reversed (or previously applied) patch detected!  Assume -R? [n] 
Apply anyway? [n] 
Skipping patch.
1 out of 1 hunk ignored -- saving rejects to file drivers/tty/hvc/hvc_opal.c.rej
---------------------------

...versus

---------------------------
$git apply -p1 0001-tty-hvc_opal-Fix-debug-function-name.patch
error: patch failed: drivers/tty/hvc/hvc_opal.c:401
error: drivers/tty/hvc/hvc_opal.c: patch does not apply
---------------------------

Maybe there is an easy way to teach git am/apply to detect "previously
applied" in a way similar to patch?  The closest I could come to that
was "git apply --check -R ..." and seeing what it said (or didn't say).

2) In maintaining linux stable releases (esp older ones), the dry-run
output, if say it says something like 23/30 chunks failed, it tells me
that the underlying baseline has probably changed too much for a simple
backport.  But if only 1/30 chunks fail or similar, I'll simply proceed
since the backport is viable and likely trivial.

Paul.
--

> 
> Independent from the answer to that question...
> 
> Running "git apply -p1" would by definition give you the same
> failure without --dry-run (because you know it already failed), no?
> Then you could ask for rejects or attempt to apply with reduced
> contexts to "git apply" all without having to say --dry-run, as
> unapplicable change will not be applied.
> 

Re: [PATCH] git-am: indicate where a failed patch is to be found.

[Junio C Hamano]

Paul Gortmaker <paul.gortmaker@windriver.com> writes:

> Sorry, that description was a bit context free.  Two typical cases:
>
> 1) applying a series of commits (e.g. preempt RT feature) to a newer
> baseline. Some of those commits may have been upstreamed and now
> present in mainline.  The "git am" failure doesn't really hint that
> "already applied" may be the case -- e.g. consider and compare the
> output when we extract and then intentionally try to re-apply something
> already in tree, created with:
>
> -------------
> $git format-patch 50fb31cf~..50fb31cf
> 0001-tty-hvc_opal-Fix-debug-function-name.patch
> -------------
>
> With "git am":
> --------------------------
> $git am 0001-tty-hvc_opal-Fix-debug-function-name.patch
> Applying: tty/hvc_opal: Fix debug function name
> error: patch failed: drivers/tty/hvc/hvc_opal.c:401
> error: drivers/tty/hvc/hvc_opal.c: patch does not apply
> Patch failed at 0001 tty/hvc_opal: Fix debug function name
> 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".
> ---------------------------
>
> ...versus 
>
> ---------------------------
> $patch -p1 --dry-run < 0001-tty-hvc_opal-Fix-debug-function-name.patch 
> patching file drivers/tty/hvc/hvc_opal.c
> Reversed (or previously applied) patch detected!  Assume -R? [n] 
> Apply anyway? [n] 
> Skipping patch.
> 1 out of 1 hunk ignored -- saving rejects to file drivers/tty/hvc/hvc_opal.c.rej
> ---------------------------

"git am -3" will give you a message "already applied" and moves on,
or if an already applied stuff is similar but not different would
stop with conflict, or fail butd the latter two cases GNU patch
would not say "reversed", so "am -3" would be a win 2 out of 3 cases
and the remaining 1 out of 3 case would be a tie.

> 2) In maintaining linux stable releases (esp older ones), the dry-run
> output, if say it says something like 23/30 chunks failed, it tells me
> that the underlying baseline has probably changed too much for a simple
> backport.  But if only 1/30 chunks fail or similar, I'll simply proceed
> since the backport is viable and likely trivial.

Perhaps "git apply" when stops upon unapplicable patch may want to
be improved to give more detailed diagnostics (I think it stops upon
first hunk per each file that is touched---it may be able to keep
going and see if other hunks might apply).  This is in "patches
welcome" category ;-).

Re: [PATCH] git-am: indicate where a failed patch is to be found.

[Junio C Hamano]

Paul Gortmaker <paul.gortmaker@windriver.com> writes:

> I think this is where our two thinking paths diverge.  You are
> suggesting I edit and fix the patch.  Yes, occasionally I do
> that, if it is a trivial context change.  But hand editing a
> patch is not for Joe Average, and gets very complicated in all
> but the trivial cases.

In your patch, you do not special case and refrain from giving the
location of the patchfile when there is only one patch in the input,
so the above does not matter anyway.

The patch does two unrelated things: reveal the location of the
actual patchfile that failed to apply which was so far kept sekrit,
and tell the user what to do with it.

Because a user who _wants to_ use a patch, once she knows where it
is, would know her favorite way of working with it (be it by editing
it and reapplying, running "git apply" with --reject or reduced
context lines, or running "patch"), an advice on _what_ to do is of
secondary importance between the two.  Perhaps we can postpone the
discussion on that and first update the code to tell _where_ the
patch is to the user?  That would be an improvement from the current
codebase no matter what your faviourite workflow is.

[PATCH v2] git-am: indicate where a failed patch is to be found.

[Paul Gortmaker]

If git am fails to apply something, the end user may need
to know where to find the patch.  This is normally known for
a single patch, but if the user is processing a mbox with
many patches, they may not have a single broken out patch
handy.  So, provide a helpful hint as to where they can
find the patch to do some sort of manual fixup, if we
are processing a mbox with more than one patch in it.

Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
[v2: drop text suggesting what to do with failed patch; only
 emit the help text if we are processing mbox with multi patches]

 git-am.sh |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/git-am.sh b/git-am.sh
index f8b7a0c..20b3b73 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -855,6 +855,11 @@ did you forget to use 'git add'?"
 	if test $apply_status != 0
 	then
 		eval_gettextln 'Patch failed at $msgnum $FIRSTLINE'
+		if test $patch_format = mbox && test "$last" -ne "1"
+		then
+			eval_gettextln "You can find the copy of the patch that failed here:
+   $dotest/patch"
+		fi
 		stop_here_user_resolve $this
 	fi
 
-- 
1.7.9.7

Re: [PATCH v2] git-am: indicate where a failed patch is to be found.

[Junio C Hamano]

Paul Gortmaker <paul.gortmaker@windriver.com> writes:

> If git am fails to apply something, the end user may need
> to know where to find the patch.  This is normally known for
> a single patch, but if the user is processing a mbox with
> many patches, they may not have a single broken out patch
> handy.  So, provide a helpful hint as to where they can
> find the patch to do some sort of manual fixup, if we
> are processing a mbox with more than one patch in it.

I would rather see this done even for a single patch mbox.  The
patch that was fed to "git apply" by "git am" and failed to apply is
that one, not the one in the mbox you gave "git am".  The latter may
be ungrokkable with GNU patch or "git apply", if the original was
sent in Quoted-Printable and such MIME funnies, which is the whole
point of having a separate file there for "git am", instead of
feeding the original.

I am not sure if we should limit $patch_format to mbox, but I think
showing this unconditionally regardless of mbox/stgit/hg will teach
the user only one location to remember, so perhaps like this?

 Documentation/config.txt | 3 +++
 git-am.sh                | 4 ++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 0e1168c..b1f0a75 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -143,6 +143,9 @@ advice.*::
 		Advice shown when you used linkgit:git-checkout[1] to
 		move to the detach HEAD state, to instruct how to create
 		a local branch after the fact.
+	amWorkDir::
+		Advice that shows the location of the patch file when
+		linkgit:git-am[1] fails to apply it.
 --
 
 core.fileMode::
diff --git a/git-am.sh b/git-am.sh
index dc48f87..f1ae932 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -834,9 +834,9 @@ did you forget to use 'git add'?"
 	if test $apply_status != 0
 	then
 		eval_gettextln 'Patch failed at $msgnum $FIRSTLINE'
-		if test $patch_format = mbox && test "$last" -ne "1"
+		if test "$(git config --bool advice.amworkdir)" != false
 		then
-			eval_gettextln "You can find the copy of the patch that failed here:
+			eval_gettextln "The copy of the patch that failed is found in:
    $dotest/patch"
 		fi
 		stop_here_user_resolve $this

Re: [PATCH v2] git-am: indicate where a failed patch is to be found.

[Paul Gortmaker]

On 12-07-13 03:58 PM, Junio C Hamano wrote:
> Paul Gortmaker <paul.gortmaker@windriver.com> writes:
> 
>> If git am fails to apply something, the end user may need
>> to know where to find the patch.  This is normally known for
>> a single patch, but if the user is processing a mbox with
>> many patches, they may not have a single broken out patch
>> handy.  So, provide a helpful hint as to where they can
>> find the patch to do some sort of manual fixup, if we
>> are processing a mbox with more than one patch in it.
> 
> I would rather see this done even for a single patch mbox.  The

OK, I got the opposite impression from your prev. mail when
you mentioned that I hadn't limited the message output at all.

I'm fine with the changes you've proposed below, and can squash that
into a v3 and resend again.

Paul.
--

> patch that was fed to "git apply" by "git am" and failed to apply is
> that one, not the one in the mbox you gave "git am".  The latter may
> be ungrokkable with GNU patch or "git apply", if the original was
> sent in Quoted-Printable and such MIME funnies, which is the whole
> point of having a separate file there for "git am", instead of
> feeding the original.
> 
> I am not sure if we should limit $patch_format to mbox, but I think
> showing this unconditionally regardless of mbox/stgit/hg will teach
> the user only one location to remember, so perhaps like this?
> 
>  Documentation/config.txt | 3 +++
>  git-am.sh                | 4 ++--
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 0e1168c..b1f0a75 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -143,6 +143,9 @@ advice.*::
>  		Advice shown when you used linkgit:git-checkout[1] to
>  		move to the detach HEAD state, to instruct how to create
>  		a local branch after the fact.
> +	amWorkDir::
> +		Advice that shows the location of the patch file when
> +		linkgit:git-am[1] fails to apply it.
>  --
>  
>  core.fileMode::
> diff --git a/git-am.sh b/git-am.sh
> index dc48f87..f1ae932 100755
> --- a/git-am.sh
> +++ b/git-am.sh
> @@ -834,9 +834,9 @@ did you forget to use 'git add'?"
>  	if test $apply_status != 0
>  	then
>  		eval_gettextln 'Patch failed at $msgnum $FIRSTLINE'
> -		if test $patch_format = mbox && test "$last" -ne "1"
> +		if test "$(git config --bool advice.amworkdir)" != false
>  		then
> -			eval_gettextln "You can find the copy of the patch that failed here:
> +			eval_gettextln "The copy of the patch that failed is found in:
>     $dotest/patch"
>  		fi
>  		stop_here_user_resolve $this
> 

Re: [PATCH v2] git-am: indicate where a failed patch is to be found.

[Junio C Hamano]

Paul Gortmaker <paul.gortmaker@windriver.com> writes:

> I'm fine with the changes you've proposed below,...

Here is what I committed for today's integration run.  Will be
pushed out on 'pu'.

Thanks.

-- >8 --
From: Paul Gortmaker <paul.gortmaker@windriver.com>
Date: Fri, 13 Jul 2012 11:51:30 -0400
Subject: [PATCH] am: indicate where a failed patch is to be found

If "git am" fails to apply something, the end user may need to know
where to find the patch that failed to apply, so that the user can
do other things (e.g. trying "GNU patch" on it, running "diffstat"
to see what it tried to change, etc.)  The input to "am" may have
contained more than one patch, or the message may have been MIME
encoded, and knowing what the user fed to "am" does not help very
much for this purpose.

Also introduce advice.amworkdir configuration to allow people who
learned where to look to squelch this message.

Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/config.txt | 3 +++
 git-am.sh                | 5 +++++
 2 files changed, 8 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 0e1168c..b1f0a75 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -143,6 +143,9 @@ advice.*::
 		Advice shown when you used linkgit:git-checkout[1] to
 		move to the detach HEAD state, to instruct how to create
 		a local branch after the fact.
+	amWorkDir::
+		Advice that shows the location of the patch file when
+		linkgit:git-am[1] fails to apply it.
 --
 
 core.fileMode::
diff --git a/git-am.sh b/git-am.sh
index cb833e2..f1ae932 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -834,6 +834,11 @@ did you forget to use 'git add'?"
 	if test $apply_status != 0
 	then
 		eval_gettextln 'Patch failed at $msgnum $FIRSTLINE'
+		if test "$(git config --bool advice.amworkdir)" != false
+		then
+			eval_gettextln "The copy of the patch that failed is found in:
+   $dotest/patch"
+		fi
 		stop_here_user_resolve $this
 	fi
 
-- 
1.7.11.2

[PATCH] Re: git-am: indicate where a failed patch is to be found.

[Nicolas Sebrecht]

The 12/07/12, Junio C Hamano wrote:

> It does not matter at all that 0001-foo.patch only has a single
> patch.  If you are going to fix up the patch after you saw "git am"
> failed, you will be fixing .git/rebase-apply/patch with your editor
> and re-run "git am" without arguments, at which point "git am" will
> not look at your 0001-foo.patch file at all.

Hugh! Didn't know that.

Is it actually expected from users to manually edit
.git/rebase-apply/patch path? I can't find any reference about that in
the documentation and it really sounds like interfering with the git
internals.

Shouldn't git-am/git-rebase expose this to the user (I'm thinking about
something like

  git am --edit-offending-patch
  git am --fix-patch

)?

-- 
Nicolas Sebrecht

Re: [PATCH] Re: git-am: indicate where a failed patch is to be found.

[Junio C Hamano]

Nicolas Sebrecht <nicolas.s.dev@gmx.fr> writes:

> The 12/07/12, Junio C Hamano wrote:
>
>> It does not matter at all that 0001-foo.patch only has a single
>> patch.  If you are going to fix up the patch after you saw "git am"
>> failed, you will be fixing .git/rebase-apply/patch with your editor
>> and re-run "git am" without arguments, at which point "git am" will
>> not look at your 0001-foo.patch file at all.
>
> Hugh! Didn't know that.
>
> Is it actually expected from users to manually edit
> .git/rebase-apply/patch path? I can't find any reference about that in
> the documentation and it really sounds like interfering with the git
> internals.
>
> Shouldn't git-am/git-rebase expose this to the user (I'm thinking about
> something like
>
>   git am --edit-offending-patch
>   git am --fix-patch

I doubt it would be very useful.  As Paul says, it is a powerful way
to work, but it is not for everybody, and more importantly, it is
not the only way to work with the patch, once the user knows where
it is.

The first problem before any of that is that we didn't tell the user
where the patch is.  You can re-run "git am" with different options
like reject, "-3", and/or with a reduced context and many cases are
handled without having to know where the patch is at all, but if the
user starts wanting to know where the patch is because she wants to
do things beyond that, we should just tell her where it is, instead
of adding a yet another option to run an editor on it, still without
telling her where it is.

Re: [PATCH] git-am: indicate where a failed patch is to be found.

[Jeff King]

On Thu, Jul 12, 2012 at 02:32:01PM -0400, Paul Gortmaker wrote:

> In case it helps any, a brief summary of my workflow is this:
> 
> git am /tmp/mbox
> <some random fail halfway in the queue>
> patch -p1 --dry-run < .git/rebase-apply/patch
> # gauge status.  Is patch really invalid, or already applied?
> # already applied; "git am --skip"
> # no, if valid, but with minor issues, apply what we can.
> patch -p1 < .git/rebase-apply/patch
> # manually deal with rejects (typically with wiggle)
> git add any_new_files
> git add -u
> git am -r

This does not in any way address your patch, but you may find it
helpful. My usual next step after "git am" fails is to run "git am -3"
and have it do a 3-way merge. This can sometimes resolve issues
entirely, and when conflicts remain, they are placed in the file with
the usual conflict markers (and the conflicted files are marked in the
index, so you can even use "git mergetool" to help you run an external
tool like xxdiff).

-Peff