Re: being nice to patch(1)

Linus Torvalds [off-list ref] writes:

So I would suggest that in quilt and other systems, you either:

 - strip all headers manually

 - forget about "patch", and use "git-apply" instead that does things 
   right and doesn't screw up like this (and can do rename diffs etc too).

I guess the second choice generally isn't an option, but dammit, 
"git-apply" really is the better program here.

Why not?  git-apply works outside of a git repo ;-)

On Mon, 2 Jul 2007, Junio C Hamano wrote:

Linus Torvalds [off-list ref] writes:

quoted

So I would suggest that in quilt and other systems, you either:

 - strip all headers manually

 - forget about "patch", and use "git-apply" instead that does things 
   right and doesn't screw up like this (and can do rename diffs etc too).

I guess the second choice generally isn't an option, but dammit, 
"git-apply" really is the better program here.

Why not?  git-apply works outside of a git repo ;-)

I was more thinking that people are not necessarily willing to install git 
just to get the "git-apply" program..

		Linus

Hi,

On Mon, 2 Jul 2007, Linus Torvalds wrote:

On Mon, 2 Jul 2007, Junio C Hamano wrote:

quoted

Linus Torvalds [off-list ref] writes:

quoted

So I would suggest that in quilt and other systems, you either:

 - strip all headers manually

 - forget about "patch", and use "git-apply" instead that does things 
   right and doesn't screw up like this (and can do rename diffs etc too).

I guess the second choice generally isn't an option, but dammit, 
"git-apply" really is the better program here.

Why not?  git-apply works outside of a git repo ;-)

I was more thinking that people are not necessarily willing to install git 
just to get the "git-apply" program..

But maybe they would be willing to install git to get that wonderful 
git-apply program, and that wonderful rename-and-mode-aware git-diff, and 
the git-merge-file program, all of which can operate outside of a git 
repository. (Take that, hg!)

Ciao,
Dscho

On 7/3/07, Johannes Schindelin [off-list ref] wrote:

Hi,
On Mon, 2 Jul 2007, Linus Torvalds wrote:

quoted

On Mon, 2 Jul 2007, Junio C Hamano wrote:

quoted

Linus Torvalds [off-list ref] writes:

quoted

So I would suggest that in quilt and other systems, you either:

 - strip all headers manually

 - forget about "patch", and use "git-apply" instead that does things
   right and doesn't screw up like this (and can do rename diffs etc too).

I guess the second choice generally isn't an option, but dammit,
"git-apply" really is the better program here.

Why not?  git-apply works outside of a git repo ;-)

I was more thinking that people are not necessarily willing to install git
just to get the "git-apply" program..

But maybe they would be willing to install git to get that wonderful
git-apply program, and that wonderful rename-and-mode-aware git-diff, and
the git-merge-file program, all of which can operate outside of a git
repository. (Take that, hg!)

How about shipping just these commands as a separate package?
Is that a cray idea?

ciao,
-- 
Paolo
"Tutto cio' che merita di essere fatto,merita di essere fatto bene"
Philip Stanhope IV conte di Chesterfield

Hi,

On Tue, 3 Jul 2007, Paolo Ciarrocchi wrote:

On 7/3/07, Johannes Schindelin [off-list ref] wrote:

quoted

Hi,
On Mon, 2 Jul 2007, Linus Torvalds wrote:

quoted

On Mon, 2 Jul 2007, Junio C Hamano wrote:

quoted

Linus Torvalds [off-list ref] writes:

quoted

So I would suggest that in quilt and other systems, you either:

 - strip all headers manually

 - forget about "patch", and use "git-apply" instead that does things
   right and doesn't screw up like this (and can do rename diffs etc

too).

quoted

quoted

quoted

I guess the second choice generally isn't an option, but dammit,
"git-apply" really is the better program here.

Why not?  git-apply works outside of a git repo ;-)

I was more thinking that people are not necessarily willing to install

git

quoted

just to get the "git-apply" program..

