unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Windows mingw64 and cygwin builds broken
@ 2015-11-12 20:06 Andy Moreton
  2015-11-12 20:42 ` Eli Zaretskii
  0 siblings, 1 reply; 18+ messages in thread
From: Andy Moreton @ 2015-11-12 20:06 UTC (permalink / raw)
  To: emacs-devel

Hi,

Changes in commit 9d43941569fc broke the Windows builds.

1) Mingw64 (64bit, msys2 hosted)

The mingw64 failures appear to be caused by using the
NOTIFYICONDATAW_V3_SIZE  macro from the mingw64 shellapi.h header,
which depends on the build-time definition of NTDDI_VERSION.

--[msys2 mingw64 64bit]-----------------------------------------------
In file included from C:/msys64/mingw64/x86_64-w64-mingw32/include/minwindef.h:163:0,
                 from C:/msys64/mingw64/x86_64-w64-mingw32/include/windef.h:8,
                 from C:/msys64/mingw64/x86_64-w64-mingw32/include/windows.h:69,
                 from ../../src/w32gui.h:21,
                 from ../../src/w32term.h:21,
                 from ../../src/w32fns.c:35:
../../src/w32fns.c: In function 'add_tray_notification':
../../src/w32fns.c:8904:16: error: 'NOTIFYICONDATAW {aka struct _NOTIFYICONDATAW}' has no member named 'hBalloonIcon'
  nidw.cbSize = NOTIFYICONDATAW_V3_SIZE;
                ^
Makefile:364: recipe for target 'w32fns.o' failed
--[msys2 mingw64 64bit]-----------------------------------------------


2) Cygwin w32 build (64bit)

The cygwin w32 build breaks because it builds w32fns.c but uses
different filename manipulation routines:

--[cygwin-w32]--------------------------------------------------------
In file included from /usr/include/w32api/minwindef.h:163:0,
                 from /usr/include/w32api/windef.h:8,
                 from /usr/include/w32api/windows.h:69,
                 from /cygdrive/c/emacs/git/emacs/master/src/w32gui.h:21,
                 from /cygdrive/c/emacs/git/emacs/master/src/w32term.h:21,
                 from /cygdrive/c/emacs/git/emacs/master/src/w32fns.c:35:
/cygdrive/c/emacs/git/emacs/master/src/w32fns.c: In function ‘add_tray_notification’:
/cygdrive/c/emacs/git/emacs/master/src/w32fns.c:8904:16: error: ‘NOTIFYICONDATAW’ has no member named ‘hBalloonIcon’
  nidw.cbSize = NOTIFYICONDATAW_V3_SIZE;
                ^
/cygdrive/c/emacs/git/emacs/master/src/w32fns.c:8921:8: warning: implicit declaration of function ‘filename_to_utf16’ [-Wimplicit-function-declaration]
        if (filename_to_utf16 (icon, icon_w) != 0)
        ^
/cygdrive/c/emacs/git/emacs/master/src/w32fns.c:8933:8: warning: implicit declaration of function ‘filename_to_ansi’ [-Wimplicit-function-declaration]
        if (filename_to_ansi (icon, icon_a) != 0)
        ^
/cygdrive/c/emacs/git/emacs/master/src/w32fns.c:8960:4: warning: implicit declaration of function ‘pMultiByteToWideChar’ [-Wimplicit-function-declaration]
    tiplen = pMultiByteToWideChar (CP_UTF8, MB_ERR_INVALID_CHARS,
    ^
Makefile:364: recipe for target 'w32fns.o' failed
--[cygwin-w32]--------------------------------------------------------




^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Windows mingw64 and cygwin builds broken
  2015-11-12 20:06 Windows mingw64 and cygwin builds broken Andy Moreton
@ 2015-11-12 20:42 ` Eli Zaretskii
  2015-11-12 22:01   ` Andy Moreton
  2015-11-12 22:12   ` Andy Moreton
  0 siblings, 2 replies; 18+ messages in thread
From: Eli Zaretskii @ 2015-11-12 20:42 UTC (permalink / raw)
  To: Andy Moreton; +Cc: emacs-devel

> From: Andy Moreton <andrewjmoreton@gmail.com>
> Date: Thu, 12 Nov 2015 20:06:51 +0000
> 
> Changes in commit 9d43941569fc broke the Windows builds.

