Missing changes in merges from emacs-25 to master

Missing changes in merges from emacs-25 to master

[martin rudalics]

Hi, Paul and friends

Something in these commits seems to have made earlier changes to master
reverted:


commit cb4e054e41cdb7e398351a5ae8224759e721349e
Merge: e6d5753 6090275
Author: Paul Eggert <eggert@cs.ucla.edu>
Date:   Sat Jan 30 11:43:26 2016 -0800

     -

commit 2b41d6a979b0ea361e078891b8763b4ae7be8092
Merge: fe9c8b6 71468e0
Author: Paul Eggert <eggert@cs.ucla.edu>
Date:   Sat Jan 30 11:28:37 2016 -0800

     -

commit 82b089783e71b2aeef950eaecfe4cbc0735e64a2
Merge: d27c807 f7dc6d8
Author: Paul Eggert <eggert@cs.ucla.edu>
Date:   Sat Jan 30 11:27:34 2016 -0800

     -


In particular, in commit aa636c8465b03c09249dccd68fca3d1df04eaa2c Eli
fixed a bug in window.c by writing

	  m->total_cols = new_size;

instead of the earlier

	  m->total_cols = size;

IIUC after commit 82b089783e71b2aeef950eaecfe4cbc0735e64a2 the earlier
line reappeared.

Can you please have a look?  I somehow have the feeling that more such
issues might have occurred but I can easily be wrong.  Worse even, I
could have introduced such a bug myself in one of my later commits but
don't see any evidence so far :-(

Thanks, martin

Re: Missing changes in merges from emacs-25 to master

[Eli Zaretskii]

> Date: Sun, 20 Mar 2016 12:36:07 +0100
> From: martin rudalics <rudalics@gmx.at>
> Cc: emacs-devel <emacs-devel@gnu.org>
> 
> Hi, Paul and friends
> 
> Something in these commits seems to have made earlier changes to master
> reverted:
> 
> 
> commit cb4e054e41cdb7e398351a5ae8224759e721349e
> Merge: e6d5753 6090275
> Author: Paul Eggert <eggert@cs.ucla.edu>
> Date:   Sat Jan 30 11:43:26 2016 -0800
> 
>      -
> 
> commit 2b41d6a979b0ea361e078891b8763b4ae7be8092
> Merge: fe9c8b6 71468e0
> Author: Paul Eggert <eggert@cs.ucla.edu>
> Date:   Sat Jan 30 11:28:37 2016 -0800
> 
>      -
> 
> commit 82b089783e71b2aeef950eaecfe4cbc0735e64a2
> Merge: d27c807 f7dc6d8
> Author: Paul Eggert <eggert@cs.ucla.edu>
> Date:   Sat Jan 30 11:27:34 2016 -0800
> 
>      -

How did you conclude that these commits are the culprit?

Re: Missing changes in merges from emacs-25 to master

[martin rudalics]

 > How did you conclude that these commits are the culprit?

Running

git diff fbce4757a874cc43806eb41b8637538b101c3c69..82b089783e71b2aeef950eaecfe4cbc0735e64a2 src/window.c

gets me

diff --git a/src/window.c b/src/window.c
index 8953d3c..add2de3 100644
--- a/src/window.c
+++ b/src/window.c
@@ -35,13 +35,15 @@
  #include "dispextern.h"
  #include "blockinput.h"
  #include "termhooks.h"		/* For FRAME_TERMINAL.  */
-#include "xwidget.h"
  #ifdef HAVE_WINDOW_SYSTEM
  #include TERM_HEADER
  #endif /* HAVE_WINDOW_SYSTEM */
  #ifdef MSDOS
  #include "msdos.h"
  #endif
+#ifdef HAVE_XWIDGETS
+# include "xwidget.h"
+#endif

  static ptrdiff_t count_windows (struct window *);
  static ptrdiff_t get_leaf_windows (struct window *, struct window **,
@@ -3971,11 +3973,9 @@ depends on the value of (window-start WINDOW), so if calling this
  }


