For some older CLang (or in combination with !intel arch) the
-fsemantic-interposition might be not present. But in that case,
we still want our mocking in test suite to work properly. Disable
optimizations as that seems to be the only way.
Also, don't forget to drop this when FreeBSD 11 and macOS 12 are
dropped.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
This still doesn't work when somebody overrides CFLAGS as those are
placed AFTER our supported_cc_flags:
$ export CFLAGS="-O2 -ggdb"
$ meson setup _build
$ ninja -v -C _build/
[1/1202] clang ... -O2 -g ... -O0 -O2 -ggdb -fPIC ... -c ../src/util/glibcompat.c
meson.build | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/meson.build b/meson.build
index a0682e8d0b..249505d474 100644
--- a/meson.build
+++ b/meson.build
@@ -492,6 +492,13 @@ if get_option('warning_level') == '2'
endif
endif
+
+if cc.get_id() == 'clang' and not supported_cc_flags.contains('-fsemantic-interposition')
+ # If CLang is old enough to not support -fsemantic-interposition
+ # then this is our best bet to make mocking in tests working properly.
+ supported_cc_flags += [ '-O0' ]
+endif
+
add_project_arguments(supported_cc_flags, language: 'c')
if cc.has_argument('-Wsuggest-attribute=format')
--
2.39.2
On Tue, Mar 21, 2023 at 04:11:33PM +0100, Michal Privoznik wrote: > For some older CLang (or in combination with !intel arch) the > -fsemantic-interposition might be not present. But in that case, > we still want our mocking in test suite to work properly. Disable > optimizations as that seems to be the only way. > > Also, don't forget to drop this when FreeBSD 11 and macOS 12 are > dropped. > > Signed-off-by: Michal Privoznik <mprivozn@redhat.com> > --- > > This still doesn't work when somebody overrides CFLAGS as those are > placed AFTER our supported_cc_flags: > > $ export CFLAGS="-O2 -ggdb" > $ meson setup _build > $ ninja -v -C _build/ > [1/1202] clang ... -O2 -g ... -O0 -O2 -ggdb -fPIC ... -c ../src/util/glibcompat.c > > meson.build | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/meson.build b/meson.build > index a0682e8d0b..249505d474 100644 > --- a/meson.build > +++ b/meson.build > @@ -492,6 +492,13 @@ if get_option('warning_level') == '2' > endif > > endif > + > +if cc.get_id() == 'clang' and not supported_cc_flags.contains('-fsemantic-interposition') > + # If CLang is old enough to not support -fsemantic-interposition > + # then this is our best bet to make mocking in tests working properly. > + supported_cc_flags += [ '-O0' ] > +endif I don't like the idea of forcing -O0 for the production builds, just to work around the problem of our broken tests. Can we approach it from the opposite POV and disable building of tests, if we see meson optimization level is > 0 eg something roughly like this: with_tests = true if cc.get_id() == 'clang' and not supported_cc_flags.contains('-fsemantic-interposition') and get_option('optimization') != 0 with_tests = false endif if with_tests subdir('tests') endif So people can choose to have tests work or not With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 3/21/23 16:25, Daniel P. Berrangé wrote: > On Tue, Mar 21, 2023 at 04:11:33PM +0100, Michal Privoznik wrote: >> <snip/> > > I don't like the idea of forcing -O0 for the production builds, just to > work around the problem of our broken tests. Can we approach it from the > opposite POV and disable building of tests, if we see meson optimization > level is > 0 > > eg something roughly like this: > > with_tests = true > if cc.get_id() == 'clang' and > not supported_cc_flags.contains('-fsemantic-interposition') and > get_option('optimization') != 0 > with_tests = false > endif > > if with_tests > subdir('tests') > endif > > > So people can choose to have tests work or not That could work too, yeah. My reasoning for going with -O0 was that it's very rare that somebody would use such old CLang, but I guess disabling tests is less invasive. I'll send v2 shortly. Michal
On Tue, Mar 21, 2023 at 04:31:00PM +0100, Michal Prívozník wrote: > On 3/21/23 16:25, Daniel P. Berrangé wrote: > > On Tue, Mar 21, 2023 at 04:11:33PM +0100, Michal Privoznik wrote: > >> <snip/> > > > > I don't like the idea of forcing -O0 for the production builds, just to > > work around the problem of our broken tests. Can we approach it from the > > opposite POV and disable building of tests, if we see meson optimization > > level is > 0 > > > > eg something roughly like this: > > > > with_tests = true > > if cc.get_id() == 'clang' and > > not supported_cc_flags.contains('-fsemantic-interposition') and > > get_option('optimization') != 0 > > with_tests = false > > endif > > > > if with_tests > > subdir('tests') > > endif > > > > > > So people can choose to have tests work or not > > That could work too, yeah. My reasoning for going with -O0 was that it's > very rare that somebody would use such old CLang, but I guess disabling > tests is less invasive. I'll send v2 shortly. It isn't just old Clang. Latest clang lacks -fsemantic-interposition on non-x86_64, which means current macOS M1 platform today. For our CI, I guess we'll want to request non-optimized builds for macOS and FreeBSD 12, so we still run unit tests. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Tue, Mar 21, 2023 at 03:33:24PM +0000, Daniel P. Berrangé wrote: > On Tue, Mar 21, 2023 at 04:31:00PM +0100, Michal Prívozník wrote: > > On 3/21/23 16:25, Daniel P. Berrangé wrote: > > > On Tue, Mar 21, 2023 at 04:11:33PM +0100, Michal Privoznik wrote: > > >> <snip/> > > > > > > I don't like the idea of forcing -O0 for the production builds, just to > > > work around the problem of our broken tests. Can we approach it from the > > > opposite POV and disable building of tests, if we see meson optimization > > > level is > 0 > > > > > > eg something roughly like this: > > > > > > with_tests = true > > > if cc.get_id() == 'clang' and > > > not supported_cc_flags.contains('-fsemantic-interposition') and > > > get_option('optimization') != 0 > > > with_tests = false > > > endif > > > > > > if with_tests > > > subdir('tests') > > > endif > > > > > > > > > So people can choose to have tests work or not > > > > That could work too, yeah. My reasoning for going with -O0 was that it's > > very rare that somebody would use such old CLang, but I guess disabling > > tests is less invasive. I'll send v2 shortly. > > It isn't just old Clang. Latest clang lacks -fsemantic-interposition > on non-x86_64, which means current macOS M1 platform today. For our > CI, I guess we'll want to request non-optimized builds for macOS > and FreeBSD 12, so we still run unit tests. We can also utilize the meson 'buildtype' option [1] to figure out if we need to disable/enable tests or modify the optimization that we use when building. When building from git the default value is 'debugoptimized' but when building using RPM it changes to 'plain'. Not sure what FreeBSD and macOS are using. The 'buildtype' affects what value will be stored in 'debug' and 'optimization' options. That could allow us to run tests from git builds where we could disable optimizations and disable tests when libvirt is compiled using package manager. Pavel [1] <https://mesonbuild.com/Builtin-options.html#details-for-buildtype>
On 3/21/23 17:04, Pavel Hrdina wrote: > On Tue, Mar 21, 2023 at 03:33:24PM +0000, Daniel P. Berrangé wrote: >> On Tue, Mar 21, 2023 at 04:31:00PM +0100, Michal Prívozník wrote: >>> On 3/21/23 16:25, Daniel P. Berrangé wrote: >>>> On Tue, Mar 21, 2023 at 04:11:33PM +0100, Michal Privoznik wrote: >>>>> <snip/> >>>> >>>> I don't like the idea of forcing -O0 for the production builds, just to >>>> work around the problem of our broken tests. Can we approach it from the >>>> opposite POV and disable building of tests, if we see meson optimization >>>> level is > 0 >>>> >>>> eg something roughly like this: >>>> >>>> with_tests = true >>>> if cc.get_id() == 'clang' and >>>> not supported_cc_flags.contains('-fsemantic-interposition') and >>>> get_option('optimization') != 0 >>>> with_tests = false >>>> endif >>>> >>>> if with_tests >>>> subdir('tests') >>>> endif >>>> >>>> >>>> So people can choose to have tests work or not >>> >>> That could work too, yeah. My reasoning for going with -O0 was that it's >>> very rare that somebody would use such old CLang, but I guess disabling >>> tests is less invasive. I'll send v2 shortly. >> >> It isn't just old Clang. Latest clang lacks -fsemantic-interposition >> on non-x86_64, which means current macOS M1 platform today. For our >> CI, I guess we'll want to request non-optimized builds for macOS >> and FreeBSD 12, so we still run unit tests. > > We can also utilize the meson 'buildtype' option [1] to figure out if we > need to disable/enable tests or modify the optimization that we use when > building. > > When building from git the default value is 'debugoptimized' but when > building using RPM it changes to 'plain'. Not sure what FreeBSD and > macOS are using. > > The 'buildtype' affects what value will be stored in 'debug' and > 'optimization' options. That could allow us to run tests from git builds > where we could disable optimizations and disable tests when libvirt is > compiled using package manager. To summarize what we want: if CLang doesn't support -fsemantic-interposition: if building from a git checkout: enforce -O0 else disable tests What worries me about this approach is that some distros might leave .git/ dir in the checkout (e.g. live ebuilds from Gentoo) in which case we detect the build as from git. But such distros surely pass --buildtype=... Now, we need to distinguish two scenarios: when the default value from the beginning of the meson.build file was used (and thus we can enforce -O0), OR when --buildtype=debugoptimized (or any other option that sets optimization level) was passed on the cmd line (in which case we need to disable tests). But I don't think there's a way to detect that. But at the same time, disabling optimization whenever .git/ dir is found feels very, very wrong. Michal
On Tue, Mar 21, 2023 at 05:34:45PM +0100, Michal Prívozník wrote: > On 3/21/23 17:04, Pavel Hrdina wrote: > > On Tue, Mar 21, 2023 at 03:33:24PM +0000, Daniel P. Berrangé wrote: > >> On Tue, Mar 21, 2023 at 04:31:00PM +0100, Michal Prívozník wrote: > >>> On 3/21/23 16:25, Daniel P. Berrangé wrote: > >>>> On Tue, Mar 21, 2023 at 04:11:33PM +0100, Michal Privoznik wrote: > >>>>> <snip/> > >>>> > >>>> I don't like the idea of forcing -O0 for the production builds, just to > >>>> work around the problem of our broken tests. Can we approach it from the > >>>> opposite POV and disable building of tests, if we see meson optimization > >>>> level is > 0 > >>>> > >>>> eg something roughly like this: > >>>> > >>>> with_tests = true > >>>> if cc.get_id() == 'clang' and > >>>> not supported_cc_flags.contains('-fsemantic-interposition') and > >>>> get_option('optimization') != 0 > >>>> with_tests = false > >>>> endif > >>>> > >>>> if with_tests > >>>> subdir('tests') > >>>> endif > >>>> > >>>> > >>>> So people can choose to have tests work or not > >>> > >>> That could work too, yeah. My reasoning for going with -O0 was that it's > >>> very rare that somebody would use such old CLang, but I guess disabling > >>> tests is less invasive. I'll send v2 shortly. > >> > >> It isn't just old Clang. Latest clang lacks -fsemantic-interposition > >> on non-x86_64, which means current macOS M1 platform today. For our > >> CI, I guess we'll want to request non-optimized builds for macOS > >> and FreeBSD 12, so we still run unit tests. > > > > We can also utilize the meson 'buildtype' option [1] to figure out if we > > need to disable/enable tests or modify the optimization that we use when > > building. > > > > When building from git the default value is 'debugoptimized' but when > > building using RPM it changes to 'plain'. Not sure what FreeBSD and > > macOS are using. > > > > The 'buildtype' affects what value will be stored in 'debug' and > > 'optimization' options. That could allow us to run tests from git builds > > where we could disable optimizations and disable tests when libvirt is > > compiled using package manager. > > To summarize what we want: > > if CLang doesn't support -fsemantic-interposition: > if building from a git checkout: > enforce -O0 > else > disable tests > > What worries me about this approach is that some distros might leave > .git/ dir in the checkout (e.g. live ebuilds from Gentoo) in which case > we detect the build as from git. But such distros surely pass > --buildtype=... > > Now, we need to distinguish two scenarios: when the default value from > the beginning of the meson.build file was used (and thus we can enforce > -O0), OR when --buildtype=debugoptimized (or any other option that sets > optimization level) was passed on the cmd line (in which case we need to > disable tests). But I don't think there's a way to detect that. I don't believe we need to check 'buildtype'. A get_option("optimization") call should reflect the results of the 'buildtype' option IIRC> > But at the same time, disabling optimization whenever .git/ dir is found > feels very, very wrong. Yeah, I wouldn't enforce -O0, just disable tests, and print a message at end of meson execution to use 'buildtype' to turn off optimization or use a newer clang, if they want to re-enable tests. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Tue, Mar 21, 2023 at 05:04:20PM +0100, Pavel Hrdina wrote: > On Tue, Mar 21, 2023 at 03:33:24PM +0000, Daniel P. Berrangé wrote: > > On Tue, Mar 21, 2023 at 04:31:00PM +0100, Michal Prívozník wrote: > > > On 3/21/23 16:25, Daniel P. Berrangé wrote: > > > > On Tue, Mar 21, 2023 at 04:11:33PM +0100, Michal Privoznik wrote: > > > >> <snip/> > > > > > > > > I don't like the idea of forcing -O0 for the production builds, just to > > > > work around the problem of our broken tests. Can we approach it from the > > > > opposite POV and disable building of tests, if we see meson optimization > > > > level is > 0 > > > > > > > > eg something roughly like this: > > > > > > > > with_tests = true > > > > if cc.get_id() == 'clang' and > > > > not supported_cc_flags.contains('-fsemantic-interposition') and > > > > get_option('optimization') != 0 > > > > with_tests = false > > > > endif > > > > > > > > if with_tests > > > > subdir('tests') > > > > endif > > > > > > > > > > > > So people can choose to have tests work or not > > > > > > That could work too, yeah. My reasoning for going with -O0 was that it's > > > very rare that somebody would use such old CLang, but I guess disabling > > > tests is less invasive. I'll send v2 shortly. > > > > It isn't just old Clang. Latest clang lacks -fsemantic-interposition > > on non-x86_64, which means current macOS M1 platform today. For our > > CI, I guess we'll want to request non-optimized builds for macOS > > and FreeBSD 12, so we still run unit tests. > > We can also utilize the meson 'buildtype' option [1] to figure out if we > need to disable/enable tests or modify the optimization that we use when > building. IIUC, we shouldn't look at 'buildtype' directly, as that's just an alias that expands to 3 other options - optimization, warning_level and debug. > When building from git the default value is 'debugoptimized' but when > building using RPM it changes to 'plain'. Not sure what FreeBSD and > macOS are using. > > The 'buildtype' affects what value will be stored in 'debug' and > 'optimization' options. That could allow us to run tests from git builds > where we could disable optimizations and disable tests when libvirt is > compiled using package manager. Right, we do that in libvirt-dbus: https://gitlab.com/libvirt/libvirt-dbus/-/blob/master/meson.build#L220 With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
© 2016 - 2024 Red Hat, Inc.