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
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to