report on a possible bug: git commit -p myfile.py unexpected output

report on a possible bug: git commit -p myfile.py unexpected output

[Joan Aguilar]

Hello there

this is the first bug report of my life and I am not a native English
speaker so, first of all, I would like to apologize for my English
skills and the report itself (if it is not precise enough).

I have already read this 'guidelines'
http://www.chiark.greenend.org.uk/~sgtatham/bugs.html and I will try
to attach to them as much as I can.

What System I am running:
* Linux 4.9.0-1-amd64 #1 SMP Debian 4.9.6-3 (2017-01-28) x86_64 GNU/Linux
* git version 2.11.0. Well actually, according to the apt-get install
output (of course after apt-get update) -> git is already the newest
version (1:2.11.0-2).
* VIM - Vi IMproved 8.0 (2016 Sep 12, compiled Feb 13 2017 00:56:16)
Included patches: 1-197, 322

What happened:
I was working on my git repository yesterday and did a lot of changes
in the file myfile.py. After a little bit of testing I was ready to
commit them. I decided not to commit all of them at once (because
these are non related changes), but instead I decided to use the -p
option (as I always do in this case). For the first patch I decided to
commit only all methods I have removed because these are not needed
anymore. So my Idea was to jump from hunk to hunk and split them
(using 's') to select (with 'y') only the parts I wanted to commit,
ie, lines I had removed because they are not useful for me anymore. So
I went for it and I did:

git commit -p myfile.py

Until this point all went as expected. The first hunk was printed in
the console and the it waited for my decision. As always. After a
couple of hunks I realized it was not possible to split all of them in
a proper way (to select only removed lines) because new methods were
mixed up with the old ones. So I cancel the process (ctrl + c) and
started again using 'e' instead to manually edit the hunks.

So I repeated the last command: git commit -p myfile.py but this time
I only used 'y', 'n' and 'e'. All in all I edited ('e') two big hunks.
By doing this, the default editor (vim in my case) opened showing the
hunk and with the instructions in how to edit at the bottom.
Everything as used to be. Until this point all went as I expected.

In the two hunk I edited I did the same. I removed all + lines by
deleting them and I make some - lines as context. Like explained in
the bottom of the file.

When I was done with each of the hunks, I saved and closed the editor.
No error was produced (I sometimes make mistakes while editing a hunk
and know you can get an error (patch does not apply) here, but in this
case were no errors there so the next hunk was printed and the commit
procedure kept going.

When I was done with all hunks, the editor (vim) started again to
write the commit message. I wrote (something like) this:

    myfile.py -> old unused methods removed...

    1) mymethod1
    2) mymethod2
    3) mymethod3
    4) mymethod4
    5) mymethod5

Yes, not the best commit ever ;) but enough for us in this case.
Anyway, I saved it and close the editor and I was surprised by the git
output.

[master 96d1c24] myfile.py -> old unused methods removed...
 1 file changed, 182 insertions(+), 302 deletions(-)
 rewrite myfile.py (60%)

What?? I thought to myself... Insertions?? This can't be true? I
removed all + lines, my idea was to commit only deletions... What did
I wrong?

To check this I did
* git show 96d1c24
and I saw only red minus lines , ie, deletions. As I expected.

To check it twice I used tig too.

tig showed the same for this specific commit
myfile.py | 120
---------------------------------------------------------------------------------------------------------------------
1 file changed, 120 deletions(-)

Conclusion:
After these two checks I am sure I did what I intended to do. What I
do not understand is the output of git when I was done with the
commit.

Maybe is just me, because I do not understand how git commit -p when
using 'e' to edit hunks works. But as user I would expect to see only
deletions in the output message if I am picking only deletions.

If this is a bug, I hope you can reproduce it on your system. If not,
do not hesitate to contact me for further information.


Kind regards,

Joan Aguilar
--
Joan Aguilar Lorente

Re: report on a possible bug: git commit -p myfile.py unexpected output

[Michael J Gruber]

