Re: [libvirt PATCH 342/351] meson: add syntax-check

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jul 29, 2020 at 12:00:46PM +0200, Peter Krempa wrote:
> On Wed, Jul 29, 2020 at 11:47:31 +0200, Pavel Hrdina wrote:
> > On Wed, Jul 29, 2020 at 10:29:46AM +0100, Daniel P. Berrangé wrote:
> > > On Wed, Jul 29, 2020 at 09:11:11AM +0200, Pavel Hrdina wrote:
> > > > So I was trying to figure out what to do with our syntax-check and this
> > > > could be one solution:
> > > > 
> > > > 
> > > > rc = run_command(
> > > >   'sed', '-n',
> > > >   's/^\\(sc_[a-zA-Z0-9_-]*\\):.*/\\1/p',
> > > >   meson.current_source_dir() / 'syntax-check.mk',
> > > >   check: true,
> > > > )
> > > > 
> > > > sc_tests = rc.stdout().strip().split()
> > > > 
> > > > 
> > > > This is how syntax-check.mk gets the list of targets to run for
> > > > syntax-check target. We can use the same list to define tests like this:
> > > > 
> > > > 
> > > > foreach target : sc_tests
> > > >   rc = run_command(
> > > >     python3_prog, '-c',
> > > >     'print("@0@".replace("sc_", ""))'.format(target),
> > > >     check: true,
> > > >     env: runutf8,
> > > >   )
> > > >   name = rc.stdout().strip()
> > > > 
> > > >   test(
> > > >     name,
> > > >     make_prog,
> > > >     args: [ '-C', meson.current_build_dir(), target ],
> > > >     depends: [
> > > >       potfiles_dep,
> > > >     ],
> > > >     suite: 'syntax-check',
> > > >   )
> > > > endforeach
> > > 
> > > I like this idea as it eliminates a little bit more of the "make"
> > > usage. BTW, can we just run them more directly instead of via
> > > "python_prog" ? The tests don't use python, so avoiding creating
> > > a python intepretor for each syntax check rule probably wins for
> > > performance a litle
> > 
> > The run_command() using python3_prog is executed during `meson setup`
> > phase and the only purpose of that is to rename `sc_test_name` to
> > `test_name`. It will not affect the performance of running meson test
> > as that one will execute only `make_prog -C builddir sc_test_name`.
> > 
> > I'm OK with dropping the run_command() part completely, which would make
> > the output of `meson test` look like this:
> > 
> > 147/154 libvirt:syntax-check / sc_prohibit_test_double_equal  OK
> 
> I think this is okay. It even helps you finding the checking rule.
> 
> > 
> > instead of
> > 
> > 147/154 libvirt:syntax-check / prohibit_test_double_equal  OK
> > 
> > I just realized a huge drawback of this approach. In order to run
> > `meson test` or `ninja test` it will first compile everything. It can be
> > disabled by running `meson test --no-rebuild` which will ignore explicit
> > dependencies as well.
> > 
> > To workaround it for our CI codestyle job we would have to run these
> > commands:
> > 
> >     meson build
> >     ninja -C build libvirt-pot-dep
> >     meson test -C build --suite syntax-check --no-rebuild
> 
> IMO we can live with this. CI can be fixed. Downstreams which
> specifically care about backporting non-conformat patches will probably
> disable the syntax-check suite anyways. For developers, you usually
> compile stuff anyways before running tests.

Perfect, I'm OK with all of the above so I'll go with it.

Thanks

Pavel

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux