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

[Martin Kletzander]

On Fri, Mar 24, 2023 at 11:35:52AM +0100, Michal Prívozník wrote:
>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 at redhat.com>
>>> ---
>>>
>>> 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 at redhat.com>
>
>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.
>

Which is another thing that needs adjustment.  I think if you adjust it
right now just for macos-12 and then update the CI files in the same
commit then we'll get a green pipeline *with* no tests skipped because
of clang for a change ;)

>Michal
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20230324/4bfe5b84/attachment-0001.sig>