-/* Resize frame F's windows when F's width or height is set to SIZE.
-   If HORFLAG is zero, F's width was set to SIZE, otherwise its height
-   was set.  SIZE is interpreted in F's canonical character units
-   (a.k.a. "columns" or "lines"), unless PIXELWISE is non-zero, which
-   means to interpret SIZE in pixel units.  */
+/* Resize frame F's windows when number of lines of F is set to SIZE.
+   HORFLAG means resize windows when number of columns of F is set to
+   SIZE.  PIXELWISE means to interpret SIZE as pixels.  */
  void
  resize_frame_windows (struct frame *f, int size, bool horflag, bool pixelwise)
  {
@@ -4076,7 +4076,7 @@ depends on the value of (window-start WINDOW), so if calling this
        m = XWINDOW (mini);
        if (horflag)
  	{
-	  m->total_cols = new_size;
+	  m->total_cols = size;
  	  m->pixel_width = new_pixel_size;
  	}
        else
@@ -4371,7 +4371,9 @@ SIDE t (or `right') specifies that the new window shall be located on

        /* Block input.  */
        block_input ();
+#ifdef HAVE_XWIDGETS
        xwidget_view_delete_all_in_window (w);
+#endif
        window_resize_apply (p, horflag);
        /* If this window is referred to by the dpyinfo's mouse
  	 highlight, invalidate that slot to be safe (Bug#9904).  */
@@ -7263,7 +7265,7 @@ Value is a list of the form (WIDTH COLUMNS VERTICAL-TYPE HEIGHT LINES

  If this variable is t, splitting a window tries to get the space
  proportionally from all windows in the same combination.  This also
-allows splitting a window that is otherwise too small or of fixed size.
+allows to split a window that is otherwise too small or of fixed size.
  Resizing and deleting a window proportionally resize all windows in the
  same combination.


What do you get?

martin

Re: Missing changes in merges from emacs-25 to master

[Eli Zaretskii]

> Date: Sun, 20 Mar 2016 19:38:17 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: eggert@cs.ucla.edu, emacs-devel@gnu.org
> 
>  > How did you conclude that these commits are the culprit?
> 
> Running
> 
> git diff fbce4757a874cc43806eb41b8637538b101c3c69..82b089783e71b2aeef950eaecfe4cbc0735e64a2 src/window.c

What is fbce4757a874cc43806eb41b8637538b101c3c69, and how is it
related to the issue?

My problem is that "git log -L" doesn't show my change at all,
although searching through "git log src/window.c" I do see it.  So I'm
inclined to trust nothing I see about this particular matter.  E.g.,
are we sure this didn't happen as part of the merges from emacs-25?

Re: Missing changes in merges from emacs-25 to master

[martin rudalics]

 > What is fbce4757a874cc43806eb41b8637538b101c3c69, and how is it
 > related to the issue?

It's the last commit before 82b089783e71b2aeef950eaecfe4cbc0735e64a2
that affects window.c (if I can trust C-x v l in this regard).

commit 82b089783e71b2aeef950eaecfe4cbc0735e64a2
Merge: d27c807 f7dc6d8
Author: Paul Eggert <eggert@cs.ucla.edu>
Date:   Sat Jan 30 11:27:34 2016 -0800

     -

commit fbce4757a874cc43806eb41b8637538b101c3c69
Author: Alan Mackenzie <acm@muc.de>
Date:   Sun Jan 24 20:30:39 2016 +0000

 > My problem is that "git log -L" doesn't show my change at all,
 > although searching through "git log src/window.c" I do see it.  So I'm
 > inclined to trust nothing I see about this particular matter.  E.g.,
 > are we sure this didn't happen as part of the merges from emacs-25?

I'm quite confident that it did happen as part of these merges.  That's
why I asked for clarification.  I mentioned earlier here that IMO some
commits were lost during the merges, but this change (and that of the
comment that precedes it) are the first ones I could locate.  I have no
idea how to find the merge that caused the loss of these two changes.

martin

Re: Missing changes in merges from emacs-25 to master

[Paul Eggert]

As near as I can make out, the changes in this commit:

commit aa636c8465b03c09249dccd68fca3d1df04eaa2c
Author: Eli Zaretskii <eliz@gnu.org>
Date:   Fri Jan 15 17:07:38 2016 +0200

     Fix crashes when mini-window has non-zero margins


were first lost in this merge commit:

commit 7823745acbe9b87eea2db4ef434e379fc903ec35
Merge: 6acd2aa bb0cd31
Author: John Wiegley <johnw@newartisans.com>
Date:   Mon Jan 18 22:56:33 2016 -0800

     ; Merge from origin/emacs-25

     The following commit was skipped:

     bb0cd31 Merge branch 'emacs-25' of git.sv.gnu.org:/srv/git/emacs into emacs-25


This merge commit skips a merge, which I think causes this particular problem. 
This merge commit is a parent of:

commit 140016558bcb462692d8b9f2f3f866e0f337d218
Merge: 7823745 3ae7934
Author: John Wiegley <johnw@newartisans.com>
Date:   Mon Jan 18 22:56:34 2016 -0800

     Merge from origin/emacs-25

     3ae7934 ; * etc/NEWS: Mark entries that don't need further treatment.
     6165c36 * lisp/files.el (dir-locals--all-files): Respect absolute file-names
     2ffdf15 * lisp/help-fns.el (describe-variable): Fix a left-over parenthesis
     71ecd62 * lisp/dired-x.el (dired-omit-here-always): Use add-dir-local-variable
     f0b82b3 * lisp/files.el (dir-locals--all-files): Use completion instead of 
wildcards
     86e4513 Fix incompatbilities with MS-Windows 2000 and older
     4e96521 Mention in PROBLEMS an issue with MS-Windows NT4
     15c23aa Ensure 8-byte aligned memory allocation on MS-Windows 9X
     39afa42 Fix tests for active region in hideif.el
     05df666 Fix interactive specs in some hideif.el commands


which in turn is a parent of:

commit bca49307c8ea3f96d6eede375a7f42091ae5d5af
Merge: 1400165 6e79b63
Author: John Wiegley <johnw@newartisans.com>
Date:   Mon Jan 18 22:56:34 2016 -0800

     ; Merge from origin/emacs-25

     The following commit was skipped:

     6e79b63 ; Auto-commit of loaddefs files.


which in turn is a parent of:

commit 1b76d9168336ede8976b980aeaed64ae2908501a
Merge: bca4930 2e5a89f
Author: John Wiegley <johnw@newartisans.com>
Date:   Mon Jan 18 22:59:51 2016 -0800

     -

which in turn is a parent of:

commit b895c72059521fec064ff27b4cfcfa4104081c4e
Merge: badcd38 1b76d91
Author: Vincent Belaïche <vincentb1@users.sourceforge.net>
Date:   Wed Jan 20 08:30:51 2016 +0100

     Merge branch 'master' of git.sv.gnu.org:/srv/git/emacs

which leads via a long thin graph to the merge commit 
82b089783e71b2aeef950eaecfe4cbc0735e64a2 that you identified as losing the 
change. One of the entries in that graph is this commit:

commit 64a568d6100e5eab5b86e8bd31846212654d1fcf
Author: Eli Zaretskii <eliz@gnu.org>
Date:   Wed Jan 20 11:47:19 2016 +0200

     Fix MS-Windows build broken by a botched merge from emacs-25

     * src/w32.c (w32_crypto_hprov): New static variable.
     (globals_of_w32): Initialize w32_crypto_hprov.
     (w32_init_crypt_random, w32_init_random): New functions.
     Include wincrypt.h.
     * src/w32.h (w32_init_random): Add prototype.


which indicates that Eli noticed a problem around then.

Re: Missing changes in merges from emacs-25 to master

[martin rudalics]

[...]
 > which in turn is a parent of:
 >
 > commit 1b76d9168336ede8976b980aeaed64ae2908501a
 > Merge: bca4930 2e5a89f
 > Author: John Wiegley <johnw@newartisans.com>
 > Date:   Mon Jan 18 22:59:51 2016 -0800
 >
 >      -
 >
 > which in turn is a parent of:
 >
 > commit b895c72059521fec064ff27b4cfcfa4104081c4e
 > Merge: badcd38 1b76d91
 > Author: Vincent Belaïche <vincentb1@users.sourceforge.net>
 > Date:   Wed Jan 20 08:30:51 2016 +0100
 >
 >      Merge branch 'master' of git.sv.gnu.org:/srv/git/emacs
 >
 > which leads via a long thin graph to the merge commit
 > 82b089783e71b2aeef950eaecfe4cbc0735e64a2 that you identified as losing
 > the change.

Thanks for the forensics.  I have no idea which tools to use to trace
such a path.  One thing that stupefies me is that in

commit fbce4757a874cc43806eb41b8637538b101c3c69
Author: Alan Mackenzie <acm@muc.de>
Date:   Sun Jan 24 20:30:39 2016 +0000

which IIUC follows the commits you cited above, the "correct" version is
still there why it should have been already lost at least on January 20.

martin

Re: Missing changes in merges from emacs-25 to master

[Paul Eggert]

martin rudalics wrote:
> Thanks for the forensics.  I have no idea which tools to use to trace
> such a path.

You might try gitk. It's not perfect, but it beats 'git log' for this sort of thing.

 > One thing that stupefies me is that in
>
> commit fbce4757a874cc43806eb41b8637538b101c3c69
> Author: Alan Mackenzie <acm@muc.de>
> Date:   Sun Jan 24 20:30:39 2016 +0000
>
> which IIUC follows the commits you cited above, the "correct" version is
> still there why it should have been already lost at least on January 20.

That patch was applied after the problematic merge 
e6d575316a42946aac6d83c9587f09afd1a59d98 that propagated the problems I 
identified in my previous email, so it is immune to those problems.

Specifically, the problematic merge combined 
fef1b3066ae02ec6dd37b0358905491fabbedfdf (which did not have the problem) and 
9304e6d3a222de6ef1c405bfd6e09eee6ed2c2aa (which did have the problem), whereas 
the commit you cite is an immediate child of 
fef1b3066ae02ec6dd37b0358905491fabbedfdf and was therefore applied in the next 
merge (cb4e054e41cdb7e398351a5ae8224759e721349e).

Re: Missing changes in merges from emacs-25 to master

[martin rudalics]

 > You might try gitk. It's not perfect, but it beats 'git log' for this
 > sort of thing.

I'm probably too silly but neither with gitk I can visualize the commit
that did reset

       m->total_cols = new_size;

to

       m->total_cols = size;

 >  > One thing that stupefies me is that in
 >>
 >> commit fbce4757a874cc43806eb41b8637538b101c3c69
 >> Author: Alan Mackenzie <acm@muc.de>
 >> Date:   Sun Jan 24 20:30:39 2016 +0000
 >>
 >> which IIUC follows the commits you cited above, the "correct" version is
 >> still there why it should have been already lost at least on January 20.
 >
 > That patch was applied after the problematic merge
 > e6d575316a42946aac6d83c9587f09afd1a59d98 that propagated the problems
 > I identified in my previous email, so it is immune to those problems.
 >
 > Specifically, the problematic merge combined
 > fef1b3066ae02ec6dd37b0358905491fabbedfdf (which did not have the
 > problem) and 9304e6d3a222de6ef1c405bfd6e09eee6ed2c2aa (which did have
 > the problem), whereas the commit you cite is an immediate child of
 > fef1b3066ae02ec6dd37b0358905491fabbedfdf and was therefore applied in
 > the next merge (cb4e054e41cdb7e398351a5ae8224759e721349e).

I'm still lost.

(1) I do C-x v l in master's window.c.

(2) I go to the fragment

commit fbce4757a874cc43806eb41b8637538b101c3c69
Author: Alan Mackenzie <acm@muc.de>
Date:   Sun Jan 24 20:30:39 2016 +0000

and on the commit line I type 'f' which gets me a buffer called
window.c.~fbce4757a874cc43806eb41b8637538b101c3c69~.  Line 4079 of that
buffer shows

	  m->total_cols = new_size;

Now IIUC this means that this line "was in master once".  Or am I wrong?

(3) Now I go to the fragment

commit 82b089783e71b2aeef950eaecfe4cbc0735e64a2
Merge: d27c807 f7dc6d8
Author: Paul Eggert <eggert@cs.ucla.edu>
Date:   Sat Jan 30 11:27:34 2016 -0800

and on the commit line I type 'f' which gets me a buffer called
window.c.~82b089783e71b2aeef950eaecfe4cbc0735e64a2~.  Line 4079 of that
buffer shows me

	  m->total_cols = size;

Since as you said 82b089783e71b2aeef950eaecfe4cbc0735e64a2 is not the
offending commit there must have been a commit (on master) in between
the commits I cited above that did reset line 4079.  If so, how do I
find that offending commit?  Apparently "git log src/window.c" won't
find it which IIUC renders this command useless for tracking merges.

Neither do

git show 9304e6d3a222de6ef1c405bfd6e09eee6ed2c2aa
git show cb4e054e41cdb7e398351a5ae8224759e721349e

show any changes to window.c.  And

git show 7823745acbe9b87eea2db4ef434e379fc903ec35

which you cited earlier as the culprit only tells me that

commit 7823745acbe9b87eea2db4ef434e379fc903ec35
Merge: 6acd2aa bb0cd31
Author: John Wiegley <johnw@newartisans.com>
Date:   Mon Jan 18 22:56:33 2016 -0800

     ; Merge from origin/emacs-25

     The following commit was skipped:

     bb0cd31 Merge branch 'emacs-25' of git.sv.gnu.org:/srv/git/emacs into emacs-25

So was that line that in (2) above apparently "was in master once"
removed silently?

martin

Re: Missing changes in merges from emacs-25 to master

[Andreas Schwab]

martin rudalics <rudalics@gmx.at> writes:

> Neither do
>
> git show 9304e6d3a222de6ef1c405bfd6e09eee6ed2c2aa
> git show cb4e054e41cdb7e398351a5ae8224759e721349e

Try git show -m to see both sides.  A diff for a merge commit by default
only shows changes that differ from both sides.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: Missing changes in merges from emacs-25 to master

[martin rudalics]

 >> Neither do
 >>
 >> git show 9304e6d3a222de6ef1c405bfd6e09eee6ed2c2aa
 >> git show cb4e054e41cdb7e398351a5ae8224759e721349e
 >
 > Try git show -m to see both sides.  A diff for a merge commit by default
 > only shows changes that differ from both sides.

Thanks, this seems to show it.  But

git show -m 9304e6d3a222de6ef1c405bfd6e09eee6ed2c2aa

gets me some 500.000 lines of output.  Is there a way to restrict
that to src/window.c?

martin

Re: Missing changes in merges from emacs-25 to master

[Andreas Schwab]

martin rudalics <rudalics@gmx.at> writes:

> git show -m 9304e6d3a222de6ef1c405bfd6e09eee6ed2c2aa
>
> gets me some 500.000 lines of output.  Is there a way to restrict
> that to src/window.c?

git diff 9304e6d3a222de6ef1c405bfd6e09eee6ed2c2aa^2 9304e6d3a222de6ef1c405bfd6e09eee6ed2c2aa -- src/window.c

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: Missing changes in merges from emacs-25 to master

[martin rudalics]

 > git diff 9304e6d3a222de6ef1c405bfd6e09eee6ed2c2aa^2  9304e6d3a222de6ef1c405bfd6e09eee6ed2c2aa -- src/window.c

With git version 1.9.4.msysgit.2 (on Windows) this gets me

git diff 9304e6d3a222de6ef1c405bfd6e09eee6ed2c2aa^2 9304e6d3a222de6ef1c405bfd6e09eee6ed2c2aa -- src/window.c
fatal: bad revision '9304e6d3a222de6ef1c405bfd6e09eee6ed2c2aa2'

martin

Re: Missing changes in merges from emacs-25 to master

[Óscar Fuentes]

martin rudalics <rudalics@gmx.at> writes:

>> git diff 9304e6d3a222de6ef1c405bfd6e09eee6ed2c2aa^2  9304e6d3a222de6ef1c405bfd6e09eee6ed2c2aa -- src/window.c
>
> With git version 1.9.4.msysgit.2 (on Windows) this gets me
>
> git diff 9304e6d3a222de6ef1c405bfd6e09eee6ed2c2aa^2 9304e6d3a222de6ef1c405bfd6e09eee6ed2c2aa -- src/window.c
> fatal: bad revision '9304e6d3a222de6ef1c405bfd6e09eee6ed2c2aa2'

Most probably that git version is too old to support the ^2 syntax.
Please install the latest Git for Windows:

https://git-for-windows.github.io/

The output is

diff --git a/src/window.c b/src/window.c
index 9e065ea..70b7e58 100644
--- a/src/window.c
+++ b/src/window.c
@@ -3971,11 +3971,9 @@ values.  */)
 }
 
 
-/* Resize frame F's windows when F's width or height is set to SIZE.
-   If HORFLAG is zero, F's width was set to SIZE, otherwise its height
-   was set.  SIZE is interpreted in F's canonical character units
-   (a.k.a. "columns" or "lines"), unless PIXELWISE is non-zero, which
-   means to interpret SIZE in pixel units.  */
+/* Resize frame F's windows when number of lines of F is set to SIZE.
+   HORFLAG means resize windows when number of columns of F is set to
+   SIZE.  PIXELWISE means to interpret SIZE as pixels.  */
 void
 resize_frame_windows (struct frame *f, int size, bool horflag, bool pixelwise)
 {
@@ -4076,7 +4074,7 @@ resize_frame_windows (struct frame *f, int size, bool horflag, bool pixelwise)
       m = XWINDOW (mini);
       if (horflag)
        {
-         m->total_cols = new_size;
+         m->total_cols = size;
          m->pixel_width = new_pixel_size;
        }
       else

Re: Missing changes in merges from emacs-25 to master

[martin rudalics]

 > Most probably that git version is too old to support the ^2 syntax.

That's what I suspected initially but it turned out that I was just too
sloppy when reading the error message.

 > diff --git a/src/window.c b/src/window.c
 > index 9e065ea..70b7e58 100644
 > --- a/src/window.c
 > +++ b/src/window.c
 > @@ -3971,11 +3971,9 @@ values.  */)
 >   }
 >
 >
 > -/* Resize frame F's windows when F's width or height is set to SIZE.
 > -   If HORFLAG is zero, F's width was set to SIZE, otherwise its height
 > -   was set.  SIZE is interpreted in F's canonical character units
 > -   (a.k.a. "columns" or "lines"), unless PIXELWISE is non-zero, which
 > -   means to interpret SIZE in pixel units.  */
 > +/* Resize frame F's windows when number of lines of F is set to SIZE.
 > +   HORFLAG means resize windows when number of columns of F is set to
 > +   SIZE.  PIXELWISE means to interpret SIZE as pixels.  */
 >   void
 >   resize_frame_windows (struct frame *f, int size, bool horflag, bool pixelwise)
 >   {
 > @@ -4076,7 +4074,7 @@ resize_frame_windows (struct frame *f, int size, bool horflag, bool pixelwise)
 >         m = XWINDOW (mini);
 >         if (horflag)
 >          {
 > -         m->total_cols = new_size;
 > +         m->total_cols = size;
 >            m->pixel_width = new_pixel_size;
 >          }
 >         else
 >
 >
 >

Exactly what I was looking for and what I now get here as well ;-)

Thanks, martin

Re: Missing changes in merges from emacs-25 to master

[Noam Postavsky]

On Tue, Mar 22, 2016 at 7:47 AM, martin rudalics <rudalics@gmx.at> wrote:
>> git diff 9304e6d3a222de6ef1c405bfd6e09eee6ed2c2aa^2
>> 9304e6d3a222de6ef1c405bfd6e09eee6ed2c2aa -- src/window.c
>
> With git version 1.9.4.msysgit.2 (on Windows) this gets me
>
> git diff 9304e6d3a222de6ef1c405bfd6e09eee6ed2c2aa^2
> 9304e6d3a222de6ef1c405bfd6e09eee6ed2c2aa -- src/window.c
> fatal: bad revision '9304e6d3a222de6ef1c405bfd6e09eee6ed2c2aa2'

If you were doing this from a cmd.exe prompt, then ^ is an escape
character so you need to enter it twice: git diff 9304...d2c2aa^^2

Re: Missing changes in merges from emacs-25 to master

[martin rudalics]

 >> git diff 9304e6d3a222de6ef1c405bfd6e09eee6ed2c2aa^2
 >> 9304e6d3a222de6ef1c405bfd6e09eee6ed2c2aa -- src/window.c
 >> fatal: bad revision '9304e6d3a222de6ef1c405bfd6e09eee6ed2c2aa2'
 >
 > If you were doing this from a cmd.exe prompt, then ^ is an escape
 > character so you need to enter it twice: git diff 9304...d2c2aa^^2

It should have been obvious from the message but I was in a hurry ...

Thank you very much for the explanation, martin

Re: Missing changes in merges from emacs-25 to master

[Eli Zaretskii]

> Cc: Eli Zaretskii <eliz@gnu.org>, emacs-devel@gnu.org,
>  John Wiegley <johnw@newartisans.com>
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Mon, 21 Mar 2016 00:36:29 -0700
> 
> commit 64a568d6100e5eab5b86e8bd31846212654d1fcf
> Author: Eli Zaretskii <eliz@gnu.org>
> Date:   Wed Jan 20 11:47:19 2016 +0200
> 
>      Fix MS-Windows build broken by a botched merge from emacs-25
> 
>      * src/w32.c (w32_crypto_hprov): New static variable.
>      (globals_of_w32): Initialize w32_crypto_hprov.
>      (w32_init_crypt_random, w32_init_random): New functions.
>      Include wincrypt.h.
>      * src/w32.h (w32_init_random): Add prototype.
> 
> 
> which indicates that Eli noticed a problem around then.

I didn't: the compiler did.  This omission caused compilation
failures, so I looked into that and found the problem.  But I didn't
try looking for other similar omissions, and even if I'd think about
that (which I didn't), I wouldn't know how to do that.  Is there some
handy set of Git commands to help in this matter?

I hope the reason(s) for these problems could be revealed, because
otherwise we don't really know what we have on master, code-wise.

Re: Missing changes in merges from emacs-25 to master

[John Wiegley]

>>>>> Eli Zaretskii <eliz@gnu.org> writes:

> I hope the reason(s) for these problems could be revealed, because otherwise
> we don't really know what we have on master, code-wise.

The only true way to ever know is:

    git diff origin/emacs-25 origin/master

And then go over it by eye. This is part of the pain of maintaining long lived
development forks.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

Re: Missing changes in merges from emacs-25 to master

[Paul Eggert]

John Wiegley wrote:
> The only true way to ever know is:
>
>      git diff origin/emacs-25 origin/master
>
> And then go over it by eye. This is part of the pain of maintaining long lived
> development forks.

Even if that is done, it's also helpful to find out what caused this specific 
problem in order to easily find and fix neighboring problems.

I'm not sure anybody has the time to do that diff right now. (I don't.)

Re: Missing changes in merges from emacs-25 to master

[Eli Zaretskii]

> From: John Wiegley <jwiegley@gmail.com>
> Cc: Paul Eggert <eggert@cs.ucla.edu>,  rudalics@gmx.at,  emacs-devel@gnu.org
> Date: Mon, 21 Mar 2016 10:55:08 -0700
> 
> >>>>> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > I hope the reason(s) for these problems could be revealed, because otherwise
> > we don't really know what we have on master, code-wise.
> 
> The only true way to ever know is:
> 
>     git diff origin/emacs-25 origin/master
> 
> And then go over it by eye.

Sorry, I'm not following: the above will just show the diffs between
the branches.  I asked about the reason(s) for the omission of some
commits from the merge.  How will "git diff" satisfy that request?

Re: Missing changes in merges from emacs-25 to master

[John Wiegley]

>>>>> Eli Zaretskii <eliz@gnu.org> writes:

> Sorry, I'm not following: the above will just show the diffs between the
> branches. I asked about the reason(s) for the omission of some commits from
> the merge. How will "git diff" satisfy that request?

As for "reason for the omission", I think there are two possible answers:

  1. gitmerge.el auto-marked it to be skipped, and despite my examination of
     each such mark, I failed to see that should not be skipped.

  2. I incorrectly resolved a merge conflict that seemed simple enough to me
     that I didn't e-mail the author of the commit about it.

So, in both case, human error pointed at me. If there's a tool error here, I'd
be surprised.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

Re: Missing changes in merges from emacs-25 to master

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 1537 bytes --]

On 03/21/2016 02:27 PM, John Wiegley wrote:
> I think there are two possible answers:
>
>    1. gitmerge.el auto-marked it to be skipped, and despite my examination of
>       each such mark, I failed to see that should not be skipped.
>
>    2. I incorrectly resolved a merge conflict that seemed simple enough to me
>       that I didn't e-mail the author of the commit about it.
>
> So, in both case, human error pointed at me.

I think you're right it wasn't a tool error per se (though the tools are 
error-prone).

Human error also points at me, since I merged the resulting bad branch 
against a good branch and didn't notice the issue. In an attempt to fix 
the problem I looked for the clump of commits near the lost one, and 
reinstalled them into master by applying the attached patches.  (I did 
not look for similar clumps yet.)

There is one patch near this clump, where I wasn't sure whether it was 
superseded by existing patches on 'master', so I left it alone. Phillip, 
can you please look into this? Thanks. The patch in question is this 
one, and the question is whether it needs to be modified and reapplied 
to the master branch:

commit 549a765efeca2748e68a5c6ce6c9238784e82535
Author: Phillip Lord <phillip.lord@russet.org.uk>
Date:   Fri Jan 15 22:11:39 2016 +0000

     Enable test selector from command line

     * test/automated/Makefile.in: Change variable manipulation to avoid
       over-writing selector.

http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=549a765efeca2748e68a5c6ce6c9238784e82535


[-- Attachment #2: 0001-etc-NEWS-Remove-unneeded-text.patch --]
[-- Type: application/x-patch, Size: 731 bytes --]

[-- Attachment #3: 0002-Fix-picture-mode-wrt-double-width-characters.patch --]
[-- Type: application/x-patch, Size: 1219 bytes --]

[-- Attachment #4: 0003-Unhide-the-no-line-directive-option-to-etags.patch --]
[-- Type: application/x-patch, Size: 3023 bytes --]

[-- Attachment #5: 0004-Merge-doc-fixes-for-random-lost-in-previous-merge.patch --]
[-- Type: application/x-patch, Size: 1807 bytes --]

[-- Attachment #6: 0005-In-comment-dwim-with-style-extra-line-respect-indent.patch --]
[-- Type: application/x-patch, Size: 1950 bytes --]

[-- Attachment #7: 0006-Ensure-positive-number-of-glyphs-for-margins-of-posi.patch --]
[-- Type: application/x-patch, Size: 1185 bytes --]

[-- Attachment #8: 0007-Don-t-confuse-with-when-trying-to-parse-member-initi.patch --]
[-- Type: application/x-patch, Size: 2421 bytes --]

Re: Missing changes in merges from emacs-25 to master

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 1424 bytes --]

On 03/21/2016 05:17 PM, Paul Eggert wrote:
> (I did not look for similar clumps yet.)

I looked for similar clumps of errors (i.e., patches lost by incorrect 
merges to master), and found two as noted below. I suspect gitmerge.el 
somehow encourages these errors, but haven't looked into how that might 
be. In the meantime I suggest looking skeptically at anything gitmerge 
does where the log message looks like "The following commit was skipped: 
... Merge branch 'A' ... into B", as skipping a merge ought to be a big 
deal.

I installed the attached patches to attempt to resurrect the two lost 
master patches.

Here are logs for the merges that lost patches:


commit 5ab243f40e4db91586ff74b01a775a5218a1d5be
Merge: f991a50 52f64cc
Author: John Wiegley <johnw@newartisans.com>
Date:   Thu Mar 3 23:53:26 2016 -0800

     ; Merge from origin/emacs-25

     The following commit was skipped:

     52f64cc Merge branch 'emacs-25' of git.sv.gnu.org:/srv/git/emacs 
into emacs-25

This lost commit 433407d8230a8a256b26c6c7615d3921b43f59ce.



commit c71e7cc113ed0d5f01aaa2e441a3e3c9fbeb9fa5
Merge: f55fc92 1fbcc8a
Author: Paul Eggert <eggert@cs.ucla.edu>
Date:   Tue Feb 9 14:23:10 2016 -0800

     ; Merge from origin/emacs-25

     The following commit was skipped:

     1fbcc8a Merge branch 'emacs-25' of 
git.savannah.gnu.org:/srv/git/emacs into emacs-25

This lost commit c1d32a65372c72d7de4808d620eefd3214a8e92a.


[-- Attachment #2: 0001-etc-TODO-Minor-quoting-and-grammar-fixes.patch --]
[-- Type: application/x-patch, Size: 4694 bytes --]

[-- Attachment #3: 0002-Fix-problems-caused-by-new-implementation-of-sub-wor.patch --]
[-- Type: application/x-patch, Size: 87470 bytes --]

Re: Missing changes in merges from emacs-25 to master

[Eli Zaretskii]

> From: John Wiegley <jwiegley@gmail.com>
> Cc: eggert@cs.ucla.edu,  rudalics@gmx.at,  emacs-devel@gnu.org
> Date: Mon, 21 Mar 2016 14:27:54 -0700
> 
> As for "reason for the omission", I think there are two possible answers:
> 
>   1. gitmerge.el auto-marked it to be skipped, and despite my examination of
>      each such mark, I failed to see that should not be skipped.
> 
>   2. I incorrectly resolved a merge conflict that seemed simple enough to me
>      that I didn't e-mail the author of the commit about it.

But in these cases, the commit we lost should have been mentioned in
the list of skipped one.  It wasn't, AFAICS.

Re: Missing changes in merges from emacs-25 to master

[Eli Zaretskii]

> Date: Tue, 22 Mar 2016 05:35:32 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: rudalics@gmx.at, eggert@cs.ucla.edu, emacs-devel@gnu.org
> 
> > From: John Wiegley <address@hidden>
> > Cc: address@hidden,  address@hidden,  address@hidden
> > Date: Mon, 21 Mar 2016 14:27:54 -0700
> > 
> > As for "reason for the omission", I think there are two possible answers:
> > 
> >   1. gitmerge.el auto-marked it to be skipped, and despite my examination of
> >      each such mark, I failed to see that should not be skipped.
> > 
> >   2. I incorrectly resolved a merge conflict that seemed simple enough to me
> >      that I didn't e-mail the author of the commit about it.
> 
> But in these cases, the commit we lost should have been mentioned in
> the list of skipped one.  It wasn't, AFAICS.

I think I know what happened.  It goes like this:

  . Phillip worked on test selector for the test suite.  When he
    finished, he made a local commit 549a765 on the emacs-25 branch.

  . Phillip then said "git pull", followed by "git push".  This
    produced the merge-commit bb0cd31, which incorporated 9 other
    commits that were pushed there since the time Phillip started
    working on 549a765.  One of these 9 commits was my commit that
    fixed resize_frame_windows.

  . When John merged all of that to master, bb0cd31 was skipped.
    Skipping it omitted all the 9 commits that were merged by bb0cd31.

So the question (to John) now becomes: why was bb0cd31 skipped?  Was
that something that gitmerge.el suggested?  If so, we should fix
gitmerge.el not to make such dangerous proposals in the future.

We should also audit all the other commits merged in bb0cd31, and
probably also in any other similar merge-commit that was skipped by
John in the merge to master.

Re: Missing changes in merges from emacs-25 to master

[Stefan Monnier]

> I think I know what happened.  It goes like this:

Sounds like a good explanation, thank you.

>   . Phillip then said "git pull", followed by "git push".

That's a problem in itself, since it leads to annoying commit diffs that
repeat other things, and it changes "git log" ordering in confusing ways
(makes it seem like mainline's development is Philip's and when happened
on master during this time was done on a branch).

>   . When John merged all of that to master, bb0cd31 was skipped.
>     Skipping it omitted all the 9 commits that were merged by bb0cd31.
> So the question (to John) now becomes: why was bb0cd31 skipped?

Probably because it's a merge commit, so its commit message contains
"merge" which is in bzrmerge-skip-regexp.  We should probably remove
"merge" from bzrmerge-skip-regexp (and use "backport" as the main magic
keyword) or at least refine it to only match things like "don't merge".


        Stefan

Re: Missing changes in merges from emacs-25 to master

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Tue, 22 Mar 2016 12:30:52 -0400
> 
> >   . Phillip then said "git pull", followed by "git push".
> 
> That's a problem in itself, since it leads to annoying commit diffs that
> repeat other things, and it changes "git log" ordering in confusing ways
> (makes it seem like mainline's development is Philip's and when happened
> on master during this time was done on a branch).

Indeed.  But sometimes this cannot be avoided, because no matter how
quick you are, there's always a chance that someone else pushes
between your local commit and your push.  The probability of this
could be brought down significantly by "git pull" before the local
commit, but it cannot be eliminated altogether.

> >   . When John merged all of that to master, bb0cd31 was skipped.
> >     Skipping it omitted all the 9 commits that were merged by bb0cd31.
> > So the question (to John) now becomes: why was bb0cd31 skipped?
> 
> Probably because it's a merge commit, so its commit message contains
> "merge" which is in bzrmerge-skip-regexp.  We should probably remove
> "merge" from bzrmerge-skip-regexp (and use "backport" as the main magic
> keyword) or at least refine it to only match things like "don't merge".

You mean, gitmerge-skip-regexp.  Yes, I think that's the reason.

Re: Missing changes in merges from emacs-25 to master

[Stefan Monnier]

>> >   . Phillip then said "git pull", followed by "git push".
>> That's a problem in itself, since it leads to annoying commit diffs that
>> repeat other things, and it changes "git log" ordering in confusing ways
>> (makes it seem like mainline's development is Philip's and when happened
>> on master during this time was done on a branch).
> Indeed.  But sometimes this cannot be avoided, because no matter how
> quick you are, there's always a chance that someone else pushes
> between your local commit and your push.  The probability of this
> could be brought down significantly by "git pull" before the local
> commit, but it cannot be eliminated altogether.

"git rebase" should do the trick.


        Stefan

Re: Missing changes in merges from emacs-25 to master

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Tue, 22 Mar 2016 12:52:48 -0400
> 
> >> >   . Phillip then said "git pull", followed by "git push".
> >> That's a problem in itself, since it leads to annoying commit diffs that
> >> repeat other things, and it changes "git log" ordering in confusing ways
> >> (makes it seem like mainline's development is Philip's and when happened
> >> on master during this time was done on a branch).
> > Indeed.  But sometimes this cannot be avoided, because no matter how
> > quick you are, there's always a chance that someone else pushes
> > between your local commit and your push.  The probability of this
> > could be brought down significantly by "git pull" before the local
> > commit, but it cannot be eliminated altogether.
> 
> "git rebase" should do the trick.

We've been through this: "git rebase" will cause much worse trouble if
you have local merge-commits from feature branches.  So I at least
will never use "git rebase" unless I have no other choice.

When we started using Git, I explicitly asked whether we cared about
the messed-up DAG due to these merge-commits, and the answer was "we
don't".  So I never looked back, and I guess others didn't, too.

Re: Missing changes in merges from emacs-25 to master

[Paul Eggert]

On 03/22/2016 10:03 AM, Eli Zaretskii wrote:
>> "git rebase" should do the trick.
> We've been through this: "git rebase" will cause much worse trouble if
> you have local merge-commits from feature branches.  So I at least
> will never use "git rebase" unless I have no other choice.

For complex clashes such as the one you describe, you might try redoing 
the failed push from a clean new master branch rather than attempting 
"git pull / git push".  Something like this:

$ git push
[ fails due to somebody else committing in the meantime ]
$ git branch -m master master-clashed
$ git branch --track master origin/master
$ git checkout master
$ git pull
[ now, merge your changes again ]
$ git push

Re: Missing changes in merges from emacs-25 to master

[Eli Zaretskii]

> Cc: emacs-devel@gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Tue, 22 Mar 2016 11:21:04 -0700
> 
> On 03/22/2016 10:03 AM, Eli Zaretskii wrote:
> >> "git rebase" should do the trick.
> > We've been through this: "git rebase" will cause much worse trouble if
> > you have local merge-commits from feature branches.  So I at least
> > will never use "git rebase" unless I have no other choice.
> 
> For complex clashes such as the one you describe, you might try redoing 
> the failed push from a clean new master branch rather than attempting 
> "git pull / git push".  Something like this:
> 
> $ git push
> [ fails due to somebody else committing in the meantime ]
> $ git branch -m master master-clashed
> $ git branch --track master origin/master
> $ git checkout master
> $ git pull
> [ now, merge your changes again ]
> $ git push

Thanks, but I don't see why I should bother with such complications,
when no one cares about the DAG keeping a clear "mainline".

Re: Missing changes in merges from emacs-25 to master

[Stefan Monnier]

> Thanks, but I don't see why I should bother with such complications,
> when no one cares about the DAG keeping a clear "mainline".

Agreed.  If we want that, we should setup some post-receive-hook which
enforces it.


        Stefan

Re: Missing changes in merges from emacs-25 to master

[Stefan Monnier]

> We've been through this: "git rebase" will cause much worse trouble if
> you have local merge-commits from feature branches.

Indeed.  The problem is very real when you want to do "git merge; git
push" and the merge is non-trivial, since you have to hurry to fix the
merge problems before someone else.

> So I at least will never use "git rebase" unless I have no
> other choice.

I do use "git rebase" when none of the local commits are merges.

When there's a merge, the only option I know of is to checkout "master",
then do a "git merge {localbranch}; git push".

I really think there should be a way to tell git to "merge in the other
direction", but Git people don't seem to listen.

> When we started using Git, I explicitly asked whether we cared about
> the messed-up DAG due to these merge-commits, and the answer was "we
> don't".  So I never looked back, and I guess others didn't, too.

What can I say, you were right.  I'm not really bothered by the shape of
the DAG itself, but the commit-diffs are pretty annoying and there are
a few other linked problems such as the one discussed in this thread.


        Stefan

Re: Missing changes in merges from emacs-25 to master

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org
> Date: Tue, 22 Mar 2016 14:41:53 -0400
> 
> > When we started using Git, I explicitly asked whether we cared about
> > the messed-up DAG due to these merge-commits, and the answer was "we
> > don't".  So I never looked back, and I guess others didn't, too.
> 
> What can I say, you were right.  I'm not really bothered by the shape of
> the DAG itself, but the commit-diffs are pretty annoying and there are
> a few other linked problems such as the one discussed in this thread.

I simply don't see how this can be fixed satisfactorily until Git
acquires some features to support such a workflow.

Re: Missing changes in merges from emacs-25 to master

[Stefan Monnier]

> I simply don't see how this can be fixed satisfactorily until Git
> acquires some features to support such a workflow.

Indeed, although I can't believe those features would have to
be complex.  A simple "git merge --reverse-parents" should be more or
less all it takes.


        Stefan

Re: Missing changes in merges from emacs-25 to master

[Andreas Schwab]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> A simple "git merge --reverse-parents" should be more or less all it
> takes.

Send patches.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: Missing changes in merges from emacs-25 to master

[Lars Magne Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> We've been through this: "git rebase" will cause much worse trouble if
> you have local merge-commits from feature branches. 

That's true, but that's a quite unusual thing to be doing.  ("Unusual"
as in "most of the time most of the people who work on Emacs won't be
doing that".)  In this instance, if he had done
"git pull --rebase; git push" then no merge commit would have been
pushed, and things would generally be less confusing.

That rebase/push thing is what I do 99.9% of the time, and its the least
noisy you can be when doing normal stuff.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: Missing changes in merges from emacs-25 to master

[Eli Zaretskii]

> From: Lars Magne Ingebrigtsen <larsi@gnus.org>
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>,  emacs-devel@gnu.org
> Date: Tue, 22 Mar 2016 20:34:52 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > We've been through this: "git rebase" will cause much worse trouble if
> > you have local merge-commits from feature branches. 
> 
> That's true, but that's a quite unusual thing to be doing.  ("Unusual"
> as in "most of the time most of the people who work on Emacs won't be
> doing that".)

I was explaining why _I_ don't want to use rebase when pulling from
upstream.  To understand that, you need to ask what _I_ do most of the
time, not what others do.

For me, working on a feature branch is entirely not unusual.

Re: Missing changes in merges from emacs-25 to master

[Phillip Lord]

Lars Magne Ingebrigtsen <larsi@gnus.org> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>> We've been through this: "git rebase" will cause much worse trouble if
>> you have local merge-commits from feature branches. 
>
> That's true, but that's a quite unusual thing to be doing.  ("Unusual"
> as in "most of the time most of the people who work on Emacs won't be
> doing that".)  In this instance, if he had done
> "git pull --rebase; git push" then no merge commit would have been
> pushed, and things would generally be less confusing.
>
> That rebase/push thing is what I do 99.9% of the time, and its the least
> noisy you can be when doing normal stuff.

If I remember correctly, I think I did the commit, but forgot to push,
till the next day. Once I got into the merge commit, my lack of git
expertise meant I couldn't work out how to get out of it again.

Mostly I develop with feature branches, so don't normally get this
problem. Ah well, live and learn.

Phil

Re: Missing changes in merges from emacs-25 to master

[Paul Eggert]

On 03/22/2016 09:30 AM, Stefan Monnier wrote:
> We should probably remove
> "merge" from bzrmerge-skip-regexp (and use "backport" as the main magic
> keyword) or at least refine it to only match things like "don't merge".

I tried gitmerge.el unchanged, found a similar bug with the proposed 
merge, removed "merge" from gitmerge-skip-regexp, redid the merge and 
found that it worked better this time, and committed the result into the 
master.

The whole gitmerge.el business is still odd, though. What's up with 
merge commits like f95bbe5a67e03fe6d05cbfb4d0c9151a754d6ccd whose commit 
message is merely "-"?

Plus, ChangeLog generation is still broken in master. I'm becoming more 
inclined to install into master the patch I proposed in 
<http://lists.gnu.org/archive/html/emacs-devel/2016-03/msg00391.html>. 
Although this patch is merely a stopgap measure until someone figures 
out a better way to edit commit messages, it's clearly better than what 
we have now in 'master'.

Re: Missing changes in merges from emacs-25 to master

[Eli Zaretskii]

> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Tue, 22 Mar 2016 11:32:12 -0700
> 
> Plus, ChangeLog generation is still broken in master. I'm becoming more 
> inclined to install into master the patch I proposed in 
> <http://lists.gnu.org/archive/html/emacs-devel/2016-03/msg00391.html>. 

No, please don't.  I don't want us to start fixing mistakes in Perl,
because this will just bring more mistakes.

John said he wanted to look into this issue, so let's wait.

Re: Missing changes in merges from emacs-25 to master

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 357 bytes --]

On 03/22/2016 11:37 AM, Eli Zaretskii wrote:
> I don't want us to start fixing mistakes in Perl

OK, I can install the attached instead. It won't fix mistakes in Perl, 
and it will still be better than what we have now (where ChangeLog.2 is 
plainly bogus in the 'master' branch). It's intended as a stopgap 
measure until we come up with something better.

[-- Attachment #2: 0001-Simplify-autogeneration-of-top-level-ChangeLog.patch --]
[-- Type: application/x-patch, Size: 8864 bytes --]

Re: Missing changes in merges from emacs-25 to master

[Eli Zaretskii]

> Cc: monnier@iro.umontreal.ca, emacs-devel@gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Tue, 22 Mar 2016 12:15:00 -0700
> 
> On 03/22/2016 11:37 AM, Eli Zaretskii wrote:
> > I don't want us to start fixing mistakes in Perl
> 
> OK, I can install the attached instead. It won't fix mistakes in Perl, 
> and it will still be better than what we have now (where ChangeLog.2 is 
> plainly bogus in the 'master' branch). It's intended as a stopgap 
> measure until we come up with something better.

Isn't it easier and simpler just to delete ChangeLog.2 from the master
branch, and leave everything else intact?

Re: Missing changes in merges from emacs-25 to master

[Paul Eggert]

On 03/22/2016 12:42 PM, Eli Zaretskii wrote:
>> OK, I can install the attached instead. It won't fix mistakes in 
>> Perl, and it will still be better than what we have now (where 
>> ChangeLog.2 is plainly bogus in the 'master' branch). It's intended 
>> as a stopgap measure until we come up with something better. 
> Isn't it easier and simpler just to delete ChangeLog.2 from the master
> branch, and leave everything else intact?

No, as that would erase the relevant emacs-25 history from the master 
branch. Also, it would leave intact the procedure for updating 
ChangeLog.2 in the master branch, even though we should not be using 
that procedure there as things stand, and Murphy's law would apply if we 
left it in there (someone would go ahead and use the procedure anyway).

Re: Missing changes in merges from emacs-25 to master

[David Engster]

Paul Eggert writes:
> The whole gitmerge.el business is still odd, though. What's up with
> merge commits like f95bbe5a67e03fe6d05cbfb4d0c9151a754d6ccd whose
> commit message is merely "-"?

It's always good to look at the merge before pushing. Anyway, this is
because of de8837599112:

      ;; FIXME: We add "-m-" because the default commit message
      ;; apparently tickles our commit hook:
      ;;    Line longer than 78 characters in commit message
      ;;    Line longer than 78 characters in commit message
      ;;    Line longer than 78 characters in commit message
      ;;    Commit aborted; please see the file CONTRIBUTE

I don't understand this. It only applies to resumed merges, whereas this
problem should happen with regular merges as well. And why not simply
fix this in `gitmerge-commit-message' through truncation, instead of
using a '-' as commit message?

-David

Re: Missing changes in merges from emacs-25 to master

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 230 bytes --]

On 03/22/2016 03:57 PM, David Engster wrote:
> why not simply
> fix this in `gitmerge-commit-message' through truncation, instead of
> using a '-' as commit message?

Thanks, good suggestion. I installed the attached into master.

[-- Attachment #2: 0001-Compute-a-better-commit-message-for-merges.txt --]
[-- Type: text/plain, Size: 2026 bytes --]

From 8c25e016ee06b2c2246986690af7ba779b9f0af8 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Tue, 22 Mar 2016 16:42:28 -0700
Subject: [PATCH] Compute a better commit message for merges
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Problem reported by David Engster in:
http://lists.gnu.org/archive/html/emacs-devel/2016-03/msg01270.html
* admin/gitmerge.el (gitmerge-commit-message):
Truncate the computed commit message to at most 72 characters per line.
(gitmerge-maybe-resume): Don’t use "-" as the commit message for
merges; use the computed commit message instead.
---
 admin/gitmerge.el | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/admin/gitmerge.el b/admin/gitmerge.el
index 74e6f80..6a52f7a 100644
--- a/admin/gitmerge.el
+++ b/admin/gitmerge.el
@@ -331,6 +331,10 @@ gitmerge-commit-message
 	   (if end (list (concat beg "~.." end))
 	     `("-1" ,beg)))
     (insert "\n")
