Re: [Mesa-dev] [PATCH] configure.ac/meson.build: Add options for library suffixes

2018-06-13 Thread Benjamin Gordon
On Wed, Jun 13, 2018 at 9:46 AM Dylan Baker  wrote:

> Quoting Eric Engestrom (2018-06-13 03:03:25)
> > On Tuesday, 2018-06-12 11:19:40 -0600, bmgor...@chromium.org wrote:
> > > From: Benjamin Gordon 
> > >
> > > When building the Chrome OS Android container, we need to build copies
> > > of mesa that don't conflict with the Android system-supplied libraries.
> > > This adds options to create suffixed versions of EGL and GLES
> libraries:
> > >
> > > libEGL.so -> libEGL${egl-lib-suffix}.so
> > > libGLESv1_CM.so -> libGLESv1_CM${gles-lib-suffix}.so
> > > libGLESv2.so -> libGLES${gles-lib-suffix}.so
> > >
> > > This is similar to what happens when --enable-libglvnd is specified,
> but
> > > without the side effects of linking against libglvnd.
> >
> > This seems reasonable, and the meson side of this patch is correct,
> > but we need to document or prevent the interaction between
> > --enable-libglvnd and --with-egl-lib-suffix.
> >
> > I can't think of a use-case for having both, so I suggest "if both are
> > enabled, error out"; scroll down for what this could look like in meson.
>
> Agreed, making it hard error to use both makes sense to me.
>

Thanks for the reviews.  I just sent a v2 that makes it an error to pass
both flags.


