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

[Pekka Paalanen]

On Thu, 20 Jul 2017 12:21:27 +0200
Quentin Glidic <sardemff7+wayland at sardemff7.net> wrote:

> On 7/18/17 3:58 PM, Pekka Paalanen wrote:
> > On Wed, 24 May 2017 11:32:21 +0300
> > Jussi Kukkonen <jussi.kukkonen at intel.com> wrote:
> >   
> >> On 23 May 2017 at 14:51, Quentin Glidic <sardemff7+wayland at sardemff7.net>
> >> wrote:  
> >   
> >>> So you want to break the simple (cross-compiling) cases to fix
> >>> something that doesn’t need fixing?  
> >>
> >> I think that is not warranted.
> >> * My issue was that every wayland-scanner user was doing something
> >> different, often in a way that required patching in Yocto
> >> * Weston was fine for us but Pekka gave me feedback that lead me to
> >> believe the weston method was not the best solution either
> >>
> >> I did my best to try and resolve both issues. It looks like my
> >> attempt was not successful but there should be no reason to say I
> >> "want to break" something.  
> > 
> > Hi,
> > 
> > I'm with Jussi here.
> >   
> >>> If you want to make a patch, it should not introduce a regression.  
> >>
> >> I think _if_ the goal is to use the same scanner lookup in e.g. mesa
> >> and weston, some use case in one or the other is just bound to have a
> >> regression.  
> > 
> > Quentin, do you see any way unifying the usage of wayland-scanner
> > without regressing at least one user?  
> 
> Yes, we can just do what we do now in Weston, *adding* the Yocto style.
> IOW, we want to support, without passing any variable:
> - native and cross-compiling as a system package
>    just wayland-scanner from (native) path is good
> - native in a custom prefix (system may be different version)
>    prefer the pkg-config file
> - cross-compiling in a custom prefix (system may be different version)
>    we need to check for the native (custom prefix) pkg-config here

Why should custom prefix vs. non-custom prefix be handled differently?
How do you tell them apart when there is no strict standard on the file
system layout?

And the default prefix in autotools is actually neither.

> In all three cases, checking for the version would be much better. We 
> could even use the native system scanner in some cases if the version 
> matches, avoiding another build in cross-compiling case.

Ensuring the version is not wrong is nice, but skipping a step on
cross-compiling based on it is not a good idea. It means that for some
people the cross-build would work, and for some it would require
additional building - which is easy to overlook by recipe writers, so
would likely become a build failure for people who need the "extra"
step.

E.g. a Yocto build depending on the host system tools (as opposed to
build tools built/downloaded by Yocto itself) is frowned upon, because
the host system should not affect how the actual build works. And it's
a very easy mistake to do.

> 
> In pseudo-code, here is what I suggest:
> 
> get_wayland_version()
> 
> # macro WL_PROG_SCANNER([version])
> if $cross_compiling; then
>      get_native_pkg_config_scanner_path()

How would get_native_pkg_config_scanner_path() work? What kind of setup
does it need from the user (or e.g. Yocto)?

> else
>      get_pkg_config_scanner_path()
> fi
> if scanner == ""; then

How could 'scanner' ever be empty here? Do you imply doing some "can it
run" check?

>      get_PATH_scanner_path()
>      if scanner == "" || ! check_scanner_version(); then
>          error()
>      fi
> fi
> # macro end
> 
> You can call "pkg-config --variable=wayland_scanner 'wayland-scanner = 
> "version'" directly to let pkg-config do the version check.
> 
> That would support all use cases listed above, with no variable. And we 
> can still add the configure switch for very specific corner cases that 
> might exist.

If the two details I pointed out are not a problem, that sounds fine to
me.

Any currently used build setup must have both PATH and pkg-config
set up to find the new scanner instead of the system scanner, since we
have projects preferring PATH and projects preferring pkg-config
variable. (Yet, I have a feeling this is not actually the case, since
less correct setups have worked by accident.)

> >>> So at this point, we should make a generic patch, ideally (IMO) in
> >>> wayland-scanner.m4, and have people test it in all their different
> >>> setup to see if it works as expected.  
> > 
> > Is anyone actually using wayland-scanner.m4?
> > 
> > I did not find any evidence of Weston using it. Certainly no-one is
> > using wayland-scanner.mk anymore because of the structure or
> > wayland-protocols and the need to search for XML files in both local
> > and system paths. Since wayland-scanner.m4 has a reference to
> > wayland-scanner.mk, I doubt anyone is using wayland-scanner.m4 either.
> > 
> > We should make a plan to get rid of wayland-scanner.mk and
> > wayland-scanner.m4, I do not think they are fixable.  
> 
> We can deprecate the current macro while adding new ones. Or have a new 
> file. It doesn’t matter much, it’s just a (autoreconf) build time dep on 
> the newer Wayland.
> I see no issue with wayland-scanner.m4 not being used. We can make *new* 
> versions use it, it’s adding API/ABI.

So the autoconf macro you are proposing would only find
wayland-scanner, right? I suppose that might work.

The problem with the old macro is that it tries to do three things
together: find wayland-scanner, find the XML file directory, and set up
make rules. Particularly the make rules part is problematic since it
assumes a single directory layout, which does not work even for weston.

> >>> If you’re not comfortable making one, I’ll do it by this week-end
> >>> so you can test it in yours.  
> > 
> > Did anything come out of that?  
> 
> We need to agree on the “good” way to do the checks first, but if you 
> feel better doing so with a real patch as a basis, I can do it.

What you outlined above sounds ok, but the devil is in the details. :-)

If you'd like to send a patch, please CC Jussi Kukkonen, Tomek Bury,
and anyone else you can find who was interested in cross-builds. And
probably also Emil Velikov for Mesa.

> > There is also the sysroot patch from Tomek Bury that no-one has
> > commented on yet.
> > https://patchwork.freedesktop.org/patch/165478/  
> 
> Ack.

Thanks for checking that one.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20170721/c53f62e9/attachment-0001.sig>