Re: [PATCH v2] Do not show "diff --git" metainfo with --no-prefix

From

Daniel Barkalow <barkalow@iabervon.org>

Date

Jan 16, 2008, 03:56 UTC

Message-ID

<alpine.LNX.1.00.0801152226000.13593@iabervon.org>

In-Reply-To

<alpine.LFD.1.00.0801151902080.2806@woody.linux-foundation.org>

On Tue, 15 Jan 2008, Linus Torvalds wrote:

> On Tue, 15 Jan 2008, Junio C Hamano wrote:
> >
> > If a non-standard prefix is used by --no-prefix, --src-prefix,
> > or --dst-prefix options, the resulting diff becomes something
> > git-apply would not grok.  In such a case, we should not trigger
> > the more strict check git-apply does for patches in "diff --git"
> > format.
> 
> I think this is wrong.
> 
> If we do any git-specific stuff, we need to have that "--git" thing there. 
> That is *not* just limited to the prefix, but to all the other things git 
> diffs can do: renames, mode changes, etc.

I suppose the other option is to have the header in this case be:

diff --git --src-prefix= --dst-prefix= filename filename

so that apply can figure out what diff did correctly.

> > Metainformation (e.g. "index", "similarity", etc.) lines will
> > safely be ignored by patch and git-apply (even when the latter
> > parses a non-git diff output), so this patch does not bother
> > stripping them away.
> 
> It's not necessarily safe to ignore some of them, like the rename info. If 
> you see a rename patch and don't understand it as a rename, it's 
> pointless.
> 
> So I would argue that you need something stronger to say "don't do a git 
> diff", and that should also disallow rename detection at a minimum. Quite 
> frankly, any program that is so stupid as to not accept current git 
> patches (ie TortoiseSVN), then we damn well shouldn't just disable the 
> most trivial part of it. We should make sure that we do not enable *any* 
> of the rather important extensions: even if ToirtoiseSVN would ignore 
> them, if ignoring them means that it mis-understands the diff, it 
> shouldn't be allowed at all.
> 
> So maybe a --standard-diff option that removes the "--git" part, but also 
> removes everything else.

That seems wise to me. We should be able to generate patches that are 
accessible to programs that can't follow any clever instructions. I think 
the point of the "Index:" header is that these programs will freak out if 
two filenames don't match (or, more likely, break in some way), and it 
means you can't sensibly generate patches that upset them for deletes or 
creates.

	-Daniel
*This .sig left intentionally blank*