Re: [PATCH xserver 0/3] meson build for xserver

[Adam Jackson]

On Tue, 2017-04-25 at 19:49 -0700, Keith Packard wrote:
> Eric Anholt  writes:
> 
> > Eric Anholt (3):
> >   dix: Remove a redundant #define
> >   Use #ifdef instead of #if for features to make Meson easier.
> 
> Reviewed-by: Keith Packard 
> 
> >   Add a Meson build system alongside autotools.
> 
> Acked-by: Keith Packard 
> 
> I'm not going to even pretend I have any idea if this is right...

Merged the first two:

remote: I: patch #152804 updated using rev 
ace6bfd5901e19045371f9bd18b997b37f55b589.
remote: I: patch #152803 updated using rev 
c7be7a688a78a34f61b90c0d95914e14b90b0cdc.
remote: I: 2 patch(es) updated to state Accepted.
To ssh://git.freedesktop.org/git/xorg/xserver
   27a6b9f7c..c7be7a688  master -> master

Will take a look at 3/3 shortly.

- ajax
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver v2] os: Handle SIGABRT

[Adam Jackson]

On Wed, 2017-04-26 at 18:31 +0900, Michel Dänzer wrote:
> From: Michel Dänzer 
> 
> Without this, assertion failures can make life hard for users and
> those
> trying to help them.
> 
> v2:
> * Change commit log wording slightly to "can make life hard", since
>   apparently e.g. logind can alleviate that somewhat.
> * Set default handler for SIGABRT in
>   hw/xfree86/common/xf86Init.c:InstallSignalHandlers() and
>   hw/xquartz/quartz.c:QuartzInitOutput() (Eric Anholt)
> 
> Reviewed-by: Eric Anholt 
> Signed-off-by: Michel Dänzer 

remote: I: patch #152880 updated using rev 
27a6b9f7c84c914d0f5909ec1069d72f5035bc04.
remote: I: 1 patch(es) updated to state Accepted.
To ssh://git.freedesktop.org/git/xorg/xserver
   23f2f1932..27a6b9f7c  master -> master

- ajax
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver 2/3] Use #ifdef instead of #if for features to make Meson easier.

[Keith Packard]

Aaron Plattner  writes:

> Waiting until autotools is nuked sounds fine to me. I just wanted to
> throw the idea of using -Wundef out there.

me too.

-- 
-keith


signature.asc
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver 2/3] Use #ifdef instead of #if for features to make Meson easier.

[Aaron Plattner]

On 04/26/2017 11:21 AM, Eric Anholt wrote:
> Aaron Plattner  writes:
> 
>> We try to do exactly the opposite in our internal driver build, because
>> it's too easy to accidentally do something like
>>
>> #ifdef GLAMOUR_HAS_GBM
>>
>> And mistakes like that don't always cause obvious build failures like
>> this would. So we build everything with -Wundef -Werror=undef and try to
>> use #if whenever possible. It's a shame that Meson makes that hard.
> 
> Meson makes what you want easy with:
> 
> set10(varname, boolean_value) is the same as above but the value is
> either true or false and will be written as 1 or 0, respectively
> 
> but autotools usually produces either #define 1 or #undef, unless you go
> through contortions.  This seemed like the easy, consistent solution
> today, given that our codebase is almost all #ifdef.  One alternative
> would be to just disable -Wundef for now.  The other alternative to this
> patch would be something like:
> 
> conf_array += ['HAVE_DBM_H', cc.has_header('dbm.h')]
> 
> then finish with something like:
> 
> foreach conf: conf_array
> if conf[1]
> conf_data.set(conf[0], '1')
> endif
> endforeach
> 
> but I think I'd rather wait and consistently swap to .set10() and #if
> once we've nuked autotools.

Waiting until autotools is nuked sounds fine to me. I just wanted to
throw the idea of using -Wundef out there.

-- Aaron
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver 2/3] Use #ifdef instead of #if for features to make Meson easier.

