threads / discuss / 34937

1.8.4 rebase regression?

Subject: 1.8.4 rebase regression?

## tl;dr

10 messages between Sep 15, 2013 and Sep 17, 2013.

replies: 9people: 2as markdown or json

Patrick Welche· Sep 15, 2013, 23:57 UTC · lore

I just upgraded (via pkgsrc) from git 1.8.3.4 to 1.8.4. With 1.8.4, I had local changes in glib, did a git pull --rebase. Some of my changes conflicted, but

$ git rebase --abort No rebase in progress?

so somehow the usual process of amending the edit, or skipping the patch no longer works.

I found a similar report at: http://mail-index.netbsd.org/pkgsrc-users/2013/09/14/msg018646.html

Another less important regression is that in an xterm, with 1.8.3.4 I see

$ /tmp/bin/git diff diff --cc glib/gmain.c index 738e69c,5aaebd0..0000000

--- a/glib/gmain.c
+++ b/glib/gmain.c
@@@ -4953,32 -4921,32 +4953,48 @@@ g_unix_signal_watch_dispatch (GSourc
  }
...

but with 1.8.4, I see

$ git diff
ESC[1mdiff --cc glib/gmain.cESC[m
ESC[1mindex 738e69c,5aaebd0..0000000ESC[m
ESC[1m--- a/glib/gmain.cESC[m
ESC[1m+++ b/glib/gmain.cESC[m
ESC[36m@@@ -4953,32 -4921,32 +4953,48 @@@ESC[m ESC[mg_unix_signal_watch_dispatch (GSourcESC[m


(same xterm, no change of TERM in both invocations above)
git status in 1.8.4 does show red, so colour does work...

Thoughts on how to help debug?

Cheers,

Patrick

Matthieu Moy· Sep 16, 2013, 07:52 UTC · re: Patrick Welche · lore

Re: 1.8.4 rebase regression?

Patrick Welche <prlw1@cam.ac.uk> writes:

Show 12 quoted lines

> I just upgraded (via pkgsrc) from git 1.8.3.4 to 1.8.4. With 1.8.4, I had
> local changes in glib, did a git pull --rebase. Some of my changes
> conflicted, but
>
> $ git rebase --abort
> No rebase in progress?
>
> so somehow the usual process of amending the edit, or skipping the patch
> no longer works.
>
> I found a similar report at:
> http://mail-index.netbsd.org/pkgsrc-users/2013/09/14/msg018646.html

Does the following patch fix this issue?

  https://github.com/git/git/commit/99855dd

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Patrick Welche· Sep 16, 2013, 10:28 UTC · re: Matthieu Moy · lore

Re: 1.8.4 rebase regression?

On Mon, Sep 16, 2013 at 09:52:35AM +0200, Matthieu Moy wrote:

Show 18 quoted lines

> Patrick Welche <prlw1@cam.ac.uk> writes:
> 
> > I just upgraded (via pkgsrc) from git 1.8.3.4 to 1.8.4. With 1.8.4, I had
> > local changes in glib, did a git pull --rebase. Some of my changes
> > conflicted, but
> >
> > $ git rebase --abort
> > No rebase in progress?
> >
> > so somehow the usual process of amending the edit, or skipping the patch
> > no longer works.
> >
> > I found a similar report at:
> > http://mail-index.netbsd.org/pkgsrc-users/2013/09/14/msg018646.html
> 
> Does the following patch fix this issue?
> 
>   https://github.com/git/git/commit/99855dd

Yes thanks!

CONFLICT (content): Merge conflict in glib/gmain.c Failed to merge in the changes.

:-)

Any thoughts of the cosmetic ESC[31m change?

Cheers,

Patrick

Matthieu Moy· Sep 16, 2013, 11:18 UTC · re: Patrick Welche · lore

Re: 1.8.4 rebase regression?

Patrick Welche <prlw1@cam.ac.uk> writes:

Show 12 quoted lines

> $ git diff
> ESC[1mdiff --cc glib/gmain.cESC[m
> ESC[1mindex 738e69c,5aaebd0..0000000ESC[m
> ESC[1m--- a/glib/gmain.cESC[m
> ESC[1m+++ b/glib/gmain.cESC[m
> ESC[36m@@@ -4953,32 -4921,32 +4953,48 @@@ESC[m ESC[mg_unix_signal_watch_dispatch (GSourcESC[m
>
>
> (same xterm, no change of TERM in both invocations above)
> git status in 1.8.4 does show red, so colour does work...
>
> Thoughts on how to help debug?

Can you try:

git -c color.ui=never diff git -c color.ui=auto diff git -c color.ui=always diff

If you have a bit of time, you can use "git bisect" on a clone of git.git to find out the guilty commit.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Matthieu Moy· Sep 17, 2013, 07:15 UTC · re: Matthieu Moy · lore

Re: 1.8.4 rebase regression?

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

Show 22 quoted lines

> Patrick Welche <prlw1@cam.ac.uk> writes:
>
>> $ git diff
>> ESC[1mdiff --cc glib/gmain.cESC[m
>> ESC[1mindex 738e69c,5aaebd0..0000000ESC[m
>> ESC[1m--- a/glib/gmain.cESC[m
>> ESC[1m+++ b/glib/gmain.cESC[m
>> ESC[36m@@@ -4953,32 -4921,32 +4953,48 @@@ESC[m ESC[mg_unix_signal_watch_dispatch (GSourcESC[m
>>
>>
>> (same xterm, no change of TERM in both invocations above)
>> git status in 1.8.4 does show red, so colour does work...
>>
>> Thoughts on how to help debug?
>
> Can you try:
>
> git -c color.ui=never diff
> git -c color.ui=auto diff
> git -c color.ui=always diff
>
> ?

... and Junio suggested offline to look for a broken pager, so, you can try this too:

git --no-pager diff

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Patrick Welche· Sep 17, 2013, 09:15 UTC · re: Matthieu Moy · lore

Re: 1.8.4 rebase regression?

On Tue, Sep 17, 2013 at 09:15:43AM +0200, Matthieu Moy wrote:

Show 29 quoted lines

> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
> 
> > Patrick Welche <prlw1@cam.ac.uk> writes:
> >
> >> $ git diff
> >> ESC[1mdiff --cc glib/gmain.cESC[m
> >> ESC[1mindex 738e69c,5aaebd0..0000000ESC[m
> >> ESC[1m--- a/glib/gmain.cESC[m
> >> ESC[1m+++ b/glib/gmain.cESC[m
> >> ESC[36m@@@ -4953,32 -4921,32 +4953,48 @@@ESC[m ESC[mg_unix_signal_watch_dispatch (GSourcESC[m
> >>
> >>
> >> (same xterm, no change of TERM in both invocations above)
> >> git status in 1.8.4 does show red, so colour does work...
> >>
> >> Thoughts on how to help debug?
> >
> > Can you try:
> >
> > git -c color.ui=never diff
> > git -c color.ui=auto diff
> > git -c color.ui=always diff
> >
> > ?
> 
> ... and Junio suggested offline to look for a broken pager, so, you can
> try this too:
> 
> git --no-pager diff

Even better - colour works here too => I must look at my pager.

Thank you!

Patrick

Patrick Welche· Sep 17, 2013, 09:13 UTC · re: Matthieu Moy · lore

Re: 1.8.4 rebase regression?

On Mon, Sep 16, 2013 at 01:18:48PM +0200, Matthieu Moy wrote:

Show 22 quoted lines

> Patrick Welche <prlw1@cam.ac.uk> writes:
> 
> > $ git diff
> > ESC[1mdiff --cc glib/gmain.cESC[m
> > ESC[1mindex 738e69c,5aaebd0..0000000ESC[m
> > ESC[1m--- a/glib/gmain.cESC[m
> > ESC[1m+++ b/glib/gmain.cESC[m
> > ESC[36m@@@ -4953,32 -4921,32 +4953,48 @@@ESC[m ESC[mg_unix_signal_watch_dispatch (GSourcESC[m
> >
> >
> > (same xterm, no change of TERM in both invocations above)
> > git status in 1.8.4 does show red, so colour does work...
> >
> > Thoughts on how to help debug?
> 
> Can you try:
> 
> git -c color.ui=never diff
> git -c color.ui=auto diff
> git -c color.ui=always diff
> 
> ?

Got it: the change between 1.8.3.4 and 1.8.4 is that colour is on by default. If I take 1.8.3.4 and git -c color.ui=always log, I see the same ESC codes => not a regression! I'll just have to sort my box out if I want colour. (The only oddity is that git status is correctly colourful.)

> If you have a bit of time, you can use "git bisect" on a clone of
> git.git to find out the guilty commit.

commit e5be297279e8ee8c503eb59da21ab17edc40e748
Merge: a3bc3d0 6897a64

but that is presumably just when the default changed )

Sorry for the confusion - it just looked like a regression to me, but isn't!

Cheers,

Patrick

Matthieu Moy· Sep 17, 2013, 09:23 UTC · re: Patrick Welche · lore

Re: 1.8.4 rebase regression?

Patrick Welche <prlw1@cam.ac.uk> writes:

Show 5 quoted lines

> Got it: the change between 1.8.3.4 and 1.8.4 is that colour is on by
> default. If I take 1.8.3.4 and git -c color.ui=always log, I see
> the same ESC codes => not a regression! I'll just have to sort my
> box out if I want colour. (The only oddity is that git status is
> correctly colourful.)

Maybe you disabled the pager for "git status"?

Show 9 quoted lines

> (
>> If you have a bit of time, you can use "git bisect" on a clone of
>> git.git to find out the guilty commit.
>
> commit e5be297279e8ee8c503eb59da21ab17edc40e748
> Merge: a3bc3d0 6897a64
>
> but that is presumably just when the default changed
> )

It doesn't seem so.

On my box, color work just fine when piped to less, but I guess not all pagers accept this, and perhaps a wrong value of $LESS can cause less to reject it.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Patrick Welche· Sep 17, 2013, 10:07 UTC · re: Matthieu Moy · lore

Re: 1.8.4 rebase regression?

On Tue, Sep 17, 2013 at 11:23:51AM +0200, Matthieu Moy wrote:

Show 9 quoted lines

> Patrick Welche <prlw1@cam.ac.uk> writes:
> 
> > Got it: the change between 1.8.3.4 and 1.8.4 is that colour is on by
> > default. If I take 1.8.3.4 and git -c color.ui=always log, I see
> > the same ESC codes => not a regression! I'll just have to sort my
> > box out if I want colour. (The only oddity is that git status is
> > correctly colourful.)
> 
> Maybe you disabled the pager for "git status"?

Interesting: I didn't disable it on purpose, but indeed, it is not
used for status.

Can't believe this:

$ echo $PAGER more

unset PAGER, and git diff is fine...

Thanks for all the help!

Cheers,

Patrick

Matthieu Moy· Sep 17, 2013, 10:59 UTC · re: Patrick Welche · lore

Re: 1.8.4 rebase regression?

Patrick Welche <prlw1@cam.ac.uk> writes:

Show 13 quoted lines

> On Tue, Sep 17, 2013 at 11:23:51AM +0200, Matthieu Moy wrote:
>> Patrick Welche <prlw1@cam.ac.uk> writes:
>> 
>> > Got it: the change between 1.8.3.4 and 1.8.4 is that colour is on by
>> > default. If I take 1.8.3.4 and git -c color.ui=always log, I see
>> > the same ESC codes => not a regression! I'll just have to sort my
>> > box out if I want colour. (The only oddity is that git status is
>> > correctly colourful.)
>> 
>> Maybe you disabled the pager for "git status"?
>
> Interesting: I didn't disable it on purpose, but indeed, it is not
> used for status.

Ah, my bad. Whether the pager should be enabled by default for status led to a lot of debates here, and I thought the conclusion was yes. But I'm the one having it enabled by default:

[pager]
        status = true

Show 6 quoted lines

> Can't believe this:
>
> $ echo $PAGER
> more
>
> unset PAGER, and git diff is fine...

less will be a much better pager than more, indeed. The default behavior of less is sometimes anoying (open full-page, and 'q' restores the terminal, which is very inconvenient for short output), but if you didn't set $LESS, then Git will set it for you to something appropriate for git pager.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/