[libvirt PATCH 037/351] meson: add readline build option

[Pavel Hrdina]

On Wed, Aug 05, 2020 at 11:52:30AM +0200, Andrea Bolognani wrote:
> On Wed, 2020-08-05 at 11:23 +0200, Erik Skultety wrote:
> > On Wed, Aug 05, 2020 at 10:59:08AM +0200, Andrea Bolognani wrote:
> > > Erik reports that there are issues with building RPMs on CentOS. He
> > > opened https://gitlab.com/libvirt/libvirt-ci/-/merge_requests/42, but
> > > it seems to me that the topic should be discussed here instead, so
> > > I'm quoting his message and replying inline.
> > > 
> > > > After libvirt switched to the meson build system, rpmbuild on
> > > > CentOS is no longer viable as meson >= 0.54.0 is required while
> > > > only 0.49.2 (at the time of this patch) is available from the
> > > > package manager. Even if --nodeps is passed, the "%meson" RPM macro
> > > > won't expand and the build will fail.
> > > 
> > > The question is, was dropping RPM build support for CentOS an
> > > intentional change? I'm not sure it was, because otherwise the spec
> > > file could have been cleaned up a bit at the same time. If it was
> > > not, then we need to make use of the `%meson` macro conditional.
> > > 
> > > As an aside, I haven't actually checked, but I think that even though
> > > CentOS 8 ships a version of Meson that's too old for us it should at
> > > least have the `%meson` macro. Maybe that won't work with a Meson
> > > installed from PyPi, though...
> > 
> > One thing I haven't tried is to install the old meson and use --nodeps with
> > rpmbuild, still, rather than having 2 version of meson installed on CentOS for
> > the rpmbuild, we should fallback to something else if %meson isn't defined.
> 
> Oh, I see now the %meson macro is shipped as part of meson itself. We
> definitely don't want to install two versions of meson, and if we end
> up agreeing that keeping RPM builds working on CentOS is important we
> will have to work around the need for the macro on CentOS 7 anyway,
> so we should be able to use that same workaround on CentOS 8 too.

We need to figure it out somehow but I'm not sure that we want to copy
the content of /usr/lib/rpm/macros.d/macros.meson file into our spec
file.

Downstream maintainers will probably have to bundle meson 0.54.0
together with libvirt in the packages, that was my original motivation
not to introduce the BuildRequires in our spec file. It was pointed out
during review that I should add it there, which actually makes sense as
we depend on the macro.

For our testing purposes we can install the system wide meson and newer
version from PyPi if required and remove the version requirement from
the spec file.

That way downstream maintainers will get the system wide meson with
meson macros for RPM and if required they will use the bundled meson to
do the compilation.

Everything should work and we will not make our spec file even more
complex.

> > > > It doesn't end there though, even if meson 0.54.0 were available,
> > > > the rpm build would still fail because of readline-devel not
> > > > providing the pkg config file on CentOS-8 which we rely on.
> > > 
> > > Looking at the lastest pipeline, all CentOS versions have "readline:
> > > NO" in their build summary, but the build still succeeded, so I'm not
> > > sure why the RPM build would fail? readline is an optional
> > > dependency, and we're not passing `-Dreadline=enabled`.
> > > 
> > > Additionally, the complex logic necessary to detect readline
> > > correctly, even in the absence of its pkg-config file, is still
> > > present in `meson.build`, so I'm not sure why it's not doing what
> > > it's supposed to do...
> > 
> > Well, I just reported what happened to me yesterday when I was doing some mdev
> > review on CentOS 8 and wanted to do an RPM install of the latest libvirt, meson
> > couldn't be re-built from the latest spec on CentOS 8, but taking the binary
> > package directly from Fedora 32 worked like a charm and then I hit the readline
> > issue for some reason, it's true I've had a bunch of packages (including
> > readline) installed manually in order to overcome the meson issues.
> > I'll re-try in a completely clean environment to see whether the missing
> > readline pkg config was only related to my environment or it actually fails the
> > build.
> 
> I don't understand exactly what issue you ran into based on your
> description, but even just looking at the pipeline shows that
> something is not quite right, since readline was detected correctly
> on CentOS before the switch and now it isn't. I'm hoping Pavel can
> shed some light there :)

Sigh, so the autotools virt-readline.m4 file had a comment that
rl_completion_quote_character function is available in all reasonable
versions.

Well, it's not a function so meson obviously fails to detect it with
cc.has_function(), in addition the readline/readline.h header requires
that the user includes stdio.h before it, again sigh.

My mistake expecting that software requiring some header file will
include it as well, I guess at some point it was not cool to do the
nice thing for users.

I'll post a patch shortly to fix the readline detection.

Pavel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20200805/081e8e07/attachment-0001.sig>