[Eric Anholt]

Aaron Plattner  writes:

> We try to do exactly the opposite in our internal driver build, because
> it's too easy to accidentally do something like
>
> #ifdef GLAMOUR_HAS_GBM
>
> And mistakes like that don't always cause obvious build failures like
> this would. So we build everything with -Wundef -Werror=undef and try to
> use #if whenever possible. It's a shame that Meson makes that hard.

Meson makes what you want easy with:

set10(varname, boolean_value) is the same as above but the value is
either true or false and will be written as 1 or 0, respectively

but autotools usually produces either #define 1 or #undef, unless you go
through contortions.  This seemed like the easy, consistent solution
today, given that our codebase is almost all #ifdef.  One alternative
would be to just disable -Wundef for now.  The other alternative to this
patch would be something like:

conf_array += ['HAVE_DBM_H', cc.has_header('dbm.h')]

then finish with something like:

foreach conf: conf_array
if conf[1]
conf_data.set(conf[0], '1')
endif
endforeach

but I think I'd rather wait and consistently swap to .set10() and #if
once we've nuked autotools.


signature.asc
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver 2/3] Use #ifdef instead of #if for features to make Meson easier.

[Aaron Plattner]

We try to do exactly the opposite in our internal driver build, because
it's too easy to accidentally do something like

#ifdef GLAMOUR_HAS_GBM

And mistakes like that don't always cause obvious build failures like
this would. So we build everything with -Wundef -Werror=undef and try to
use #if whenever possible. It's a shame that Meson makes that hard.

