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

Fri Feb 16 19:28:20 UTC 2018

Quoting Alexander von Gluck IV (2018-02-16 11:08:33)
> 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)

dep_pthread.found() returns true on anything that has pthreads, so for us that
means anything except windows (non-cygwin windows). I think this is the right
way to handle this, haiku does have pthreads, they're just not a separate
library. macOS has some weird pthreads stuff too, and this is how we handle
macOS.

> 
> >> 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.

Until autotools is dropped we'll be using it to make the tarballs, after it's
dropped we'll use meson, so for now it does need to be in the extra dist.

> 
>  -- Alex
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: signature
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180216/bba1c25d/attachment.sig>