Re: Filenames and prefixes in extended diffs

From: Andreas Gruenbacher <agruen@suse.de>
Date: Jan 18, 2010, 23:22 UTC
Message-ID: <201001190022.07580.agruen@suse.de>
In-Reply-To: <7vfx69k0bu.fsf@alter.siamese.dyndns.org>

On Thursday 14 January 2010 01:16:21 am Junio C Hamano wrote:

Show 27 quoted lines

> Andreas Gruenbacher <agruen@suse.de> writes:
> > Can git be changed to ...
> 
> Just to save your time coming up with more ways to *change* git diff...
> 
> Even though I wouldn't say _any_ change is too late to bring in, change in
> the output format from "git diff" family _must_ be usable by "git apply"
> people have been using for the last 4 years or so.
> 
> Suppose your updated version of "git diff" with a certain set of options
> produces output A, which is different from the output B you would get out
> of today's "git diff" that is run with the same set of options.
> 
> If "git apply" people have been using understands B (i.e. current output)
> and does something, the format change between A and B must be designed in
> such a way that the same "git apply" accepts A (i.e. your output) and do
> the same thing.
> 
> Two examples:
> 
>  - "git diff -M" (or "git show -M") is _defined_ to show the filenames
>    without prefix on "rename from" line, and deployed "git apply" relies
>    on this definition to apply the patch to the file the patch was meant
>    to apply.  If your modified "git diff -M" changes it to add the prefix,
>    and existing "git apply" changes behaviour (either by rejecting your
>    output, or applying the patch to a wrong file), then such a change has
>    *no chance* of getting in.  It is merely a breakage.

Git apply is currently broken in some (uncommon) cases. Consider the following two patches:

	$ cat add.diff
	diff --git a/d/f b/d/f
	new file mode 100644
	index 0000000..6a69f92
	--- /dev/null
	+++ b/d/f
	@@ -0,0 +1 @@
	+f

	$ cat rename.diff
	diff --git a/d/f b/d/g
	similarity index 100%
	rename from d/f
	rename to d/g

They apply fine after each other with plain "git apply". When you try to apply them to a different location, things break though:

	$ git apply --directory e -p2 add.diff
	$ git apply --directory e -p2 rename.diff
	error: e/d/f: No such file or directory

Had the second patch not been generated with "git diff -M", things would just have worked; in other words, the -M format is broken.

I think the easiest way to fix this in "git apply" would be to figure out what the missing prefixes are in the rename and copy lines (in this case, just "a/" and "b/"), and to prepend those prefixes to the decoded filenames before stripping off pathname components. This is the same as just taking the filenames from the "diff --git" line, and ignoring the filenames in the "copy from", "copy to", "rename from", and "rename to" headers.

I don't see a way how to fix this in the existing headers in a backwards compatible way. Do you?

Independent of whether and how this is addressed, can I please at least have the "diff --git" line parsing problem fixed so that filenames which contain spaces are put in double quotes there? Then I can at least ignore all the prefix-less filenames in GNU patch and still make it understand git's output.

>  - If you say "git diff --src-prefix=a/b/c --dst-prefix=x/y", it _might_
>    produce something "git apply" won't grok (I haven't checked this,
>    though).  You can suggest to change the output from such a case to work
>    better.  We didn't work as expected so a change _could_ be a fix.

The output format for that is fine.

Show 13 quoted lines

> The output from "git diff --no-index" is an exception to the above rule.
> It is primarily for people who have unmanaged contents and want to use
> features of the git diff engine that are not found in other people's diff
> implementations (e.g. wordwise colored diff), and the header part of its
> output does not currently follow "git diff" convention to be grokkable by
> "git apply".
> 
> Fixing _that_ is a welcome change, but I suspect that there are corner
> cases, e.g. "git diff --no-index frotz-1.2.36/ /tmp/frotz/" (i.e. you have
> a pristine version in frotz-1.2.36 directory, but your modified version is
> in /tmp/frtoz/) that might make fixing it fundamentally impossible (I
> haven't looked into it for a long time, so it could be easy, but my gut
> feeling is it isn't).

Patches with a different number of components in the from and to prefixes are a really bad idea. (GNU patch will prefer the pathname with the fewer components, but this yould just as well be the wrong one.)

Thanks, Andreas