Why doesn't git-apply remove empty file

Why doesn't git-apply remove empty file

[Francis Moreau]

Hi,

Or at least has an option to do so ?

thanks
-- 
Francis

Re: Why doesn't git-apply remove empty file

[Junio C Hamano]

If you want to create an empty file, the patch would look like this:

        diff --git a/bar b/bar
        index 257cc56..e69de29 100644
        --- a/bar
        +++ b/bar
        @@ -1 +0,0 @@
        -foo

On the other hand, if you want to remove an empty file, the patch would
look like this:

        diff --git a/bar b/bar
        deleted file mode 100644
        index 257cc56..0000000
        --- a/bar
        +++ /dev/null
        @@ -1 +0,0 @@
        -foo

A patch generated by non-git tools would lack "index" and "deleted" lines,
but they will still have difference between "bar" and "/dev/null" on the
postimage filename (i.e. "+++" line), and git-apply leaves an empty file
for the former, and removes the file for the latter patch.

Re: Why doesn't git-apply remove empty file

[Francis Moreau]

Hello,

On Wed, Aug 13, 2008 at 2:25 AM, Junio C Hamano <gitster@pobox.com> wrote:
> On the other hand, if you want to remove an empty file, the patch would
> look like this:
>
>        diff --git a/bar b/bar
>        deleted file mode 100644
>        index 257cc56..0000000
>        --- a/bar
>        +++ /dev/null
>        @@ -1 +0,0 @@
>        -foo
>
> A patch generated by non-git tools would lack "index" and "deleted" lines,
> but they will still have difference between "bar" and "/dev/null" on the
> postimage filename (i.e. "+++" line), and git-apply leaves an empty file
> for the former, and removes the file for the latter patch.
>

Hm this is somehow wrong, please see:

    $ mkdir a b
    $ date > a/f
    $ diff -Nurp a/f b/f
    --- a/f 2008-08-13 09:27:29.000000000 +0200
    +++ b/f 1970-01-01 01:00:00.000000000 +0100
    @@ -1 +0,0 @@
    -Wed Aug 13 09:27:29 CEST 2008

So '/dev/null' doesn't appear here. I think patch(1) uses the date of
b/f for removing
the file.

If we keep going on:

    $ diff -Nurp a b > test.patch
    $ ( cd a && git apply ../test.patch )
    $ ls a
    f
    $ cat a/f
    $

of course patch(1) does remove the file.

Thanks
-- 
Francis

Re: Why doesn't git-apply remove empty file

[René Scharfe]

Francis Moreau schrieb:
>     $ mkdir a b
>     $ date > a/f
>     $ diff -Nurp a/f b/f
>     --- a/f 2008-08-13 09:27:29.000000000 +0200
>     +++ b/f 1970-01-01 01:00:00.000000000 +0100
>     @@ -1 +0,0 @@
>     -Wed Aug 13 09:27:29 CEST 2008
> 
> So '/dev/null' doesn't appear here. I think patch(1) uses the date of
> b/f for removing
> the file.
> 
> If we keep going on:
> 
>     $ diff -Nurp a b > test.patch
>     $ ( cd a && git apply ../test.patch )
>     $ ls a
>     f
>     $ cat a/f
>     $
> 
> of course patch(1) does remove the file.

I bet you are using GNU patch.  It removes files that are empty after
patching and you need to specify --posix to make it keep empty files.

Larry Wall's original version of patch keeps empty files by default and
you need to use the option option -E (or --remove-empty-files) to make
it remove them.

René

Re: Why doesn't git-apply remove empty file

[Linus Torvalds]

On Wed, 13 Aug 2008, René Scharfe wrote:
> 
> I bet you are using GNU patch.  It removes files that are empty after
> patching and you need to specify --posix to make it keep empty files.

GNU patch' behavior wrt empty files is a bit more complex than that. It's 
true that you can disable it all with the POSIX mode (not that anybody 
ever does), but it's not an unconditional removal, I think.

It does look at the date of the destination if there is one, ie according 
to the man-page:

      "You can create a file by sending out a diff that compares /dev/null  or
       an empty file dated the Epoch (1970-01-01 00:00:00 UTC) to the file you
       want to create.  This only works if the file you want to create doesn’t
       exist  already  in  the target directory.  Conversely, you can remove a
       file by sending out a context diff that compares the file to be deleted
       with  an  empty  file dated the Epoch.  The file will be removed unless
       patch is conforming to POSIX and the -E or --remove-empty-files  option
       is  not  given.  An easy way to generate patches that create and remove
       files is to use GNU diff’s -N or --new-file option."

and no, git never did that file date thing, so git acts differently from 
GNU patch in this thing (as in so many others, for that matter).

