Re: git full diff output issues..

quoted

quoted

quoted

quoted

"JCH" == Junio C Hamano [off-list ref] writes:
"LT" == Linus Torvalds [off-list ref] writes:

LT> This means, btw, that the "git --diff" format must _not_ do

LT> diff --git a/file /dev/null
LT> deleted file mode 100644

JCH> I just checked, and both built-in and git-external-diff-script
JCH> should be safe about this issue.

Sorry, I spoke too soon about a week and half ago X-<, and I am
bugging you about this because this clearly belongs to "fix"
category not "new stuff".

The case you mentioned (i.e. /dev/null) is fine but rename/copy
is "broken" according to the definition by git-apply.

What do you want the diff-patch format to say for this one?

    :100644 100644 SHA1-OLD SHA1-NEW R frotz.c nitfol.c

Currently I am saying:

    diff --git a/frotz.c b/nitfol.c
    similarity index 89%
    rename old frotz.c
    rename new nitfol.c
    --- a/frotz.c
    +++ b/nitfol.c
    @@ ...

and this makes git-apply barf, because a/ and b/ names are
different.  Is the following what you want?  That is, do you
always want p->two->path (name in the right hand side tree)?

    diff --git a/nitfol.c b/nitfol.c
    similarity index 89%
    rename old frotz.c
    rename new nitfol.c
    --- a/frotz.c
    +++ b/nitfol.c
    @@ ...

According to the current apply.c, git_header_name() does not
care as long as a/ and b/ names are the same (that is, I could
even say "diff --git a/junkio b/junkio" to make it grok the
above example, as long as I have the correct "rename old" and
"rename new" in the extended header part).  In that sense, it
all boils down to which name you, as a human consumer of the
patch, would want to see on the header, if we go the route of
making a/ and b/ name always the same.  However I suspect that
this slightly breaks patch reversibility.

If we do care about patch reversibility, having a/ and b/ names
to show the pre- and post- paths like my current output does
(which _does_ break the current apply.c name checking) is
probably the most sensible thing to keep things symmetric.  I am
not sure if it is worth it to make the name checking logic in
apply.c more complicated only to support this rename symmetry,
though.

Another possibility; since "diff --git" is a git-specific header
format anyway, we could quote things to help apply.c parsing it,
without introducing too much clutter for ordinary cases.  How
about taking advantage of the fact that most pathnames do not
contain spaces nor backslashes, and if we see them we simply
quote, like this?

    # no need for quote
    diff --git a/frotz.c b/nitfol.c
    rename old frotz.c
    rename new nitfol.c
    
    # patch for "frotz and nitfol.c"
    diff --git a/frotz\ and\ nitfol.c b/frotz\ and\ nitfol.c

    # rename but filename has spaces and a backslash
    diff --git a/old\ name\\with\ bs b/new\ name\\with\ bs
    rename old old name\with bs
    rename new new name\with bs

On Sun, 5 Jun 2005, Junio C Hamano wrote:

The case you mentioned (i.e. /dev/null) is fine but rename/copy
is "broken" according to the definition by git-apply.

No problem, the renames always get the names from the "rename" line, not 
the header. Same goes for copies.

It's only modified files that keep the same name _and_ the same content 
that don't have the name uniquely on a line somewhere.

What do you want the diff-patch format to say for this one?

    :100644 100644 SHA1-OLD SHA1-NEW R frotz.c nitfol.c

Currently I am saying:

    diff --git a/frotz.c b/nitfol.c
    similarity index 89%
    rename old frotz.c
    rename new nitfol.c
    --- a/frotz.c
    +++ b/nitfol.c

This finds the old names unambiguously in _two_ places: in the "--- " line 
(no question about where it begins: it's -p1, or where it ends - at the 
newline) _and_ on the "rename old xxxx" line.

The only case that was special was literally the "same name, no content 
changes, new mode" case, which looked like

	diff --git a/oldname.c b/oldname.c
	new mode 100755
	old mode 100644

and thus _only_ had the name in the (normally ambiguous wrt whitepsace)  
header line.

But by having the requirement that the format of the header line for that
case is "-p1" together with both names being the same, it's not ambigious 
any more.

		Linus