+    ;; Truncate to 72 chars so that the resulting ChangeLog line fits in 80.
+    (goto-char (point-min))
+    (while (re-search-forward "^\\(.\\{69\\}\\).\\{4,\\}" nil t)
+      (replace-match "\\1..."))
     (buffer-string)))
 
 (defun gitmerge-apply (missing from)
@@ -432,14 +436,8 @@ gitmerge-maybe-resume
 	(when mergehead
 	  (with-current-buffer (get-buffer-create gitmerge-output-buffer)
 	    (erase-buffer)
-            ;; FIXME: We add "-m-" because the default commit message
-            ;; apparently tickles our commit hook:
-            ;;    Line longer than 78 characters in commit message
-            ;;    Line longer than 78 characters in commit message
-            ;;    Line longer than 78 characters in commit message
-            ;;    Commit aborted; please see the file CONTRIBUTE
 	    (unless (zerop (call-process "git" nil t nil
-					 "commit" "--no-edit" "-m-"))
+					 "commit" "--no-edit"))
 	      (error "Git error during merge - fix it manually"))))
 	;; Successfully resumed.
 	t))))
-- 
2.5.5

Re: Missing changes in merges from emacs-25 to master

[Eli Zaretskii]

There seems to be another issue with merging to master: we get back
commits that were cherry-picked from master to the release branch.
Here's one example; I don't know whether there are others:

  commit ad879b7f7e049160c45361fe8a12580801ba035b
  Author: K. Handa <handa@gnu.org>
  Date:   Thu Jan 14 21:48:44 2016 +0900

      Backport:fix previous change of src/ftfont.c (ftfont_shape_by_flt)

      * src/ftfont.c (ftfont_shape_by_flt): Fix previous change.  Access the
      second glyph only when there are enough glyphs.

      (cherry picked from commit 9835757013569673854b692ccbb58bfb3c3ed1f7)