I don't think it would necessarily be wrong to try to emulate GNU patch 
for the case where git is guessing at removal, though (ie for the 
"traditional diff" case - for a "git diff", the removal question is 
unambiguous thanks to the git extensions, of course).

That said, I'm also not personally very motivated to add yet another odd 
GNU patch behavior quirk. Especially as we very much try to avoid parsing 
that insane and not-well-specified date format anyway, and just ignore it. 
But if somebody sends out a tested patch to add such logic, I wouldn't 
think it _wrong_ either.

			Linus

Re: Why doesn't git-apply remove empty file

[Francis Moreau]

On Thu, Aug 14, 2008 at 1:09 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> and no, git never did that file date thing, so git acts differently from
> GNU patch in this thing (as in so many others, for that matter).
>

Well patch(1) is so used out there that makes git-apply often do the
wrong thing for such corner cases when applying a patch made by
patch(1).

Maybe git-apply would be more friendly regarding patch(1) if it has an
option to emulate GNU patch for some situations. Or if this means
adding too many quirks in git-apply code, maybe give the possibity for
git-am to use patch(1) instead of git-apply.

-- 
Francis

Re: Why doesn't git-apply remove empty file

[Linus Torvalds]

On Thu, 14 Aug 2008, Francis Moreau wrote:
> 
> Well patch(1) is so used out there that makes git-apply often do the
> wrong thing for such corner cases when applying a patch made by
> patch(1).
> 
> Maybe git-apply would be more friendly regarding patch(1) if it has an
> option to emulate GNU patch for some situations. Or if this means
> adding too many quirks in git-apply code, maybe give the possibity for
> git-am to use patch(1) instead of git-apply.

The thing is, "patch" is a total piece of utterly unbelievable SH*T.

git-apply acts differently, yes, but it acts differently for a damn good 
reason. No, you cannot replace git-apply with that horrible crap that is 
GNU patch.

Some of the reasons are purely trivial implementation issues:

 - git-apply knows about the index, and knows about updating it properly, 
   including tracking new files automatically.

That's an important thing, but yeah, it's an implementation issue.

The other things that git-apply do right are much more fundamental:

 - git apply doesn't leave half-applied state turds around when a patch 
   fails.

   People who actually use "patch" for large projects will know the pain 
   here: if a diff fails in the middle, GNU patch will have applied the 
   previous parts (including to other files), and it's now your problem to 
   fix it up. There's no way to do an all-or-nothing patch, which is often 
   a huge requirement.

 - git apply doesn't guess (unless you really tell it to, and even then it 
   will guess a whole lot less than GNU patch). If a "git apply" succeeds, 
   it was probably good. If a GNU patch invocation succeeds, it might have 
   been total and utter crap, but hey, it tried to apply that piece of 
   shit very aggressively even when it made no sense and the context 
   didn't actually match even _remotely_.

   Yeah, context diffs can still mis-apply even with git apply, but they 
   do so a hell of a lot less than with GNU patch, and if you want it to 
   just generate a random end result, you at least have to _ask_ for it.

So no. There's no way in hell that git am should use GNU patch.

But as mentioned, if somebody wants to parse the dates, we could do that. 

			Linus

Re: Why doesn't git-apply remove empty file

[Linus Torvalds]

On Thu, 14 Aug 2008, Linus Torvalds wrote:
>
>   There's no way to do an all-or-nothing patch, which is often 
>    a huge requirement.

Side note: there's a way to do a "nothing or nothing" thing, and then test 
the return value of whether the do-nothing was successful or not.

So a reasonably common operation thing for scripting GNU patch is to 
literally do

	patch --dry-run

first to see if the patch will succeed.

..or to play games with backup and reject files after-the-fact, of course.

			Linus

Re: Why doesn't git-apply remove empty file

[Stephan Beyer]

Hi,

while at the git-apply topic...

Linus Torvalds wrote:
> On Thu, 14 Aug 2008, Linus Torvalds wrote:
> >
> >   There's no way to do an all-or-nothing patch, which is often 
> >    a huge requirement.
[...]
> 
> ..or to play games with backup and reject files after-the-fact, of course.

What I missed when I first used git-apply (git-am) with some not-so-
well-done patches was something like a "simulated merge" (of course,
only when you ask for it), i.e. something like a user-friendly
--reject behavior:
Instead of generating reject files it puts conflict markers into the file.
(If no context matches at all, then perhaps just insert them at the lines
that the hunk header says.)
And then declaring the files as "unmerged", so that you can see it in
git status.

I've seen two or three people asking on #git for such a feature, so
it looked like I am not the only one who misses this. But yet I've had
no time to implement such a thing.  Or is it even there, hidden?

Regards,
  Stephan

-- 
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F

Re: Why doesn't git-apply remove empty file

[Jeff King]

On Thu, Aug 14, 2008 at 10:21:59PM +0200, Stephan Beyer wrote:

> Instead of generating reject files it puts conflict markers into the file.
> (If no context matches at all, then perhaps just insert them at the lines
> that the hunk header says.)
> And then declaring the files as "unmerged", so that you can see it in
> git status.

I use "git am -3" for that. Then you get conflict markers, or you can
use git-mergetool.

-Peff

Re: Why doesn't git-apply remove empty file

[Stephan Beyer]

Hi,

Jeff King wrote:
> On Thu, Aug 14, 2008 at 10:21:59PM +0200, Stephan Beyer wrote:
> 
> > Instead of generating reject files it puts conflict markers into the file.
> > (If no context matches at all, then perhaps just insert them at the lines
> > that the hunk header says.)
> > And then declaring the files as "unmerged", so that you can see it in
> > git status.
> 
> I use "git am -3" for that. Then you get conflict markers, or you can
> use git-mergetool.

git am -3 looks for the base blob in the "index" line of the patch.

Now imagine you do not have this blob in git, e.g. because you've not
fetched some large side branch from another repo.
Then git am -3 won't help, too. (Yes, in this example fetching the
branch may help, but there may be other examples.)

Regards,
  Stephan

-- 
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F

Re: Why doesn't git-apply remove empty file

[Jeff King]

On Thu, Aug 14, 2008 at 11:10:34PM +0200, Stephan Beyer wrote:

> git am -3 looks for the base blob in the "index" line of the patch.
> 
> Now imagine you do not have this blob in git, e.g. because you've not
> fetched some large side branch from another repo.

Ah, you're right. That is much trickier (though it can still be used in
some cases, depending on how broken the patch is).

> Then git am -3 won't help, too. (Yes, in this example fetching the
> branch may help, but there may be other examples.)

An easy example that breaks: the patch submitter didn't use git at all,
so there is no branch to fetch from. :)

-Peff

Re: Why doesn't git-apply remove empty file

[Linus Torvalds]

On Thu, 14 Aug 2008, Stephan Beyer wrote:
> 
> What I missed when I first used git-apply (git-am) with some not-so-
> well-done patches was something like a "simulated merge" (of course,
> only when you ask for it), i.e. something like a user-friendly
> --reject behavior:

It's really hard to do. Largely impossible, I think. The reason is that 
when a patch fails, you - by definition - don't really know where it is 
supposed to apply and what the original code is. Because if you did, the 
patch wouldn't fail!

Git *can* do it (git am -3, as people have said), when you have a git 
patch, and the patch indicates the original blob. Then git can just see 
what the original state was, reconstruct the patched file, and do a normal 
three-way merge. But it's not doable in general.

Now, we could do other things, like try to _search_ for an original blob 
that it applies cleanly to (or even have a "guessing mode" that does a 
really rough patch-application with the GNU patch defaults and then does a 
three-way merge of a fudged version with no fuzz as the base), so I'm not 
saying that it would be fundamentally impossible to try to go further than 
what we do now, but it's definitely not trivial to do.

> Instead of generating reject files it puts conflict markers into the file.
> (If no context matches at all, then perhaps just insert them at the lines
> that the hunk header says.)
> And then declaring the files as "unmerged", so that you can see it in
> git status.

I do agree that it would be very cool. I'm so used to editing patches to 
match the target files by hand that I don't personally mind. I absolutely 
detest reject files personally because of the way they mean that you have 
a half-way done thing, so I will literally always edit the original patch 
in place instead.

But I agree that what you describe would be absolutely lovely.

			Linus

Re: Why doesn't git-apply remove empty file

[Francis Moreau]

On Thu, Aug 14, 2008 at 9:57 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> The thing is, "patch" is a total piece of utterly unbelievable SH*T.
>
> git-apply acts differently, yes, but it acts differently for a damn good
> reason. No, you cannot replace git-apply with that horrible crap that is
> GNU patch.
>
> Some of the reasons are purely trivial implementation issues:
>
>  - git-apply knows about the index, and knows about updating it properly,
>   including tracking new files automatically.
>
> That's an important thing, but yeah, it's an implementation issue.
>
> The other things that git-apply do right are much more fundamental:
>

agreed with all of them...

> So no. There's no way in hell that git am should use GNU patch.
>

Just for clarification, I'm not saying to replace git-apply by patch in git-am !

But in few situations it might be better to use GNU patch for git-am.
So we could have such option:

              $ git am --applier="/usr/bin/patch" ...

thanks
-- 
Francis