On 04/25/2017 04:03 PM, Eric Anholt wrote:
> We mostly use #ifdef throughout the tree, and this lets the generated
> config.h files just be #define TOKEN instead of #define TOKEN 1.
> 
> Signed-off-by: Eric Anholt 
> ---
>  glamor/glamor_priv.h|  4 ++--
>  hw/xfree86/common/xf86.h|  2 +-
>  hw/xfree86/drivers/modesetting/driver.c | 16 
>  hw/xfree86/loader/loadmod.c |  4 ++--
>  hw/xfree86/sdksyms.sh   |  8 
>  hw/xwayland/xwayland.c  |  2 +-
>  include/dixstruct.h |  2 +-
>  include/os.h|  2 +-
>  include/xserver_poll.h  |  2 +-
>  os/utils.c  | 16 
>  10 files changed, 29 insertions(+), 29 deletions(-)
> 
> diff --git a/glamor/glamor_priv.h b/glamor/glamor_priv.h
> index 7b92f35705f4..a90879a7bbfd 100644
> --- a/glamor/glamor_priv.h
> +++ b/glamor/glamor_priv.h
> @@ -38,7 +38,7 @@
>  #endif
>  
>  #include 
> -#if GLAMOR_HAS_GBM
> +#ifdef GLAMOR_HAS_GBM
>  #define MESA_EGL_NO_X11_HEADERS
>  #include 
>  #endif
> @@ -342,7 +342,7 @@ typedef struct glamor_pixmap_private {
>  GLuint pbo;
>  RegionRec prepare_region;
>  Bool prepared;
> -#if GLAMOR_HAS_GBM
> +#ifdef GLAMOR_HAS_GBM
>  EGLImageKHR image;
>  #endif
>  /** block width of this large pixmap. */
> diff --git a/hw/xfree86/common/xf86.h b/hw/xfree86/common/xf86.h
> index f1a5bd6741bd..877b9e9e768e 100644
> --- a/hw/xfree86/common/xf86.h
> +++ b/hw/xfree86/common/xf86.h
> @@ -35,7 +35,7 @@
>  #ifndef _XF86_H
>  #define _XF86_H
>  
> -#if HAVE_XORG_CONFIG_H
> +#ifdef HAVE_XORG_CONFIG_H
>  #include 
>  #elif HAVE_DIX_CONFIG_H
>  #include 
> diff --git a/hw/xfree86/drivers/modesetting/driver.c 
> b/hw/xfree86/drivers/modesetting/driver.c
> index d7030e5c2117..a1451fe471e8 100644
> --- a/hw/xfree86/drivers/modesetting/driver.c
> +++ b/hw/xfree86/drivers/modesetting/driver.c
> @@ -55,7 +55,7 @@
>  #ifdef XSERVER_PLATFORM_BUS
>  #include "xf86platformBus.h"
>  #endif
> -#if XSERVER_LIBPCIACCESS
> +#ifdef XSERVER_LIBPCIACCESS
>  #include 
>  #endif
>  
> @@ -227,7 +227,7 @@ check_outputs(int fd, int *count)
>  *count = res->count_connectors;
>  
>  ret = res->count_connectors > 0;
> -#if defined DRM_CAP_PRIME && GLAMOR_HAS_GBM_LINEAR
> +#if defined(DRM_CAP_PRIME) && defined(GLAMOR_HAS_GBM_LINEAR)
>  if (ret == FALSE) {
>  uint64_t value = 0;
>  if (drmGetCap(fd, DRM_CAP_PRIME, ) == 0 &&
> @@ -244,7 +244,7 @@ probe_hw(const char *dev, struct xf86_platform_device 
> *platform_dev)
>  {
>  int fd;
>  
> -#if XF86_PDEV_SERVER_FD
> +#ifdef XF86_PDEV_SERVER_FD
>  if (platform_dev && (platform_dev->flags & XF86_PDEV_SERVER_FD)) {
>  fd = xf86_platform_device_odev_attributes(platform_dev)->fd;
>  if (fd == -1)
> @@ -366,7 +366,7 @@ ms_setup_entity(ScrnInfoPtr scrn, int entity_num)
>  pPriv->ptr = xnfcalloc(sizeof(modesettingEntRec), 1);
>  }
>  
> -#if XSERVER_LIBPCIACCESS
> +#ifdef XSERVER_LIBPCIACCESS
>  static Bool
>  ms_pci_probe(DriverPtr driver,
>   int entity_num, struct pci_device *dev, intptr_t match_data)
> @@ -826,7 +826,7 @@ ms_get_drm_master_fd(ScrnInfoPtr pScrn)
>  return TRUE;
>  }
>  
> -#if XSERVER_PLATFORM_BUS
> +#ifdef XSERVER_PLATFORM_BUS
>  if (pEnt->location.type == BUS_PLATFORM) {
>  #ifdef XF86_PDEV_SERVER_FD
>  if (pEnt->location.id.plat->flags & XF86_PDEV_SERVER_FD)
> @@ -844,7 +844,7 @@ ms_get_drm_master_fd(ScrnInfoPtr pScrn)
>  }
>  else
>  #endif
> -#if XSERVER_LIBPCIACCESS
> +#ifdef XSERVER_LIBPCIACCESS
>  if (pEnt->location.type == BUS_PCI) {
>  char *BusID = NULL;
>  struct pci_device *PciInfo;
> @@ -1018,7 +1018,7 @@ PreInit(ScrnInfoPtr pScrn, int flags)
>  if (ms->drmmode.glamor)
>  pScrn->capabilities |= RR_Capability_SinkOffload;
>  }
> -#if GLAMOR_HAS_GBM_LINEAR
> +#ifdef GLAMOR_HAS_GBM_LINEAR
>  if (value & DRM_PRIME_CAP_EXPORT && ms->drmmode.glamor)
>  pScrn->capabilities |= RR_Capability_SourceOutput | 
> RR_Capability_SourceOffload;
>  #endif
> @@ -1189,7 +1189,7 @@ msEnableSharedPixmapFlipping(RRCrtcPtr crtc, PixmapPtr 
> front, PixmapPtr back)
>  if (ms->drmmode.reverse_prime_offload_mode)
>  return FALSE;
>  
> -#if XSERVER_PLATFORM_BUS
> +#ifdef XSERVER_PLATFORM_BUS
>  if (pEnt->location.type == BUS_PLATFORM) {
>  char *syspath =
>  xf86_platform_device_odev_attributes(pEnt->location.id.plat)->
> diff --git 

Re: [PATCH xserver 3/3] Add a Meson build system alongside autotools.

[Jon Turney]

On 26/04/2017 00:03, Eric Anholt wrote:

This is a work in progress that builds Xvfb, Xephyr, Xwayland, Xnest,
and Xdmx so far.  The outline of Xquartz/Xwin support is in tree, but
hasn't been built yet.  The unit tests are also not done.


man pages and documentation are also not done.

I've more or less written XWin, so don't feel the need to do that :)


diff --git a/glx/meson.build b/glx/meson.build

[...]

+libxserver_glx = ''
+if build_glx
+libxserver_glx = static_library('libxserver_glx',


This doesn't seem to be right.  If you mesonconf with -Dglx=false, this 
causes "Link target '' is not library." when it appears in link_with:.


Attached is a patch with a few small fixes I made.

Adding project(..., meson_version: '>0.40') seems like a good idea as well.

From f7e55ca8051242de2d1782cbf85cda87b0a8f123 Mon Sep 17 00:00:00 2001
From: Jon Turney 
Date: Tue, 18 Apr 2017 13:37:35 +0100
Subject: [PATCH xserver 1/6] Various fixes and tweaks to meson.build

Restore stub ossupport
Make xf86vidmode optional, set XF86VIDMODE
Make DGA optional, always disabled
Nettle is required if it's the only SHA1 choice
Test to build xserver_poll.c was inverted
Typos
---
 hw/xfree86/os-support/meson.build | 17 -
 include/meson.build   |  4 ++--
 meson.build   |  7 ---
 meson_options.txt |  2 +-
 os/meson.build|  4 ++--
 5 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/hw/xfree86/os-support/meson.build 
b/hw/xfree86/os-support/meson.build
index a3c3b5688..4cef618b0 100644
--- a/hw/xfree86/os-support/meson.build
+++ b/hw/xfree86/os-support/meson.build
@@ -73,7 +73,7 @@ elif host_machine.system() == 'solaris'
 endif
 
 os_support_flags += '-DHAVE_SYSV_IPC'
-else
+elif (host_machine.system() == 'freebsd') or (host_machine.system() == 
'netbsd') or (host_machine.system() == 'openbsd')
 srcs_xorg_os_support += [
 'bsd/bsd_VTsw.c',
 'bsd/bsd_bell.c',
@@ -109,6 +109,21 @@ else
 else
 srcs_xorg_os_support += 'shared/agp_noop.c'
 endif
+else
+# stub ossupport
+srcs_xorg_os_support += [
+'shared/VTsw_noop.c',
+'shared/agp_noop.c',
+'shared/ioperm_noop.c',
+'shared/kmod_noop.c',
+'shared/pm_noop.c',
+'shared/vidmem.c',
+'shared/posix_tty.c',
+'shared/sigio.c',
+'stub/stub_bell.c',
+'stub/stub_init.c',
+'stub/stub_video.c',
+]
 endif
 
 xorg_os_support = static_library('xorg_os_support',
diff --git a/include/meson.build b/include/meson.build
index 75469ff0a..e066fc469 100644
--- a/include/meson.build
+++ b/include/meson.build
@@ -161,8 +161,8 @@ conf_data.set('XCSECURITY', build_xsecurity)
 conf_data.set('XDMCP', get_option('xdmcp'))
 conf_data.set('XF86BIGFONT', build_xf86bigfont)
 conf_data.set('XF86DRI', build_dri1)
-conf_data.set('XF86VIDMODE', '1')
-conf_data.set('XFIXES   ', '1')
+conf_data.set('XF86VIDMODE', build_xf86vidmode)
+conf_data.set('XFIXES', '1')
 conf_data.set('XINERAMA', build_xinerama)
 conf_data.set('XINPUT', '1')
 conf_data.set('XRECORD', '1')
diff --git a/meson.build b/meson.build
index 30103f8d6..3955ad0bd 100644
--- a/meson.build
+++ b/meson.build
@@ -35,8 +35,8 @@ dri2proto_dep = dependency('dri2proto', version: '>= 2.8')
 dri3proto_dep = dependency('dri3proto', version: '>= 1.0')
 xineramaproto_dep = dependency('xineramaproto')
 xf86bigfontproto_dep = dependency('xf86bigfontproto', version: '>= 1.2.0')
-xf86dgaproto_dep = dependency('xf86dgaproto', version: '>= 2.0.99.1')
-xf86vidmodeproto_dep = dependency('xf86vidmodeproto', version: '>= 2.2.99.1')
+xf86dgaproto_dep = dependency('xf86dgaproto', version: '>= 2.0.99.1', 
required: false)
+xf86vidmodeproto_dep = dependency('xf86vidmodeproto', version: '>= 2.2.99.1', 
required: false)
 windowswmproto_dep = dependency('windowswmproto', required: false)
 applewmproto_dep = dependency('applewmproto', version: '>= 1.4', required: 
false)
 xshmfence_dep = dependency('xshmfence', version: '>= 1.1')
@@ -45,7 +45,7 @@ pixman_dep = dependency('pixman-1')
 libbsd_dep = dependency('libbsd', required: false)
 xkbfile_dep = dependency('xkbfile')
 xfont2_dep = dependency('xfont2', version: '>= 2.0')
-nettle_dep = dependency('nettle', required: false)
+nettle_dep = dependency('nettle')
 
 dbus_required = get_option('systemd_logind') == 'yes'
 dbus_dep = dependency('dbus-1', version: '>= 1.0', required: dbus_required)
@@ -233,6 +233,7 @@ build_xinerama = true
 build_xselinux = false
 build_xv = true
 build_dga = false
+build_xf86vidmode = xf86vidmodeproto_dep.found()
 
 m_dep = cc.find_library('m', required : false)
 dl_dep = cc.find_library('dl', required : false)
diff --git a/meson_options.txt b/meson_options.txt
index b89fdaa2c..a5d7de6f4 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -1,7 +1,7 @@
 option('xorg', type: 'combo', choices: ['yes', 'no', 'auto'], value: 'auto',

[PATCH xserver v2] os: Handle SIGABRT

[Michel Dänzer]

From: Michel Dänzer 

Without this, assertion failures can make life hard for users and those
trying to help them.

v2:
* Change commit log wording slightly to "can make life hard", since
  apparently e.g. logind can alleviate that somewhat.
* Set default handler for SIGABRT in
  hw/xfree86/common/xf86Init.c:InstallSignalHandlers() and
  hw/xquartz/quartz.c:QuartzInitOutput() (Eric Anholt)

Reviewed-by: Eric Anholt 
Signed-off-by: Michel Dänzer 
---

Thanks for the review and suggestions, Eric!

 hw/xfree86/common/xf86Init.c | 1 +
 hw/xquartz/quartz.c  | 1 +
 os/osinit.c  | 1 +
 os/utils.c   | 6 ++
 4 files changed, 9 insertions(+)

diff --git a/hw/xfree86/common/xf86Init.c b/hw/xfree86/common/xf86Init.c
index d3c7c47b0..d0bd6e95b 100644
--- a/hw/xfree86/common/xf86Init.c
+++ b/hw/xfree86/common/xf86Init.c
@@ -309,6 +309,7 @@ InstallSignalHandlers(void)
 }
 else {
 OsSignal(SIGSEGV, SIG_DFL);
+OsSignal(SIGABRT, SIG_DFL);
 OsSignal(SIGILL, SIG_DFL);
 #ifdef SIGEMT
 OsSignal(SIGEMT, SIG_DFL);
diff --git a/hw/xquartz/quartz.c b/hw/xquartz/quartz.c
index c8b6f966d..c8ea3bf8b 100644
--- a/hw/xquartz/quartz.c
+++ b/hw/xquartz/quartz.c
@@ -178,6 +178,7 @@ QuartzInitOutput(int argc,
 {
 /* For XQuartz, we want to just use the default signal handler to work 
better with CrashTracer */
 signal(SIGSEGV, SIG_DFL);
+signal(SIGABRT, SIG_DFL);
 signal(SIGILL, SIG_DFL);
 #ifdef SIGEMT
 signal(SIGEMT, SIG_DFL);
diff --git a/os/osinit.c b/os/osinit.c
index 5b2f6b546..cd769d181 100644
--- a/os/osinit.c
+++ b/os/osinit.c
@@ -173,6 +173,7 @@ OsInit(void)
 int i;
 
 int siglist[] = { SIGSEGV, SIGQUIT, SIGILL, SIGFPE, SIGBUS,
+SIGABRT,
 SIGSYS,
 SIGXCPU,
 SIGXFSZ,
diff --git a/os/utils.c b/os/utils.c
index 3f8bac5c6..226b9c817 100644
--- a/os/utils.c
+++ b/os/utils.c
@@ -1353,6 +1353,12 @@ OsAbort(void)
 #ifndef __APPLE__
 OsBlockSignals();
 #endif
+#if !defined(WIN32) || defined(__CYGWIN__)
+/* abort() raises SIGABRT, so we have to stop handling that to prevent
+ * recursion
+ */
+OsSignal(SIGABRT, SIG_DFL);
+#endif
 abort();
 }
 
-- 
2.11.0

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver v2 2/7] xwayland: Allow pointer warp on the root window

[Olivier Fourdan]

Hey Carlos,

> Of sorts, as we can't honor pointer warping across the whole root window
> coordinates, peek the pointer focus in this case.
> 
> Signed-off-by: Carlos Garnacho 
> ---
>  v2: Check that requester and focus window clients are the same
> 
>  hw/xwayland/xwayland.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/hw/xwayland/xwayland.c b/hw/xwayland/xwayland.c
> index 9e1ecf8..9e24011 100644
> --- a/hw/xwayland/xwayland.c
> +++ b/hw/xwayland/xwayland.c
> @@ -180,6 +180,11 @@ xwl_cursor_warped_to(DeviceIntPtr device,
>  xwl_seat = xwl_screen_get_default_seat(xwl_screen);
>  
>  xwl_window = xwl_window_from_window(window);
> +if (!xwl_window && !window->parent &&
> +client == wClient(xwl_seat->focus_window->window)) {
> +DebugF("Warp on root window, assuming pointer focus\n");
> +xwl_window = xwl_seat->focus_window;
> +}
>  if (!xwl_window)
>  return;

Just so you know, I was evaluating your patches while taking a look at bug 
100740 [1] and noticed they actually crash Xwayland with the reproducer steps 
(basically blender with fly-mode ) [2] - that's because the dest_win 
passed by blender for XIWarpPointer() is None (which is legit), so 
window->parent is a NULL pointer dereference.

Good news though is a small change in your patch can not only fix the crash but 
also fix the issue with blender as well, that's [3].

Only problem is that the client matching part needs to be avoided as in the 
case of blender, the client is blender but the focused surface belongs to 
gnome-shell [4], I am not sure why...

[1] https://bugs.freedesktop.org/show_bug.cgi?id=100740
[2] https://bugs.freedesktop.org/show_bug.cgi?id=100740#c6
[3] https://bugs.freedesktop.org/show_bug.cgi?id=100740#c9
[4] https://bugs.freedesktop.org/show_bug.cgi?id=100740#c10

Cheers,
Olivier


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel