Possible bug: --ext-diff ignored with --cc in git log

 Hello,

 I use a combination of git attributes and a custom diff driver to ignore
the changes to the generated files (that we unfortunately need to keep in
our repository) from appearing in "git diff" and "git log" output, i.e.:

	% cat .gitattributes
	# Use a custom diff driver for bakefile generated files
	# This allows ignoring them in git diff for example by doing
	#     $ git config diff.generated.command true
	# pass --no-ext-diff to git diff to ignore the custom driver
	*.sln              diff=generated
	*.vcproj           diff=generated
	*.vcxproj          diff=generated
	*.vcxproj.filters  diff=generated
	% git config diff.generated.command
	echo Diff of generated file "$1" suppressed; true

 This works quite nicely most of the time provided you use "--ext-diff"
with "git log", but not when showing the merges with "--cc". I.e. the
command "git log --ext-diff -p --cc" still outputs the real diff even for
the generated files, as if "--ext-diff" were not given. If I use "git log
--ext-diff -p -m", the generated files are ignored, which is nice, but the
diff for the other files becomes much more verbose and less readable and
I'd really like to combine "--ext-diff" with "--cc" if possible.

 Is the current behaviour intentional? I see it with all the git versions I
tried (1.7.10, 2.1.0, 2.7.0 and v2.8.0-rc1), but I don't really see why
would it need to work like this, so I hope it's an oversight and could be
corrected.

 Or is there perhaps some other way to do what I want already?

 Thanks in advance for any help,
VZ

Vadim Zeitlin <vz-git@zeitlins.org> writes:

I think this is "intentional" in the sense that "--cc" feature is fundamentally and conceptually incompatible with "--ext-diff".

 - The "external diff" feature is to allow third-party tools to
   produce output that is vastly different from the usual "diff"
   output, e.g. unlike the usual "diff", the output may not even be
   line-oriented, and certainly would not have to follow the
   convention of denoting the contents on old and new lines with "-"
   and "+" prefixes.

 - The "--cc" feature is to show multiple "diff" outputs in the
   usual format with post-processing to coalesce them into a more
   concise form, and fundamentally depends on (1) the output being
   line-oriented and (2) the contents of old and new lines denoted
   by "-"/"+" prefixes to be able to do so.

I haven't tried it myself, but if the contents you are using ext-diff on can be compared in a format that is easy-to-read for humans by passing them first to "textconv" filter and then running the normal "diff" on, that may be a viable approach to do what you are trying to do, as "textconv" feature is meant to still produce the output that still follows the usual "diff" convention. Its output should be usable by any tool (e.g. diffstat) meant to post-process patch output, and would be a better match for the "--cc" mechanism.

On Thu, 10 Mar 2016 14:33:55 -0800 Junio C Hamano <gitster@pobox.com> wrote:

JCH> Vadim Zeitlin <vz-git@zeitlins.org> writes: JCH> JCH> > I.e. the JCH> > command "git log --ext-diff -p --cc" still outputs the real diff even for JCH> > the generated files, as if "--ext-diff" were not given. ... JCH> > Is the current behaviour intentional? I see it with all the git versions I JCH> > tried (1.7.10, 2.1.0, 2.7.0 and v2.8.0-rc1), but I don't really see why JCH> > would it need to work like this, so I hope it's an oversight and could be JCH> > corrected. JCH> JCH> I think this is "intentional" in the sense that "--cc" feature is JCH> fundamentally and conceptually incompatible with "--ext-diff".

 Thank you for your reply, Junio, I hadn't realized that --cc was dependent
on textual diff output format before, but now I understand why it can't
respect --ext-diff.

JCH> I haven't tried it myself, but if the contents you are using JCH> ext-diff on can be compared in a format that is easy-to-read for JCH> humans by passing them first to "textconv" filter and then running JCH> the normal "diff" on, that may be a viable approach to do what you JCH> are trying to do, as "textconv" feature is meant to still produce JCH> the output that still follows the usual "diff" convention. Its JCH> output should be usable by any tool (e.g. diffstat) meant to JCH> post-process patch output, and would be a better match for the JCH> "--cc" mechanism.

 I can't think of a way to make the output as concise as it is now (i.e.
just a single line saying that a generated file has been modified but the
changes to it are not being shown) with this approach.

 Maybe I'm clutching at straws here, but I wonder if it could be possible