Joan Aguilar venit, vidit, dixit 24.03.2017 11:27:
> Hello there
> 
> this is the first bug report of my life and I am not a native English
> speaker so, first of all, I would like to apologize for my English
> skills and the report itself (if it is not precise enough).
> 
> I have already read this 'guidelines'
> http://www.chiark.greenend.org.uk/~sgtatham/bugs.html and I will try
> to attach to them as much as I can.
> 
> What System I am running:
> * Linux 4.9.0-1-amd64 #1 SMP Debian 4.9.6-3 (2017-01-28) x86_64 GNU/Linux
> * git version 2.11.0. Well actually, according to the apt-get install
> output (of course after apt-get update) -> git is already the newest
> version (1:2.11.0-2).
> * VIM - Vi IMproved 8.0 (2016 Sep 12, compiled Feb 13 2017 00:56:16)
> Included patches: 1-197, 322
> 
> What happened:
> I was working on my git repository yesterday and did a lot of changes
> in the file myfile.py. After a little bit of testing I was ready to
> commit them. I decided not to commit all of them at once (because
> these are non related changes), but instead I decided to use the -p
> option (as I always do in this case). For the first patch I decided to
> commit only all methods I have removed because these are not needed
> anymore. So my Idea was to jump from hunk to hunk and split them
> (using 's') to select (with 'y') only the parts I wanted to commit,
> ie, lines I had removed because they are not useful for me anymore. So
> I went for it and I did:
> 
> git commit -p myfile.py
> 
> Until this point all went as expected. The first hunk was printed in
> the console and the it waited for my decision. As always. After a
> couple of hunks I realized it was not possible to split all of them in
> a proper way (to select only removed lines) because new methods were
> mixed up with the old ones. So I cancel the process (ctrl + c) and
> started again using 'e' instead to manually edit the hunks.
> 
> So I repeated the last command: git commit -p myfile.py but this time
> I only used 'y', 'n' and 'e'. All in all I edited ('e') two big hunks.
> By doing this, the default editor (vim in my case) opened showing the
> hunk and with the instructions in how to edit at the bottom.
> Everything as used to be. Until this point all went as I expected.
> 
> In the two hunk I edited I did the same. I removed all + lines by
> deleting them and I make some - lines as context. Like explained in
> the bottom of the file.
> 
> When I was done with each of the hunks, I saved and closed the editor.
> No error was produced (I sometimes make mistakes while editing a hunk
> and know you can get an error (patch does not apply) here, but in this
> case were no errors there so the next hunk was printed and the commit
> procedure kept going.
> 
> When I was done with all hunks, the editor (vim) started again to
> write the commit message. I wrote (something like) this:
> 
>     myfile.py -> old unused methods removed...
> 
>     1) mymethod1
>     2) mymethod2
>     3) mymethod3
>     4) mymethod4
>     5) mymethod5
> 
> Yes, not the best commit ever ;) but enough for us in this case.
> Anyway, I saved it and close the editor and I was surprised by the git
> output.
> 
> [master 96d1c24] myfile.py -> old unused methods removed...
>  1 file changed, 182 insertions(+), 302 deletions(-)
>  rewrite myfile.py (60%)
> 
> What?? I thought to myself... Insertions?? This can't be true? I
> removed all + lines, my idea was to commit only deletions... What did
> I wrong?
> 
> To check this I did
> * git show 96d1c24
> and I saw only red minus lines , ie, deletions. As I expected.

What dows "git show --stat" say?

> To check it twice I used tig too.
> 
> tig showed the same for this specific commit
> myfile.py | 120
> ---------------------------------------------------------------------------------------------------------------------
> 1 file changed, 120 deletions(-)
> 
> Conclusion:
> After these two checks I am sure I did what I intended to do. What I
> do not understand is the output of git when I was done with the
> commit.
> 
> Maybe is just me, because I do not understand how git commit -p when
> using 'e' to edit hunks works. But as user I would expect to see only
> deletions in the output message if I am picking only deletions.
> 
> If this is a bug, I hope you can reproduce it on your system. If not,
> do not hesitate to contact me for further information.
> 
> 
> Kind regards,
> 
> Joan Aguilar
> --
> Joan Aguilar Lorente
> 

182-302 = -120

Did you make any changes in the lines that you left? Apparantly, that's
what the rewrite looked like to git commit.

Michael

Re: report on a possible bug: git commit -p myfile.py unexpected output

[Jeff King]

On Fri, Mar 24, 2017 at 03:59:07PM +0100, Michael J Gruber wrote:

> > [master 96d1c24] myfile.py -> old unused methods removed...
> >  1 file changed, 182 insertions(+), 302 deletions(-)
> >  rewrite myfile.py (60%)
> [...]
> > myfile.py | 120
> > ---------------------------------------------------------------------------------------------------------------------
> > 1 file changed, 120 deletions(-)
> 
> 182-302 = -120
> 
> Did you make any changes in the lines that you left? Apparantly, that's
> what the rewrite looked like to git commit.