>
> > With that (and the corresponding autotools hunk):
> > Reviewed-by: Eric Engestrom 
> >
> > >
> > > Change-Id: I0a534d3921a24c031e2532ee7d5ba9813740b33b
> >
> > (Note to whoever merges this patch: drop this line ^)
> >
> > > Signed-off-by: Benjamin Gordon 
> > > ---
> > >  configure.ac| 14 ++
> > >  meson_options.txt   | 12 
> > >  src/egl/Makefile.am |  8 
> > >  src/egl/meson.build |  2 +-
> > >  src/mapi/Makefile.am| 28 ++--
> > >  src/mapi/es1api/meson.build |  2 +-
> > >  src/mapi/es2api/meson.build |  2 +-
> > >  7 files changed, 47 insertions(+), 21 deletions(-)
> > >
> > > diff --git a/configure.ac b/configure.ac
> > > index 35ade986d1..6070a2146b 100644
> > > --- a/configure.ac
> > > +++ b/configure.ac
> > > @@ -1511,12 +1511,24 @@ AC_ARG_WITH([gl-lib-name],
> > >  [specify GL library name @<:@default=GL@:>@])],
> > >[GL_LIB=$withval],
> > >[GL_LIB="$DEFAULT_GL_LIB_NAME"])
> > > +AC_ARG_WITH([egl-lib-suffix],
> > > +  [AS_HELP_STRING([--with-egl-lib-suffix@<:@=NAME@:>@],
> > > +[specify EGL library suffix @<:@default=none@:>@])],
> > > +  [EGL_LIB_SUFFIX=$withval],
> > > +  [EGL_LIB_SUFFIX=""])
> > > +AC_ARG_WITH([gles-lib-suffix],
> > > +  [AS_HELP_STRING([--with-gles-lib-suffix@<:@=NAME@:>@],
> > > +[specify GLES library suffix @<:@default=none@:>@])],
> > > +  [GLES_LIB_SUFFIX=$withval],
> > > +  [GLES_LIB_SUFFIX=""])
> > >  AC_ARG_WITH([osmesa-lib-name],
> > >[AS_HELP_STRING([--with-osmesa-lib-name@<:@=NAME@:>@],
> > >  [specify OSMesa library name @<:@default=OSMesa@:>@])],
> > >[OSMESA_LIB=$withval],
> > >[OSMESA_LIB=OSMesa])
> > >  AS_IF([test "x$GL_LIB" = xyes], [GL_LIB="$DEFAULT_GL_LIB_NAME"])
> > > +AS_IF([test "x$EGL_LIB_SUFFIX" = xyes], [EGL_LIB_SUFFIX=""])
> > > +AS_IF([test "x$GLES_LIB_SUFFIX" = xyes], [GLES_LIB_SUFFIX=""])
> > >  AS_IF([test "x$OSMESA_LIB" = xyes], [OSMESA_LIB=OSMesa])
> > >
> > >  dnl
> > > @@ -1534,6 +1546,8 @@ if test "x${enable_mangling}" = "xyes" ; then
> > >OSMESA_LIB="Mangled${OSMESA_LIB}"
> > >  fi
> > >  AC_SUBST([GL_LIB])
> > > +AC_SUBST([EGL_LIB_SUFFIX])
> > > +AC_SUBST([GLES_LIB_SUFFIX])
> > >  AC_SUBST([OSMESA_LIB])
> > >
> > >  # Check for libdrm
> > > diff --git a/meson_options.txt b/meson_options.txt
> > > index ce7d87f1eb..9d84c3b5bb 100644
> > > --- a/meson_options.txt
> > > +++ b/meson_options.txt
> > > @@ -298,3 +298,15 @@ option(
> > >choices : ['freedreno', 'glsl', 'intel', 'nir', 'nouveau', 'all'],
> > >description : 'List of tools to build.',
> > >  )
> > > +option(
> > > +  'egl-lib-suffix',
> > > +  type : 'string',
> > > +  value : '',
> > > +  description : 'Suffix to append to EGL library name.  Default:
> none.'
> > > +)
> > > +option(
> > > +  'gles-lib-suffix',
> > > +  type : 'string',
> > > +  value : '',
> > > +  description : 'Suffix to append to GLES library names.  Default:
> none.'
> > > +)
> > > diff --git a/src/egl/Makefile.am b/src/egl/Makefile.am
> > > index 086a4a1e63..c3aeeea007 100644
> > > --- a/src/egl/Makefile.am
> > > +++ b/src/egl/Makefile.am
> > > @@ -184,12 +184,12 @@ libEGL_mesa_la_LDFLAGS = \
> > >
> > >  else # USE_LIBGLVND
> > >
> > > -lib_LTLIBRARIES = libEGL.la
> > > -libEGL_la_SOURCES =
> > > -libEGL_la_LIBADD = \
> > > +lib_LTLIBRARIES = libEGL@EGL_LIB_SUFFIX@.la
> > > +libEGL@EGL_LIB_SUFFIX@_la_SOURCES =
> > > +libEGL@EGL_LIB_SUFFIX@_la_LIBADD = \
> > >   libEGL_common.la \
> > >   $(top_builddir)/src/mapi/shared-glapi/libglapi.la
> > > -libEGL_la_LDFLAGS = \
> > > +libEGL@EGL_LIB_SUFFIX@_la_LDFLAGS = \
> > >   -no-undefined \
> > >   -version-number 1:0 \
> > >   $(BSYMBOLIC) \
> > > diff --git a/src/egl/

Re: [Mesa-dev] [PATCH] configure.ac/meson.build: Add options for library suffixes

2018-06-13 Thread Dylan Baker
Quoting Eric Engestrom (2018-06-13 03:03:25)
> On Tuesday, 2018-06-12 11:19:40 -0600, bmgor...@chromium.org wrote:
> > From: Benjamin Gordon 
> > 
> > When building the Chrome OS Android container, we need to build copies
> > of mesa that don't conflict with the Android system-supplied libraries.
> > This adds options to create suffixed versions of EGL and GLES libraries:
> > 
> > libEGL.so -> libEGL${egl-lib-suffix}.so
> > libGLESv1_CM.so -> libGLESv1_CM${gles-lib-suffix}.so
> > libGLESv2.so -> libGLES${gles-lib-suffix}.so
> > 
> > This is similar to what happens when --enable-libglvnd is specified, but
> > without the side effects of linking against libglvnd.
> 
> This seems reasonable, and the meson side of this patch is correct,
> but we need to document or prevent the interaction between
> --enable-libglvnd and --with-egl-lib-suffix.
> 
> I can't think of a use-case for having both, so I suggest "if both are
> enabled, error out"; scroll down for what this could look like in meson.

Agreed, making it hard error to use both makes sense to me.