Re: Missing changes in merges from emacs-25 to master

[Andreas Schwab]

Eli Zaretskii <eliz@gnu.org> writes:

> There seems to be another issue with merging to master: we get back
> commits that were cherry-picked from master to the release branch.

What do you mean with "get back"?  Since they are part of the history of
the branch and merging incorporates the whole history you will of course
see them from master.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: Missing changes in merges from emacs-25 to master

[Eli Zaretskii]

> From: Andreas Schwab <schwab@linux-m68k.org>
> Cc: John Wiegley <johnw@gnu.org>,  emacs-devel@gnu.org
> Date: Fri, 25 Mar 2016 10:14:10 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > There seems to be another issue with merging to master: we get back
> > commits that were cherry-picked from master to the release branch.
> 
> What do you mean with "get back"?  Since they are part of the history of
> the branch and merging incorporates the whole history you will of course
> see them from master.

They were supposed to be skipped during merging, so if what we have
currently is the expected result, then what exactly does "skip" mean
in this context?

Re: Missing changes in merges from emacs-25 to master

[Andreas Schwab]

Eli Zaretskii <eliz@gnu.org> writes:

> They were supposed to be skipped during merging

They cannot be "skipped".

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: Missing changes in merges from emacs-25 to master

[Eli Zaretskii]