Even without changes to the remaining lines, a rewrite diff would
consider them removed from the preimage and added again in the
post-image.

The difference between the two commands is that "commit" turns on "-B"
break detection by default, and "git show", "tig", etc, do not.

Looking at the actual diff with "git show -B" should show something
like:

  -old
  -lines
  -that
  -weren't
  -touched
  -some
  -lines
  -that
  -were
  -deleted
  +old
  +lines
  +that
  +weren't
  +touched

The change is the same no matter how you view it; the "-B" flag just
asks Git to show a non-minimal diff when the file was substantially
changed.

-Peff

Re: report on a possible bug: git commit -p myfile.py unexpected output

[Joan Aguilar]

Hello Michael, hello Jeff,

I was writing a response to Michael and I received the Email from
Jeff, so I decided to reply to the second one, with copy to both of
you (and the mailing list too, of course). I hope this is ok for you.

It works exactly as Jeff said.

If I do git show --stat 96d1c24 the output is:
user@machine:~/mygitrepo$ git show --stat 96d1c24
commit 96d1c24*******
Author: Joan Aguilar Lorente <joan.aguilar.lorente@gmail.com>
Date:   Thu Mar 23 18:15:07 2017 +0100

    myfile.py -> old unused methods removed...

    1) mymethod1
    2) mymethod2
    3) mymethod3
    4) mymethod4
    5) mymethod5

 myfile.py | 120
------------------------------------------------------------------------------------------------------------------------
 1 file changed, 120 deletions(-)

But if I add the flag -B (git show --stat -B 96d1c24) the last two
lines are different (as already expected by Jeff) and match exactly
the output of git commit I got yesterday.
 myfile.py | 484
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------------------------------------------------------------------------------------------------------------------------------------------------------------
 1 file changed, 182 insertions(+), 302 deletions(-)

The output of git show -B is, of course, like the one expected by Jeff too.

Thank you! I learned a little bit about git. And most of all, I
realize there are a lot of options and flags I am not aware of, and
not using at all! I have to read the documentation. I am missing a lot
of git!!

I am sorry that I reported this as possible bug. I guess I was just
confused because the "standard behavior" of "git commit" differs from
the one of "tig" or "git show".

Thank you again and I see you around.

Best regards
Joan Aguilar Lorente
--
Joan Aguilar Lorente


On Fri, Mar 24, 2017 at 4:09 PM, Jeff King <peff@peff.net> wrote:
> On Fri, Mar 24, 2017 at 03:59:07PM +0100, Michael J Gruber wrote:
>
>> > [master 96d1c24] myfile.py -> old unused methods removed...
>> >  1 file changed, 182 insertions(+), 302 deletions(-)
>> >  rewrite myfile.py (60%)
>> [...]
>> > myfile.py | 120
>> > ---------------------------------------------------------------------------------------------------------------------
>> > 1 file changed, 120 deletions(-)
>>
>> 182-302 = -120
>>
>> Did you make any changes in the lines that you left? Apparantly, that's
>> what the rewrite looked like to git commit.
>
> Even without changes to the remaining lines, a rewrite diff would
> consider them removed from the preimage and added again in the
> post-image.
>
> The difference between the two commands is that "commit" turns on "-B"
> break detection by default, and "git show", "tig", etc, do not.
>
> Looking at the actual diff with "git show -B" should show something
> like:
>
>   -old
>   -lines
>   -that
>   -weren't
>   -touched
>   -some
>   -lines
>   -that
>   -were
>   -deleted
>   +old
>   +lines
>   +that
>   +weren't
>   +touched
>
> The change is the same no matter how you view it; the "-B" flag just
> asks Git to show a non-minimal diff when the file was substantially
> changed.
>
> -Peff

Re: report on a possible bug: git commit -p myfile.py unexpected output

[Jeff King]

On Fri, Mar 24, 2017 at 04:31:09PM +0100, Joan Aguilar wrote:

> Thank you! I learned a little bit about git. And most of all, I
> realize there are a lot of options and flags I am not aware of, and
> not using at all! I have to read the documentation. I am missing a lot
> of git!!
> 
> I am sorry that I reported this as possible bug. I guess I was just
> confused because the "standard behavior" of "git commit" differs from
> the one of "tig" or "git show".

No problem. Thanks for a thorough report; it was easy to see what the
problem was from reading it.

-Peff