diffstat wierdness with 'git format-patch' output

diffstat wierdness with 'git format-patch' output

[Greg KH]

I was trying to use the built-in git tools to send patches off, instead
of my horribly hacked up scripts that use the git low-level stuff, when
I noticed that git format-patch's output confuses diffstat a bit, and
causes it to add another line to it's count.

This isn't good when I do a 'diffstat -p1 *.txt' of the output and add
it to an email to send off for someone to pull from, as the result will
be off from what is really there.

Here's what I get:

 $ git format-patch -n origin..HEAD
 0001-USB-fix-up-the-usb-early-handoff-logic-for-EHCI.txt
 0002-USB-add-new-device-ids-to-ldusb.txt
 0003-USB-change-ldusb-s-experimental-state.txt
 0004-USB-PL2303-Leadtek-9531-GPS-Mouse.txt
 0005-USB-sl811_cs-needs-platform_device-conversion-too.txt
 0006-usb-storage-new-unusual_devs-entry.txt
 0007-usb-storage-unusual_devs-entry.txt
 0008-USB-unusual_devs.h-entry-TrekStor-i.Beat.txt
 0009-USB-unusual_devs.h-entry-iAUDIO-M5.txt
 0010-USB-unusual-devs-bugfix.txt

 $ git log | head -n 1
 commit 16f05be7be0bf121491d83bd97337fe179b3b323

 $ git show 16f05be7be0bf121491d83bd97337fe179b3b323 | diffstat -p1
  drivers/usb/storage/unusual_devs.h |   25 ++++++++++++++++++-------
  1 file changed, 18 insertions(+), 7 deletions(-)

 $ diffstat -p1 0010-USB-unusual-devs-bugfix.txt
  drivers/usb/storage/unusual_devs.h |   26 ++++++++++++++++++--------
  1 file changed, 18 insertions(+), 8 deletions(-)

Any thoughts?

thanks,

greg k-h

Re: diffstat wierdness with 'git format-patch' output

[Greg KH]

On Mon, Feb 13, 2006 at 09:54:25PM -0800, Greg KH wrote:
> I was trying to use the built-in git tools to send patches off, instead
> of my horribly hacked up scripts that use the git low-level stuff, when
> I noticed that git format-patch's output confuses diffstat a bit, and
> causes it to add another line to it's count.
> 
> This isn't good when I do a 'diffstat -p1 *.txt' of the output and add
> it to an email to send off for someone to pull from, as the result will
> be off from what is really there.
> 
> Here's what I get:
> 
>  $ git format-patch -n origin..HEAD
>  0001-USB-fix-up-the-usb-early-handoff-logic-for-EHCI.txt
>  0002-USB-add-new-device-ids-to-ldusb.txt
>  0003-USB-change-ldusb-s-experimental-state.txt
>  0004-USB-PL2303-Leadtek-9531-GPS-Mouse.txt
>  0005-USB-sl811_cs-needs-platform_device-conversion-too.txt
>  0006-usb-storage-new-unusual_devs-entry.txt
>  0007-usb-storage-unusual_devs-entry.txt
>  0008-USB-unusual_devs.h-entry-TrekStor-i.Beat.txt
>  0009-USB-unusual_devs.h-entry-iAUDIO-M5.txt
>  0010-USB-unusual-devs-bugfix.txt
> 
>  $ git log | head -n 1
>  commit 16f05be7be0bf121491d83bd97337fe179b3b323
> 
>  $ git show 16f05be7be0bf121491d83bd97337fe179b3b323 | diffstat -p1
>   drivers/usb/storage/unusual_devs.h |   25 ++++++++++++++++++-------
>   1 file changed, 18 insertions(+), 7 deletions(-)
> 
>  $ diffstat -p1 0010-USB-unusual-devs-bugfix.txt
>   drivers/usb/storage/unusual_devs.h |   26 ++++++++++++++++++--------
>   1 file changed, 18 insertions(+), 8 deletions(-)
> 
> Any thoughts?

