git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Junio C Hamano <gitster@pobox.com>
Cc: david@lang.hm, git@vger.kernel.org, rob@landley.net
Subject: Re: possible bug in git apply?
Date: Sun, 5 Aug 2007 09:59:03 -0700 (PDT)	[thread overview]
Message-ID: <alpine.LFD.0.999.0708050949220.5037@woody.linux-foundation.org> (raw)
In-Reply-To: <7vvebuh8g8.fsf@assigned-by-dhcp.cox.net>



On Sat, 4 Aug 2007, Junio C Hamano wrote:
> >
> > Does this fix it? Totally untested, but it _looks_ obvious enough..
> 
> That would regress the fix made in aea19457, I am afraid.

The fix in aea19457 is broken anyway.

Why? 

That whole "do it in two phases" thing breaks it.

What can happen is that you have a directory with 100 files, and:
 - a patch modifies 99 of them
 - and removes one

What happens is that during phase 0, we'll remove all the files (and *not* 
write new ones), and then beause the last patch entry is a removal, we'll 
also remove the directory (which succeeds, because all the files that got 
modified are all long gone).

Then, in phase 1, we'll re-create the files that we modified, and create a 
whole new directory.

IOW, as far as I can see we _already_ delete and then recreate the 
directory structure under some circumstances.

I just extended it to also do it for "rename" and not just delete, since a 
rename may be renaming it to another directory.

So I'd say that my patch is a clear improvement on the current situation. 

That said, if we really wanted to get it right, we should do this as a 
three-phase thing: (1) remove old files (2) create new files (3) for all 
removals and renames, try to remove source directories that might have 
become empty.

That would fix it properly and for all cases.

		Linus

  parent reply	other threads:[~2007-08-05 17:00 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-08-04 19:45 possible bug in git apply? david
2007-08-04 20:00 ` Junio C Hamano
2007-08-04 20:08 ` David Kastrup
2007-08-05  4:48 ` Linus Torvalds
2007-08-05  4:53   ` Linus Torvalds
2007-08-05  5:11   ` Junio C Hamano
2007-08-05  7:55     ` David Kastrup
2007-08-05 16:59     ` Linus Torvalds [this message]
2007-08-05 17:53       ` David Kastrup
2007-08-05 18:18         ` Linus Torvalds
2007-08-05 18:50           ` David Kastrup
2007-08-05 19:20             ` Linus Torvalds
2007-08-05 19:37               ` David Kastrup
2007-08-06  8:29       ` Junio C Hamano
2007-08-06  9:37         ` 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=alpine.LFD.0.999.0708050949220.5037@woody.linux-foundation.org \
    --to=torvalds@linux-foundation.org \
    --cc=david@lang.hm \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=rob@landley.net \
    /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;
as well as URLs for NNTP newsgroup(s).