But maybe they would be willing to install git to get that wonderful
git-apply program, and that wonderful rename-and-mode-aware git-diff, and
the git-merge-file program, all of which can operate outside of a git
repository. (Take that, hg!)

How about shipping just these commands as a separate package?
Is that a cray idea?

Heh, all three programs are "builtins", which means that you get almost 
the whole package of git anyway ;-)

Ciao,
Dscho

Johannes Schindelin [off-list ref] writes:

But maybe they would be willing to install git to get that wonderful
git-apply program, and that wonderful rename-and-mode-aware
git-diff, and the git-merge-file program, all of which can operate
outside of a git repository. (Take that, hg!)

As long as git-diff lists all added files in a second non-git dirtree
as "/dev/null" when doing
git-diff --name-status -B -M -C dir1 dir2
its usefulness is limited.

git-diff --name-status -B -M -C dir1 dir2
D	dir1/auctex-11.84/CHANGES
D	dir1/auctex-11.84/COPYING
D	dir1/auctex-11.84/ChangeLog

[...]

D	dir1/auctex-11.84/preview/preview-latex.spec
D	dir1/auctex-11.84/preview/prv-emacs.el
D	dir1/auctex-11.84/preview/prv-install.el
D	dir1/auctex-11.84/tex-site.el.in
D	dir1/auctex-11.84/tex-wizard.el
A	/dev/null
A	/dev/null
R100	dir1/auctex-11.84/images/amstex.xpm	dir2/etc/auctex/images/amstex.xpm
R100	dir1/auctex-11.84/images/bibtex.xpm	dir2/etc/auctex/images/bibtex.xpm
R100	dir1/auctex-11.84/images/dropdown.xpm	dir2/etc/auctex/images/dropdown.xpm

[...]

R100	dir1/auctex-11.84/images/viewdvi.xpm	dir2/etc/auctex/images/viewdvi.xpm
R100	dir1/auctex-11.84/images/viewpdf.xpm	dir2/etc/auctex/images/viewpdf.xpm
R100	dir1/auctex-11.84/images/viewps.xpm	dir2/etc/auctex/images/viewps.xpm
A	/dev/null
A	/dev/null
A	/dev/null
A	/dev/null
A	/dev/null
A	/dev/null

and so on.

git --version
git version 1.5.2.2.565.gde09

-- 
David Kastrup

Hi David,

[please Cc me, since I will be more likely to miss replies if you do not]

On Tue, 3 Jul 2007, David Kastrup wrote:

Johannes Schindelin [off-list ref] writes:

quoted

But maybe they would be willing to install git to get that wonderful 
git-apply program, and that wonderful rename-and-mode-aware git-diff, 
and the git-merge-file program, all of which can operate outside of a 
git repository. (Take that, hg!)

As long as git-diff lists all added files in a second non-git dirtree
as "/dev/null" when doing
git-diff --name-status -B -M -C dir1 dir2
its usefulness is limited.

git-diff --name-status -B -M -C dir1 dir2
D	dir1/auctex-11.84/CHANGES
D	dir1/auctex-11.84/COPYING
D	dir1/auctex-11.84/ChangeLog

[...]

Yes, directories are a problem. There our DWIMery does not really help. 
But there is a solution: say

	git diff --name-status --no-index -B -M -C dir1 dir2

Hth,
Dscho

On Tue, Jul 03, 2007 at 02:21:51PM +0200, Paolo Ciarrocchi wrote:

quoted

But maybe they would be willing to install git to get that wonderful
git-apply program, and that wonderful rename-and-mode-aware git-diff, and
the git-merge-file program, all of which can operate outside of a git
repository. (Take that, hg!)

How about shipping just these commands as a separate package?
Is that a cray idea?

Or people could submit a bug report/feature request/patch to the
patch(1) maintainer.  :-)

						- Ted

[ Paul Eggert added to Cc: I'm not sure he actually maintains "patch" 
  or cares any more, but hopefully he at least knows who does ]

On Tue, 3 Jul 2007, Theodore Tso wrote:

Or people could submit a bug report/feature request/patch to the
patch(1) maintainer.  :-)