Thanks, I tried to fix those problems in c1bc6e5.



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Windows mingw64 and cygwin builds broken
  2015-11-12 20:42 ` Eli Zaretskii
@ 2015-11-12 22:01   ` Andy Moreton
  2015-11-12 22:12   ` Andy Moreton
  1 sibling, 0 replies; 18+ messages in thread
From: Andy Moreton @ 2015-11-12 22:01 UTC (permalink / raw)
  To: emacs-devel

On Thu 12 Nov 2015, Eli Zaretskii wrote:

>> From: Andy Moreton <andrewjmoreton@gmail.com>
>> Date: Thu, 12 Nov 2015 20:06:51 +0000
>> 
>> Changes in commit 9d43941569fc broke the Windows builds.
>
> Thanks, I tried to fix those problems in c1bc6e5.

Thanks Eli.

Something else is still wrong though: in the mingw64 build,
bootstrapping from a clean tree results in a SEGV in temacs.
Attaching gdb shows:

Attaching to process 2024
[New Thread 2024.0x12bc]
[New Thread 2024.0x1304]
Reading symbols from C:\emacs\git\emacs\master\obj-mingw64\src\temacs.exe...done.
0x000000007769cc91 in ntdll!DbgBreakPoint () from C:\Windows\SYSTEM32\ntdll.dll
warning: /c/emacs/git/emacs/master/obj-mingw64/../src/.gdbinit: No such file or directory
Environment variable "DISPLAY" not defined.
TERM = xterm-256color
Breakpoint 1 at 0x400136118: file ../../src/emacs.c, line 371.
Temporary breakpoint 2 at 0x4001638ed: file ../../src/sysdep.c, line 905.
(gdb) cont
Continuing.
[Thread 2024.0x1304 exited with code 0]

Program received signal SIGTRAP, Trace/breakpoint trap.
[Switching to Thread 2024.0x12bc]
0x000007fefd773c63 in KERNELBASE!DebugBreak () from C:\Windows\system32\KernelBase.dll
(gdb) bt full
#0  0x000007fefd773c63 in KERNELBASE!DebugBreak () from C:\Windows\system32\KernelBase.dll
No symbol table info available.
#1  0x00000004002919c7 in emacs_abort () at ../../src/w32fns.c:9791
        button = 0x6
#2  0x00000004001f2786 in specbind (symbol=..., value=...) at ../../src/eval.c:3021
        sym = 0x401ba3f50 <lispsym+32704>
#3  0x000000040013d6b6 in safe_run_hooks (hook=...) at ../../src/keyboard.c:1831
        count = 0x0
#4  0x000000040019443f in Fdo_auto_save (no_message=..., current_only=...) at ../../src/fileio.c:5502
        old = 0x0
        b = 0xa
        tail = {
          i = 0xfffffffdffffffff
        }
        buf = {
          i = 0x7fefdf22ae0
        }
        hook = {
          i = 0xfffffffffebefed0
        }
        auto_saved = 0x0
        do_handled_files = 0xfdf22ae0
        oquit = {
          i = 0x0
        }
        stream = 0x0
        count = 0x0
        orig_minibuffer_auto_raise = 0x0
        old_message_p = 0x0
        auto_save_unwind = {
          stream = 0x1,
          auto_raise = 0x1
        }
#5  0x0000000400138502 in shut_down_emacs (sig=0x16, stuff=...) at ../../src/emacs.c:2018
No locals.
#6  0x0000000400136189 in terminate_due_to_signal (sig=0x16, backtrace_limit=0x7fffffff) at ../../src/emacs.c:     382
No locals.
#7  0x00000004001cae93 in die (msg=0x4006e531b <DEFAULT_REHASH_SIZE+2931> "INTEGERP (bucket)", file=0x4006e47b     0 <DEFAULT_REHASH_SIZE+8> "../../src/lread.c", line=0xecb) at ../../src/alloc.c:7000
No locals.
#8  0x000000040022b06e in define_symbol (sym=..., str=0x4006de8d2 <NaN_string.60208+370> ":timeout") at ../../     src/lread.c:3787
        bucket = {
          i = 0x6c8
        }
        len = 0x8
        string = {
          i = 0x40031a4ac
        }
#9  0x000000040022ba0b in init_obarray () at ../../src/lread.c:4007
        i = 0x94
        oblength = {
          i = 0x179e
        }
        size = 0x69
#10 0x00000004001375ba in main (argc=0x5, argv=0x8f7310) at ../../src/emacs.c:1180
        dummy = {
          i = 0x28
        }
        stack_bottom_variable = 0x0
        do_initial_setlocale = 0x1
        dumping = 0x1
        skip_args = 0x1
        no_loadup = 0x0
        junk = 0x0
        dname_arg = 0x0
        ch_to_dir = 0x0
        original_pwd = 0x0

Program received signal SIGSEGV, Segmentation fault.
0x00000004001e9761 in backtrace_top () at ../../src/eval.c:184
184       while (backtrace_p (pdl) && pdl->kind != SPECPDL_BACKTRACE)
The program being debugged was signaled while in a function called from GDB.
GDB remains in the frame where the signal was received.
To change this behavior use "set unwindonsignal on".
Evaluation of the expression containing the function
(backtrace_top) will be abandoned.
When the function is done executing, GDB will silently stop.
(gdb)




^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Windows mingw64 and cygwin builds broken
  2015-11-12 20:42 ` Eli Zaretskii
  2015-11-12 22:01   ` Andy Moreton
@ 2015-11-12 22:12   ` Andy Moreton
  2015-11-13  2:11     ` Andy Moreton
  2015-11-13  9:11     ` Eli Zaretskii
  1 sibling, 2 replies; 18+ messages in thread
From: Andy Moreton @ 2015-11-12 22:12 UTC (permalink / raw)
  To: emacs-devel

On Thu 12 Nov 2015, Eli Zaretskii wrote:

