[PATCH v3] configure.ac: Change in build system to use the path from pkg-config for wayland-scanner.

[Pekka Paalanen]

On Mon, 19 May 2014 11:56:39 +0200
Thierry Reding <thierry.reding at gmail.com> wrote:

> On Mon, May 19, 2014 at 11:59:51AM +0300, Pekka Paalanen wrote:
> > On Sun, 18 May 2014 00:39:16 +0200 Thierry Reding <thierry.reding at gmail.com> wrote:
> [...]

> > >      be made to behave the same way. I think a more appropriate sequence
> > >      would be:
> > > 
> > > 	AC_PATH_PROG([wayland_scanner], [wayland-scanner])
> > > 	if test "x$wayland_scanner" = "x"; then
> > > 		wayland_scanner=`$PKG_CONFIG --variable=wayland_scanner wayland-scanner`
> > > 	fi
> > 
> > I'd really like to have the .pc file take priority, because of the
> > above.
> 
> I don't think that's really an option. If, as proposed earlier, we allow
> cross builds to ship wayland-scanner, then having the .pc file take
> priority will cause the cross-built version of wayland-scanner to be
> picked up. The reason is that wayland-scanner.pc will be installed in
> ${prefix}/lib/pkgconfig, along with any other wayland-*.pc. In order to
> pick up the necessary include paths and libraries, people will need to
> set PKG_CONFIG_PATH or PKG_CONFIG_LIBDIR appropriately. But doing so
> will pick up wayland-scanner.pc at the same time.

Gah, right.

So there is no way a .pc file for build tools will ever work for
cross-builds.

> > > I think it would further be useful to make all of that logic available
> > > via an m4 snippet (like what we already have in wayland-scanner.m4) so
> > > that all packages that need wayland-scanner handle this consistently.
> > > weston should use that snippet rather than implement its own version.
> > 
> > Actually that .m4 snippet has caused a lot of grief. Now, most projects
> > just don't bother with it. It is much easier and more flexible to just
> > copy the few lines that the wayland-scanner build rules need.
> 
> Unless something causes these rules to need modification, in which case
> all projects now need to adapt manually. I think if at least weston used
> that snippet there'd be enough regular testing to make sure nothing
> breaks. Otherwise if it's really causing too much grief to be useful by
> any project and if it can't be fixed, then perhaps a better option would
> be to remove it altogether.

One basic problem with this particular snippet is, that it cannot deal
with multiple locations for the XML files. It also used to produce
mysterious error messages, when when the macro wasn't really found or
something else was missing.

Grepping Weston for WAYLAND_SCANNER_RULES or wayland-scanner.mk produces
nothing, so looks like it is already abandoned by Weston.

The wayland-scanner.m4 insists for the .pc file to give the absolute
path to wayland-scanner, with no way to override AFAICS.

> > Finding wayland-scanner is slightly different though, because build
> > rules you may want to customize, but finding a binary not so much.
> > 
> > To summarize, I'd prefer this search order:
> > 
> > 1. AC_ARG_VAR if set, otherwise
> > 2. .pc file, with fallback to
> > 3. searching the PATH
> > 
> > Would that be ok?
> 
> I'm pretty sure that it could be made to work. However I think it makes
> it more difficult than necessary. Using AC_PROG_PATH already gives you
> two ways of overriding the automatic value:
> 
> 	AC_PROG_PATH([WAYLAND_SCANNER], [wayland-scanner])
> 
> Will check both WAYLAND_SCANNER and ac_cv_path_WAYLAND_SCANNER before
> trying to lookup an executable on the path.
> 
> And like I said, having wayland-scanner.pc take priority will break
> cross-compilation unless you explicitly override using AC_ARG_VAR.
> 
> There's also the matter of consistency. Other tools (gcc, pkg-config,
> ...) used during the build are also looked up on the path, so following
> the principle of least suprise I'd expect any build tools to follow the
> same conventions.

Ok, you convinced me. Especially with the fact, that the .pc file can
never be correct when cross-building. We need a totally different
approach.

> That still leaves open the question of how to handle stale versions of
> the wayland-scanner binary. Like I said, I think telling people to add
> it to the PATH wouldn't be much of a burden. For many users that might
> already be the case anyway. For instance I have a set of scripts to
> cross-build distributions from scratch (similar in spirit to buildroot
> and friends) where tools such as wayland-scanner get installed to a
> prefix that's always first in PATH. When I do native builds out of git
> I usually install to /usr/local/stow/$PACKAGE and let stow manage the
> symlink hierarchy in /usr/local for me. The distribution that I use has
> /usr/local/bin in PATH (so that it takes precedence over /usr/bin), so
> any binaries that I install manually override whatever the distribution
> installed.
> 
> If the exact version of wayland-scanner really matters, perhaps a better
> solution would be to check for the required version at configure time.

Ok, we can also just ignore that little problem. I'm not sure how you
would check the wayland-scanner version. You can't use the .pc file,
and wayland-scanner itself cannot tell you. Good thing it is not
supposed to be version-picky even though there are some rare failure
modes (too new wayland-scanner with old libwayland, or too old
wayland-scanner not supporting the latest XML mark-up).

Will you take Hebbar's patch review to the end?


Thanks,
pq