> With that (and the corresponding autotools hunk):
> Reviewed-by: Eric Engestrom 
> 
> > 
> > Change-Id: I0a534d3921a24c031e2532ee7d5ba9813740b33b
> 
> (Note to whoever merges this patch: drop this line ^)
> 
> > Signed-off-by: Benjamin Gordon 
> > ---
> >  configure.ac| 14 ++
> >  meson_options.txt   | 12 
> >  src/egl/Makefile.am |  8 
> >  src/egl/meson.build |  2 +-
> >  src/mapi/Makefile.am| 28 ++--
> >  src/mapi/es1api/meson.build |  2 +-
> >  src/mapi/es2api/meson.build |  2 +-
> >  7 files changed, 47 insertions(+), 21 deletions(-)
> > 
> > diff --git a/configure.ac b/configure.ac
> > index 35ade986d1..6070a2146b 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -1511,12 +1511,24 @@ AC_ARG_WITH([gl-lib-name],
> >  [specify GL library name @<:@default=GL@:>@])],
> >[GL_LIB=$withval],
> >[GL_LIB="$DEFAULT_GL_LIB_NAME"])
> > +AC_ARG_WITH([egl-lib-suffix],
> > +  [AS_HELP_STRING([--with-egl-lib-suffix@<:@=NAME@:>@],
> > +[specify EGL library suffix @<:@default=none@:>@])],
> > +  [EGL_LIB_SUFFIX=$withval],
> > +  [EGL_LIB_SUFFIX=""])
> > +AC_ARG_WITH([gles-lib-suffix],
> > +  [AS_HELP_STRING([--with-gles-lib-suffix@<:@=NAME@:>@],
> > +[specify GLES library suffix @<:@default=none@:>@])],
> > +  [GLES_LIB_SUFFIX=$withval],
> > +  [GLES_LIB_SUFFIX=""])
> >  AC_ARG_WITH([osmesa-lib-name],
> >[AS_HELP_STRING([--with-osmesa-lib-name@<:@=NAME@:>@],
> >  [specify OSMesa library name @<:@default=OSMesa@:>@])],
> >[OSMESA_LIB=$withval],
> >[OSMESA_LIB=OSMesa])
> >  AS_IF([test "x$GL_LIB" = xyes], [GL_LIB="$DEFAULT_GL_LIB_NAME"])
> > +AS_IF([test "x$EGL_LIB_SUFFIX" = xyes], [EGL_LIB_SUFFIX=""])
> > +AS_IF([test "x$GLES_LIB_SUFFIX" = xyes], [GLES_LIB_SUFFIX=""])
> >  AS_IF([test "x$OSMESA_LIB" = xyes], [OSMESA_LIB=OSMesa])
> >  
> >  dnl
> > @@ -1534,6 +1546,8 @@ if test "x${enable_mangling}" = "xyes" ; then
> >OSMESA_LIB="Mangled${OSMESA_LIB}"
> >  fi
> >  AC_SUBST([GL_LIB])
> > +AC_SUBST([EGL_LIB_SUFFIX])
> > +AC_SUBST([GLES_LIB_SUFFIX])
> >  AC_SUBST([OSMESA_LIB])
> >  
> >  # Check for libdrm
> > diff --git a/meson_options.txt b/meson_options.txt
> > index ce7d87f1eb..9d84c3b5bb 100644
> > --- a/meson_options.txt
> > +++ b/meson_options.txt
> > @@ -298,3 +298,15 @@ option(
> >choices : ['freedreno', 'glsl', 'intel', 'nir', 'nouveau', 'all'],
> >description : 'List of tools to build.',
> >  )
> > +option(
> > +  'egl-lib-suffix',
> > +  type : 'string',
> > +  value : '',
> > +  description : 'Suffix to append to EGL library name.  Default: none.'
> > +)
> > +option(
> > +  'gles-lib-suffix',
> > +  type : 'string',
> > +  value : '',
> > +  description : 'Suffix to append to GLES library names.  Default: none.'
> > +)
> > diff --git a/src/egl/Makefile.am b/src/egl/Makefile.am
> > index 086a4a1e63..c3aeeea007 100644
> > --- a/src/egl/Makefile.am
> > +++ b/src/egl/Makefile.am
> > @@ -184,12 +184,12 @@ libEGL_mesa_la_LDFLAGS = \
> >  
> >  else # USE_LIBGLVND
> >  
> > -lib_LTLIBRARIES = libEGL.la
> > -libEGL_la_SOURCES =
> > -libEGL_la_LIBADD = \
> > +lib_LTLIBRARIES = libEGL@EGL_LIB_SUFFIX@.la
> > +libEGL@EGL_LIB_SUFFIX@_la_SOURCES =
> > +libEGL@EGL_LIB_SUFFIX@_la_LIBADD = \
> >   libEGL_common.la \
> >   $(top_builddir)/src/mapi/shared-glapi/libglapi.la
> > -libEGL_la_LDFLAGS = \
> > +libEGL@EGL_LIB_SUFFIX@_la_LDFLAGS = \
> >   -no-undefined \
> >   -version-number 1:0 \
> >   $(BSYMBOLIC) \
> > diff --git a/src/egl/meson.build b/src/egl/meson.build
> > index 6537e4bdee..b833fd1729 100644
> > --- a/src/egl/meson.build
> > +++ b/src/egl/meson.build
> > @@ -148,7 +148,7 @@ if cc.has_function('mincore')
> >  endif
> >  
> 
>   if with_glvnd and get_option('egl-lib-suffix') != ''
> error('''EGL lib suffix can't be used with libglvnd''')
>   endif
> 
> >  if not with_glvnd
> > -  egl_lib