>> From: Andy Moreton <andrewjmoreton@gmail.com>
>> Date: Thu, 12 Nov 2015 20:06:51 +0000
>> 
>> Changes in commit 9d43941569fc broke the Windows builds.
>
> Thanks, I tried to fix those problems in c1bc6e5.

The cygwin w32 build needs an additional patch:

diff --git a/src/w32fns.c b/src/w32fns.c
index b71002f8bfc9..93b7152a34f8 100644
--- a/src/w32fns.c
+++ b/src/w32fns.c
@@ -9635,8 +9635,10 @@ This variable has effect only on Windows Vista and later.  */);
   defsubr (&Sw32_window_exists_p);
   defsubr (&Sw32_battery_status);
   defsubr (&Sw32__menu_bar_in_use);
+#ifndef CYGWIN
   defsubr (&Sw32_notification_notify);
   defsubr (&Sw32_notification_close);
+#endif

 #ifdef WINDOWSNT
   defsubr (&Sfile_system_info);


However, after applying the patch. temacs hits a problem:

make[2]: Leaving directory '/cygdrive/c/emacs/git/emacs/master/obj-cygwin64-w32/lisp'
./temacs --batch --load loadup bootstrap

/cygdrive/c/emacs/git/emacs/master/src/lread.c:3787: Emacs fatal error: assertion failed: INTEGERP (bucket)
Fatal error 6: Aborted: paxctl -zex emacs.exe
mv -f emacs.exe bootstrap-emacs.exe
mv: cannot stat ‘emacs.exe’: No such file or directory
Makefile:707: recipe for target 'bootstrap-emacs.exe' failed




^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: Windows mingw64 and cygwin builds broken
  2015-11-12 22:12   ` Andy Moreton
@ 2015-11-13  2:11     ` Andy Moreton
  2015-11-13  9:11       ` Eli Zaretskii
  2015-11-13  9:11     ` Eli Zaretskii
  1 sibling, 1 reply; 18+ messages in thread
From: Andy Moreton @ 2015-11-13  2:11 UTC (permalink / raw)
  To: emacs-devel

On Thu 12 Nov 2015, Andy Moreton wrote:

> On Thu 12 Nov 2015, Eli Zaretskii wrote:
>
>>> From: Andy Moreton <andrewjmoreton@gmail.com>
>>> Date: Thu, 12 Nov 2015 20:06:51 +0000
>>> 
>>> Changes in commit 9d43941569fc broke the Windows builds.
>>
>> Thanks, I tried to fix those problems in c1bc6e5.

> make[2]: Leaving directory '/cygdrive/c/emacs/git/emacs/master/obj-cygwin64-w32/lisp'
> ./temacs --batch --load loadup bootstrap
>
> /cygdrive/c/emacs/git/emacs/master/src/lread.c:3787: Emacs fatal error: assertion failed: INTEGERP (bucket)
> Fatal error 6: Aborted: paxctl -zex emacs.exe
> mv -f emacs.exe bootstrap-emacs.exe
> mv: cannot stat ‘emacs.exe’: No such file or directory
> Makefile:707: recipe for target 'bootstrap-emacs.exe' failed

This appears to be caused by a clash between symbols:

./dbusbind.c:1704:  DEFSYM (QCdbus_timeout, ":timeout");
./w32fns.c:9302:  DEFSYM (QCtimeout, ":timeout");

Renaming QCdbus_timeout to QCtimeout allows the cygwin-w32 and mingw64
builds to bootstrap successfully (I don't know if that is the right
fix though). Should the other keyword argument symbols in dbusbind.c
also be renamed QCdbus_* -> QC* ?

Here's a minimal working patch:

diff --git a/src/dbusbind.c b/src/dbusbind.c
index ce0465dcb8b9..d3ace51cf1eb 100644
--- a/src/dbusbind.c
+++ b/src/dbusbind.c
@@ -1405,7 +1405,7 @@ usage: (dbus-message-internal &rest REST)  */)
     }
 
   /* Check for timeout parameter.  */
-  if ((count+2 <= nargs) && (EQ ((args[count]), QCdbus_timeout)))
+  if ((count+2 <= nargs) && (EQ ((args[count]), QCtimeout)))
     {
       CHECK_NATNUM (args[count+1]);
       timeout = min (XFASTINT (args[count+1]), INT_MAX);
@@ -1701,7 +1701,7 @@ syms_of_dbusbind (void)
   DEFSYM (QCdbus_session_bus, ":session");
 
   /* Lisp symbol for method call timeout.  */
-  DEFSYM (QCdbus_timeout, ":timeout");
+  DEFSYM (QCtimeout, ":timeout");
 
   /* Lisp symbols of D-Bus types.  */
   DEFSYM (QCdbus_type_byte, ":byte");
diff --git a/src/w32fns.c b/src/w32fns.c
index b71002f8bfc9..07102e477245 100644
--- a/src/w32fns.c
+++ b/src/w32fns.c
@@ -9635,8 +9635,10 @@ This variable has effect only on Windows Vista and later.  */);
   defsubr (&Sw32_window_exists_p);
   defsubr (&Sw32_battery_status);
   defsubr (&Sw32__menu_bar_in_use);
