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

[Emil Velikov]

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