All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paul Gortmaker <paul.gortmaker@windriver.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: <git@vger.kernel.org>
Subject: Re: [PATCH] git-am: indicate where a failed patch is to be found.
Date: Fri, 13 Jul 2012 13:40:30 -0400	[thread overview]
Message-ID: <50005D8E.1020407@windriver.com> (raw)
In-Reply-To: <7v629sbvh8.fsf@alter.siamese.dyndns.org>

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.
> 

  reply	other threads:[~2012-07-13 17:40 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=50005D8E.1020407@windriver.com \
    --to=paul.gortmaker@windriver.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.