Re: [PATCH xserver 2/3] meson: Distribute more SDK headers

2018-04-04 Thread Adam Jackson
On Tue, 2018-04-03 at 12:16 +0200, Thierry Reding wrote:

> Yeah, looking more closely at the automake file it has XORG guards
> around all of the SDK headers. There's also some inconsistencies there
> with regards to which files are included depending on other extensions
> being enabled. For example, xvdix.h and xvmcext.h are unconditionally
> installed, even if Xv is not enabled. That doesn't make much sense to
> me, but maybe it is something that we should keep around for the Meson
> build? Or do we want to "clean" this up while at it and only distribute
> the SDK headers that really make sense?

The SDK shouldn't install anything conditionally if it's being
installed at all, it's a description of every interface the server
might have (as well as xorg-server.h to say exactly which sets you can
expect on the runtime you're building against). Granted it _does_
install many things conditionally, but I think that's an error.

- 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] meson: Distribute more SDK headers

2018-04-03 Thread Thierry Reding
On Fri, Mar 30, 2018 at 09:39:29AM -0700, Aaron Plattner wrote:
> Thanks, Thierry! I started working on a change to do this, but didn't
> get very far before you beat me to it.
> 
> On 03/29/2018 04:07 AM, Thierry Reding wrote:
> > From: Thierry Reding 
> > 
> > Install missing headers to the SDK directory to allow external modules
> > to properly build against the SDK. After this commit, the list of files
> > installed in the SDK include directory is the same as the list of files
> > installed by the autotools-based build.
> > 
> > Signed-off-by: Thierry Reding 
> > ---
> >  Xext/meson.build  | 12 
> >  composite/meson.build |  6 ++
> >  dbe/meson.build   |  6 ++
> >  dri3/meson.build  |  6 ++
> >  fb/meson.build| 10 ++
> >  glx/meson.build   |  6 ++
> >  hw/xfree86/os-support/meson.build |  9 -
> >  include/meson.build   |  1 +
> >  mi/meson.build| 15 +++
> >  miext/damage/meson.build  |  7 +++
> >  miext/shadow/meson.build  |  6 ++
> >  miext/sync/meson.build|  9 +
> >  present/meson.build   |  7 +++
> >  randr/meson.build |  7 +++
> >  render/meson.build|  9 +
> >  15 files changed, 115 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Xext/meson.build b/Xext/meson.build
> > index 9968f2a9e312..a7217371871d 100644
> > --- a/Xext/meson.build
> > +++ b/Xext/meson.build
> > @@ -8,12 +8,19 @@ srcs_xext = [
> >  'xtest.c',
> >  ]
> >  
> > +hdrs_xext = [
> > +'geext.h',
> > +'geint.h',
> > +'syncsdk.h',
> > +]
> > +
> >  if build_dpms
> >  srcs_xext += 'dpms.c'
> >  endif
> >  
> >  if build_mitshm
> >  srcs_xext += 'shm.c'
> > +hdrs_xext += ['shmint.h']
> >  endif
> >  
> >  if build_res
> > @@ -26,6 +33,7 @@ endif
> >  
> >  if build_xace
> >  srcs_xext += 'xace.c'
> > +hdrs_xext += ['xace.h', 'xacestr.h']
> >  endif
> >  
> >  if build_xf86bigfont
> > @@ -34,6 +42,7 @@ endif
> >  
> >  if build_xinerama
> >  srcs_xext += ['panoramiX.c', 'panoramiXprocs.c', 'panoramiXSwap.c']
> > +hdrs_xext += ['panoramiX.h', 'panoramiXsrv.h']
> >  endif
> >  
> >  if build_xsecurity
> > @@ -46,6 +55,7 @@ endif
> >  
> >  if build_xv
> >  srcs_xext += ['xvmain.c', 'xvdisp.c', 'xvmc.c']
> > +hdrs_xext += ['xvdix.h', 'xvmcext.h']
> >  endif
> >  
> >  libxserver_xext = static_library('libxserver_xext',
> > @@ -59,3 +69,5 @@ libxserver_xext_vidmode = 
> > static_library('libxserver_xext_vidmode',
> >  include_directories: inc,
> >  dependencies: common_dep,
> >  )
> > +
> > +install_data(hdrs_xext, install_dir: xorgsdkdir)
> 
> Do these new install_data() directives need to be behind an 'if
> build_xorg'? It looks like the other two instances of this that weren't
> behind the build_xorg check that guards subdir('xfree86') in
> hw/meson.build had their own build_xorg checks.
> 
> Prior to this change, building with "meson configure -Dxorg=false" only
> installs xorg-server.h to $prefix/include/xorg.

Yeah, looking more closely at the automake file it has XORG guards
around all of the SDK headers. There's also some inconsistencies there
with regards to which files are included depending on other extensions
being enabled. For example, xvdix.h and xvmcext.h are unconditionally
installed, even if Xv is not enabled. That doesn't make much sense to
me, but maybe it is something that we should keep around for the Meson
build? Or do we want to "clean" this up while at it and only distribute
the SDK headers that really make sense?

[...]
> > diff --git a/hw/xfree86/os-support/meson.build 
> > b/hw/xfree86/os-support/meson.build
> > index 2b96e7e4adee..901422786586 100644
> > --- a/hw/xfree86/os-support/meson.build
> > +++ b/hw/xfree86/os-support/meson.build
> > @@ -4,6 +4,13 @@ srcs_xorg_os_support = [
> >  'shared/sigio.c',
> >  'shared/vidmem.c',
> >  ]
> > +
> > +hdrs_xorg_os_support = [
> > +'bus/xf86Pci.h',
> 
> Should this one go inside the if get_option('pciaccess') block below?

That would probably make sense, but it would deviate from the autotools
build, which installs this file unconditionally.


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] meson: Distribute more SDK headers

2018-03-30 Thread Aaron Plattner
Thanks, Thierry! I started working on a change to do this, but didn't
get very far before you beat me to it.

On 03/29/2018 04:07 AM, Thierry Reding wrote:
> From: Thierry Reding 
> 
> Install missing headers to the SDK directory to allow external modules
> to properly build against the SDK. After this commit, the list of files
> installed in the SDK include directory is the same as the list of files
> installed by the autotools-based build.
> 
> Signed-off-by: Thierry Reding 
> ---
>  Xext/meson.build  | 12 
>  composite/meson.build |  6 ++
>  dbe/meson.build   |  6 ++
>  dri3/meson.build  |  6 ++
>  fb/meson.build| 10 ++
>  glx/meson.build   |  6 ++
>  hw/xfree86/os-support/meson.build |  9 -
>  include/meson.build   |  1 +
>  mi/meson.build| 15 +++
>  miext/damage/meson.build  |  7 +++
>  miext/shadow/meson.build  |  6 ++
>  miext/sync/meson.build|  9 +
>  present/meson.build   |  7 +++
>  randr/meson.build |  7 +++
>  render/meson.build|  9 +
>  15 files changed, 115 insertions(+), 1 deletion(-)
> 
> diff --git a/Xext/meson.build b/Xext/meson.build
> index 9968f2a9e312..a7217371871d 100644
> --- a/Xext/meson.build
> +++ b/Xext/meson.build
> @@ -8,12 +8,19 @@ srcs_xext = [
>  'xtest.c',
>  ]
>  
> +hdrs_xext = [
> +'geext.h',
> +'geint.h',
> +'syncsdk.h',
> +]
> +
>  if build_dpms
>  srcs_xext += 'dpms.c'
>  endif
>  
>  if build_mitshm
>  srcs_xext += 'shm.c'
> +hdrs_xext += ['shmint.h']
>  endif
>  
>  if build_res
> @@ -26,6 +33,7 @@ endif
>  
>  if build_xace
>  srcs_xext += 'xace.c'
> +hdrs_xext += ['xace.h', 'xacestr.h']
>  endif
>  
>  if build_xf86bigfont
> @@ -34,6 +42,7 @@ endif
>  
>  if build_xinerama
>  srcs_xext += ['panoramiX.c', 'panoramiXprocs.c', 'panoramiXSwap.c']
> +hdrs_xext += ['panoramiX.h', 'panoramiXsrv.h']
>  endif
>  
>  if build_xsecurity
> @@ -46,6 +55,7 @@ endif
>  
>  if build_xv
>  srcs_xext += ['xvmain.c', 'xvdisp.c', 'xvmc.c']
> +hdrs_xext += ['xvdix.h', 'xvmcext.h']
>  endif
>  
>  libxserver_xext = static_library('libxserver_xext',
> @@ -59,3 +69,5 @@ libxserver_xext_vidmode = 
> static_library('libxserver_xext_vidmode',
>  include_directories: inc,
>  dependencies: common_dep,
>  )
> +
> +install_data(hdrs_xext, install_dir: xorgsdkdir)

Do these new install_data() directives need to be behind an 'if
build_xorg'? It looks like the other two instances of this that weren't
behind the build_xorg check that guards subdir('xfree86') in
hw/meson.build had their own build_xorg checks.

Prior to this change, building with "meson configure -Dxorg=false" only
installs xorg-server.h to $prefix/include/xorg.

> diff --git a/composite/meson.build b/composite/meson.build
> index 6c4a03fb80c2..7547f0e7edce 100644
> --- a/composite/meson.build
> +++ b/composite/meson.build
> @@ -6,8 +6,14 @@ srcs_composite = [
>   'compwindow.c',
>  ]
>  
> +hdrs_composite = [
> + 'compositeext.h',
> +]
> +
>  libxserver_composite = static_library('libxserver_composite',
>   srcs_composite,
>   include_directories: inc,
>   dependencies: common_dep,
>  )
> +
> +install_data(hdrs_composite, install_dir: xorgsdkdir)
> diff --git a/dbe/meson.build b/dbe/meson.build
> index e10bde19913d..76a2d3f85d2b 100644
> --- a/dbe/meson.build
> +++ b/dbe/meson.build
> @@ -3,8 +3,14 @@ srcs_dbe = [
>   'midbe.c',
>  ]
>  
> +hdrs_dbe = [
> + 'dbestruct.h',
> +]
> +
>  libxserver_dbe = static_library('libxserver_dbe',
>   srcs_dbe,
>   include_directories: inc,
>   dependencies: common_dep,
>  )
> +
> +install_data(hdrs_dbe, install_dir: xorgsdkdir)
> diff --git a/dri3/meson.build b/dri3/meson.build
> index 0deec32aafbe..48ce0d9d6aa1 100644
> --- a/dri3/meson.build
> +++ b/dri3/meson.build
> @@ -4,6 +4,10 @@ srcs_dri3 = [
>   'dri3_screen.c',
>  ]
>  
> +hdrs_dri3 = [
> + 'dri3.h',
> +]
> +
>  libxserver_dri3 = []
>  if build_dri3
>  libxserver_dri3 = static_library('libxserver_dri3',
> @@ -13,3 +17,5 @@ if build_dri3
>  c_args: '-DHAVE_XORG_CONFIG_H'
>  )
>  endif
> +
> +install_data(hdrs_dri3, install_dir: xorgsdkdir)
> diff --git a/fb/meson.build b/fb/meson.build
> index bf85141f980f..477ab047dfd6 100644
> --- a/fb/meson.build
> +++ b/fb/meson.build
> @@ -28,6 +28,14 @@ srcs_fb = [
>   'fbwindow.c',
>  ]
>  
> +hdrs_fb = [
> + 'fb.h',
> + 'fboverlay.h',
> + 'fbpict.h',
> + 'fbrop.h',
> + 'wfbrename.h'
> +]
> +
>  libxserver_fb = static_library('libxserver_fb',
>   srcs_fb,
>   include_directories: inc,
> @@ -45,3 +53,5 @@ libxserver_wfb = static_library('libxserver_wfb',
>   pic: true,
>   build_by_default: false,
>  )
> +
> +install_data(hdrs_fb, install_dir: xorgsd