Is there such a thing?

The latest official version of patch from GNU is 2.5.4 from 1999, I think.

I'm finding references to 2.5.9 in distributions (from 2003), but 2.5.4 is 
the latest I see on the GNU mirror at kernel.org, and that's also what 
Fedora 7 has too, it seems, so the 2.5.9 thing seems to be something 
unofficial or at least not widely known about..

Anyway, I tried to look at the patch sources, but I had to stop. That 
whole "intuit_diff_type()" function is probably designed as an initiation 
rite for any patch programmers, and to make sure that you have to be 
really serious about wanting to send patches before you can become part of 
the "in crowd". It's "mental hazing".

Yeah, git-apply sources aren't necessarily a thing of great beauty either, 
but in comparison to patch, I think it's a work of art. Of course, part of 
it is that it doesn't try to parse 'ed' scripts etc, but a large part of 
it really is that "patch" is an old program that has grown over time, and 
not seen a lot of cleanups, I suspect.

IOW, I tried to see how easy it would be to dismiss the code that 
takes care of "indent", but it wasn't totally obvious. It's set in many 
different places, and the logic for "skip_this_patch" is a bit confusing.

Anyway, with Paul Eggert Cc'd, maybe he can help us sort it out.

Paul - the issue here isn't actually with git at all, but the fact that 
Andrew Morton noticed that he cannot apply one of the series of patches he 
has with "patch" (well, with his scripts that are designed _around_ 
patch, to be exact).

The reason? Part of the patch *description* looked like this:

    [SCSI] 53c700: Amiga 4000T NCR53c710 SCSI

    New driver for the Amiga 4000T built-in NCR53c710 SCSI controller, using the
    53c700 SCSI core.

where it really *was* indented by four characters (and that's where git 
comes in: git indents the patch descriptions exactly so that you cannot 
*possibly* confuse the patch itself with the description).

It turns out that "patch" would actually think there is a patch there: the 
line

    53c700 SCSI core.

was determined to be an ed-script ("53c700") _despite_ the fact that it's 
indented.

Andrew was able to fix that particular damage by using "-u" and forcing 
anything but unified diffs to be ignored, but that isn't an option for all 
quilt users, since some projects use old-fashioned context diffs or a 
mixture.

Besides, the explanations can certainly contain patch fragments anyway (in 
the kernel, we put things like example code in them).

And it really boils down to a really simple thing: when scripting, you DO 
NOT WANT "patch" to make random guesses. And that whole "indentation" 
thing by patch is a pure guess, and should simply NOT BE DONE. And there's 
no way to tell patch to not do it.

So Paul, you're our only hope.

I'm personally trying to tell people not to use "patch" at all (this isn't 
the first time patch has done insane things by default, but it's the first 
time you cannot even _disable_ the insane behaviour), but Ted has a point: 
regardless of whether people learn to use "git-apply" to apply patches, 
the old "patch" binary would be better off just improved.

In this case, the improvement would be to simply ignore indented patches 
(preferably by default, but at least have the option to do so).

		Linus

Johannes Schindelin [off-list ref] writes:

quoted

quoted

quoted

I guess the second choice generally isn't an option, but dammit, 
"git-apply" really is the better program here.

Why not?  git-apply works outside of a git repo ;-)

I was more thinking that people are not necessarily willing to install git 
just to get the "git-apply" program..

But maybe they would be willing to install git to get that wonderful
git-apply program, and that wonderful rename-and-mode-aware
git-diff, and the git-merge-file program, all of which can operate
outside of a git repository. (Take that, hg!)

Well, hmph!  I just rewrote my git-diff-using script to not check
stuff into a throw-away git repository, and guess what: with real-life
use cases (diffing trees of about 500MB size), git-diff runs out of
memory (the machine probably has something like 1.5GB of virtual memory
size) when operating outside of a git repository.

So the usefulness still seems limited, even now that the output format
of --name-status has been fixed.

Any idea whether this is a bug, sloppy programming, or an inherent
restriction/necessity?

