Re: [PATCH] git-am: indicate where a failed patch is to be found.
On 12-07-12 01:45 PM, Junio C Hamano wrote: Paul Gortmaker paul.gortma...@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. -- -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-am: indicate where a failed patch is to be found.
Paul Gortmaker paul.gortma...@windriver.com writes: On 12-07-12 01:45 PM, Junio C Hamano wrote: Paul Gortmaker paul.gortma...@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. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-am: indicate where a failed patch is to be found.
On 12-07-12 02:53 PM, Junio C Hamano wrote: Paul Gortmaker paul.gortma...@windriver.com writes: On 12-07-12 01:45 PM, Junio C Hamano wrote: Paul Gortmaker paul.gortma...@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. --- -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-am: indicate where a failed patch is to be found.
Paul Gortmaker paul.gortma...@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. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-am: indicate where a failed patch is to be found.
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 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-am: indicate where a failed patch is to be found.
Paul Gortmaker paul.gortma...@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. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
