Re: [PATCH] builtin-apply: keep information about files to be deleted

From: Michał Kiedrowicz <michal.kiedrowicz@gmail.com>
Date: Apr 17, 2009, 17:23 UTC
Message-ID: <20090417192324.3a888abf@gmail.com>
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ł:

Show 16 quoted lines

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