All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michał Kiedrowicz" <michal.kiedrowicz@gmail.com>
To: git@vger.kernel.org
Subject: Re: [PATCH] builtin-apply: keep information about files to be deleted
Date: Fri, 17 Apr 2009 19:23:24 +0200	[thread overview]
Message-ID: <20090417192324.3a888abf@gmail.com> (raw)
In-Reply-To: <7v1vrwdyxx.fsf@gitster.siamese.dyndns.org>


W dniu 13 kwietnia 2009 23:30 użytkownik Junio C Hamano
<gitster@pobox.com> napisał:
>  
> >
> > As far as I understand the code, diffs are applied independently
> > (for every file apply_patch() is called) and for every apply_patch()
> > call fn_table is cleared. So situation you described in only
> > possible in a *single* diff and I don't think it is possible to
> > happen.  
>
> Yes, one invocation of "git format-patch -1" will not produce such a
> situation.
>
> A single diff file that is concatenation of two "git format-patch -1"
> output (or just a plain-old "diff -ru" output from outside git,
> perhaps managed in quilt) was what introduced fn_table mechanism.
>  Apparently people use "git apply" to apply such a patch.
>  

I have been thinking about that and IMO something is not right in
handling multiple patches. I'm still new to git, so I may be wrong.
Look:

Suppose I have 3 patches:

patch #1: modify A
patch #2: rename A to B
patch #3: modify B

These patches will be applied correctly.

But, if I swap patches #1 and #3, none of them will be applied. This
is because of 2 rules, implemented in add_to_fn_table():

1. If a file was renamed/deleted, applying a patch is not possible.
2. If a file is new/modified, applying a patch is possible.

They seem reasonable. In previous example, file A comes under rule #1
and file B under rule #2. However, there are some cases when these two
rules may cause problems:

patch #1: rename A to B
patch #2: rename C to A
patch #3: modify A

Should patch #3 modify B (which was A) or A (which was C)?

patch #1: rename A to B
patch #2: rename B to A
patch #3: modify A
patch #4: modify B

Which files should be patched by #3 and #4?

In my opinion both #3 and #4 should fail (or both should succeed) --
with my patch only #3 will work and #4 will be rejected, because in #2
B was marked as deleted.

  reply	other threads:[~2009-04-17 17:32 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-04-11 19:31 [PATCH] builtin-apply: keep information about files to be deleted Michał Kiedrowicz
2009-04-13 13:51 ` Michał Kiedrowicz
2009-04-13 18:51 ` Junio C Hamano
2009-04-13 21:03   ` Michał Kiedrowicz
2009-04-13 21:30     ` Junio C Hamano
2009-04-17 17:23       ` Michał Kiedrowicz [this message]
2009-04-18  4:59         ` Junio C Hamano
2009-04-18 11:27           ` Andreas Ericsson
2009-04-18 19:56             ` Junio C Hamano
2009-04-18 20:58           ` Michał Kiedrowicz
2009-04-18 21:03             ` [PATCH] tests: make test-apply-criss-cross-rename more robust Michał Kiedrowicz
2009-04-18 22:05             ` [PATCH] builtin-apply: keep information about files to be deleted Junio C Hamano

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=20090417192324.3a888abf@gmail.com \
    --to=michal.kiedrowicz@gmail.com \
    --cc=git@vger.kernel.org \
    /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.