After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 773710 - Build failure with --disable-x11: keymasks.h should not include Xlib.h
Build failure with --disable-x11: keymasks.h should not include Xlib.h
Status: RESOLVED FIXED
Product: at-spi
Classification: Platform
Component: at-spi2-core
unspecified
Other Mac OS
: Normal major
: ---
Assigned To: At-spi maintainer(s)
At-spi maintainer(s)
Depends on:
Blocks: 774453
 
 
Reported: 2016-10-31 05:14 UTC by Philip Chimento
Modified: 2016-11-27 20:16 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Copy in key mask defines from Xlib.h (2.20 KB, patch)
2016-10-31 05:14 UTC, Philip Chimento
none Details | Review
Copy in key mask defines from Xlib.h (3.18 KB, patch)
2016-10-31 05:25 UTC, Philip Chimento
committed Details | Review
Block out X11 API with #ifdefs (2.89 KB, patch)
2016-10-31 05:25 UTC, Philip Chimento
committed Details | Review
registryd: Include config.h before checking HAVE_X11 (666 bytes, patch)
2016-11-17 10:58 UTC, Philip Withnall
rejected Details | Review
registryd/display: Include config.h before checking HAVE_X11 (645 bytes, patch)
2016-11-18 06:18 UTC, Philip Chimento
rejected Details | Review
Revert "Block out X11 API with #ifdefs" (2.85 KB, patch)
2016-11-18 11:16 UTC, Emmanuele Bassi (:ebassi)
committed Details | Review
Ensure that X11 symbols are only used when necessary (3.07 KB, patch)
2016-11-18 11:16 UTC, Emmanuele Bassi (:ebassi)
committed Details | Review

Description Philip Chimento 2016-10-31 05:14:04 UTC
keymasks.h includes Xlib.h even when compiling with --disable-x11.

I think you can probably just put the values from Xlib.h verbatim into keymasks.h since that's what GDK does.
Comment 1 Philip Chimento 2016-10-31 05:14:44 UTC
Created attachment 338809 [details] [review]
Copy in key mask defines from Xlib.h

This is to avoid including Xlib.h when building with --disable-x11.
Comment 2 Philip Chimento 2016-10-31 05:25:23 UTC
Created attachment 338810 [details] [review]
Copy in key mask defines from Xlib.h

This is to avoid including Xlib.h when building with --disable-x11.
Comment 3 Philip Chimento 2016-10-31 05:25:26 UTC
Created attachment 338811 [details] [review]
Block out X11 API with #ifdefs

We should not include any X11 headers when building with --disable-x11,
so put them inside #ifdef guards, and don't use X symbols.
Comment 4 Mike Gorse 2016-11-17 02:01:59 UTC
Attachment 338810 [details] pushed as commit 66ac36
Attachment 338811 [details] pushed as commit 27e7b2

Thanks for the patches.
Comment 5 Philip Withnall 2016-11-17 10:58:43 UTC
Created attachment 340125 [details] [review]
registryd: Include config.h before checking HAVE_X11
Comment 6 Philip Withnall 2016-11-17 10:59:06 UTC
Here’s a follow-up patch from the other Philip.
Comment 7 fakey 2016-11-17 17:38:04 UTC
There still seems to be some X11 usage that wasn't properly #ifdef'd out:



*** Building at-spi2-core *** [12/18]
make -j 5
make  all-recursive
make[1]: Entering directory '/home/william/.cache/jhbuild/build/at-spi2-core'
Making all in po
make[2]: Entering directory '/home/william/.cache/jhbuild/build/at-spi2-core/po'
make[2]: Nothing to be done for 'all'.
make[2]: Leaving directory '/home/william/.cache/jhbuild/build/at-spi2-core/po'
Making all in dbind
make[2]: Entering directory '/home/william/.cache/jhbuild/build/at-spi2-core/dbind'
make[2]: Leaving directory '/home/william/.cache/jhbuild/build/at-spi2-core/dbind'
Making all in xml
make[2]: Entering directory '/home/william/.cache/jhbuild/build/at-spi2-core/xml'
make[2]: Leaving directory '/home/william/.cache/jhbuild/build/at-spi2-core/xml'
Making all in atspi
make[2]: Entering directory '/home/william/.cache/jhbuild/build/at-spi2-core/atspi'
make  all-am
make[3]: Entering directory '/home/william/.cache/jhbuild/build/at-spi2-core/atspi'
make[3]: Nothing to be done for 'all-am'.
make[3]: Leaving directory '/home/william/.cache/jhbuild/build/at-spi2-core/atspi'
make[2]: Leaving directory '/home/william/.cache/jhbuild/build/at-spi2-core/atspi'
Making all in bus
make[2]: Entering directory '/home/william/.cache/jhbuild/build/at-spi2-core/bus'
make[2]: Leaving directory '/home/william/.cache/jhbuild/build/at-spi2-core/bus'
Making all in registryd
make[2]: Entering directory '/home/william/.cache/jhbuild/build/at-spi2-core/registryd'
  CC       at_spi2_registryd-event-source.o