+#ifndef CYGWIN
   defsubr (&Sw32_notification_notify);
   defsubr (&Sw32_notification_close);
+#endif
 
 #ifdef WINDOWSNT
   defsubr (&Sfile_system_info);




^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: Windows mingw64 and cygwin builds broken
  2015-11-13  2:11     ` Andy Moreton
@ 2015-11-13  9:11       ` Eli Zaretskii
  2015-11-13  9:58         ` Andy Moreton
  2015-11-13 10:13         ` Michael Albinus
  0 siblings, 2 replies; 18+ messages in thread
From: Eli Zaretskii @ 2015-11-13  9:11 UTC (permalink / raw)
  To: Andy Moreton, Michael Albinus; +Cc: emacs-devel

> From: Andy Moreton <andrewjmoreton@gmail.com>
> Date: Fri, 13 Nov 2015 02:11:36 +0000
> 
> > ./temacs --batch --load loadup bootstrap
> >
> > /cygdrive/c/emacs/git/emacs/master/src/lread.c:3787: Emacs fatal error: assertion failed: INTEGERP (bucket)
> > Fatal error 6: Aborted: paxctl -zex emacs.exe
> > mv -f emacs.exe bootstrap-emacs.exe
> > mv: cannot stat ‘emacs.exe’: No such file or directory
> > Makefile:707: recipe for target 'bootstrap-emacs.exe' failed
> 
> This appears to be caused by a clash between symbols:
> 
> ./dbusbind.c:1704:  DEFSYM (QCdbus_timeout, ":timeout");
> ./w32fns.c:9302:  DEFSYM (QCtimeout, ":timeout");

Does this mean that your MinGW64 build uses D-Bus?  If so, it
shouldn't use the native w32 tray notifications.  I've pushed a change
to that effect, please test.  If you can afford testing MinGW64 also
without D-Bus, I'd appreciate that.

> Renaming QCdbus_timeout to QCtimeout allows the cygwin-w32 and mingw64
> builds to bootstrap successfully (I don't know if that is the right
> fix though). Should the other keyword argument symbols in dbusbind.c
> also be renamed QCdbus_* -> QC* ?

I don't understand why dbusbind.c uses such a non-standard naming
convention.  Michael?

Thanks.




^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Windows mingw64 and cygwin builds broken
  2015-11-12 22:12   ` Andy Moreton
  2015-11-13  2:11     ` Andy Moreton
@ 2015-11-13  9:11     ` Eli Zaretskii
  2015-11-13 10:18       ` Andy Moreton
  1 sibling, 1 reply; 18+ messages in thread
From: Eli Zaretskii @ 2015-11-13  9:11 UTC (permalink / raw)
  To: Andy Moreton; +Cc: emacs-devel

> From: Andy Moreton <andrewjmoreton@gmail.com>
> Date: Thu, 12 Nov 2015 22:12:09 +0000
> 
> On Thu 12 Nov 2015, Eli Zaretskii wrote:
> 
> >> From: Andy Moreton <andrewjmoreton@gmail.com>
> >> Date: Thu, 12 Nov 2015 20:06:51 +0000
> >> 
> >> Changes in commit 9d43941569fc broke the Windows builds.
> >
> > Thanks, I tried to fix those problems in c1bc6e5.
> 
> The cygwin w32 build needs an additional patch:

Thanks, fixed.



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Windows mingw64 and cygwin builds broken
  2015-11-13  9:11       ` Eli Zaretskii
@ 2015-11-13  9:58         ` Andy Moreton
  2015-11-13 10:10           ` Eli Zaretskii
  2015-11-13 10:13         ` Michael Albinus
  1 sibling, 1 reply; 18+ messages in thread
From: Andy Moreton @ 2015-11-13  9:58 UTC (permalink / raw)
  To: emacs-devel

On Fri 13 Nov 2015, Eli Zaretskii wrote:

>> From: Andy Moreton <andrewjmoreton@gmail.com>
>> Date: Fri, 13 Nov 2015 02:11:36 +0000
>> 
>> > ./temacs --batch --load loadup bootstrap
>> >
>> > /cygdrive/c/emacs/git/emacs/master/src/lread.c:3787: Emacs fatal error: assertion failed: INTEGERP (bucket)
>> > Fatal error 6: Aborted: paxctl -zex emacs.exe
>> > mv -f emacs.exe bootstrap-emacs.exe
>> > mv: cannot stat ‘emacs.exe’: No such file or directory
>> > Makefile:707: recipe for target 'bootstrap-emacs.exe' failed
>> 
>> This appears to be caused by a clash between symbols:
>> 
>> ./dbusbind.c:1704:  DEFSYM (QCdbus_timeout, ":timeout");
>> ./w32fns.c:9302:  DEFSYM (QCtimeout, ":timeout");
>
> Does this mean that your MinGW64 build uses D-Bus?  If so, it
> shouldn't use the native w32 tray notifications.  I've pushed a change
> to that effect, please test.  If you can afford testing MinGW64 also
> without D-Bus, I'd appreciate that.

