[Mesa-dev] [PATCH 2/3] gbm: Don't traverse backwards for includes

[Dylan Baker]

Quoting Emil Velikov (2017-11-02 11:04:44)
> On 2 November 2017 at 16:53, Dylan Baker <dylan at pnwbakers.com> wrote:
> > Quoting Emil Velikov (2017-11-02 06:21:09)
> >> On 21 October 2017 at 02:00, Dylan Baker <dylan at pnwbakers.com> wrote:
> >> > This is just a terrible idea, but it also needs to be fixed for the next
> >> > patch to work, so let's fix it right.
> >> >
> >> Please rework the commit message. Perhaps something alike
> >> 1b1bb6ee103a79de11aa4941ccbcd34f0a158276?
> >>
> >> > Signed-off-by: Dylan Baker <dylanx.c.baker at intel.com>
> >> > ---
> >> >  src/gbm/Makefile.am            | 4 +++-
> >> >  src/gbm/backends/dri/gbm_dri.c | 2 +-
> >> >  src/gbm/meson.build            | 6 +++---
> >> >  3 files changed, 7 insertions(+), 5 deletions(-)
> >> >
> >> > diff --git a/src/gbm/Makefile.am b/src/gbm/Makefile.am
> >> > index 805208a3ca9..05d861ff999 100644
> >> > --- a/src/gbm/Makefile.am
> >> > +++ b/src/gbm/Makefile.am
> >> > @@ -31,7 +31,9 @@ libgbm_la_LIBADD = \
> >> >         $(DLOPEN_LIBS)
> >> >
> >> >  if HAVE_PLATFORM_WAYLAND
> >> > -AM_CFLAGS += $(WAYLAND_SERVER_CFLAGS)
> >> > +AM_CFLAGS += \
> >> > +       $(WAYLAND_SERVER_CFLAGS) \
> >> > +       -I$(top_srcdir)/src/egl/wayland/wayland-drm/
> >> >  libgbm_la_LIBADD += $(top_builddir)/src/egl/wayland/wayland-drm/libwayland-drm.la $(WAYLAND_SERVER_LIBS)
> >> This might cause a trivial merge/rebase conflict.
> >>
> >> >  endif
> >> >
> >> > diff --git a/src/gbm/backends/dri/gbm_dri.c b/src/gbm/backends/dri/gbm_dri.c
> >> > index 0a4853bf63d..b2121cbc340 100644
> >> > --- a/src/gbm/backends/dri/gbm_dri.c
> >> > +++ b/src/gbm/backends/dri/gbm_dri.c
> >> > @@ -53,7 +53,7 @@
> >> >
> >> >  /* For importing wl_buffer */
> >> >  #if HAVE_WAYLAND_PLATFORM
> >> > -#include "../../../egl/wayland/wayland-drm/wayland-drm.h"
> >> > +#include "wayland-drm.h"
> >> >  #endif
> >> >
> >> >  #ifndef DRM_FORMAT_MOD_INVALID
> >> > diff --git a/src/gbm/meson.build b/src/gbm/meson.build
> >> > index 1bb3c94c387..2910fa2390c 100644
> >> > --- a/src/gbm/meson.build
> >> > +++ b/src/gbm/meson.build
> >> > @@ -31,6 +31,7 @@ deps_gbm = []
> >> >  args_gbm = []
> >> >  links_gbm = []
> >> >  deps_gbm = []
> >> > +incs_gbm = [include_directories('main'), inc_include, inc_src, inc_loader]
> >> >
> >> >  if with_dri2
> >> >    files_gbm += files('backends/dri/gbm_dri.c', 'backends/dri/gbm_driint.h')
> >> > @@ -40,6 +41,7 @@ endif
> >> >  if with_platform_wayland
> >> >    deps_gbm += dep_wayland_server
> >> >    links_gbm += libwayland_drm
> >> > +  incs_gbm += include_directories('../egl/wayland/wayland-drm')
> >> This looks quite ugly IMHO... Surely meson has the concept of
> >> top_{build,src}dir?
> >> Quick grep shows a few move cases like these. Were those
> >> butchered/inspired by the autotools build?
> >
> > meson specifically does not allow passing absolute paths to include_directories,
> > since you can assign includes to variables I assume. I can just create an
> > "inc_wayland_drm" instead.
> >
> top_{build,src}dir are not absolute - top_abs{build,src}dir are.
> 
> Normally one would want to consistently use
> $top/relative/path/to/something as opposed to having things relative
> to pwd.
> Regardless, I'm not the best person to listen to about meson best practises :-)
> 
> -Emil

Meson's equivalents (meson.source_dir(), meson.build_dir()) return absolute
paths. Meson upstream also considers their uses a hack, and in general if you
need them it means that meson itself is missing some functionality. In general
the way the meson wants you to do this is assign an include_directories() object
at a lower level, like we do with the include directory (as inc_include).

Dylan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: signature
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20171102/7fed4b3b/attachment-0001.sig>