* [PATCH] libvirt: disable Werror for non-libvirt flights
@ 2022-09-13 10:03 Roger Pau Monne
2022-09-13 12:54 ` Ian Jackson
2022-09-15 14:10 ` Anthony PERARD
0 siblings, 2 replies; 7+ messages in thread
From: Roger Pau Monne @ 2022-09-13 10:03 UTC (permalink / raw)
To: xen-devel; +Cc: Roger Pau Monne, Ian Jackson, Anthony PERARD, Julien Grall
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
Signed-off-by: Roger Pau Monné <roger.pau@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 <iwj@xenproject.org>
Cc: Anthony PERARD <anthony.perard@citrix.com>
Cc: Julien Grall <julien@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";;
esac
job_create_build build-$arch-libvirt build-libvirt \
diff --git a/ts-libvirt-build b/ts-libvirt-build
index 16b45cfd..e4faa1d7 100755
--- a/ts-libvirt-build
+++ b/ts-libvirt-build
@@ -73,7 +73,7 @@ sub config() {
--with-libxl --without-xen --without-xenapi --without-selinux \\
--without-lxc --without-vbox --without-uml \\
--without-qemu --without-openvz --without-vmware \\
- --sysconfdir=/etc --localstatedir=/var #/
+ --sysconfdir=/etc --localstatedir=/var $r{autogen_options} #/
END
} else {
target_cmd_build($ho, 3600, $builddir, <<END);
@@ -87,6 +87,7 @@ END
-Ddriver_libvirtd=enabled \\
-Ddriver_remote=enabled \\
--sysconfdir=/etc --localstatedir=/var \\
+ $r{meson_options} \\
build
END
}
--
2.37.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] libvirt: disable Werror for non-libvirt flights
2022-09-13 10:03 [PATCH] libvirt: disable Werror for non-libvirt flights Roger Pau Monne
@ 2022-09-13 12:54 ` Ian Jackson
2022-09-13 13:15 ` Roger Pau Monné
2022-09-15 14:10 ` Anthony PERARD
1 sibling, 1 reply; 7+ messages in thread
From: Ian Jackson @ 2022-09-13 12:54 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel, Anthony PERARD, Julien Grall
Roger Pau Monne writes ("[PATCH] libvirt: disable Werror for non-libvirt flights"):
> 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.
..
> +build-i386-libvirt autogen_options --disable-werror
We have no way to specify -Wno-error-switch-enum specifically ?
(I'm not sure if that would be desirable.)
> I'm unsure whether we want o disable Werror even for libvirt flights,
> but this seems more conservative.
Probably disabling it only for Xen is right.
Ian.
--
Ian Jackson <ijackson@chiark.greenend.org.uk> These opinions are my own.
Pronouns: they/he. If I emailed you from @fyvzl.net or @evade.org.uk,
that is a private address which bypasses my fierce spamfilter.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] libvirt: disable Werror for non-libvirt flights
2022-09-13 12:54 ` Ian Jackson
@ 2022-09-13 13:15 ` Roger Pau Monné
0 siblings, 0 replies; 7+ messages in thread
From: Roger Pau Monné @ 2022-09-13 13:15 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel, Anthony PERARD, Julien Grall
On Tue, Sep 13, 2022 at 01:54:12PM +0100, Ian Jackson wrote:
> Roger Pau Monne writes ("[PATCH] libvirt: disable Werror for non-libvirt flights"):
> > 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.
> ..
> > +build-i386-libvirt autogen_options --disable-werror
>
> We have no way to specify -Wno-error-switch-enum specifically ?
> (I'm not sure if that would be desirable.)
Hm, maybe playing with CFLAGS, but not from the autogen/meson options
AFAIK. Using the autogen/meson flags seems cleaner and less error
prone (albeit the disabling of Werror is more wide than what we
strictly require).
> > I'm unsure whether we want o disable Werror even for libvirt flights,
> > but this seems more conservative.
>
> Probably disabling it only for Xen is right.
Thanks, let's try this first then.
Roger.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] libvirt: disable Werror for non-libvirt flights
2022-09-13 10:03 [PATCH] libvirt: disable Werror for non-libvirt flights Roger Pau Monne
2022-09-13 12:54 ` Ian Jackson
@ 2022-09-15 14:10 ` Anthony PERARD
2022-09-15 15:47 ` Roger Pau Monné
1 sibling, 1 reply; 7+ messages in thread
From: Anthony PERARD @ 2022-09-15 14:10 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel, Ian Jackson, Julien Grall
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?
>
> Signed-off-by: Roger Pau Monné <roger.pau@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 <iwj@xenproject.org>
> Cc: Anthony PERARD <anthony.perard@citrix.com>
> Cc: Julien Grall <julien@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.
Thanks,
--
Anthony PERARD
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] libvirt: disable Werror for non-libvirt flights
2022-09-15 14:10 ` Anthony PERARD
@ 2022-09-15 15:47 ` Roger Pau Monné
2022-09-16 0:20 ` Ian Jackson
0 siblings, 1 reply; 7+ messages in thread
From: Roger Pau Monné @ 2022-09-15 15:47 UTC (permalink / raw)
To: Anthony PERARD, Ian Jackson; +Cc: xen-devel, Julien Grall
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.pau@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 <iwj@xenproject.org>
> > Cc: Anthony PERARD <anthony.perard@citrix.com>
> > Cc: Julien Grall <julien@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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] libvirt: disable Werror for non-libvirt flights
2022-09-15 15:47 ` Roger Pau Monné
@ 2022-09-16 0:20 ` Ian Jackson
2022-09-16 8:19 ` Roger Pau Monné
0 siblings, 1 reply; 7+ messages in thread
From: Ian Jackson @ 2022-09-16 0:20 UTC (permalink / raw)
To: Roger Pau Monné; +Cc: Anthony PERARD, xen-devel, Julien Grall
Roger Pau Monné writes ("Re: [PATCH] libvirt: disable Werror for non-libvirt flights"):
> I don't mind using -Dwerror=false if that's considered better. Ian, do
> you have an opinion?
No, I don't think I do. I think it depends on what kinds of things
are likely to change, or go wrong, in libvirt.
- *) libvirt_build_runvars+=" make_njobs=1";;
+ *) libvirt_build_runvars+=" make_njobs=1 meson_options=-Dwerror=false autogen_options=--disable-werror";;
I wonder if it would be better to abstract this away and instead have
a runvar like "libvrit_build_werror=false". But maybe that is
gold-plating it.
If you choose to keep that the way it is, then for either version of
this patch:
Acked-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
--
Ian Jackson <ijackson@chiark.greenend.org.uk> These opinions are my own.
Pronouns: they/he. If I emailed you from @fyvzl.net or @evade.org.uk,
that is a private address which bypasses my fierce spamfilter.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] libvirt: disable Werror for non-libvirt flights
2022-09-16 0:20 ` Ian Jackson
@ 2022-09-16 8:19 ` Roger Pau Monné
0 siblings, 0 replies; 7+ messages in thread
From: Roger Pau Monné @ 2022-09-16 8:19 UTC (permalink / raw)
To: Ian Jackson; +Cc: Anthony PERARD, xen-devel, Julien Grall
On Fri, Sep 16, 2022 at 01:20:11AM +0100, Ian Jackson wrote:
> Roger Pau Monné writes ("Re: [PATCH] libvirt: disable Werror for non-libvirt flights"):
> > I don't mind using -Dwerror=false if that's considered better. Ian, do
> > you have an opinion?
>
> No, I don't think I do. I think it depends on what kinds of things
> are likely to change, or go wrong, in libvirt.
>
> - *) libvirt_build_runvars+=" make_njobs=1";;
> + *) libvirt_build_runvars+=" make_njobs=1 meson_options=-Dwerror=false autogen_options=--disable-werror";;
>
> I wonder if it would be better to abstract this away and instead have
> a runvar like "libvrit_build_werror=false". But maybe that is
> gold-plating it.
Did consider this initially, but decided to use specific options for
mesa/autogen in case we need to add further switches to specific
configuration systems (unlikely I guess, because autogen won't be used
anymore).
> If you choose to keep that the way it is, then for either version of
> this patch:
>
> Acked-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
Thanks, Roger.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-09-16 8:20 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-13 10:03 [PATCH] libvirt: disable Werror for non-libvirt flights Roger Pau Monne
2022-09-13 12:54 ` Ian Jackson
2022-09-13 13:15 ` Roger Pau Monné
2022-09-15 14:10 ` Anthony PERARD
2022-09-15 15:47 ` Roger Pau Monné
2022-09-16 0:20 ` Ian Jackson
2022-09-16 8:19 ` Roger Pau Monné
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.