[PATCH][weston] configure.ac: Add --with-wayland-scanner-path

[Quentin Glidic]

On 5/23/17 10:05 AM, Jussi Kukkonen wrote:
> Modify wayland-scanner lookup: Use the path given by pkg-config
> but offer an option to override the path with
> "--with-wayland-scanner-path=PATH". The latter is useful for
> cross-compile situations.

I would rather use --with-wayland-scanner.


> AC_PATH_PROG is no longer used.

I disagree with that, but you can at least make it work for 
cross-compiling (to an incompatible host) *without* the need to add 
--with-wayland-scanner-path in most cases.


> Also add a AC_SUBST-call (it seems previously the pkg-config value was
> never substituted into Makefiles).

It was, AC_PATH_PROG() does call AC_SUBST().

> ---
> 
> My goal is to standardize wayland-scanner usage in a way that does
> not require patching when cross-compiling in Yocto. I'm sending a
> similar patch to mesa and will fix other projects if these two patches
> are well received.
> 
> I did not check that wayland-scanner can actually run as pq suggested:
> I don't think that's typically a problem and the error on make should
> be fairly good in that case.
> 
> Thanks,
>   Jussi
> 
>   configure.ac | 19 ++++++++++++++-----
>   1 file changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index db757f20..e17135a9 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -653,11 +653,20 @@ if test "x$enable_lcms" != "xno"; then
>   fi
>   AM_CONDITIONAL(HAVE_LCMS, [test "x$enable_lcms" = xyes])
>   
> -AC_PATH_PROG([wayland_scanner], [wayland-scanner])
> -if test x$wayland_scanner = x; then
> -	PKG_CHECK_MODULES(WAYLAND_SCANNER, [wayland-scanner])
> -	wayland_scanner=`$PKG_CONFIG --variable=wayland_scanner wayland-scanner`
> -fi
> +AC_ARG_WITH([wayland-scanner-path],
> +            [AS_HELP_STRING([--with-wayland-scanner-path=PATH],
> +                            [Path to wayland-scanner (by default the path from
> +                             'pkg-config --variable=wayland_scanner wayland-scanner'
> +                             is used)])],
> +            [wayland_scanner="$withval"],

Drop that [].

> +            [wayland_scanner="auto"])

And use "with_wayland_scanner=yes" here. Then, (see post-fi comment)


> +if test x$wayland_scanner = xauto; then
> +        PKG_CHECK_MODULES([WAYLAND_SCANNER],
> +                          [wayland-scanner],
> +                          [wayland_scanner=`$PKG_CONFIG --variable=wayland_scanner wayland-scanner`],

Here, $PKG_CONFIG is wrong. Specifically in cross-compiling case, which 
is what your patch is supposed to fix.
Having a way to override the value is nice, but we already had.
Running "./configure 
ac_cv_path_wayland_scanner=/usr/yocto/bin/wayland-scanner" should work. 
Doing nothing works in most cases.

With this patch, you’re breaking cross-compilation to incompatible host 
in the default case, to add something you can already do.

At the very least, you should be using the *build* pkg-config, with 
something like that:
AC_CANONICAL_BUILD
AC_ARG_VAR([BUILD_PKG_CONFIG])
AC_PATH_PROGS([BUILD_PKG_CONFIG],
[${build}-pkg-config pkg-config], [${PKG_CONFIG}])
# We’re safe, ${PKG_CONFIG} is used elsewhere so it’s a good fallback
wayland_scanner=`${BUILD_PKG_CONFIG} --variable=wayland_scanner 
wayland-scanner`
# But using ${PKG_CONFIG} can lead to unusable executable here, we 
*must* check it works
wl_scanner_version=`${wayland_scanner} --version 2>/dev/null`
AS_IF([test "x$?" != "x0"], [AC_MSG_ERROR([${wayland_scanner} is not 
usable])])


> +                          [AC_MSG_ERROR([wayland-scanner was not found and --with-wayland-scanner-path was not used])])
> +fi

Use AS_CASE, for "yes" (= "auto"), "no" (= error, to make 
--without-wayland-scanner fails) and "*" (= do nothing, consider the 
path is ok).


> +AC_SUBST(wayland_scanner)

Adding a run test would be nice, indeed, but we should have a working value.

>   AC_ARG_ENABLE(systemd_notify,
>                 AS_HELP_STRING([--enable-systemd-notify],


I think this patch should be in Wayland, to wayland-scanner.m4 for other 
projects to use (they would depend on a recent Wayland for "make dist" 
and Git builds only, so the “extra dep” should be fine, one can always 
backport the .m4 to their tree if needed).
The proper macro name is WAYLAND_PROG_WAYLAND_SCANNER (or WL_PROG…).

A nice thing would be to check the version too.

Once we’ve figured out the proper algorithm to use, I’ll make a match to 
create a wayland module in Meson to do the same thing.

-- 

Quentin “Sardem FF7” Glidic