Re: [Mesa-dev] [PATCH] configure.ac/meson.build: Add options for library suffixes

2018-06-13 Thread Eric Engestrom
On Tuesday, 2018-06-12 11:19:40 -0600, bmgor...@chromium.org wrote:
> From: Benjamin Gordon 
> 
> When building the Chrome OS Android container, we need to build copies
> of mesa that don't conflict with the Android system-supplied libraries.
> This adds options to create suffixed versions of EGL and GLES libraries:
> 
> libEGL.so -> libEGL${egl-lib-suffix}.so
> libGLESv1_CM.so -> libGLESv1_CM${gles-lib-suffix}.so
> libGLESv2.so -> libGLES${gles-lib-suffix}.so
> 
> This is similar to what happens when --enable-libglvnd is specified, but
> without the side effects of linking against libglvnd.

This seems reasonable, and the meson side of this patch is correct,
but we need to document or prevent the interaction between
--enable-libglvnd and --with-egl-lib-suffix.

I can't think of a use-case for having both, so I suggest "if both are
enabled, error out"; scroll down for what this could look like in meson.

With that (and the corresponding autotools hunk):
Reviewed-by: Eric Engestrom 

> 
> Change-Id: I0a534d3921a24c031e2532ee7d5ba9813740b33b

(Note to whoever merges this patch: drop this line ^)

