Re: [Mesa-dev] [PATCH 03/18] glx: meson: build src/glx only with -Dglx=dri
On Tue, 18 Dec 2018 at 21:52, Dylan Baker wrote:
>
> Quoting Emil Velikov (2018-12-18 02:33:56)
> > On Mon, 17 Dec 2018 at 19:44, Dylan Baker wrote:
> > >
> > > Quoting Emil Velikov (2018-12-17 10:58:11)
> > > > On Fri, 14 Dec 2018 at 17:13, Dylan Baker wrote:
> > > > >
> > > > > Quoting Emil Velikov (2018-12-13 08:05:52)
> > > > > > From: Emil Velikov
> > > > > >
> > > > > > 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.
> > >
> > > Wait, why shouldn't they run? The tests pass (at least with the
> > > gallium-xlib
> > > glx), and running tests seems useful.
> > >
> > Fully agree running tests is useful and welcome.
> >
> > The tests flex the -Dglx=dri code in src/glx/. As-is it's rather
> > confusing, misleading even, to request one thing then run tests for
> > something that looks the same but isn't.
> > Silly analogy - running ANV tests, when -Dvulkan-drivers=amd is used.
> >
> > Hope this is clear - not sure if the coffee has kicked in ;-)
> > -Emil
>
> Sorry, I meant to respond yesterday. Ian explained why these tests are only
> useful if building DRI based GLX.
>
> Maybe make the commit message something like:
> "meson: Ensure tests for dri based glx are only built when dri based glx is
> enabled"
> or something like that? I'll leave it up to you what you think is best. As
> long
> as you note that the real functional change is not building tests when we
> don't
> want/need them.
>
> Reviewed-by: Dylan Baker
Ack will do. Thanks.
-Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 03/18] glx: meson: build src/glx only with -Dglx=dri
Quoting Emil Velikov (2018-12-18 02:33:56)
> On Mon, 17 Dec 2018 at 19:44, Dylan Baker wrote:
> >
> > Quoting Emil Velikov (2018-12-17 10:58:11)
> > > On Fri, 14 Dec 2018 at 17:13, Dylan Baker wrote:
> > > >
> > > > Quoting Emil Velikov (2018-12-13 08:05:52)
> > > > > From: Emil Velikov
> > > > >
> > > > > 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.
> >
> > Wait, why shouldn't they run? The tests pass (at least with the gallium-xlib
> > glx), and running tests seems useful.
> >
> Fully agree running tests is useful and welcome.
>
> The tests flex the -Dglx=dri code in src/glx/. As-is it's rather
> confusing, misleading even, to request one thing then run tests for
> something that looks the same but isn't.
> Silly analogy - running ANV tests, when -Dvulkan-drivers=amd is used.
>
> Hope this is clear - not sure if the coffee has kicked in ;-)
> -Emil
Sorry, I meant to respond yesterday. Ian explained why these tests are only
useful if building DRI based GLX.
Maybe make the commit message something like:
"meson: Ensure tests for dri based glx are only built when dri based glx is
enabled"
or something like that? I'll leave it up to you what you think is best. As long
as you note that the real functional change is not building tests when we don't
want/need them.
Reviewed-by: Dylan Baker
signature.asc
Description: signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 03/18] glx: meson: build src/glx only with -Dglx=dri
On Mon, 17 Dec 2018 at 19:44, Dylan Baker wrote:
>
> Quoting Emil Velikov (2018-12-17 10:58:11)
> > On Fri, 14 Dec 2018 at 17:13, Dylan Baker wrote:
> > >
> > > Quoting Emil Velikov (2018-12-13 08:05:52)
> > > > From: Emil Velikov
> > > >
> > > > 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.
>
> Wait, why shouldn't they run? The tests pass (at least with the gallium-xlib
> glx), and running tests seems useful.
>
Fully agree running tests is useful and welcome.
The tests flex the -Dglx=dri code in src/glx/. As-is it's rather
confusing, misleading even, to request one thing then run tests for
something that looks the same but isn't.
Silly analogy - running ANV tests, when -Dvulkan-drivers=amd is used.
Hope this is clear - not sure if the coffee has kicked in ;-)
-Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 03/18] glx: meson: build src/glx only with -Dglx=dri
Quoting Emil Velikov (2018-12-17 10:58:11)
> On Fri, 14 Dec 2018 at 17:13, Dylan Baker wrote:
> >
> > Quoting Emil Velikov (2018-12-13 08:05:52)
> > > From: Emil Velikov
> > >
> > > 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.
Wait, why shouldn't they run? The tests pass (at least with the gallium-xlib
glx), and running tests seems useful.
> With this in mind, do you think we should drop the fixes tag?
>
> > > Signed-off-by: Emil Velikov
> > > ---
> > > 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
> >
>
> Thanks.
> Emil
signature.asc
Description: signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 03/18] glx: meson: build src/glx only with -Dglx=dri
On Fri, 14 Dec 2018 at 17:13, Dylan Baker wrote:
>
> Quoting Emil Velikov (2018-12-13 08:05:52)
> > From: Emil Velikov
> >
> > 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
> > ---
> > 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
>
Thanks.
Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 03/18] glx: meson: build src/glx only with -Dglx=dri
Quoting Emil Velikov (2018-12-13 08:05:52)
> From: Emil Velikov
>
> 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
> Signed-off-by: Emil Velikov
> ---
> 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.
With those changes:
Reviewed-by: Dylan Baker
>
> if with_tests
>subdir('tests')
> diff --git a/src/meson.build b/src/meson.build
> index 915441fb2ce..ae094fccf6c 100644
> --- a/src/meson.build
> +++ b/src/meson.build
> @@ -74,7 +74,7 @@ subdir('loader')
> if with_platform_haiku
>subdir('hgl')
> endif
> -if with_glx != 'disabled'
> +if with_glx == 'dri'
>subdir('glx')
> endif
> if with_gbm
> --
> 2.19.2
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
signature.asc
Description: signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 03/18] glx: meson: build src/glx only with -Dglx=dri
From: Emil Velikov
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")
Signed-off-by: Emil Velikov
---
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,
+)
if with_tests
subdir('tests')
diff --git a/src/meson.build b/src/meson.build
index 915441fb2ce..ae094fccf6c 100644
--- a/src/meson.build
+++ b/src/meson.build
@@ -74,7 +74,7 @@ subdir('loader')
if with_platform_haiku
subdir('hgl')
endif
-if with_glx != 'disabled'
+if with_glx == 'dri'
subdir('glx')
endif
if with_gbm
--
2.19.2
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
