On Fri, Sep 20, 2019 at 01:28:28PM +0200, Fabiano Fidêncio wrote:
> On Fri, Sep 20, 2019 at 11:05 AM Pavel Hrdina <phrdina@xxxxxxxxxx> 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@xxxxxxxxxx>
> > Tested-by: Ján Tomko <jtomko@xxxxxxxxxx>
> > ---
> >
> > 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.
This is what was done by Autotools, at least for AUTHORS. If you are
compiling libvirt-dbus from tarball you don't have git so you will not
be able to generate AUTHORS. For the spec file my idea was that there
is no need to regenerate it from tarball as that one is usually used as
it is.
> > - 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.
Ignored per following email :).
> > - 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.
Good point, should have mentioned it in the commit message.
The --system option is usually used only by libvirt developers if they
need to run libvirt from git with system configuration files, I doubt
that anyone is actually using it with libvirt-dbus as there is probably
no need at all.
> > -
> > -$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 = '@0@_dir'.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 = [
> > + '-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-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.
Right, I knew that I've missed something, will fix, thanks.
> > +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.
Works for me, thanks for review, I'll fix the dist script to use a shell
script.
Pavel
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