to have a file attribute specifying whether --cc or -m should be used for
it when showing merges? Because this is, basically, what I want here: --cc
for normal files for readability but -m for the files I'm not interested
in. It's probably too specific to my particular hack^H^H^H^H use case to
add support for it to Git itself, but I wanted to mention it on a chance
that somebody else might think it's a good idea.

 Anyhow, thanks again for your explanation,
VZ

Vadim Zeitlin <vz-git@zeitlins.org> writes:

Having established that, I should also add that "--cc fundamentally is incompatible with --ext-diff" does not justify that "--cc when given with --ext-diff just ignores and uses the usual diff".

An equally (or even more) valid consequence could have been to disable "--cc" processing for paths that would trigger an external diff driver. After all, the user told us that the contents would not compare well with the usual "diff"; we know that "--cc" output that summarizes the usual diff output is useless.

What we show instead is an interesting thing to think about.

For example, we _could_ also ignore what external diff driver produces in this case (as we know it won't be producing an appropriate input to the "--cc" post-processing), and pretend as if comparing an old version of foo.sln with a new version of foo.sln produced a diff like this:

    diff --git a/foo.sln b/foo.sln
    index d7ff46e,b829410
    --- a/foo.sln
    +++ b/foo.sln
    @@ 1,1 @@
    -d7ff46ec4a016c6ab7d233b9d4a196ecde623528  - generated file
    +b829410f6da0afc14353b4621d2fdf874181a9f7  - generated file

then you might see in a merge that merges two versions of foo.sln and result in another version of foo.sln in your "--cc" output a hunk that is like this:

    diff --cc foo.sln
    index d7ff46e,6c9aaa1..b829410
    --- a/foo.sln
    +++ b/foo.sln
    @@@ 1,1 @@@
    - d7ff46ec4a016c6ab7d233b9d4a196ecde623528  - generated file
     -6c9aaa1ae63a2255a215c1287e38e75fcc5fc5d3  - generated file
    ++b829410f6da0afc14353b4621d2fdf874181a9f7  - generated file

which would at least tell you that there was a merge, and if the merge took the full contents of the file from one of the commits and recorded as the result of the merge, then you wouldn't see them in the "--cc" output.

It happens that the above is fairly easily doable with today's Git without any modification. Here is how.

(1) Have this in your .git/config

    [diff "uninteresting"]
    	textconv = /path/to/uninteresting-textconv-script

(2) Mark your .sln paths as uninteresting in your .gitattributes

    *.sln	diff=uninteresting

(3) Have this textconv filter in /path/to/uninteresting-textconv-script

    #!/bin/sh
    printf "%s generated file\n" "$(sha1sum <"$1")"

On Fri, Mar 11, 2016 at 10:20:42AM -0800, Junio C Hamano wrote:

I think an even easier way is:

  git log --cc --raw

I know that is somewhat beside the point you are making, which is how we should handle "--cc" with ext-diff. But I would much rather have us show nothing for that case, and let the user turn on "--raw", than to invent a diff-looking format that does not actually represent the file contents.

-Peff

Jeff King <peff@peff.net> writes:

Sorry, but I am not sure where you are trying to go with this.

I understand that the original issue was that Vadim wants to suppress reams of differences for _some_ paths but still wants to benefit from the textual summarized diff for all the other paths. Giving "--raw" would be global, and would affect other paths, no?

On Fri, Mar 11, 2016 at 11:08:32AM -0800, Junio C Hamano wrote:

Ah, sorry, I thought the problem was the opposite: that there was no output for ext-diff paths, and we needed to add something back in. Doing "--raw" is a much easier way than textconv of the "add back in" part, but it does not suppress the ordinary combined diff.

-Peff

On Fri, 11 Mar 2016 10:20:42 -0800 Junio C Hamano <gitster@pobox.com> wrote:

JCH> Vadim Zeitlin <vz-git@zeitlins.org> writes: JCH> JCH> > Thank you for your reply, Junio, I hadn't realized that --cc was dependent JCH> > on textual diff output format before, but now I understand why it can't JCH> > respect --ext-diff. JCH> JCH> Having established that, I should also add that "--cc fundamentally JCH> is incompatible with --ext-diff" does not justify that JCH> "--cc when given with --ext-diff just ignores and uses the usual JCH> diff". JCH> JCH> An equally (or even more) valid consequence could have been to JCH> disable "--cc" processing for paths that would trigger an external JCH> diff driver.

 FWIW I agree that this would make more sense than the current behaviour.
But it still wouldn't be ideal if disabling "--cc" meant not showing any
output for these files at all, we still want to know that the file has been
modified as part of the commit, even if we don't care about its contents.

JCH> After all, the user told us that the contents would not compare well JCH> with the usual "diff"; we know that "--cc" output that summarizes the JCH> usual diff output is useless.

 This is so logical that it made me check how did "--cc" behave with the
binary files because this argument seems to apply perfectly well to them
too. And (unsurprisingly?) it already works just fine with them:

	# I have alias g=git and I also suppress all successful output
	$ g init
	$ echo 'Binary\0file' > binary
	$ g add binary
	$ g commit -m 'Added'
	$ echo '2nd line' >> binary
	$ g commit -a -m 'Added 2nd line'
	$ g checkout -b another HEAD~
	$ echo 'another line' >> binary
	$ g commit -a -m 'Added another line'
	$ g checkout master
	$ g merge
	warning: Cannot merge binary files: binary (HEAD vs. another)
	Auto-merging binary
	CONFLICT (content): Merge conflict in binary
	Automatic merge failed; fix conflicts and then commit the result.
	$ vi binary # combine both versions
	$ g commit
	$ g show # finally I can show what all this is about
	commit d30ae002cb52974228d50723fc8c9d7077e760da
	Merge: ae542d2 3204f35
	Author: Vadim Zeitlin <vz-xxx@zeitlins.org>
	Date:   Sat Mar 12 01:57:55 2016 +0100

	    Merge branch 'another'

	diff --cc binary
	index 31499e2,1730dfd..1eda50a
	Binary files differ

So it looks like it shouldn't be too difficult to make it also output "Files using custom diff viewer differ", what do you think?

JCH> For example, we could also ignore what external diff driver JCH> produces in this case (as we know it won't be producing an JCH> appropriate input to the "--cc" post-processing), and pretend JCH> as if comparing an old version of foo.sln with a new version of JCH> foo.sln produced a diff like this: JCH> JCH> diff --git a/foo.sln b/foo.sln JCH> index d7ff46e,b829410 JCH> --- a/foo.sln JCH> +++ b/foo.sln JCH> @@ 1,1 @@ JCH> -d7ff46ec4a016c6ab7d233b9d4a196ecde623528 - generated file JCH> +b829410f6da0afc14353b4621d2fdf874181a9f7 - generated file JCH> JCH> then you might see in a merge that merges two versions of foo.sln JCH> and result in another version of foo.sln in your "--cc" output a JCH> hunk that is like this: JCH> JCH> diff --cc foo.sln JCH> index d7ff46e,6c9aaa1..b829410 JCH> --- a/foo.sln JCH> +++ b/foo.sln JCH> @@@ 1,1 @@@ JCH> - d7ff46ec4a016c6ab7d233b9d4a196ecde623528 - generated file JCH> -6c9aaa1ae63a2255a215c1287e38e75fcc5fc5d3 - generated file JCH> ++b829410f6da0afc14353b4621d2fdf874181a9f7 - generated file JCH> JCH> which would at least tell you that there was a merge, and if the JCH> merge took the full contents of the file from one of the commits and JCH> recorded as the result of the merge, then you wouldn't see them in JCH> the "--cc" output.

 Interesting, but I admit I don't really see any advantage of showing the
SHA-1s here compared to what already happens with the binary files. Is
there anything I'm missing?

JCH> It happens that the above is fairly easily doable with today's Git JCH> without any modification. Here is how. JCH> JCH> (1) Have this in your .git/config JCH> JCH> [diff "uninteresting"] JCH> textconv = /path/to/uninteresting-textconv-script JCH> JCH> (2) Mark your .sln paths as uninteresting in your .gitattributes JCH> JCH> *.sln diff=uninteresting JCH> JCH> (3) Have this textconv filter in /path/to/uninteresting-textconv-script JCH> JCH> #!/bin/sh JCH> printf "%s generated file\n" "$(sha1sum <"$1")"

 This is really ingenious, thanks! I'm probably indeed going to put this in
place at least for now for our mail notification script because it's just
too annoying to receive emails with thousands of lines of diffs to the
generated files.

 But I still think that it would make sense for "--cc" to behave as it does
for the binary files for the ext-diffable ones too. I've never touched git
code before but if you think it's a good idea and if you don't see any
insurmountable difficulties in implementing this, I could try to make a
patch doing it, please let me know if you think it could be useful.

 And thanks again for your textconv hint!
VZ