Re: [libvirt PATCH v4] ci: Switch to meson build system
On Fri, 2020-11-27 at 11:28 +0100, Erik Skultety wrote:
On Fri, Nov 27, 2020 at 10:44:56AM +0100, Andrea Bolognani wrote:
> On Fri, 2020-11-27 at 09:46 +0100, Erik Skultety wrote:
> > +++ b/ci/Makefile
> > -ci-check@%:
> > - $(MAKE) -C $(CI_ROOTDIR) ci-build@$* CI_MAKE_ARGS="check"
> > +ci-test@%:
> > + $(MAKE) -C $(CI_ROOTDIR) ci-build@$* CI_NINJA_ARGS+=test
>
> I don't know why this last bit turned from =test in v3 to +=test in
> v4, but I don't think it should have. Please change it back.
Deliberate decision...the whole point of ci-test is to test, you may want to
add more flags to ninja but still want to test, we don't document anywhere that
the flags user sets override our defaults - I've seen user flags to be appended
to global env variables in Makefiles so I did the same here. If you want me to
change it back without documenting that when user selects a CI target expecting
something to happen turning out completely different because they set a certain
variable according to the help, I can sure do that (I don't care that much),
but it's IMO confusing and this way it's more foolproof.
Since you're just converting from autotools to meson in this commit,
the underlying behavior should probably not change, so I'd leave it
as it is regardless of whether or not you consider the change to be
for the better.
ci-test was always intended to be a convenient shorthand anyway, the
idea being that you'd use it for the basic case and then switch to
ci-build CI_NINJA_ARGS=... as soon as you started to outgrow it.
--
Andrea Bolognani / Red Hat / Virtualization