I dont use D-Bus, but it may be detected by configure in the mingw64
build. Your patch fails to build on mingw64:

./temacs --batch --load loadup bootstrap

../../src/lread.c:3787: Emacs fatal error: assertion failed: INTEGERP (bucket)
Fatal error 6: Aborted
Backtrace:
0x100694f9a
0x100695011
0x1005f8cad
0x10054149b
0x100599c8d
0x10053c757
0x10053a216
0x1005d092f
0x1006315bc
0x100631f58
0x10053b7ed
0x180048365
0x180046074
0x18004610c
0x1006ed999
0x100401008
0x76cd5a45
0x76e0b829
Makefile:707: recipe for target 'bootstrap-emacs.exe' failed

>> Renaming QCdbus_timeout to QCtimeout allows the cygwin-w32 and mingw64
>> builds to bootstrap successfully (I don't know if that is the right
>> fix though). Should the other keyword argument symbols in dbusbind.c
>> also be renamed QCdbus_* -> QC* ?
>
> I don't understand why dbusbind.c uses such a non-standard naming
> convention.  Michael?

If the patch renamed the C symbols to use the normal convention, then it
appears that there is no harm in havng two modules declare identical
symbols in syms_of_*(). 

    AndyM




^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Windows mingw64 and cygwin builds broken
  2015-11-13  9:58         ` Andy Moreton
@ 2015-11-13 10:10           ` Eli Zaretskii
  2015-11-13 11:23             ` Andy Moreton
  0 siblings, 1 reply; 18+ messages in thread
From: Eli Zaretskii @ 2015-11-13 10:10 UTC (permalink / raw)
  To: Andy Moreton; +Cc: emacs-devel

> From: Andy Moreton <andrewjmoreton@gmail.com>
> Date: Fri, 13 Nov 2015 09:58:33 +0000
> 
> >> ./dbusbind.c:1704:  DEFSYM (QCdbus_timeout, ":timeout");
> >> ./w32fns.c:9302:  DEFSYM (QCtimeout, ":timeout");
> >
> > Does this mean that your MinGW64 build uses D-Bus?  If so, it
> > shouldn't use the native w32 tray notifications.  I've pushed a change
> > to that effect, please test.  If you can afford testing MinGW64 also
> > without D-Bus, I'd appreciate that.
> 
> I dont use D-Bus, but it may be detected by configure in the mingw64
> build.

Is HAVE_DBUS defined in src/config.h?  Is src/dbusbind.c compiled, and
do you see src/dbusbind.o in your MinGW64 build?

> Your patch fails to build on mingw64:
> 
> ./temacs --batch --load loadup bootstrap
> 
> ../../src/lread.c:3787: Emacs fatal error: assertion failed: INTEGERP (bucket)
> Fatal error 6: Aborted

With or without commit 508e77b?  I had one more thinko fixed there.

If this is with 508e77b, then I don't understand what's going on:
there should be only one definition of ':timeout', the one in
dbusbind.c.

> Backtrace:
> 0x100694f9a
> 0x100695011
> 0x1005f8cad
> 0x10054149b
> 0x100599c8d
> 0x10053c757
> 0x10053a216
> 0x1005d092f
> 0x1006315bc
> 0x100631f58
> 0x10053b7ed
> 0x180048365
> 0x180046074
> 0x18004610c
> 0x1006ed999
> 0x100401008
> 0x76cd5a45
> 0x76e0b829

Can you convert this to human-readable backtrace, or run the same
command under GDB and show a backtrace?

> >> Renaming QCdbus_timeout to QCtimeout allows the cygwin-w32 and mingw64
> >> builds to bootstrap successfully (I don't know if that is the right
> >> fix though). Should the other keyword argument symbols in dbusbind.c
> >> also be renamed QCdbus_* -> QC* ?
> >
> > I don't understand why dbusbind.c uses such a non-standard naming
> > convention.  Michael?
> 
> If the patch renamed the C symbols to use the normal convention, then it
> appears that there is no harm in havng two modules declare identical
> symbols in syms_of_*(). 

Yes, but I still want to understand the reasons for this naming
convention, and I think we need to avoid calling DEFSYM twice even for
the same name.



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Windows mingw64 and cygwin builds broken
  2015-11-13  9:11       ` Eli Zaretskii
  2015-11-13  9:58         ` Andy Moreton
@ 2015-11-13 10:13         ` Michael Albinus
  1 sibling, 0 replies; 18+ messages in thread
From: Michael Albinus @ 2015-11-13 10:13 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Andy Moreton, emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> Renaming QCdbus_timeout to QCtimeout allows the cygwin-w32 and mingw64
>> builds to bootstrap successfully (I don't know if that is the right
>> fix though). Should the other keyword argument symbols in dbusbind.c
>> also be renamed QCdbus_* -> QC* ?
>
> I don't understand why dbusbind.c uses such a non-standard naming
> convention.  Michael?

Well, it's eight years ago that I wrote dbusbind.c ... likely no special
reason, just the pedantic German who has prefixed every symbol with
"QCdbus_".

Feel free to change it.

> Thanks.

Best regards, Michael.



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Windows mingw64 and cygwin builds broken
  2015-11-13  9:11     ` Eli Zaretskii
@ 2015-11-13 10:18       ` Andy Moreton
  2015-11-13 13:25         ` Eli Zaretskii
  0 siblings, 1 reply; 18+ messages in thread
From: Andy Moreton @ 2015-11-13 10:18 UTC (permalink / raw)
  To: emacs-devel

On Fri 13 Nov 2015, Eli Zaretskii wrote:

>> From: Andy Moreton <andrewjmoreton@gmail.com>
>> Date: Thu, 12 Nov 2015 22:12:09 +0000
>> 
>> On Thu 12 Nov 2015, Eli Zaretskii wrote:
>> 
>> >> From: Andy Moreton <andrewjmoreton@gmail.com>
>> >> Date: Thu, 12 Nov 2015 20:06:51 +0000
>> >> 
>> >> Changes in commit 9d43941569fc broke the Windows builds.
>> >
>> > Thanks, I tried to fix those problems in c1bc6e5.
>> 
>> The cygwin w32 build needs an additional patch:
>
> Thanks, fixed.

Both the cygwin-w32 and mingw64 builds now need to be configured with
--without-dbus to avoid a crash when dumping.

As users may want to use D-Bus with cygwin-w32 emacs (and possibly with
mingw64 emacs), a configure option is needed for the w32-notifications.
The change in requirements to build emacs also needs to be documented in
NEWS.

Can you also please explain what was wrong with the patch I sent, which
was simpler than your solution. Does it make sense to build emacs with
support for both D-Bus and w32-notifications, and for the user to
choose which is enabled at runtime ?

    AndyM




^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Windows mingw64 and cygwin builds broken
  2015-11-13 10:10           ` Eli Zaretskii
@ 2015-11-13 11:23             ` Andy Moreton
  0 siblings, 0 replies; 18+ messages in thread
From: Andy Moreton @ 2015-11-13 11:23 UTC (permalink / raw)
  To: emacs-devel

On Fri 13 Nov 2015, Eli Zaretskii wrote:

>> From: Andy Moreton <andrewjmoreton@gmail.com>
>> Date: Fri, 13 Nov 2015 09:58:33 +0000
>> 
>> >> ./dbusbind.c:1704:  DEFSYM (QCdbus_timeout, ":timeout");
>> >> ./w32fns.c:9302:  DEFSYM (QCtimeout, ":timeout");
>> >
>> > Does this mean that your MinGW64 build uses D-Bus?  If so, it
>> > shouldn't use the native w32 tray notifications.  I've pushed a change
>> > to that effect, please test.  If you can afford testing MinGW64 also
>> > without D-Bus, I'd appreciate that.
>> 
>> I dont use D-Bus, but it may be detected by configure in the mingw64
>> build.
>
> Is HAVE_DBUS defined in src/config.h?  Is src/dbusbind.c compiled, and
> do you see src/dbusbind.o in your MinGW64 build?

configure was detecting dbus support (shown in the summary at the end),
so I think it was being compiled in. mingw64 bootstrap of commit
2b4c0c0cefa4 works after adding "--without-dbus" to my build script.

[ backtracce snipped]
> Can you convert this to human-readable backtrace, or run the same
> command under GDB and show a backtrace?

Sorry, I don't have that build any more. I will look again at the weekend.

>> >> Renaming QCdbus_timeout to QCtimeout allows the cygwin-w32 and mingw64
>> >> builds to bootstrap successfully (I don't know if that is the right
>> >> fix though). Should the other keyword argument symbols in dbusbind.c
>> >> also be renamed QCdbus_* -> QC* ?
>> >
>> > I don't understand why dbusbind.c uses such a non-standard naming
>> > convention.  Michael?
>> 
>> If the patch renamed the C symbols to use the normal convention, then it
>> appears that there is no harm in havng two modules declare identical
>> symbols in syms_of_*(). 
>
> Yes, but I still want to understand the reasons for this naming
> convention, and I think we need to avoid calling DEFSYM twice even for
> the same name.

If we want to avoid calling DEFSYM twice (which seems a good idea), then
the C level sumbol names should be available in a header, and common
keyword argument symbols should be DEFSYMed in a module that is always
present regardless of build options.

    AndyM




^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Windows mingw64 and cygwin builds broken
  2015-11-13 10:18       ` Andy Moreton