Also an idea which of the following scenarios would be best for
catching all of moves/renames/deletes/adds?  Note: any repository is
strictly throw-away.

Experiments are somewhat time-consuming, so every hunch helps.

a) diff directories outside of git (works, but fatal memory footprint
                                    for large cases)
b) diff index against work directory
c) diff revision against work directory
d) diff revision against index
e) diff revision against revision (works, but high disk footprint and
                                   likely slower than alternatives)

Thanks,

-- 
David Kastrup

Some more experiments:

David Kastrup [off-list ref] writes:

Johannes Schindelin [off-list ref] writes:

quoted

quoted

quoted

quoted

I guess the second choice generally isn't an option, but dammit, 
"git-apply" really is the better program here.

Why not?  git-apply works outside of a git repo ;-)

I was more thinking that people are not necessarily willing to install git 
just to get the "git-apply" program..

But maybe they would be willing to install git to get that wonderful
git-apply program, and that wonderful rename-and-mode-aware
git-diff, and the git-merge-file program, all of which can operate
outside of a git repository. (Take that, hg!)

Well, hmph!  I just rewrote my git-diff-using script to not check
stuff into a throw-away git repository, and guess what: with real-life
use cases (diffing trees of about 500MB size), git-diff runs out of
memory (the machine probably has something like 1.5GB of virtual memory
size) when operating outside of a git repository.

So the usefulness still seems limited, even now that the output format
of --name-status has been fixed.

Any idea whether this is a bug, sloppy programming, or an inherent
restriction/necessity?

Also an idea which of the following scenarios would be best for
catching all of moves/renames/deletes/adds?  Note: any repository is
strictly throw-away.

Experiments are somewhat time-consuming, so every hunch helps.

a) diff directories outside of git (works, but fatal memory footprint
                                    for large cases)
b) diff index against work directory

fatal memory footprint

c) diff revision against work directory

fatal memory footprint

d) diff revision against index

does not detect copies/renames

e) diff revision against revision (works, but high disk footprint and
                                   likely slower than alternatives)

So it seems like option e) is the only feasible option.  In the total
numbers, git-add is by far the slowest operation, followed by
git-commit.  git-diff on revisions is quite fast and with moderate
memory footprint.

Committing itself does not seem to add much disk space: adding into
the index seems to be the main disk space allocation.

So while the behavior of d) appears puzzling, doing another commit
before the diff is cheap, so the motivation for asking people to find
out the problems with d) is low for me.

Somewhat dissatisfactory that rewriting my script for using the
repository-less variant of git-diff fails for seriously large use
cases due to out-of-memory conditions.

I suppose that's life.

-- 
David Kastrup

On Fri, 6 Jul 2007, David Kastrup wrote:

Well, hmph!  I just rewrote my git-diff-using script to not check
stuff into a throw-away git repository, and guess what: with real-life
use cases (diffing trees of about 500MB size), git-diff runs out of
memory (the machine probably has something like 1.5GB of virtual memory
size) when operating outside of a git repository.

Ok, that's probably some huge memory leak that just doesn't show up with 
any normal git operations, likely simply because all the normal git 
operations will have thrown out the case of "identical files" without ever 
even looking at the file.

I'd guess that when using the diff logic on outside files, we'll read them 
all in, compare them, and keep them all in memory even though they are 
identical.

Generally, though, "git diff" has a much higher memory footprint than any 
normal file-by-file recursive diff, exactly because of the rename logic. 
An external "diff" won't ever have any reason to keep more than two files 
in memory at a time, but because git diff does rename and copy detection, 
it wants to keep the file data in memory over much longer times.

But I bet there is some stupid bug where we just make it much much worse 
for the "no git tree/index" case, and keep the whole tree in memory or 
something.

(The same is true of "git apply", btw, for a different reason: because 
git-apply will refuse to write out partial results in case some later 
patch fails, git-apply will keep the whole result in memory until the very 
end, and then do the write-out in one go. Again, that obviously means 
that it will potentially use a lot more memory than the "one patch at a 
time" approach that regular "patch" does)

			Linus