/home/william/Code/jhbuild/checkout/at-spi2-core/registryd/event-source.c:31:3: error: unknown type name ‘Display’
   Display *display;
   ^~~~~~~
/home/william/Code/jhbuild/checkout/at-spi2-core/registryd/event-source.c:37:37: error: unknown type name ‘XEvent’
 static void (*_spi_default_filter) (XEvent*, void*) = NULL;
                                     ^~~~~~
/home/william/Code/jhbuild/checkout/at-spi2-core/registryd/event-source.c: In function ‘event_prepare’:
/home/william/Code/jhbuild/checkout/at-spi2-core/registryd/event-source.c:45:3: error: unknown type name ‘Display’
   Display *display = ((DisplaySource *)source)->display;
   ^~~~~~~
/home/william/Code/jhbuild/checkout/at-spi2-core/registryd/event-source.c:49:12: error: implicit declaration of function ‘XPending’ [-Werror=implicit-function-declaration]
   retval = XPending (display);
            ^~~~~~~~
/home/william/Code/jhbuild/checkout/at-spi2-core/registryd/event-source.c: In function ‘event_dispatch’:
/home/william/Code/jhbuild/checkout/at-spi2-core/registryd/event-source.c:71:3: error: unknown type name ‘Display’
   Display *display = ((DisplaySource*)source)->display;
   ^~~~~~~
/home/william/Code/jhbuild/checkout/at-spi2-core/registryd/event-source.c:72:3: error: unknown type name ‘XEvent’
   XEvent xevent;
   ^~~~~~
/home/william/Code/jhbuild/checkout/at-spi2-core/registryd/event-source.c:80:7: error: implicit declaration of function ‘XNextEvent’ [-Werror=implicit-function-declaration]
       XNextEvent (display, &xevent);
       ^~~~~~~~~~
/home/william/Code/jhbuild/checkout/at-spi2-core/registryd/event-source.c:82:21: error: request for member ‘type’ in something not a structure or union
       switch (xevent.type)
                     ^
/home/william/Code/jhbuild/checkout/at-spi2-core/registryd/event-source.c:84:7: error: ‘KeyPress’ undeclared (first use in this function)
  case KeyPress:
       ^~~~~~~~
/home/william/Code/jhbuild/checkout/at-spi2-core/registryd/event-source.c:84:7: note: each undeclared identifier is reported only once for each function it appears in
/home/william/Code/jhbuild/checkout/at-spi2-core/registryd/event-source.c:85:7: error: ‘KeyRelease’ undeclared (first use in this function)
  case KeyRelease:
       ^~~~~~~~~~
/home/william/Code/jhbuild/checkout/at-spi2-core/registryd/event-source.c:88:8: error: implicit declaration of function ‘XFilterEvent’ [-Werror=implicit-function-declaration]
    if (XFilterEvent (&xevent, None))
        ^~~~~~~~~~~~
/home/william/Code/jhbuild/checkout/at-spi2-core/registryd/event-source.c:88:31: error: ‘None’ undeclared (first use in this function)
    if (XFilterEvent (&xevent, None))
                               ^~~~
/home/william/Code/jhbuild/checkout/at-spi2-core/registryd/event-source.c:92:11: error: ‘_spi_default_filter’ undeclared (first use in this function)
       if (_spi_default_filter)
           ^~~~~~~~~~~~~~~~~~~
/home/william/Code/jhbuild/checkout/at-spi2-core/registryd/event-source.c:94:11: error: implicit declaration of function ‘_spi_default_filter’ [-Werror=implicit-function-declaration]
           _spi_default_filter (&xevent, _spi_default_filter_data);
           ^~~~~~~~~~~~~~~~~~~
/home/william/Code/jhbuild/checkout/at-spi2-core/registryd/event-source.c: At top level:
/home/william/Code/jhbuild/checkout/at-spi2-core/registryd/event-source.c:111:21: error: unknown type name ‘Display’
 display_source_new (Display *display)
                     ^~~~~~~
/home/william/Code/jhbuild/checkout/at-spi2-core/registryd/event-source.c:127:18: error: unknown type name ‘Display’
 spi_events_init (Display *display)
                  ^~~~~~~
/home/william/Code/jhbuild/checkout/at-spi2-core/registryd/event-source.c: In function ‘spi_set_events’:
/home/william/Code/jhbuild/checkout/at-spi2-core/registryd/event-source.c:160:22: error: ‘StructureNotifyMask’ undeclared (first use in this function)
   long xevent_mask = StructureNotifyMask | PropertyChangeMask;
                      ^~~~~~~~~~~~~~~~~~~
/home/william/Code/jhbuild/checkout/at-spi2-core/registryd/event-source.c:160:44: error: ‘PropertyChangeMask’ undeclared (first use in this function)
   long xevent_mask = StructureNotifyMask | PropertyChangeMask;
                                            ^~~~~~~~~~~~~~~~~~