> From: Andreas Schwab <schwab@linux-m68k.org>
> Cc: johnw@gnu.org,  emacs-devel@gnu.org
> Date: Fri, 25 Mar 2016 12:50:00 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > They were supposed to be skipped during merging
> 
> They cannot be "skipped".

Maybe I'm naïve, but I hoped for a more informative and helpful
answer.  I think the problem is real.  Maybe I'm mistaken, but with
such responses I will never know.

Re: Missing changes in merges from emacs-25 to master

[David Engster]

Eli Zaretskii writes:
> There seems to be another issue with merging to master: we get back
> commits that were cherry-picked from master to the release branch.

When you merge a branch, you have to merge all of it. But when they are
marked as 'skipped', they will be merged with strategy "ours",
effectively ignoring their content. This is how it worked with Bazaar as
well.

-David

Re: Missing changes in merges from emacs-25 to master

[Eli Zaretskii]

> From: David Engster <deng@randomsample.de>
> Cc: John Wiegley <johnw@gnu.org>,  emacs-devel@gnu.org
> Date: Fri, 25 Mar 2016 10:15:41 +0100
> 
> Eli Zaretskii writes:
> > There seems to be another issue with merging to master: we get back
> > commits that were cherry-picked from master to the release branch.
> 
> When you merge a branch, you have to merge all of it. But when they are
> marked as 'skipped', they will be merged with strategy "ours",
> effectively ignoring their content.