@ 2015-11-13 13:25         ` Eli Zaretskii
  2015-11-13 13:39           ` Eli Zaretskii
  2015-11-13 14:18           ` Andy Moreton
  0 siblings, 2 replies; 18+ messages in thread
From: Eli Zaretskii @ 2015-11-13 13:25 UTC (permalink / raw)
  To: Andy Moreton; +Cc: emacs-devel

> From: Andy Moreton <andrewjmoreton@gmail.com>
> Date: Fri, 13 Nov 2015 10:18:22 +0000
> 
> Both the cygwin-w32 and mingw64 builds now need to be configured with
> --without-dbus to avoid a crash when dumping.

That's not what I intended.  It was supposed to work even when D-Bus
is compiled in.  The code that supports w32 notifications and the
related DEFSYM's are conditioned on non-Cygwin build and on D-Bus not
being available.  When D-Bus _is_ available, then only the DEFSYM's in
dbusbind.c were supposed to be visible, and those in w32fns.c
invisible, together with their supporting code.  But somehow this is
not working...

Ah, I think I see the problem: makedoc doesn't take preprocessor
directives into account.  I need to think how to fix that.

> As users may want to use D-Bus with cygwin-w32 emacs (and possibly with
> mingw64 emacs), a configure option is needed for the w32-notifications.

No, I don't think we need a configure option.  The intent was to
provide the native w32 notifications only if D-Bus is not available,
because the former support only a small subset of the functionality.

> Can you also please explain what was wrong with the patch I sent, which
> was simpler than your solution.

It used the same DEFSYM twice in certain configurations, which I
don't like.

> Does it make sense to build emacs with support for both D-Bus and
> w32-notifications, and for the user to choose which is enabled at
> runtime ?

Not to me, it doesn't.  And anyway, this is a separate issue; how to
fix the double definition of ':timeout' is unrelated to whether we
want to support both D-Bus notifications and w32 notifications in the
same build.  We could discuss the latter after we fix the former.

Thanks.



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Windows mingw64 and cygwin builds broken
  2015-11-13 13:25         ` Eli Zaretskii
@ 2015-11-13 13:39           ` Eli Zaretskii
  2015-11-13 14:18           ` Andy Moreton
  1 sibling, 0 replies; 18+ messages in thread
From: Eli Zaretskii @ 2015-11-13 13:39 UTC (permalink / raw)
  To: andrewjmoreton; +Cc: emacs-devel

> Date: Fri, 13 Nov 2015 15:25:45 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: emacs-devel@gnu.org
> 
> > From: Andy Moreton <andrewjmoreton@gmail.com>
> > Date: Fri, 13 Nov 2015 10:18:22 +0000
> > 
> > Both the cygwin-w32 and mingw64 builds now need to be configured with
> > --without-dbus to avoid a crash when dumping.
> 
> That's not what I intended.  It was supposed to work even when D-Bus
> is compiled in.  The code that supports w32 notifications and the
> related DEFSYM's are conditioned on non-Cygwin build and on D-Bus not
> being available.  When D-Bus _is_ available, then only the DEFSYM's in
> dbusbind.c were supposed to be visible, and those in w32fns.c
> invisible, together with their supporting code.  But somehow this is
> not working...
> 
> Ah, I think I see the problem: makedoc doesn't take preprocessor
> directives into account.  I need to think how to fix that.

I simply deleted the support for ':timeout' from the native w32
notifications.  It isn't worth the hassle, as it is only supported on
versions of Windows before Vista, and is silently ignored on newer
versions.

Please see if the original problems are solved, even without using
"--without-dbus" at configure time.

Thanks.



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Windows mingw64 and cygwin builds broken
  2015-11-13 13:25         ` Eli Zaretskii
  2015-11-13 13:39           ` Eli Zaretskii
@ 2015-11-13 14:18           ` Andy Moreton
  2015-11-13 14:44             ` Eli Zaretskii
  1 sibling, 1 reply; 18+ messages in thread
From: Andy Moreton @ 2015-11-13 14:18 UTC (permalink / raw)
  To: emacs-devel

On Fri 13 Nov 2015, Eli Zaretskii wrote:

>> From: Andy Moreton <andrewjmoreton@gmail.com>
>> Date: Fri, 13 Nov 2015 10:18:22 +0000
>> 
>> Both the cygwin-w32 and mingw64 builds now need to be configured with
>> --without-dbus to avoid a crash when dumping.
>
> That's not what I intended.  It was supposed to work even when D-Bus
> is compiled in.  The code that supports w32 notifications and the
> related DEFSYM's are conditioned on non-Cygwin build and on D-Bus not
> being available.  When D-Bus _is_ available, then only the DEFSYM's in
> dbusbind.c were supposed to be visible, and those in w32fns.c
> invisible, together with their supporting code.  But somehow this is
> not working...
>
> Ah, I think I see the problem: makedoc doesn't take preprocessor
> directives into account.  I need to think how to fix that.
>
>> As users may want to use D-Bus with cygwin-w32 emacs (and possibly with
>> mingw64 emacs), a configure option is needed for the w32-notifications.
>
> No, I don't think we need a configure option.  The intent was to
> provide the native w32 notifications only if D-Bus is not available,
> because the former support only a small subset of the functionality.

That still leaves users without the ability to turn both off (unless
they patch the sources before building).

>> Can you also please explain what was wrong with the patch I sent, which
>> was simpler than your solution.
>
> It used the same DEFSYM twice in certain configurations, which I
> don't like.

Fair enough - I agree that while it "works" at the moment, it is
potentially fragile.

>> Does it make sense to build emacs with support for both D-Bus and
>> w32-notifications, and for the user to choose which is enabled at
>> runtime ?
>
> Not to me, it doesn't.  And anyway, this is a separate issue; how to
> fix the double definition of ':timeout' is unrelated to whether we
> want to support both D-Bus notifications and w32 notifications in the
> same build.  We could discuss the latter after we fix the former.