/home/william/Code/jhbuild/checkout/at-spi2-core/registryd/event-source.c:163:3: error: implicit declaration of function ‘XSelectInput’ [-Werror=implicit-function-declaration]
   XSelectInput (spi_display_source->display,
   ^~~~~~~~~~~~
/home/william/Code/jhbuild/checkout/at-spi2-core/registryd/event-source.c:164:17: error: implicit declaration of function ‘DefaultRootWindow’ [-Werror=implicit-function-declaration]
                 DefaultRootWindow (spi_display_source->display),
                 ^~~~~~~~~~~~~~~~~
/home/william/Code/jhbuild/checkout/at-spi2-core/registryd/event-source.c: At top level:
/home/william/Code/jhbuild/checkout/at-spi2-core/registryd/event-source.c:169:33: error: unknown type name ‘XEvent’
 spi_set_filter (void (*filter) (XEvent*, void*), void* data)
                                 ^~~~~~
/home/william/Code/jhbuild/checkout/at-spi2-core/registryd/event-source.c:169:50: error: expected ‘;’, ‘,’ or ‘)’ before ‘void’
 spi_set_filter (void (*filter) (XEvent*, void*), void* data)
                                                  ^~~~
cc1: some warnings being treated as errors
Makefile:708: recipe for target 'at_spi2_registryd-event-source.o' failed
make[2]: *** [at_spi2_registryd-event-source.o] Error 1
make[2]: Leaving directory '/home/william/.cache/jhbuild/build/at-spi2-core/registryd'
Makefile:543: recipe for target 'all-recursive' failed
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory '/home/william/.cache/jhbuild/build/at-spi2-core'
Makefile:450: recipe for target 'all' failed
make: *** [all] Error 2
*** Error during phase build of at-spi2-core: ########## Error running make -j 5  *** [12/18]

  [1] Rerun phase build
  [2] Ignore error and continue to install
  [3] Give up on module
  [4] Start shell
  [5] Reload configuration
  [6] Go to phase "wipe directory and start over"
  [7] Go to phase "configure"
  [8] Go to phase "clean"
  [9] Go to phase "distclean"
choice:
Comment 8 Philip Chimento 2016-11-18 06:18:43 UTC
Created attachment 340222 [details] [review]
registryd/display: Include config.h before checking HAVE_X11
Comment 9 Philip Chimento 2016-11-18 06:19:12 UTC
You probably need it in registry/display.h as well
Comment 10 Emmanuele Bassi (:ebassi) 2016-11-18 10:48:51 UTC
Not really.

This is a case for conditional compilation: display.[ch] should only be built (and included) when X11 is available.
Comment 11 Emmanuele Bassi (:ebassi) 2016-11-18 11:11:55 UTC
I've just checked, and the "no X11" case is *highly* suspicious. This looks like it has never been tested.

Why is at-spi2-core built on !X11?

Personally, I'd revert 66ac36c66a420f497262e37f5553d6fea5635454 and 27e7b242d65a78d40d68f20ac375929914c3b7a8 and figure out how to avoid building at-spi2-core entirely on non-X11.
Comment 12 Emmanuele Bassi (:ebassi) 2016-11-18 11:16:16 UTC
Created attachment 340228 [details] [review]
Revert "Block out X11 API with #ifdefs"

This reverts commit 27e7b242d65a78d40d68f20ac375929914c3b7a8.

The build breaks with X11 support enabled.
Comment 13 Emmanuele Bassi (:ebassi) 2016-11-18 11:16:21 UTC
Created attachment 340229 [details] [review]
Ensure that X11 symbols are only used when necessary

Since at-spi2-core can be build with X11 support disabled, we need to
ensure that we include and build the X11-specific bits only when needed.
Comment 14 Emmanuele Bassi (:ebassi) 2016-11-18 11:18:25 UTC
I've reverted only commit 27e7b242 and then added a commit on top that ensures the build for both the --disable-X11 and normal cases.

In reality, though, at-spi2-core's code base is a disaster area that should be nuked from orbit.
Comment 15 Emmanuele Bassi (:ebassi) 2016-11-18 11:20:40 UTC
Review of attachment 340222 [details] [review]:

display.h can only be built with X11 enabled, so this is wholly unnecessary.
Comment 16 Emmanuele Bassi (:ebassi) 2016-11-18 11:21:15 UTC
Review of attachment 340125 [details] [review]:

event-source.h contains only X11 API, so it should not be even included if X11 is disabled.
Comment 17 Mike Gorse 2016-11-19 09:19:17 UTC
Attachment 340228 [details] pushed as commit e6ec6d
Attachment 340229 [details] pushed as commit 0f3d97
Comment 18 Mike Gorse 2016-11-19 09:23:27 UTC
(In reply to William Hua from comment #7)
> There still seems to be some X11 usage that wasn't properly #ifdef'd out:

Do you still get these errors after updating?
event-source.c shouoldn't be compiled in if --disable-x11 is passed to configure.
Comment 19 fakey 2016-11-20 06:53:22 UTC
Thanks all, it builds properly now (forgot to mention I'm actually building *with* X11).
Comment 20 Philip Chimento 2016-11-27 20:16:39 UTC
This seems to have fixed it for me as well, so I'll go ahead and close it.