Hm, in looking at it closer, it's probably the last two lines of the
file, the signature that git format-patch adds to the message:
	-- 
	1.2.0

Any way to suppress these?

thanks,

greg k-h

> 
> thanks,
> 
> greg k-h

Re: diffstat wierdness with 'git format-patch' output

[Junio C Hamano]

Greg KH <greg@kroah.com> writes:

> Hm, in looking at it closer, it's probably the last two lines of the
> file, the signature that git format-patch adds to the message:
> 	-- 
> 	1.2.0

If that is the case, it's unfortunate that diffstat is broken
and is not properly counting lines to tell which lines are part
of the patch and which lines are not.

Have you tried "git apply --stat" instead?

> Any way to suppress these?

Sorry, there is no option to disable that, but the stuff is
GPLv2 so you can do whatever ;-).

The string "-- \n" is an established convention to mark the
beginning of the signature (or whatever inmaterial stuff that
follow the message contents), so changing the marker is
pointless -- if we want the option it should be to delete those
two lines altogether.

I personally find it useful to see the trend of version of tools
people use on the public mailing list, and that was the primary
reason it is there.

Have you tried "git apply --stat --summary" instead?

Re: diffstat wierdness with 'git format-patch' output

[Greg KH]

On Mon, Feb 13, 2006 at 10:09:30PM -0800, Junio C Hamano wrote:
> Greg KH <greg@kroah.com> writes:
> 
> > Hm, in looking at it closer, it's probably the last two lines of the
> > file, the signature that git format-patch adds to the message:
> > 	-- 
> > 	1.2.0
> 
> If that is the case, it's unfortunate that diffstat is broken
> and is not properly counting lines to tell which lines are part
> of the patch and which lines are not.
> 
> Have you tried "git apply --stat" instead?

How would that work after I've allready applied the patches to a branch?

> > Any way to suppress these?
> 
> Sorry, there is no option to disable that, but the stuff is
> GPLv2 so you can do whatever ;-).

Bah, make me go create a patch :)

> The string "-- \n" is an established convention to mark the
> beginning of the signature (or whatever inmaterial stuff that
> follow the message contents), so changing the marker is
> pointless -- if we want the option it should be to delete those
> two lines altogether.

Oh I understand what it is and it does follow the proper convention.
Maybe we just need an extra line of padding in there before the
signature or something... I'll go play with it.

> I personally find it useful to see the trend of version of tools
> people use on the public mailing list, and that was the primary
> reason it is there.

Why not just add a "X-Git-Version: 1.2.0" flag to the message instead?

thanks,

greg k-h

Re: diffstat wierdness with 'git format-patch' output

[Junio C Hamano]

Greg KH <greg@kroah.com> writes:

>> Have you tried "git apply --stat" instead?
>
> How would that work after I've allready applied the patches to a branch?

Sorry I was unclear.  By "instead", I meant "instead of running
diffstat -p1".

Re: diffstat wierdness with 'git format-patch' output

[Greg KH]

On Mon, Feb 13, 2006 at 10:34:20PM -0800, Greg KH wrote:
> > I personally find it useful to see the trend of version of tools
> > people use on the public mailing list, and that was the primary
> > reason it is there.
> 
> Why not just add a "X-Git-Version: 1.2.0" flag to the message instead?

And here's a patch that does just that.

-----------------

Subject: change git-format-patch to put the git version in the mail header

This is more like other tools that create mail messages, and fixes an
issue of the diffstat program thinking that an extra line was removed
from the patch.

Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

diff --git a/git-format-patch.sh b/git-format-patch.sh
index e54c9e4..5f6a921 100755
--- a/git-format-patch.sh
+++ b/git-format-patch.sh
@@ -216,6 +216,7 @@ while (<FH>) {
 
 	    print "From: $author_ident\n";
 	    print "Date: $author_date\n";
+	    print "X-Git-Version: @@GIT_VERSION@@\n";
 	}
 	next;
     }
