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

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

[Vadim Zeitlin]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1549 bytes --]

 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

[-- Attachment #2: Type: APPLICATION/PGP-SIGNATURE, Size: 196 bytes --]

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

[Junio C Hamano]

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

> 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. ...
> 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.

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.

Re[2]: Possible bug: --ext-diff ignored with --cc in git log

[Vadim Zeitlin]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2192 bytes --]

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

[-- Attachment #2: Type: APPLICATION/PGP-SIGNATURE, Size: 196 bytes --]

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

[Junio C Hamano]

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

> 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.

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")"

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

[Jeff King]

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

>     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.
> [...]

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

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

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

>> It happens that the above is fairly easily doable with today's Git
>> without any modification.  Here is how.
>> [...]
>
> 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.

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?

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

[Jeff King]

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

> Jeff King <peff@peff.net> writes:
> 
> >> It happens that the above is fairly easily doable with today's Git
> >> without any modification.  Here is how.
> >> [...]
> >
> > 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.
> 
> 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?

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

Re[2]: Possible bug: --ext-diff ignored with --cc in git log

[Vadim Zeitlin]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 5159 bytes --]

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

[-- Attachment #2: Type: APPLICATION/PGP-SIGNATURE, Size: 196 bytes --]