What does "their content" include, exactly?

> This is how it worked with Bazaar as well.

With Bazaar, there was a clear mainline, displaying which these
commits wouldn't appear at all.  We don't have that with Git, so the
analogy doesn't really help.

Re: Missing changes in merges from emacs-25 to master

[David Engster]

Eli Zaretskii writes:
>> From: David Engster <deng@randomsample.de>
>> Cc: John Wiegley <johnw@gnu.org>,  emacs-devel@gnu.org
>> Date: Fri, 25 Mar 2016 10:15:41 +0100
>
>> 
>> Eli Zaretskii writes:
>> > There seems to be another issue with merging to master: we get back
>> > commits that were cherry-picked from master to the release branch.
>> 
>> When you merge a branch, you have to merge all of it. But when they are
>> marked as 'skipped', they will be merged with strategy "ours",
>> effectively ignoring their content.
>
> What does "their content" include, exactly?

The patch.

The merge-strategy "ours" means: merge the commit, but take "our"
version of everything that would be changed by it. The commit is seen as
merged afterwards, but without applying the patch it includes.

>> This is how it worked with Bazaar as well.
>
> With Bazaar, there was a clear mainline, displaying which these
> commits wouldn't appear at all. We don't have that with Git, so the
> analogy doesn't really help.

Bazaar didn't display *any* commits from merged branches by default,
whether they were "skipped" or not. So again: gitmerge does that same as
bzrmerge did. That we don't invest the effort to keep our mainline on
the "left side" is another matter.

