On Thu, Sep 15, 2022 at 03:10:59PM +0100, Anthony PERARD wrote:
> On Tue, Sep 13, 2022 at 12:03:28PM +0200, Roger Pau Monne wrote:
> > Current usage of Werror=switch-enum by default for libvirt builds out
> > of the git tree causes issues when new items are added to libxl public
> > API enums if those are used in a switch statement in libvirt code.
> > This leads to libvirt build failures for seemingly unrelated libxl
> > changes.
> >
> > In order to prevent those errors from blocking the push gate, disable
> > Werror for libvirt builds when not in a libvirt specific flight.
> >
> > The errors will be reported on the libvirt flight, and block the
> > pushes there. So the author of the changes in libxl is still expected
> > to send a fix to libvirt code. This is no ideal, but the other option
> > is to just disable Werror for all libvirt builds and let libvirt
> > developers fix the breakage when they notice it.
> >
> > runvar differences for a xen-unstable flight are:
> >
> > --- /dev/fd/63 2022-09-13 09:53:58.044441678 +0000
> > +++ /dev/fd/62 2022-09-13 09:53:58.044441678 +0000
> > @@ -574,6 +574,10 @@
> > test-xtf-amd64-amd64-3 arch
> > amd64
> > test-xtf-amd64-amd64-4 arch
> > amd64
> > test-xtf-amd64-amd64-5 arch
> > amd64
> > +build-amd64-libvirt autogen_options
> > --disable-werror
> > +build-arm64-libvirt autogen_options
> > --disable-werror
> > +build-armhf-libvirt autogen_options
> > --disable-werror
> > +build-i386-libvirt autogen_options
> > --disable-werror
> > test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm bios
> > seabios
> > test-amd64-amd64-qemuu-nested-amd bios
> > seabios
> > test-amd64-amd64-qemuu-nested-intel bios
> > seabios
> > @@ -1217,6 +1221,10 @@
> > build-arm64-libvirt make_njobs
> > 1
> > build-armhf-libvirt make_njobs
> > 1
> > build-i386-libvirt make_njobs
> > 1
> > +build-amd64-libvirt meson_options
> > -Dgit_werror=disabled
> > +build-arm64-libvirt meson_options
> > -Dgit_werror=disabled
> > +build-armhf-libvirt meson_options
> > -Dgit_werror=disabled
> > +build-i386-libvirt meson_options
> > -Dgit_werror=disabled
> > test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict recipe_dmrestrict
> > true
> > test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict recipe_dmrestrict
> > true
> > test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict recipe_nomigrate
> > true
>
> For "osstest" flight or "xen-unstable-smoke" flight, we would have the
> same difference, right?
>
> The only branch with no change would be libvirt, right?
Indeed, that's the intention.
> >
> > Signed-off-by: Roger Pau Monné <roger....@citrix.com>
> > ---
> > I'm unsure whether we want o disable Werror even for libvirt flights,
> > but this seems more conservative.
> >
> > This does at least unblock the libvirt builds for both the
> > xen-unstable and the libvirt flights.
> > ---
> > Cc: Ian Jackson <i...@xenproject.org>
> > Cc: Anthony PERARD <anthony.per...@citrix.com>
> > Cc: Julien Grall <jul...@xen.org>
> > ---
> > mfi-common | 2 +-
> > ts-libvirt-build | 3 ++-
> > 2 files changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/mfi-common b/mfi-common
> > index 59e712f4..450229e9 100644
> > --- a/mfi-common
> > +++ b/mfi-common
> > @@ -459,7 +459,7 @@ create_build_jobs () {
> > libvirt_build_runvars=''
> > case "$branch" in
> > libvirt*) ;;
> > - *) libvirt_build_runvars+=" make_njobs=1";;
> > + *) libvirt_build_runvars+=" make_njobs=1
> > meson_options=-Dgit_werror=disabled autogen_options=--disable-werror";;
>
> For meson, I think '-Dwerror=false' would be enough, instead of the
> unusual 'git_werror' configuration.
>
> But, we might not need to disable all errors, for meson we can have:
> -Dc_args='-Wno-error=switch -Wno-error=switch-enum'
>
> But disabling werror is fine too, as less likely to be an issue later.
>
> Both 'werror' and 'c_args' seems to be meson built-in options rather
> than options implemented for only libvirt.
> https://mesonbuild.com/Builtin-options.html
> While 'git_werror' is libvirt only.
I don't have a strong opinion really, I've used git_werror because
that's the first thing that I found in:
https://libvirt.org/git/?p=libvirt.git;a=blob;f=meson_options.txt
I don't mind using -Dwerror=false if that's considered better. Ian, do
you have an opinion?
Thanks, Roger.
