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

On Fri, Sep 20, 2019 at 11:05 AM Pavel Hrdina <phrdina(a)redhat.com&gt; 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 > and build definition files are more readable and easier to maintain. > > There are some major changes with Meson build system: > > - there is no syntax-check target, the syntax-check is part of Meson > test suite but it's still possible to run it separately, > > - Meson forces separation between source and build directories > > Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; > Tested-by: Ján Tomko <jtomko(a)redhat.com&gt; > --- > > Notes: > changes in v2: > > - add -Werror if we are building from git > - fixed -Wframe-larger-than > - removed unrelated fix > - added comment for flake8 ignore warning > - added 'suite' labels 'syntax' and 'unit' for tests > - AUTHORS and libvirt-dbus.spec are generated only when building from git Why? Not opposed to this decision, just want to understand the reason for that.

> - run.in is no longer executable, there is a helper script to fix permissions > for the generated run script > - fixed include_directories for test executable, direct paths can be used > since meson 0.50.0 By using 0.50.0 you're dropping support on: - Debian 10; - Ubuntu 19.04; Personally, I would stick to 0.49.0.

> - flake8 is optional as it was with autotools > - added meson version into spec file

> --- a/autogen.sh > +++ /dev/null > @@ -1,52 +0,0 @@ > -#!/bin/sh > -# Run this to generate all the initial makefiles, etc. > - > -set -e > -srcdir=`dirname $0` > -test -z "$srcdir" && srcdir=. > - > -THEDIR=`pwd` > -cd $srcdir > - > -DIE=0 > - > -for prog in autoreconf automake autoconf libtoolize > -do > - ($prog --version) < /dev/null > /dev/null 2>&1 || { > - echo > - echo "You must have $prog installed to compile libvirt-dbus." > - DIE=1 > - } > -done > - > -if test "$DIE" -eq 1; then > - exit 1 > -fi > - > -if test -z "$*"; then > - echo "I am going to run ./configure with no args - if you " > - echo "wish to pass any extra arguments to it, please specify them on " > - echo "the $0 command line." > -fi > - > -mkdir -p build-aux > -autoreconf -if > - > -cd $THEDIR > - > -if test "x$1" = "x--system"; then > - shift > - prefix=/usr > - libdir=$prefix/lib > - sysconfdir=/etc > - localstatedir=/var > - if [ -d /usr/lib64 ]; then > - libdir=$prefix/lib64 > - fi > - EXTRA_ARGS="--prefix=$prefix --sysconfdir=$sysconfdir --localstatedir=$localstatedir --libdir=$libdir" > -fi --system ended up being remove here but never added back. AFAIU that's a pattern followed by libvirt* projects and, if that's the case, may be worth to adding it back.

> - > -$srcdir/configure $EXTRA_ARGS "$@" && { > - echo > - echo "Now type 'make' to compile libvirt-dbus." > -}