-David

Re: Missing changes in merges from emacs-25 to master

[Eli Zaretskii]

> From: David Engster <deng@randomsample.de>
> Cc: johnw@gnu.org,  emacs-devel@gnu.org
> Date: Fri, 25 Mar 2016 12:38:15 +0100
> 
> >> When you merge a branch, you have to merge all of it. But when they are
> >> marked as 'skipped', they will be merged with strategy "ours",
> >> effectively ignoring their content.
> >
> > What does "their content" include, exactly?
> 
> The patch.
> 
> The merge-strategy "ours" means: merge the commit, but take "our"
> version of everything that would be changed by it. The commit is seen as
> merged afterwards, but without applying the patch it includes.

I'm not sure I understand the meaning of this.  Let me ask a more
specific question, about the particular commit that bothers me.  Once
again, it is this commit from the emacs-25 branch:

  commit ad879b7f7e049160c45361fe8a12580801ba035b
  Author: K. Handa <handa@gnu.org>
  Date:   Thu Jan 14 21:48:44 2016 +0900

      Backport:fix previous change of src/ftfont.c (ftfont_shape_by_flt)

      * src/ftfont.c (ftfont_shape_by_flt): Fix previous change.  Access the
      second glyph only when there are enough glyphs.

      (cherry picked from commit 9835757013569673854b692ccbb58bfb3c3ed1f7)

It was originally made on master, as 9835757, then cherry-picked to
emacs-25 as above and committed there as ad879b, and then merged back
to master in 3005605, with this log message:

  commit 3005605776955593e0b416de9e1ebf158114343e
  Merge: cb4e054 ad879b7
  Author: Paul Eggert <eggert@cs.ucla.edu>
  Date:   Sat Jan 30 11:43:27 2016 -0800

      ; Merge from origin/emacs-25

      The following commits were skipped:

      ad879b7 Backport:fix previous change of src/ftfont.c (ftfont_shape_by_flt)
      4a3db0f support rendering of wider range of combinging characters by ftfont

After all this, what would you expect "git annotate" to show on the
master branch as the commit that introduced the affected source lines?
Shouldn't it show the original commit, i.e. 9835757?  Because it
doesn't show that, it shows ad879b7 instead.  If that's expected
behavior, then I _really_ don't understand what "skip" means here,
since it sounds like it didn't use the "ours" strategy, according to
your definition.  What am I missing?

> > With Bazaar, there was a clear mainline, displaying which these
> > commits wouldn't appear at all. We don't have that with Git, so the
> > analogy doesn't really help.
> 
> Bazaar didn't display *any* commits from merged branches by default,
> whether they were "skipped" or not.

Of course: those commits were on other branches, so they are
irrelevant for the mainline.  What Bazaar would show is the single
merge-commit, which (with Bazaar) "represents" the commits on the
merged branch.

> So again: gitmerge does that same as bzrmerge did. That we don't
> invest the effort to keep our mainline on the "left side" is another
> matter.

I didn't yet voice any complaints about what gitmerge does, I'm just
saying that something seems wrong in the DAG.  Whether it was gitmerge
that produced that remains to be seen; for now, we cannot even agree
on what we actually see there, and whether it constitutes a problem ;-)

Re: Missing changes in merges from emacs-25 to master

[David Engster]

Eli Zaretskii writes:
>> From: David Engster <deng@randomsample.de>
>> Cc: johnw@gnu.org,  emacs-devel@gnu.org
>> Date: Fri, 25 Mar 2016 12:38:15 +0100
>
>> 
>> >> When you merge a branch, you have to merge all of it. But when they are
>> >> marked as 'skipped', they will be merged with strategy "ours",
>> >> effectively ignoring their content.
>> >
>> > What does "their content" include, exactly?
>> 
>> The patch.
>> 
>> The merge-strategy "ours" means: merge the commit, but take "our"
>> version of everything that would be changed by it. The commit is seen as
>> merged afterwards, but without applying the patch it includes.
>
> I'm not sure I understand the meaning of this.  Let me ask a more
> specific question, about the particular commit that bothers me.  Once
> again, it is this commit from the emacs-25 branch:

[...]

> After all this, what would you expect "git annotate" to show on the
> master branch as the commit that introduced the affected source lines?
> Shouldn't it show the original commit, i.e. 9835757?  Because it
> doesn't show that, it shows ad879b7 instead.  If that's expected
> behavior, then I _really_ don't understand what "skip" means here,
> since it sounds like it didn't use the "ours" strategy, according to
> your definition.  What am I missing?

Well, the cherry-picked commit ad879b7 is now in master's history. The
patch of that diff shows that it changes those lines. That we chose to
ignore the patch can only be seen by looking at the merge commit
300560577, which references the same 'tree' as the previous merge
'cb4e054e', meaning they both reference they same set of files:

  > git cat-file 300560577 -p                            
    tree ae2bec4f10425bd61e2a90563edc178d382bb4b8
    [...]

  > git cat-file cb4e054e41c -p
    tree ae2bec4f10425bd61e2a90563edc178d382bb4b8

My best guess is that 'git annotate' does not incorporate that
information, but I'm not familiar with the inner workings of that
command. Maybe no one bothered to implement it, or maybe it would make
annotation even slower.

As you well know, Bazaar had a data model which was much more amenable
for annotating a file, so I guess it could deal with this issue
correctly (I haven't checked, though).

>> > With Bazaar, there was a clear mainline, displaying which these
>> > commits wouldn't appear at all. We don't have that with Git, so the
>> > analogy doesn't really help.
>> 
>> Bazaar didn't display *any* commits from merged branches by default,
>> whether they were "skipped" or not.
>
> Of course: those commits were on other branches, so they are
> irrelevant for the mainline.

Those commits were made part of mainline's history, so how can they
possibly be "irrelevant"? It's just that Bazaar had the default to only
show mainline (which also confused plenty of people).

> What Bazaar would show is the single merge-commit, which (with Bazaar)
> "represents" the commits on the merged branch.

I forgot how it worked in Bazaar. Did a merge commit hold the full patch
information of all merged commits? In any case, that's now the case for
Git: a merge commit is a normal commit, simply with more than one
parent.

>> So again: gitmerge does that same as bzrmerge did. That we don't
>> invest the effort to keep our mainline on the "left side" is another
>> matter.
>
> I didn't yet voice any complaints about what gitmerge does, I'm just
> saying that something seems wrong in the DAG.  Whether it was gitmerge
> that produced that remains to be seen; for now, we cannot even agree
> on what we actually see there, and whether it constitutes a problem ;-)

I can at least assure you that the patches from those commits are
ignored. And I would claim that it worked essentially the same during
the Bazaar days. Just fire up 'gitk' and look at old merges from
emacs-24 into master; you'll find plenty of backported commits in the
merged branches, and bzrmerge dealt with it the same way (with the
exception that it did the whole merge in one go, while gitmerge does
separate merges).

-David

Re: Missing changes in merges from emacs-25 to master

[David Engster]

My proofreader is on holiday...

David Engster writes:
> Well, the cherry-picked commit ad879b7 is now in master's history. The
> patch of that diff shows that it changes those lines.
                ^^^^ commit

[...]

> I forgot how it worked in Bazaar. Did a merge commit hold the full patch
> information of all merged commits? In any case, that's now the case for
                                                         ^^^ not

> Git: a merge commit is a normal commit, simply with more than one
> parent.


-David

Re: Missing changes in merges from emacs-25 to master

[Eli Zaretskii]

> From: David Engster <deng@randomsample.de>
> Cc: johnw@gnu.org,  emacs-devel@gnu.org
> Date: Fri, 25 Mar 2016 17:00:45 +0100
> 
> > After all this, what would you expect "git annotate" to show on the
> > master branch as the commit that introduced the affected source lines?
> > Shouldn't it show the original commit, i.e. 9835757?  Because it
> > doesn't show that, it shows ad879b7 instead.  If that's expected
> > behavior, then I _really_ don't understand what "skip" means here,
> > since it sounds like it didn't use the "ours" strategy, according to
> > your definition.  What am I missing?
> 
> Well, the cherry-picked commit ad879b7 is now in master's history. The
> patch of that diff shows that it changes those lines. That we chose to
> ignore the patch can only be seen by looking at the merge commit
> 300560577, which references the same 'tree' as the previous merge
> 'cb4e054e', meaning they both reference they same set of files:
> 
>   > git cat-file 300560577 -p                            
>     tree ae2bec4f10425bd61e2a90563edc178d382bb4b8
>     [...]
> 
>   > git cat-file cb4e054e41c -p
>     tree ae2bec4f10425bd61e2a90563edc178d382bb4b8
> 
> My best guess is that 'git annotate' does not incorporate that
> information, but I'm not familiar with the inner workings of that
> command. Maybe no one bothered to implement it, or maybe it would make
> annotation even slower.

