Re: [PATCH] meson: Don't build tests when CLang lacks -fsemantic-interposition

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 3/24/23 08:33, Martin Kletzander wrote:
> On Tue, Mar 21, 2023 at 05:47:19PM +0100, Michal Privoznik wrote:
>> There are some CLang versions that do not support
>> -fsemantic-interposition. If that's the case, the code is
>> optimized so much that our mocking no longer works.
>>
>> Therefore, disable tests and produce a warning.
>>
>> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
>> ---
>>
>> Technically, this is a v2 of:
>>
>> https://listman.redhat.com/archives/libvir-list/2023-March/238943.html
>>
>> but a different approach is implemented, so I'm sending it anew.
>>
>> meson.build | 14 ++++++++++++--
>> 1 file changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/meson.build b/meson.build
>> index a0682e8d0b..c15003ce02 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -2035,8 +2035,18 @@ subdir('src')
>>
>> subdir('tools')
>>
>> -build_tests = not get_option('tests').disabled()
>> -if build_tests
>> +build_tests = [ not get_option('tests').disabled() ]
>> +if build_tests[0] and \
>> +   cc.get_id() == 'clang' and \
>> +   not supported_cc_flags.contains('-fsemantic-interposition') \
>> +   and get_option('optimization') != '0'
>> +   # If CLang doesn't support -fsemantic-interposition then our
>> +   # mocking doesn't work. The best we can do is to not run the
>> +   # test suite.
>> +   build_tests = [ false, '!!! Forcibly disabling tests because CLang
>> lacks -fsemantic-interposition. Update CLang or disable optimization
>> !!!' ]
>> +endif
>> +
> 
> So at first I wanted to suggest something like:
> 
> if cc.get_id() == 'clang' and \
>    not supported_cc_flags.contains('-fsemantic-interposition') \
>    and get_option('optimization') != '0'
>   # If CLang doesn't support -fsemantic-interposition then our
>   # mocking doesn't work. The best we can do is to not run the
>   # test suite.
>   if get_option('tests').enabled()
>     error('Cannot enable tests because CLang lacks
> -fsemantic-interposition. Update CLang or disable optimization')
>   else
>     build_tests = false
>   endif
> else
>   build_tests = not get_option('tests').disabled()
> endif
> 
> Which would simply error out if you want the tests to be enabled on such
> system.  However that has the downside that some potential contributor
> might not notice the tests are being disabled and we would also have to
> have another place where to track the enablement of tests/optimization,
> in our CI.  It is possible, but after some tries I think your approach
> is better and the main thing is that we need something to be pushed in
> order for us not to get into failing CI fatigue which I've seen in other
> projects where CI failures are just being dismissed after couple of
> issues.  So:
> 
> Reviewed-by: Martin Kletzander <mkletzan@xxxxxxxxxx>

Thanks. I think it's important to note that this affects systems with
old CLang and I don't expect many developers there (in fact none).
Except for our CI which can be easily fixed - just pass -O0 in CFLAGS.

Michal




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux