1:04 p.m.
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(a)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(a)"'.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(a)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(a)redhat.com>
Thanks for the review, I'll fix the issues pointed out.
Pavel