So I guess the fact that a commit was skipped during a merge is
something that is so hard to see that perhaps the entire issue of
skipping is moot with Git, and we shouldn't be bothered by it.

> >> > With Bazaar, there was a clear mainline, displaying which these
> >> > commits wouldn't appear at all. We don't have that with Git, so the
> >> > analogy doesn't really help.
> >> 
> >> Bazaar didn't display *any* commits from merged branches by default,
> >> whether they were "skipped" or not.
> >
> > Of course: those commits were on other branches, so they are
> > irrelevant for the mainline.
> 
> Those commits were made part of mainline's history

Not in Bazaar's philosophy.  They are part of the DAG, but not of the
branch's mainline.

> > What Bazaar would show is the single merge-commit, which (with Bazaar)
> > "represents" the commits on the merged branch.
> 
> I forgot how it worked in Bazaar. Did a merge commit hold the full patch
> information of all merged commits?

Yes.

> I can at least assure you that the patches from those commits are
> ignored.

I believe you.  It's just strange and confusing that so many Git
commands show information that indicates the opposite, and the truth
is only shown by looking at the low-level objects and comparing
hashes.

Re: Missing changes in merges from emacs-25 to master

[David Engster]

Eli Zaretskii writes:
>> From: David Engster <deng@randomsample.de>
>> Cc: johnw@gnu.org,  emacs-devel@gnu.org
>> Date: Fri, 25 Mar 2016 17:00:45 +0100
>
>> 
>> > After all this, what would you expect "git annotate" to show on the
>> > master branch as the commit that introduced the affected source lines?
>> > Shouldn't it show the original commit, i.e. 9835757?  Because it
>> > doesn't show that, it shows ad879b7 instead.  If that's expected
>> > behavior, then I _really_ don't understand what "skip" means here,
>> > since it sounds like it didn't use the "ours" strategy, according to
>> > your definition.  What am I missing?
>> 
>> Well, the cherry-picked commit ad879b7 is now in master's history. The
>> patch of that diff shows that it changes those lines. That we chose to
>> ignore the patch can only be seen by looking at the merge commit
>> 300560577, which references the same 'tree' as the previous merge
>> 'cb4e054e', meaning they both reference they same set of files:
>> 
>>   > git cat-file 300560577 -p                            
>>     tree ae2bec4f10425bd61e2a90563edc178d382bb4b8
>>     [...]
>> 
>>   > git cat-file cb4e054e41c -p
>>     tree ae2bec4f10425bd61e2a90563edc178d382bb4b8
>> 
>> My best guess is that 'git annotate' does not incorporate that
>> information, but I'm not familiar with the inner workings of that
>> command. Maybe no one bothered to implement it, or maybe it would make
>> annotation even slower.
>
> So I guess the fact that a commit was skipped during a merge is
> something that is so hard to see that perhaps the entire issue of
> skipping is moot with Git, and we shouldn't be bothered by it.

Well, it's not *that* hard to see, but it usually requires to look at a
span of commits. In this case,

  git diff cb4e054e41c..300560577

will correctly produce nothing. But I agree it's unfortunate that you
cannot see this from the merge commit alone. But I'm not sure what you
mean with the entire issue being moot. How do you suggest we deal with
commits that shouldn't land in master?

-David

Re: Missing changes in merges from emacs-25 to master

[Eli Zaretskii]

> From: David Engster <deng@randomsample.de>
> Cc: johnw@gnu.org,  emacs-devel@gnu.org
> Date: Fri, 25 Mar 2016 18:52:06 +0100
> 
> > So I guess the fact that a commit was skipped during a merge is
> > something that is so hard to see that perhaps the entire issue of
> > skipping is moot with Git, and we shouldn't be bothered by it.
> 
> Well, it's not *that* hard to see, but it usually requires to look at a
> span of commits. In this case,
> 
>   git diff cb4e054e41c..300560577
> 
> will correctly produce nothing. But I agree it's unfortunate that you
> cannot see this from the merge commit alone. But I'm not sure what you
> mean with the entire issue being moot. How do you suggest we deal with
> commits that shouldn't land in master?

Sorry, I should have made it clear I was talking only about the
backported commits.

Re: Missing changes in merges from emacs-25 to master

[Óscar Fuentes]

Eli Zaretskii <eliz@gnu.org> writes:

>> will correctly produce nothing. But I agree it's unfortunate that you
>> cannot see this from the merge commit alone. But I'm not sure what you
>> mean with the entire issue being moot. How do you suggest we deal with
>> commits that shouldn't land in master?
>
> Sorry, I should have made it clear I was talking only about the
> backported commits.

A backported commit still can produce a conflict if the code affected by
that commit is modified again later on master.

Re: Missing changes in merges from emacs-25 to master

[Paul Eggert]

On 03/25/2016 10:33 AM, Eli Zaretskii wrote:
> So I guess the fact that a commit was skipped during a merge is
> something that is so hard to see that perhaps the entire issue of
> skipping is moot with Git, and we shouldn't be bothered by it.

I think that's basically it, yes. I agree that 'git blame' should be 
blaming the mainline commit rather than the skipped backported commit. I 
don't know why it's blaming the skipped commit. On the other hand, this 
is consistent with the philosophy of not caring about how beautiful the 
shape of the history is, so long as each commit is accurate.

Re: Missing changes in merges from emacs-25 to master

[Stefan Monnier]

> So I guess the fact that a commit was skipped during a merge is
> something that is so hard to see that perhaps the entire issue of
> skipping is moot with Git, and we shouldn't be bothered by it.

Skipping is not something really relevant from the history point
of view.  It's rather relevant for the one performing a merge since the
main purpose to avoid spurious conflicts.


        Stefan

Re: Missing changes in merges from emacs-25 to master

[Phillip Lord]

Eli Zaretskii <eliz@gnu.org> writes:
>> But in these cases, the commit we lost should have been mentioned in
>> the list of skipped one.  It wasn't, AFAICS.
>
> I think I know what happened.  It goes like this:
>
>   . Phillip worked on test selector for the test suite.  When he
>     finished, he made a local commit 549a765 on the emacs-25 branch.
>
>   . Phillip then said "git pull", followed by "git push".  This
>     produced the merge-commit bb0cd31, which incorporated 9 other
>     commits that were pushed there since the time Phillip started
>     working on 549a765.  One of these 9 commits was my commit that
>     fixed resize_frame_windows.

Oh dear, should have made those changes on a branch and rebased from
emacs-25.

Phil

Re: Missing changes in merges from emacs-25 to master

[Eli Zaretskii]

> From: phillip.lord@russet.org.uk (Phillip Lord)
> Cc: John Wiegley <johnw@gnu.org>,  rudalics@gmx.at,  eggert@cs.ucla.edu,  emacs-devel@gnu.org
> Date: Tue, 22 Mar 2016 17:00:49 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> >> But in these cases, the commit we lost should have been mentioned in
> >> the list of skipped one.  It wasn't, AFAICS.
> >
> > I think I know what happened.  It goes like this:
> >
> >   . Phillip worked on test selector for the test suite.  When he
> >     finished, he made a local commit 549a765 on the emacs-25 branch.
> >
> >   . Phillip then said "git pull", followed by "git push".  This
> >     produced the merge-commit bb0cd31, which incorporated 9 other
> >     commits that were pushed there since the time Phillip started
> >     working on 549a765.  One of these 9 commits was my commit that
> >     fixed resize_frame_windows.
> 
> Oh dear, should have made those changes on a branch and rebased from
> emacs-25.

It's not your fault.  Quite a few others (myself included) produce
these merge-commits from time to time, this cannot be avoided.  The
problem is in the merge procedure from emacs-25 to master -- it
shouldn't skip such merge-commits.

Re: Missing changes in merges from emacs-25 to master

[John Wiegley]

>>>>> Eli Zaretskii <eliz@gnu.org> writes:

> It's not your fault. Quite a few others (myself included) produce these
> merge-commits from time to time, this cannot be avoided. The problem is in
> the merge procedure from emacs-25 to master -- it shouldn't skip such
> merge-commits.

One take away from this is to be very cautious in assessing whether merge
commits should really be skipped or not.  What is the criteria for knowing
when they *should* be skipped?  Checking to ensure that all those commits are
already on master?

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

Re: Missing changes in merges from emacs-25 to master

[Eli Zaretskii]

> From: John Wiegley <jwiegley@gmail.com>
> Cc: phillip.lord@russet.org.uk (Phillip Lord),  rudalics@gmx.at,  eggert@cs.ucla.edu,  emacs-devel@gnu.org
> Date: Tue, 22 Mar 2016 12:21:10 -0700
> 
> >>>>> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > It's not your fault. Quite a few others (myself included) produce these
> > merge-commits from time to time, this cannot be avoided. The problem is in
> > the merge procedure from emacs-25 to master -- it shouldn't skip such
> > merge-commits.
> 
> One take away from this is to be very cautious in assessing whether merge
> commits should really be skipped or not.  What is the criteria for knowing
> when they *should* be skipped?  Checking to ensure that all those commits are
> already on master?

That would be a lot of work, IMO.

I actually don't understand why we should skip _any_ merge-commits.
Can you (or someone else) describe a situation where merging such
commits to master wouldn't be TRT?