> Signed-off-by: Benjamin Gordon 
> ---
>  configure.ac| 14 ++
>  meson_options.txt   | 12 
>  src/egl/Makefile.am |  8 
>  src/egl/meson.build |  2 +-
>  src/mapi/Makefile.am| 28 ++--
>  src/mapi/es1api/meson.build |  2 +-
>  src/mapi/es2api/meson.build |  2 +-
>  7 files changed, 47 insertions(+), 21 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 35ade986d1..6070a2146b 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1511,12 +1511,24 @@ AC_ARG_WITH([gl-lib-name],
>  [specify GL library name @<:@default=GL@:>@])],
>[GL_LIB=$withval],
>[GL_LIB="$DEFAULT_GL_LIB_NAME"])
> +AC_ARG_WITH([egl-lib-suffix],
> +  [AS_HELP_STRING([--with-egl-lib-suffix@<:@=NAME@:>@],
> +[specify EGL library suffix @<:@default=none@:>@])],
> +  [EGL_LIB_SUFFIX=$withval],
> +  [EGL_LIB_SUFFIX=""])
> +AC_ARG_WITH([gles-lib-suffix],
> +  [AS_HELP_STRING([--with-gles-lib-suffix@<:@=NAME@:>@],
> +[specify GLES library suffix @<:@default=none@:>@])],
> +  [GLES_LIB_SUFFIX=$withval],
> +  [GLES_LIB_SUFFIX=""])
>  AC_ARG_WITH([osmesa-lib-name],
>[AS_HELP_STRING([--with-osmesa-lib-name@<:@=NAME@:>@],
>  [specify OSMesa library name @<:@default=OSMesa@:>@])],
>[OSMESA_LIB=$withval],
>[OSMESA_LIB=OSMesa])
>  AS_IF([test "x$GL_LIB" = xyes], [GL_LIB="$DEFAULT_GL_LIB_NAME"])
> +AS_IF([test "x$EGL_LIB_SUFFIX" = xyes], [EGL_LIB_SUFFIX=""])
> +AS_IF([test "x$GLES_LIB_SUFFIX" = xyes], [GLES_LIB_SUFFIX=""])
>  AS_IF([test "x$OSMESA_LIB" = xyes], [OSMESA_LIB=OSMesa])
>  
>  dnl
> @@ -1534,6 +1546,8 @@ if test "x${enable_mangling}" = "xyes" ; then
>OSMESA_LIB="Mangled${OSMESA_LIB}"
>  fi
>  AC_SUBST([GL_LIB])
> +AC_SUBST([EGL_LIB_SUFFIX])
> +AC_SUBST([GLES_LIB_SUFFIX])
>  AC_SUBST([OSMESA_LIB])
>  
>  # Check for libdrm
> diff --git a/meson_options.txt b/meson_options.txt
> index ce7d87f1eb..9d84c3b5bb 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -298,3 +298,15 @@ option(
>choices : ['freedreno', 'glsl', 'intel', 'nir', 'nouveau', 'all'],
>description : 'List of tools to build.',
>  )
> +option(
> +  'egl-lib-suffix',
> +  type : 'string',
> +  value : '',
> +  description : 'Suffix to append to EGL library name.  Default: none.'
> +)
> +option(
> +  'gles-lib-suffix',
> +  type : 'string',
> +  value : '',
> +  description : 'Suffix to append to GLES library names.  Default: none.'
> +)
> diff --git a/src/egl/Makefile.am b/src/egl/Makefile.am
> index 086a4a1e63..c3aeeea007 100644
> --- a/src/egl/Makefile.am
> +++ b/src/egl/Makefile.am
> @@ -184,12 +184,12 @@ libEGL_mesa_la_LDFLAGS = \
>  
>  else # USE_LIBGLVND
>  
> -lib_LTLIBRARIES = libEGL.la
> -libEGL_la_SOURCES =
> -libEGL_la_LIBADD = \
> +lib_LTLIBRARIES = libEGL@EGL_LIB_SUFFIX@.la
> +libEGL@EGL_LIB_SUFFIX@_la_SOURCES =
> +libEGL@EGL_LIB_SUFFIX@_la_LIBADD = \
>   libEGL_common.la \
>   $(top_builddir)/src/mapi/shared-glapi/libglapi.la
> -libEGL_la_LDFLAGS = \
> +libEGL@EGL_LIB_SUFFIX@_la_LDFLAGS = \
>   -no-undefined \
>   -version-number 1:0 \
>   $(BSYMBOLIC) \
> diff --git a/src/egl/meson.build b/src/egl/meson.build
> index 6537e4bdee..b833fd1729 100644
> --- a/src/egl/meson.build
> +++ b/src/egl/meson.build
> @@ -148,7 +148,7 @@ if cc.has_function('mincore')
>  endif
>  

  if with_glvnd and get_option('egl-lib-suffix') != ''
error('''EGL lib suffix can't be used with libglvnd''')
  endif

>  if not with_glvnd
> -  egl_lib_name = 'EGL'
> +  egl_lib_name = 'EGL' + get_option('egl-lib-suffix')
>egl_lib_version = '1.0.0'
>  else
>egl_lib_name = 'EGL_mesa'
> diff --git a/src/mapi/Makefile.am b/src/mapi/Makefile.am
> index 3da1a193d2..a2b108adc9 100644
> --- a/src/mapi/Makefile.am
> +++ b/src/mapi/Makefile.am
> @@ -178,24 +178,24 @@ GLES_include_HEADERS = \
>   $(top_srcdir)/inc