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.

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.

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.