> diff --git a/meson.build b/meson.build > new file mode 100644 > index 0000000..30eeebe > --- /dev/null > +++ b/meson.build > @@ -0,0 +1,279 @@ > +project( > + 'libvirt-dbus', 'c', > + version: '1.4.0', > + license: 'LGPLv2+', > + meson_version: '>= 0.49.0', In case you're really going for 0.50.0, this needs to be bumped ... > + default_options: [ > + 'buildtype=debugoptimized', > + 'c_std=gnu99', > + ], > +) > + > + > +conf = configuration_data() > +conf.set('MESON_VERSION', '0.49.0') ... and this one as well ... > +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) > + > +git = run_command('test', '-d', '.git').returncode() == 0 > + > + > +# 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 = &#39;@0(a)_dir&#39;.format(opt_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 = [ > + &#39;-DVIRT_DBUS_INTERFACES_DIR=&quot;@0(a)&quot;&#39;.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-than=1024', > + '-Wfree-nonheap-object', > + '-Whsa', > + '-Wignored-attributes', > + '-Wignored-qualifiers', > + '-Wimplicit', > + '-Wimplicit-function-declaration', > + '-Wimplicit-int', > + '-Wincompatible-pointer-types', > + '-Winit-self', > + '-Winline', > + '-Wint-conversion', > + '-Wint-to-pointer-cast', > + '-Winvalid-memory-model', > + '-Winvalid-pch', > + '-Wjump-misses-init', > + '-Wlogical-not-parentheses', > + '-Wlogical-op', > + '-Wmain', > + '-Wmaybe-uninitialized', > + '-Wmemset-transposed-args', > + '-Wmisleading-indentation', > + '-Wmissing-braces', > + '-Wmissing-declarations', > + '-Wmissing-field-initializers', > + '-Wmissing-include-dirs', > + '-Wmissing-parameter-type', > + '-Wmissing-prototypes', > + '-Wmultichar', > + '-Wnarrowing', > + '-Wnested-externs', > + '-Wno-cast-function-type', > + '-Wnonnull', > + '-Wnonnull-compare', > + '-Wnormalized=nfc', > + '-Wnull-dereference', > + '-Wodr', > + '-Wold-style-declaration', > + '-Wold-style-definition', > + '-Wopenmp-simd', > + '-Woverflow', > + '-Woverlength-strings', > + '-Woverride-init', > + '-Wpacked', > + '-Wpacked-bitfield-compat', > + '-Wparentheses', > + '-Wpointer-arith', > + '-Wpointer-sign', > + '-Wpointer-to-int-cast', > + '-Wpragmas', > + '-Wreturn-local-addr', > + '-Wreturn-type', > + '-Wscalar-storage-order', > + '-Wsequence-point', > + '-Wshadow', > + '-Wshift-count-negative', > + '-Wshift-count-overflow', > + '-Wshift-negative-value', > + '-Wshift-overflow=2', > + '-Wsizeof-array-argument', > + '-Wsizeof-pointer-memaccess', > + '-Wstack-protector', > + '-Wstrict-aliasing', > + '-Wstrict-prototypes', > + '-Wsuggest-attribute=const', > + '-Wsuggest-attribute=format', > + '-Wsuggest-attribute=noreturn', > + '-Wsuggest-attribute=pure', > + '-Wsuggest-final-methods', > + '-Wsuggest-final-types', > + '-Wswitch', > + '-Wswitch-bool', > + '-Wsync-nand', > + '-Wtautological-compare', > + '-Wtrampolines', > + '-Wtrigraphs', > + '-Wtype-limits', > + '-Wuninitialized', > + '-Wunknown-pragmas', > + '-Wunused', > + '-Wunused-but-set-parameter', > + '-Wunused-but-set-variable', > + '-Wunused-const-variable=2', > + '-Wunused-function', > + '-Wunused-label', > + '-Wunused-local-typedefs', > + '-Wunused-macros', > + '-Wunused-parameter', > + '-Wunused-result', > + '-Wunused-value', > + '-Wunused-variable', > + '-Wvarargs', > + '-Wvariadic-macros', > + '-Wvector-operation-performance', > + '-Wvla', > + '-Wvolatile-register-var', > + '-Wwrite-strings', > + '-fasynchronous-unwind-tables', > + '-fexceptions', > + '-fipa-pure-const', > + '-fno-common', > +] > +ld_flags = [ > + '-Wl,-z,now', > + '-Wl,-z,relro', > +] > + > +if git > + cc_flags += ['-Werror'] > +endif > + > +if host_machine.cpu_family() != 'aarch64' > + if host_machine.system() == 'linux' > + cc_flags += ['-fstack-protector-strong'] > + endif > + if host_machine.system() == 'freebsd' > + cc_flags += ['-fstack-protector'] > + endif > +endif > + > +if host_machine.system() not in ['cygwin', 'windows'] > + cc_flags += ['-fPIE', '-DPIE'] > + ld_flags += ['-pie'] > +endif > + > +cc = meson.get_compiler('c') > +common_flags += cc.get_supported_arguments(cc_flags) > +link_flags = cc.get_supported_link_arguments(ld_flags) > + > +add_project_arguments(common_flags, language: 'c') > +add_project_link_arguments(link_flags, language: 'c') > + > + > +# Generate run helper > + > +configure_file( > + input: 'run.in', > + output: 'run', > + configuration: conf, > +) > +run_command('tools/fix-perm.sh', 'a+x', 'run') > + > + > +# Generate dist files > + > +if git > + configure_file( > + input: 'libvirt-dbus.spec.in', > + output: 'libvirt-dbus.spec', > + configuration: conf, > + ) > + > + authors = run_command('tools/gen-authors.sh') > + configure_file( > + configuration: { 'contributorslist': authors.stdout() }, > + input: 'AUTHORS.in', > + output: 'AUTHORS', > + ) > + > + foreach file : [ 'libvirt-dbus.spec', 'AUTHORS' ] > + path = join_paths(meson.build_root(), file) > + meson.add_dist_script( > + 'sh', > + '-c', > + 'cp "@0@" "$MESON_DIST_ROOT/"'.format(path), > + ) > + endforeach You're using scripts everywhere, I'd use one here as well, as suggested by Andrea during our face-to-face meeting.

> +endif > + > + > +# Include sub-directories > + > +subdir('data') > +subdir('docs') > +subdir('src') > +subdir('tests')

In general, it looks good and works as expected. I will add my "Reviewed-by: " after we discuss the points raised. Another thing, please, let's sync to have the libvirt-jenkins-ci work done and merged before this one gets merged.