I've since tried the cygwin-w32 and mingw64 builds on a different
machine, where both builds succeed (however the two machines may have
dfferent sets of packages installed in each environment).

    AndyM




^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Windows mingw64 and cygwin builds broken
  2015-11-13 14:18           ` Andy Moreton
@ 2015-11-13 14:44             ` Eli Zaretskii
  2015-11-13 22:30               ` Andy Moreton
  0 siblings, 1 reply; 18+ messages in thread
From: Eli Zaretskii @ 2015-11-13 14:44 UTC (permalink / raw)
  To: Andy Moreton; +Cc: emacs-devel

> From: Andy Moreton <andrewjmoreton@gmail.com>
> Date: Fri, 13 Nov 2015 14:18:14 +0000
> 
> >> As users may want to use D-Bus with cygwin-w32 emacs (and possibly with
> >> mingw64 emacs), a configure option is needed for the w32-notifications.
> >
> > No, I don't think we need a configure option.  The intent was to
> > provide the native w32 notifications only if D-Bus is not available,
> > because the former support only a small subset of the functionality.
> 
> That still leaves users without the ability to turn both off (unless
> they patch the sources before building).

Why would they want to?  We don't let them turn, say,
w32-shell-execute off, do we?  How is this feature different?

> I've since tried the cygwin-w32 and mingw64 builds on a different
> machine, where both builds succeed (however the two machines may have
> dfferent sets of packages installed in each environment).

Thanks.  Please also see if it works on the same machine where it
failed before that.



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Windows mingw64 and cygwin builds broken
  2015-11-13 14:44             ` Eli Zaretskii
@ 2015-11-13 22:30               ` Andy Moreton
  2015-11-14  8:15                 ` Eli Zaretskii
  0 siblings, 1 reply; 18+ messages in thread
From: Andy Moreton @ 2015-11-13 22:30 UTC (permalink / raw)
  To: emacs-devel

On Fri 13 Nov 2015, Eli Zaretskii wrote:

>> From: Andy Moreton <andrewjmoreton@gmail.com>
>> Date: Fri, 13 Nov 2015 14:18:14 +0000
>> 
>> >> As users may want to use D-Bus with cygwin-w32 emacs (and possibly with
>> >> mingw64 emacs), a configure option is needed for the w32-notifications.
>> >
>> > No, I don't think we need a configure option.  The intent was to
>> > provide the native w32 notifications only if D-Bus is not available,
>> > because the former support only a small subset of the functionality.
>> 
>> That still leaves users without the ability to turn both off (unless
>> they patch the sources before building).
>
> Why would they want to?  We don't let them turn, say,
> w32-shell-execute off, do we?  How is this feature different?
>
>> I've since tried the cygwin-w32 and mingw64 builds on a different
>> machine, where both builds succeed (however the two machines may have
>> dfferent sets of packages installed in each environment).
>
> Thanks.  Please also see if it works on the same machine where it
> failed before that.

The machine with previously failing builds is now ok, so all seems to be
fixed.

    AndyM




^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Windows mingw64 and cygwin builds broken
  2015-11-13 22:30               ` Andy Moreton
@ 2015-11-14  8:15                 ` Eli Zaretskii
  0 siblings, 0 replies; 18+ messages in thread
From: Eli Zaretskii @ 2015-11-14  8:15 UTC (permalink / raw)
  To: Andy Moreton; +Cc: emacs-devel

> From: Andy Moreton <andrewjmoreton@gmail.com>
> Date: Fri, 13 Nov 2015 22:30:51 +0000
> 
> The machine with previously failing builds is now ok, so all seems to be
> fixed.

Thank you.  (In my defense, I should say that I did look for other
definitions of the symbols I introduced, but didn't find the
':timeout' one because I looked for "QCtimeout", and dbusbind.c uses a
different convention.  I should have looked for ":timeout" instead.)

If you'd like to start (or continue ;-) the discussion about run-time
selection between the D-Bus and w32 notifications, please do.



^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2015-11-14  8:15 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-11-12 20:06 Windows mingw64 and cygwin builds broken Andy Moreton
2015-11-12 20:42 ` Eli Zaretskii
2015-11-12 22:01   ` Andy Moreton
2015-11-12 22:12   ` Andy Moreton
2015-11-13  2:11     ` Andy Moreton
2015-11-13  9:11       ` Eli Zaretskii
2015-11-13  9:58         ` Andy Moreton
2015-11-13 10:10           ` Eli Zaretskii
2015-11-13 11:23             ` Andy Moreton
2015-11-13 10:13         ` Michael Albinus
2015-11-13  9:11     ` Eli Zaretskii
2015-11-13 10:18       ` Andy Moreton
2015-11-13 13:25         ` Eli Zaretskii
2015-11-13 13:39           ` Eli Zaretskii
2015-11-13 14:18           ` Andy Moreton
2015-11-13 14:44             ` Eli Zaretskii
2015-11-13 22:30               ` Andy Moreton
2015-11-14  8:15                 ` Eli Zaretskii

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).