[Mesa-dev] [PATCH 03/18] glx: meson: build src/glx only with -Dglx=dri

[Emil Velikov]

On Fri, 14 Dec 2018 at 17:13, Dylan Baker <dylan at pnwbakers.com> wrote:
>
> Quoting Emil Velikov (2018-12-13 08:05:52)
> > From: Emil Velikov <emil.velikov at collabora.com>
> >
> > The library is the dri capable one, push the check src/meson.build,
> > instead of the current partial handling in src/glx/meson.build.
> >
> > Fixes: a47c525f328 ("meson: build glx")
>
> This is just a refactor, the Fixes: is unnecessary. More on that below
>
If I'm reading things correctly, building with -Dglx=xlib/gallium-xlib
-Dshared-glapi -Denable-tests - will attempt to build the tests in
src/glx/tests/.
Which is something that shouldn't happen IMHO.

With this in mind, do you think we should drop the fixes tag?

> > Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
> > ---
> >  src/glx/meson.build | 32 +++++++++++++++-----------------
> >  src/meson.build     |  2 +-
> >  2 files changed, 16 insertions(+), 18 deletions(-)
> >
> > diff --git a/src/glx/meson.build b/src/glx/meson.build
> > index 3fd74439b11..898ed1f5826 100644
> > --- a/src/glx/meson.build
> > +++ b/src/glx/meson.build
> > @@ -152,23 +152,21 @@ libglx = static_library(
> >    build_by_default : false,
> >  )
> >
> > -if with_glx == 'dri'
> > -  libgl = shared_library(
> > -    gl_lib_name,
> > -    [],
> > -    include_directories : [inc_common, inc_glapi, inc_loader, inc_gl_internal],
> > -    link_with : [libglapi_static, libglapi],
> > -    link_whole : libglx,
> > -    link_args : [ld_args_bsymbolic, ld_args_gc_sections, extra_ld_args_libgl],
> > -    dependencies : [
> > -      dep_libdrm, dep_dl, dep_m, dep_thread, dep_x11, dep_xcb_glx, dep_xcb,
> > -      dep_x11_xcb, dep_xcb_dri2, dep_xext, dep_xfixes, dep_xdamage, dep_xxf86vm,
> > -      extra_deps_libgl,
> > -    ],
> > -    version : gl_lib_version,
> > -    install : true,
> > -  )
> > -endif
> > +libgl = shared_library(
> > +  gl_lib_name,
> > +  [],
> > +  include_directories : [inc_common, inc_glapi, inc_loader, inc_gl_internal],
> > +  link_with : [libglapi_static, libglapi],
> > +  link_whole : libglx,
> > +  link_args : [ld_args_bsymbolic, ld_args_gc_sections, extra_ld_args_libgl],
> > +  dependencies : [
> > +    dep_libdrm, dep_dl, dep_m, dep_thread, dep_x11, dep_xcb_glx, dep_xcb,
> > +    dep_x11_xcb, dep_xcb_dri2, dep_xext, dep_xfixes, dep_xdamage, dep_xxf86vm,
> > +    extra_deps_libgl,
> > +  ],
> > +  version : gl_lib_version,
> > +  install : true,
> > +)
>
> All you're doing is refactoring out the build_by_default into an if statement.
> If you're going to do this, please remove `build_by_default : false` from the
> libglx in src/glx.
>
Sure thing.

> With those changes:
> Reviewed-by: Dylan Baker <dylan at pnwbakers.com>
>

Thanks.
Emil