@@ -250,8 +251,6 @@ close FH or die "close $commsg pipe";
 	git-diff-tree -p $diff_opts "$commit" | git-apply --stat --summary
 	echo
 	git-diff-tree -p $diff_opts "$commit"
-	echo "-- "
-	echo "@@GIT_VERSION@@"
 
 	echo
 }

Re: diffstat wierdness with 'git format-patch' output

[Greg KH]

On Mon, Feb 13, 2006 at 10:37:59PM -0800, Junio C Hamano wrote:
> Greg KH <greg@kroah.com> writes:
> 
> >> Have you tried "git apply --stat" instead?
> >
> > How would that work after I've allready applied the patches to a branch?
> 
> Sorry I was unclear.  By "instead", I meant "instead of running
> diffstat -p1".

That doesn't give me a summary of all of the changes I made to the tree:

$ git apply --stat *.txt
 drivers/hwmon/vt8231.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)
 drivers/hwmon/w83781d.c |   43 +++++++++++++++++++++++++------------------
 1 files changed, 25 insertions(+), 18 deletions(-)
 Documentation/hwmon/w83627hf |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
 drivers/hwmon/it87.c         |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
 drivers/i2c/busses/i2c-isa.c |   12 ------------
 1 files changed, 0 insertions(+), 12 deletions(-)


Instead of:

 Documentation/hwmon/w83627hf |    4 ++++
 drivers/hwmon/it87.c         |    3 ++-
 drivers/hwmon/vt8231.c       |    8 ++++----
 drivers/hwmon/w83781d.c      |   43 +++++++++++++++++++++++++------------------
 drivers/i2c/busses/i2c-isa.c |   12 ------------
 5 files changed, 35 insertions(+), 35 deletions(-)

Which is what I need to put into a "please pull from this tree" email
message.

thanks,

greg k-h

Re: diffstat wierdness with 'git format-patch' output

[Greg KH]

On Mon, Feb 13, 2006 at 10:42:56PM -0800, Greg KH wrote:
> On Mon, Feb 13, 2006 at 10:34:20PM -0800, Greg KH wrote:
> > > I personally find it useful to see the trend of version of tools
> > > people use on the public mailing list, and that was the primary
> > > reason it is there.
> > 
> > Why not just add a "X-Git-Version: 1.2.0" flag to the message instead?
> 
> And here's a patch that does just that.
> 
> -----------------
> 
> Subject: change git-format-patch to put the git version in the mail header
> 
> This is more like other tools that create mail messages, and fixes an
> issue of the diffstat program thinking that an extra line was removed
> from the patch.
> 
> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
> 
> diff --git a/git-format-patch.sh b/git-format-patch.sh
> index e54c9e4..5f6a921 100755
> --- a/git-format-patch.sh
> +++ b/git-format-patch.sh
> @@ -216,6 +216,7 @@ while (<FH>) {
>  
>  	    print "From: $author_ident\n";
>  	    print "Date: $author_date\n";
> +	    print "X-Git-Version: @@GIT_VERSION@@\n";

Hm, git-send-email doesn't see this, so it doesn't get sent out if you
use that tool.

I can fix that to just put the version number in the message too, like
it adds the "X-Mailer" field, if people think this is really worth it...

thanks,

greg k-h

Re: diffstat wierdness with 'git format-patch' output

[Junio C Hamano]

Greg KH <greg@kroah.com> writes:

> Hm, git-send-email doesn't see this, so it doesn't get sent out if you
> use that tool.

And people are very likely to remove it by hand.

Another possibility would be to use "git apply --numstat" and
add numbers up in your script, but that would not give you a
nice graph output either X-<.

I'd say if you really care we should just remove those two
lines, and remember I am _very_ receptive from such suggestion
from prominent kernel people.

Re: diffstat wierdness with 'git format-patch' output

[Fredrik Kuivinen]

On Mon, Feb 13, 2006 at 09:54:25PM -0800, Greg KH wrote:
> I was trying to use the built-in git tools to send patches off, instead
> of my horribly hacked up scripts that use the git low-level stuff, when
> I noticed that git format-patch's output confuses diffstat a bit, and
> causes it to add another line to it's count.
> 
> This isn't good when I do a 'diffstat -p1 *.txt' of the output and add
> it to an email to send off for someone to pull from, as the result will
> be off from what is really there.
> 
> Here's what I get:
> 
>  $ git format-patch -n origin..HEAD
>  0001-USB-fix-up-the-usb-early-handoff-logic-for-EHCI.txt
>  0002-USB-add-new-device-ids-to-ldusb.txt
>  0003-USB-change-ldusb-s-experimental-state.txt
>  0004-USB-PL2303-Leadtek-9531-GPS-Mouse.txt
>  0005-USB-sl811_cs-needs-platform_device-conversion-too.txt
>  0006-usb-storage-new-unusual_devs-entry.txt
>  0007-usb-storage-unusual_devs-entry.txt
>  0008-USB-unusual_devs.h-entry-TrekStor-i.Beat.txt
>  0009-USB-unusual_devs.h-entry-iAUDIO-M5.txt
>  0010-USB-unusual-devs-bugfix.txt
> 
>  $ git log | head -n 1
>  commit 16f05be7be0bf121491d83bd97337fe179b3b323
> 
>  $ git show 16f05be7be0bf121491d83bd97337fe179b3b323 | diffstat -p1
>   drivers/usb/storage/unusual_devs.h |   25 ++++++++++++++++++-------
>   1 file changed, 18 insertions(+), 7 deletions(-)
> 
>  $ diffstat -p1 0010-USB-unusual-devs-bugfix.txt
>   drivers/usb/storage/unusual_devs.h |   26 ++++++++++++++++++--------
>   1 file changed, 18 insertions(+), 8 deletions(-)
> 
> Any thoughts?
> 

If you don't have to generate the diffstat from the individual patches
then

    git-diff-tree -p origin HEAD | git-apply --stat

should give you what you want.


BTW when I tried the above command on two random tags in the git tree
I got:

    $ git-diff-tree -p v1.2.0 v1.0.8 | diffstat -p1 | tail -1
    165 files changed, 3913 insertions(+), 8092 deletions(-)

and

    $ git-diff-tree -p v1.2.0 v1.0.8 | git apply --stat | tail -1
    160 files changed, 3925 insertions(+), 8092 deletions(-)

The difference seems to be caused by diffstat which seems to think
that all lines starting with '---' are diff headers. Hence, there are
some bogus files in the diffstat output such as:

    [master]                                          |  177 ++
    [master^2~4]                                      |    9 
    [master^]                                         |   24 
    [mybranch]                                        |   56 
    [mybranch^]                                       |  616 +++++++

- Fredrik

Re: diffstat wierdness with 'git format-patch' output

[Greg KH]

On Mon, Feb 13, 2006 at 11:10:18PM -0800, Junio C Hamano wrote:
> Greg KH <greg@kroah.com> writes:
> 
> > Hm, git-send-email doesn't see this, so it doesn't get sent out if you
> > use that tool.
> 
> And people are very likely to remove it by hand.
> 
> Another possibility would be to use "git apply --numstat" and
> add numbers up in your script, but that would not give you a
> nice graph output either X-<.
> 
> I'd say if you really care we should just remove those two
> lines, and remember I am _very_ receptive from such suggestion
> from prominent kernel people.

Hm, in thinking about this some more, I realized that it only requires
me to change my script to generate the emails for Linus to pull from a
tiny bit, making this whole thing not a bit deal at all.

Thanks a lot for being so receptive, if I have further problems with it,
I'll let you know.

greg k-h