[Mesa-dev] [PATCH] meson: Add Haiku platform support v3

Fri Feb 16 19:08:33 UTC 2018

February 16, 2018 11:49 AM, "Emil Velikov" <emil.l.velikov at gmail.com> wrote:
> Hi Alexander,
> 
> Did you drop the ball on the autotools patches? I could re-spin them
> but have no way to test.

I've been focused on meson since it's the future.
Feel free to re-spin them if you have the bandwidth.

> There's a couple of comments inline, but nothing major.
> 
> On 16 February 2018 at 00:32, Alexander von Gluck IV
> <kallisti5 at unixzen.com> wrote:
> 
>> --- a/meson.build
>> +++ b/meson.build
>> 
>> gl_priv_libs = []
>> -if dep_thread.found()
>> +if dep_thread.found() and host_machine.system() != 'haiku'
>> gl_priv_libs += ['-lpthread', '-pthread']
>> endif
> 
> There's a bug report/fix in Meson for this one right?
> Please mention the ticket number of Meson version where the fix
> landed. Thus way one can drop this when we bump the Meson requirement.

I did a bump of Meson on my Haiku build system and still saw the same
incorrect searching for -lpthread issues.  I think there are places where
-lpthread -pthread were specified without looking at dep_thread.found()
though which could be at play.  (see below)

>> if dep_m.found()
>> diff --git a/src/egl/meson.build b/src/egl/meson.build
>> index 6cd04567b0..09c28e9f83 100644
>> --- a/src/egl/meson.build
>> +++ b/src/egl/meson.build
> 
> There are three unrelated things happening here:
> 
>> @@ -21,9 +21,8 @@
>> c_args_for_egl = []
>> link_for_egl = []
>> deps_for_egl = []
>> -incs_for_egl = [
>> - inc_include, inc_src, inc_loader, inc_gbm, include_directories('main'),
>> -]
>> +incs_for_egl = [inc_include, inc_src, include_directories('main')]
>> +
> 
> a) inc_gbm should be moved in the with_platform_drm hunk

Can-do.  I wasn't 100% sure on when it was included so didn't want to disrupt
any non-Haiku build conditions.

>> files_egl = files(
>> 'main/eglapi.c',
>> 'main/eglapi.h',
>> @@ -53,9 +52,6 @@ files_egl = files(
>> 'main/eglsync.h',
>> 'main/eglentrypoint.h',
>> 'main/egltypedefs.h',
>> - 'drivers/dri2/egl_dri2.c',
>> - 'drivers/dri2/egl_dri2.h',
>> - 'drivers/dri2/egl_dri2_fallbacks.h',
>> )
>> 
>> linux_dmabuf_unstable_v1_protocol_c = custom_target(
>> @@ -100,6 +96,21 @@ g_egldispatchstubs_h = custom_target(
>> capture : true,
>> )
>> 
>> +if with_dri2
>> + files_egl += files(
>> + 'drivers/dri2/egl_dri2.c',
>> + 'drivers/dri2/egl_dri2.h',
>> + 'drivers/dri2/egl_dri2_fallbacks.h',
>> + )
>> + incs_for_egl += [inc_loader, inc_gbm]
>> + c_args_for_egl += [
>> + '-DDEFAULT_DRIVER_DIR="@0@"'.format(dri_search_path),
>> + '-D_EGL_BUILT_IN_DRIVER_DRI2',
>> + ]
>> + link_for_egl += [libloader, libxmlconfig]
>> + deps_for_egl += dep_libdrm
>> +endif
>> +
> 
> b) This should be within the with_platform_x11 hunk

So with_platform_x11 always means with_dri2 regardless
of what with_dri2 is set to?

>> if with_platform_x11
>> files_egl += files('drivers/dri2/platform_x11.c')
>> if with_dri3
>> @@ -133,6 +144,15 @@ if with_platform_android
>> deps_for_egl += dep_android
>> files_egl += files('drivers/dri2/platform_android.c')
>> endif
>> +if with_platform_haiku
>> + incs_for_egl += inc_haikugl
>> + c_args_for_egl += [
>> + '-D_EGL_BUILT_IN_DRIVER_HAIKU',
>> + ]
>> + files_egl += files('drivers/haiku/egl_haiku.cpp')
>> + link_for_egl += libgl
>> + deps_for_egl += cpp.find_library('be')
>> +endif
> 
> c) this is the patch introducing haiku support

These are all changes needed to properly build.

>> --- /dev/null
>> +++ b/src/gallium/targets/haiku-softpipe/meson.build
>> 
>> +libswpipe = shared_library(
>> 
>> + dependencies : [
>> + driver_swrast, cpp.find_library('be'), cpp.find_library('translation'),
>> + cpp.find_library('network'), dep_unwind
> 
> Some of these find_library calls are duplicated throughout. It would
> be more efficient and robust to have it done once.
> Say in the top level meson.build?

That can be done.  Focused on functionality before optimizing in this first
commit.

>> index 8aaf58a623..7a4bcd3329 100644
>> --- a/src/hgl/GLDispatcher.h
>> +++ b/src/hgl/GLDispatcher.h
>> @@ -15,7 +15,7 @@
>> #include <GL/gl.h>
>> #include <SupportDefs.h>
>> 
>> -#include "glheader.h"
>> +#include "main/glheader.h"
> 
> Please keep source changes separate from build system bits.

Can-do. This was a requirement and matches the other dispatchers like glx.

>> diff --git a/src/mapi/es1api/meson.build b/src/mapi/es1api/meson.build
>> index ea14654d2c..38a5747e9a 100644
>> --- a/src/mapi/es1api/meson.build
>> +++ b/src/mapi/es1api/meson.build
>> 
>> --- a/src/mapi/es2api/meson.build
>> +++ b/src/mapi/es2api/meson.build
>> @@ -48,7 +48,7 @@ pkg.generate(
>> description : 'Mesa OpenGL ES 2.0 library',
>> version : meson.project_version(),
>> libraries : libgles2,
>> - libraries_private : '-lm -ldl -lpthread -pthread',
>> + libraries_private : gl_priv_libs,
>> )
> 
> Unrelated fixes - make that a separate patch.
>

This is actually needed to compile EGL under Haiku since
it is assuming -lpthread and -lm... so related.

>> diff --git a/src/meson.build b/src/meson.build
>> index 730b2ff6e4..4d5637f0aa 100644
>> --- a/src/meson.build
>> +++ b/src/meson.build
>> 
>> +if with_glx != 'disabled'
>> + subdir('glx')
>> +endif
> 
> Another unrelated fix?

If you try and compile glx on Haiku you're gonna have a bad
day.  Setting glx to disabled should mean glx isn't built.

> Last but not least, please add a meson.build to the respective
> Makefile.ams' EXTRA_DIST.

Can-do.  I wasn't making any changes to Makefile / Autotools
since the plan is to migrate away from it.

 -- Alex