Git development
 help / color / mirror / Atom feed
From: Mikko Perttunen <mikko.perttunen@kapsi.fi>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Tuomas Tynkkynen <tuomas.tynkkynen@iki.fi>,
	Tomeu Vizoso <tomeu.vizoso@collabora.com>
Subject: Re: Kernel bug caused by 'git apply' misapplying a patch with ambiguous chunk
Date: Thu, 02 Apr 2015 19:13:08 +0300	[thread overview]
Message-ID: <551D6A94.3050707@kapsi.fi> (raw)
In-Reply-To: <xmqqy4majzb4.fsf@gitster.dls.corp.google.com>

On 04/02/2015 06:59 PM, Junio C Hamano wrote:
> Mikko Perttunen <mikko.perttunen@kapsi.fi> writes:
>
>> Of course this is probably rather hard to fix on the applying end; but
>> perhaps format-patch could check for ambiguous chunks and either warn
>> the user or increase the context size automatically, or apply could
>> warn about the chunk being ambiguous?
>
> Interesting thought.  Let me rephrase to make sure I got your
> thought process correctly.
>
> Imagine you started from an original that had two cut-and-pasted
> codeblocks A and B in the same file and updated one of them, say A,
> and then sent out the patch that turns A into A1.
>
> Meanwhile, somebody started from the same original and updated the
> same codeblock A in the upstream already to A2. Your patch applies
> cleanly to codeblock B and turns it to A1, which is a mispatch.  And
> you cannot even detect the problem while applying.
>
> But if you are starting from the original with idential A and B,
> format-patch can see that the resulting patch to turn A to A1 can be
> misunderstood to be a patch to change B to A1 instead. So in that
> case, you _could_ detect.
>
> But imagine if you started from an original that had A and C, that
> are clearly different.  Your change turns A into A1.  In the
> meantime, the upstream started from the same original, and changed C
> into B that looks identical to A.
>
> The same thing would happen to your patch when you try to apply it.
> "git apply" could try to diagnose this situation and warn.  But you
> cannot check when your format-patch produces a patch that turns <A,C>
> into <A1,C>, as there is no ambiguity in the original.
>
> So,
>
>   - format-patch could try to help, but it won't be a complete
>     solution.
>
>   - apply could try to help, but it won't be a complete solution.
>
> I am not sure if having "both" would make it complete, but I doubt
> it.
>
>

I agree. I think you can also create a situation where neither would 
detect the problem, if the upstream changes C such that A->A1 can be 
applied on it and at the same time changes A such that A->A1 no longer 
applies. However, such a situation should be rare, so I think these 
checks would be useful.

In this case, the ambiguity existed the whole time both on the 
submitter's end and in upstream. Upstream just added more stuff into the 
file causing the line numbers to shift. So in this case a check in 
either subcommand would have caught it.

Mikko

      reply	other threads:[~2015-04-02 16:13 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-02 15:25 Kernel bug caused by 'git apply' misapplying a patch with ambiguous chunk Mikko Perttunen
2015-04-02 15:59 ` Junio C Hamano
2015-04-02 16:13   ` Mikko Perttunen [this message]

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=551D6A94.3050707@kapsi.fi \
    --to=mikko.perttunen@kapsi.fi \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=tomeu.vizoso@collabora.com \
    --cc=tuomas.tynkkynen@iki.fi \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox