[libvirt] [dbus PATCH] build: convert to Meson/Ninja build system

Pavel Hrdina phrdina at redhat.com
Tue Sep 17 18:04:19 UTC 2019


On Tue, Sep 17, 2019 at 06:39:17PM +0200, Ján Tomko wrote:
> On Tue, Sep 17, 2019 at 05:20:43PM +0200, Pavel Hrdina wrote:
> > Meson build system is simple and quick compared to Autotools and it's
> > able to fully replace our Autotools usage.  There are few drawbacks as
> > it's a fairly new build system, it requires Python 3.5 and Ninja 1.5.0,
> > it's still evolving and the user base is not that large and there were
> > some tweaks required to achieve the same functionality.
> > 
> > However, there are benefits, the configure and build time is way shorter
> 
> Neat, despite the colors
> 
> > and build definition files are more readable and easier to maintain.
> > 
> > Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
> > ---
> > .gitignore                  |   1 +
> > AUTHORS.in                  |   2 +-
> > HACKING.md                  |  16 +--
> > Makefile.am                 |  51 -------
> > README.md                   |  12 +-
> > autogen.sh                  |  52 -------
> > configure.ac                |  87 ------------
> > data/Makefile.am            |  83 -----------
> > data/meson.build            |  15 ++
> > data/session/meson.build    |   6 +
> > data/system/meson.build     |  18 +++
> > docs/Makefile.am            |  21 ---
> > docs/meson.build            |   8 ++
> > libvirt-dbus.spec.in        |   8 +-
> > m4/manywarnings.m4          | 276 ------------------------------------
> > m4/virt-arg.m4              | 154 --------------------
> > m4/virt-compile-pie.m4      |  35 -----
> > m4/virt-compile-warnings.m4 | 203 --------------------------
> > m4/virt-linker-relro.m4     |  35 -----
> > m4/warnings.m4              |  79 -----------
> > meson.build                 | 271 +++++++++++++++++++++++++++++++++++
> > meson_options.txt           |   6 +
> > run.in                      |   4 +-
> > src/Makefile.am             |  66 ---------
> > src/domain.c                |   2 +-
> > src/meson.build             |  54 +++++++
> > tests/Makefile.am           |  57 --------
> > tests/meson.build           |  49 +++++++
> > tools/gen-authors.sh        |   8 ++
> > 29 files changed, 454 insertions(+), 1225 deletions(-)
> > delete mode 100644 Makefile.am
> > delete mode 100755 autogen.sh
> > delete mode 100644 configure.ac
> > delete mode 100644 data/Makefile.am
> > create mode 100644 data/meson.build
> > create mode 100644 data/session/meson.build
> > create mode 100644 data/system/meson.build
> > delete mode 100644 docs/Makefile.am
> > create mode 100644 docs/meson.build
> > delete mode 100644 m4/manywarnings.m4
> > delete mode 100644 m4/virt-arg.m4
> > delete mode 100644 m4/virt-compile-pie.m4
> > delete mode 100644 m4/virt-compile-warnings.m4
> > delete mode 100644 m4/virt-linker-relro.m4
> > delete mode 100644 m4/warnings.m4
> > create mode 100644 meson.build
> > create mode 100644 meson_options.txt
> > mode change 100644 => 100755 run.in
> > delete mode 100644 src/Makefile.am
> > create mode 100644 src/meson.build
> > delete mode 100644 tests/Makefile.am
> > create mode 100644 tests/meson.build
> > create mode 100755 tools/gen-authors.sh
> > 
> > -dist-hook: gen-AUTHORS
> > -
> > -# Generate the AUTHORS file (with all entries since the switch to git)
> > -# and insert it into the directory we're about to use to create a tarball.
> > -.PHONY: gen-AUTHORS
> > -gen-AUTHORS:
> > -	$(AM_V_GEN)\
> > -	if test -d $(srcdir)/.git; then \
> > -	  ( \
> > -	    cd $(srcdir) && \
> > -	    git log --pretty=format:'    %aN <%aE>' | sort -u \
> > -	  ) > all.list && \
> > -	  sort -u $(srcdir)/AUTHORS.in > maint.list && \
> > -	  comm -23 all.list maint.list > contrib.list && \
> 
> This filtering is not present in the meson version, but I can live with
> that "regression"

I can fix that, will look into it.

> > -	  contrib="`cat contrib.list`" && \
> > -	  perl -p -e "s/#contributorslist#// and print '$$contrib'" \
> > -	    < $(srcdir)/AUTHORS.in > $(distdir)/AUTHORS-tmp && \
> > -	  mv -f $(distdir)/AUTHORS-tmp $(distdir)/AUTHORS && \
> > -	  rm -f all.list maint.list contrib.list; \
> > -	fi
> > -
> 
> 
> > diff --git a/configure.ac b/configure.ac
> > deleted file mode 100644
> > index 24ebb26..0000000
> > --- a/configure.ac
> > +++ /dev/null
> > -LIBVIRT_DBUS_MAJOR_VERSION=`echo $VERSION | awk -F. '{print $1}'`
> > -LIBVIRT_DBUS_MINOR_VERSION=`echo $VERSION | awk -F. '{print $2}'`
> > -LIBVIRT_DBUS_MICRO_VERSION=`echo $VERSION | awk -F. '{print $3}'`
> > -LIBVIRT_DBUS_VERSION=$LIBVIRT_DBUS_MAJOR_VERSION.$LIBVIRT_DBUS_MINOR_VERSION.$LIBVIRT_DBUS_MICRO_VERSION$LIBVIRT_DBUS_MICRO_VERSION_SUFFIX
> > -LIBVIRT_DBUS_VERSION_NUMBER=`expr $LIBVIRT_DBUS_MAJOR_VERSION \* 1000000 + $LIBVIRT_dbus_MINOR_VERSION \* 1000 + $LIBVIRT_DBUS_MICRO_VERSION`
> > -
> > -AC_SUBST([LIBVIRT_DBUS_MAJOR_VERSION])
> > -AC_SUBST([LIBVIRT_DBUS_MINOR_VERSION])
> > -AC_SUBST([LIBVIRT_DBUS_MICRO_VERSION])
> > -AC_SUBST([LIBVIRT_DBUS_VERSION])
> > -AC_SUBST([LIBVIRT_DBUS_VERSION_INFO])
> > -AC_SUBST([LIBVIRT_DBUS_VERSION_NUMBER])
> > -
> 
> We lose these, but they were unusued.
> 
> > -AC_PROG_CC
> > -AC_PROG_MKDIR_P
> > -AM_PROG_CC_C_O
> > -AC_PROG_CC_STDC
> > -AC_PROG_LIBTOOL
> > -AC_PATH_PROGS([FLAKE8], [flake8 flake8-3.6])
> > -
> > diff --git a/meson.build b/meson.build
> > new file mode 100644
> > index 0000000..9cc4417
> > --- /dev/null
> > +++ b/meson.build
> > @@ -0,0 +1,271 @@
> > +project(
> > +    'libvirt-dbus', 'c',
> > +    version:'1.4.0',
> > +    license: 'LGPLv2+',
> > +    meson_version: '>= 0.49.0',
> > +    default_options: [
> > +        'buildtype=debugoptimized',
> > +        'c_std=gnu99',
> > +    ]
> > +)
> > +
> > +
> > +conf = configuration_data()
> > +conf.set('PACKAGE', meson.project_name())
> > +conf.set('VERSION', meson.project_version())
> > +conf.set('build_root', meson.build_root())
> > +conf.set('sbindir', get_option('sbindir'))
> > +conf.set('source_root', meson.source_root())
> > +
> > +# Dependencies
> > +
> > +glib2_version = '2.44.0'
> > +libvirt_version = '3.0.0'
> > +libvirt_glib_version = '0.0.7'
> > +
> > +dep_gio_unix = dependency('gio-unix-2.0', version: '>=' + glib2_version)
> > +dep_glib = dependency('glib-2.0', version: '>=' + glib2_version)
> > +dep_libvirt = dependency('libvirt', version: '>=' + libvirt_version)
> > +dep_libvirt_glib = dependency('libvirt-glib-1.0', version: '>=' + libvirt_glib_version)
> > +
> > +conf.set('GLIB2_REQUIRED', glib2_version)
> > +conf.set('LIBVIRT_REQUIRED', libvirt_version)
> > +conf.set('LIBVIRT_GLIB_REQUIRED', libvirt_glib_version)
> > +
> > +
> > +# Configure options
> > +
> > +conf.set('SYSTEM_USER', get_option('system_user'))
> > +
> > +opt_dirs = [
> > +    'dbus_interfaces',
> > +    'dbus_services',
> > +    'dbus_system_services',
> > +    'dbus_system_policies',
> > +    'polkit_rules',
> > +]
> > +
> > +foreach opt_dir : opt_dirs
> > +    value = get_option(opt_dir)
> > +    varname = opt_dir + '_dir'
> > +    if opt_dir.startswith('/')
> > +        set_variable(varname, value)
> > +    else
> > +        set_variable(varname, join_paths(get_option('datadir'), value))
> > +    endif
> > +endforeach
> > +
> > +
> > +# Compile flags
> > +
> > +common_flags = [
> > +    '-DVIRT_DBUS_INTERFACES_DIR="@0@"'.format(dbus_interfaces_dir),
> > +]
> > +
> > +cc_flags = [
> > +    '-W',
> > +    '-Waddress',
> > +    '-Waggressive-loop-optimizations',
> > +    '-Wall',
> > +    '-Warray-bounds=2',
> > +    '-Wattributes',
> > +    '-Wbad-function-cast',
> > +    '-Wbool-compare',
> > +    '-Wbuiltin-macro-redefined',
> > +    '-Wcast-align',
> > +    '-Wchar-subscripts',
> > +    '-Wclobbered',
> > +    '-Wcomment',
> > +    '-Wcomments',
> > +    '-Wcoverage-mismatch',
> > +    '-Wcpp',
> > +    '-Wdate-time',
> > +    '-Wdeprecated-declarations',
> > +    '-Wdesignated-init',
> > +    '-Wdiscarded-array-qualifiers',
> > +    '-Wdiscarded-qualifiers',
> > +    '-Wdiv-by-zero',
> > +    '-Wdouble-promotion',
> > +    '-Wduplicated-cond',
> > +    '-Wempty-body',
> > +    '-Wendif-labels',
> > +    '-Wextra',
> > +    '-Wformat',
> > +    '-Wformat-contains-nul',
> > +    '-Wformat-extra-args',
> > +    '-Wformat-nonliteral',
> > +    '-Wformat-security',
> > +    '-Wformat-y2k',
> > +    '-Wformat-zero-length',
> > +    '-Wframe-address',
> 
> > +    '-Wframe-larger-then=256',
> 
> s/then/than/
> 
> Also, 256 is not enough:
> ../src/connect.c:1613:1: error: stack frame size of 552 bytes in function 'virtDBusConnectNodeGetSecurityModel' [-Werror,-Wframe-larger-than=]

That explains why it was not working, will fix that.

> 
> and -Werror is missing from the list

It should not be there by default, but I'll add it conditionally if we
are running meson from git.

> > +    '-Wfree-nonheap-object',
> > +    '-Whsa',
> 
> > diff --git a/src/domain.c b/src/domain.c
> > index e8b1a0e..10fa8de 100644
> > --- a/src/domain.c
> > +++ b/src/domain.c
> > @@ -136,7 +136,7 @@ virtDBusDomainGVariantToCpumap(GVariantIter *iter,
> >     guint cnt = 0;
> > 
> >     *cpumaplen = VIR_CPU_MAPLEN(cpus);
> > -    *cpumap = g_new0(guchar, cpumaplen);
> > +    *cpumap = g_new0(guchar, *cpumaplen);
> > 
> >     while (g_variant_iter_loop(iter, "b", &usable)) {
> >         if (usable)
> 
> Unrelated fix that should go in as a separate patch.
> To this hunk:
> Reviewed-by: Ján Tomko <jtomko at redhat.com>

Right, I don't know how it slipped into the meson work. Thanks, I'll
push it separately.

> > +flake8 = find_program('flake8', 'flake8-3', required: false)
> > +if flake8.found()
> > +    test(
> > +        'flake8', flake8,
> > +        args: [
> > +            '--show-source',
> > +            '--ignore=E501',
> > +            meson.source_root(),
> > +        ]
> > +    )
> 
> Can you include the text representation of E501?
> # E501: (line too long) warning is ignored.

Good idea, I'll add it there.

> The rest looks good to me, but I presume someone who actually touched
> meson before should take a look.
> 
> Tested-by: Ján Tomko <jtomko at redhat.com>

Thanks for the review, I'll fix the issues pointed out.

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/20190917/39674700/attachment-0001.sig>


More information about the libvir-list mailing list