* [PATCH] git-am: indicate where a failed patch is to be found. @ 2012-07-12 15:50 Paul Gortmaker 2012-07-12 17:45 ` Junio C Hamano 0 siblings, 1 reply; 16+ messages in thread From: Paul Gortmaker @ 2012-07-12 15:50 UTC (permalink / raw) To: git; +Cc: 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 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] git-am: indicate where a failed patch is to be found. 2012-07-12 15:50 [PATCH] git-am: indicate where a failed patch is to be found Paul Gortmaker @ 2012-07-12 17:45 ` Junio C Hamano 2012-07-12 18:32 ` Paul Gortmaker 0 siblings, 1 reply; 16+ messages in thread From: Junio C Hamano @ 2012-07-12 17:45 UTC (permalink / raw) To: Paul Gortmaker; +Cc: git 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). ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] git-am: indicate where a failed patch is to be found. 2012-07-12 17:45 ` Junio C Hamano @ 2012-07-12 18:32 ` Paul Gortmaker 2012-07-12 18:53 ` Junio C Hamano 2012-07-12 20:33 ` [PATCH] " Jeff King 0 siblings, 2 replies; 16+ messages in thread From: Paul Gortmaker @ 2012-07-12 18:32 UTC (permalink / raw) To: Junio C Hamano; +Cc: git 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. -- > > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] git-am: indicate where a failed patch is to be found. 2012-07-12 18:32 ` Paul Gortmaker @ 2012-07-12 18:53 ` Junio C Hamano 2012-07-12 19:36 ` Paul Gortmaker 2012-07-12 21:18 ` [PATCH] " Nicolas Sebrecht 2012-07-12 20:33 ` [PATCH] " Jeff King 1 sibling, 2 replies; 16+ messages in thread From: Junio C Hamano @ 2012-07-12 18:53 UTC (permalink / raw) To: Paul Gortmaker; +Cc: git 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". ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] git-am: indicate where a failed patch is to be found. 2012-07-12 18:53 ` Junio C Hamano @ 2012-07-12 19:36 ` Paul Gortmaker 2012-07-12 20:00 ` Junio C Hamano 2012-07-12 21:07 ` Junio C Hamano 2012-07-12 21:18 ` [PATCH] " Nicolas Sebrecht 1 sibling, 2 replies; 16+ messages in thread From: Paul Gortmaker @ 2012-07-12 19:36 UTC (permalink / raw) To: Junio C Hamano; +Cc: git 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. --- ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] git-am: indicate where a failed patch is to be found. 2012-07-12 19:36 ` Paul Gortmaker @ 2012-07-12 20:00 ` Junio C Hamano 2012-07-13 17:40 ` Paul Gortmaker 2012-07-12 21:07 ` Junio C Hamano 1 sibling, 1 reply; 16+ messages in thread From: Junio C Hamano @ 2012-07-12 20:00 UTC (permalink / raw) To: Paul Gortmaker; +Cc: git 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. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] git-am: indicate where a failed patch is to be found. 2012-07-12 20:00 ` Junio C Hamano @ 2012-07-13 17:40 ` Paul Gortmaker 2012-07-13 18:06 ` Junio C Hamano 0 siblings, 1 reply; 16+ messages in thread From: Paul Gortmaker @ 2012-07-13 17:40 UTC (permalink / raw) To: Junio C Hamano; +Cc: git 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. > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] git-am: indicate where a failed patch is to be found. 2012-07-13 17:40 ` Paul Gortmaker @ 2012-07-13 18:06 ` Junio C Hamano 0 siblings, 0 replies; 16+ messages in thread From: Junio C Hamano @ 2012-07-13 18:06 UTC (permalink / raw) To: Paul Gortmaker; +Cc: git 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 ;-). ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] git-am: indicate where a failed patch is to be found. 2012-07-12 19:36 ` Paul Gortmaker 2012-07-12 20:00 ` Junio C Hamano @ 2012-07-12 21:07 ` Junio C Hamano 2012-07-13 15:51 ` [PATCH v2] " Paul Gortmaker 1 sibling, 1 reply; 16+ messages in thread From: Junio C Hamano @ 2012-07-12 21:07 UTC (permalink / raw) To: Paul Gortmaker; +Cc: git 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. ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2] git-am: indicate where a failed patch is to be found. 2012-07-12 21:07 ` Junio C Hamano @ 2012-07-13 15:51 ` Paul Gortmaker 2012-07-13 19:58 ` Junio C Hamano 0 siblings, 1 reply; 16+ messages in thread From: Paul Gortmaker @ 2012-07-13 15:51 UTC (permalink / raw) To: git; +Cc: 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 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2] git-am: indicate where a failed patch is to be found. 2012-07-13 15:51 ` [PATCH v2] " Paul Gortmaker @ 2012-07-13 19:58 ` Junio C Hamano 2012-07-13 22:46 ` Paul Gortmaker 0 siblings, 1 reply; 16+ messages in thread From: Junio C Hamano @ 2012-07-13 19:58 UTC (permalink / raw) To: Paul Gortmaker; +Cc: git 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 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2] git-am: indicate where a failed patch is to be found. 2012-07-13 19:58 ` Junio C Hamano @ 2012-07-13 22:46 ` Paul Gortmaker 2012-07-13 23:02 ` Junio C Hamano 0 siblings, 1 reply; 16+ messages in thread From: Paul Gortmaker @ 2012-07-13 22:46 UTC (permalink / raw) To: Junio C Hamano; +Cc: git 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 > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] git-am: indicate where a failed patch is to be found. 2012-07-13 22:46 ` Paul Gortmaker @ 2012-07-13 23:02 ` Junio C Hamano 0 siblings, 0 replies; 16+ messages in thread From: Junio C Hamano @ 2012-07-13 23:02 UTC (permalink / raw) To: Paul Gortmaker; +Cc: git 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 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH] Re: git-am: indicate where a failed patch is to be found. 2012-07-12 18:53 ` Junio C Hamano 2012-07-12 19:36 ` Paul Gortmaker @ 2012-07-12 21:18 ` Nicolas Sebrecht 2012-07-12 21:55 ` Junio C Hamano 1 sibling, 1 reply; 16+ messages in thread From: Nicolas Sebrecht @ 2012-07-12 21:18 UTC (permalink / raw) To: Junio C Hamano; +Cc: Paul Gortmaker, git, 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Re: git-am: indicate where a failed patch is to be found. 2012-07-12 21:18 ` [PATCH] " Nicolas Sebrecht @ 2012-07-12 21:55 ` Junio C Hamano 0 siblings, 0 replies; 16+ messages in thread From: Junio C Hamano @ 2012-07-12 21:55 UTC (permalink / raw) To: Nicolas Sebrecht; +Cc: Paul Gortmaker, git 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. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] git-am: indicate where a failed patch is to be found. 2012-07-12 18:32 ` Paul Gortmaker 2012-07-12 18:53 ` Junio C Hamano @ 2012-07-12 20:33 ` Jeff King 1 sibling, 0 replies; 16+ messages in thread From: Jeff King @ 2012-07-12 20:33 UTC (permalink / raw) To: Paul Gortmaker; +Cc: Junio C Hamano, git 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2012-07-13 23:02 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-07-12 15:50 [PATCH] git-am: indicate where a failed patch is to be found Paul Gortmaker 2012-07-12 17:45 ` Junio C Hamano 2012-07-12 18:32 ` Paul Gortmaker 2012-07-12 18:53 ` Junio C Hamano 2012-07-12 19:36 ` Paul Gortmaker 2012-07-12 20:00 ` Junio C Hamano 2012-07-13 17:40 ` Paul Gortmaker 2012-07-13 18:06 ` Junio C Hamano 2012-07-12 21:07 ` Junio C Hamano 2012-07-13 15:51 ` [PATCH v2] " Paul Gortmaker 2012-07-13 19:58 ` Junio C Hamano 2012-07-13 22:46 ` Paul Gortmaker 2012-07-13 23:02 ` Junio C Hamano 2012-07-12 21:18 ` [PATCH] " Nicolas Sebrecht 2012-07-12 21:55 ` Junio C Hamano 2012-07-12 20:33 ` [PATCH] " Jeff King
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.