[libcamera-devel] [PATCH] test: threads: Fix link failure due to missing dependency

[Umang Jain]

Hi

On 10/5/22 6:32 PM, Jacopo Mondi via libcamera-devel wrote:
> Hi Laurent
>
> On Wed, Oct 05, 2022 at 01:56:21PM +0300, Laurent Pinchart via libcamera-devel wrote:
>> Commit 036d26d6677e ("test: threads: Test thread cleanup upon abnormal
>> termination") added calls to functions provided by the pthread library
>> in the threads test, but didn't add the corresponding dependency. This
>> caused a link breakage on some platforms:
>>
>> /usr/bin/ld: test/threads.p/threads.cpp.o: undefined reference to symbol 'pthread_cancel@@GLIBC_2.4'
>> /usr/bin/ld: /lib/arm-linux-gnueabihf/libpthread.so.0: error adding symbols: DSO missing from command line
>> collect2: error: ld returned 1 exit status
>>
>> Fix it by adding the missing dependency.
>>
>> Fixes: 036d26d6677e ("test: threads: Test thread cleanup upon abnormal termination")
>> Reported-by: Naushir Patuck <naush at raspberrypi.com>
>> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>> ---
>>   src/libcamera/base/meson.build |  2 +-
>>   src/libcamera/meson.build      |  1 +
>>   test/meson.build               | 23 +++++++++++++++++++----
>>   3 files changed, 21 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/libcamera/base/meson.build b/src/libcamera/base/meson.build
>> index 3b9d74efe935..7a75914ab2a8 100644
>> --- a/src/libcamera/base/meson.build
>> +++ b/src/libcamera/base/meson.build
>> @@ -38,9 +38,9 @@ if libunwind.found()
>>   endif
>>
>>   libcamera_base_deps = [
>> -    dependency('threads'),
>>       libatomic,
>>       libdw,
>> +    libthreads,
>>       libunwind,
>>   ]
>>
>> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
>> index 63b47b177fd2..7fcbb2ddc9e7 100644
>> --- a/src/libcamera/meson.build
>> +++ b/src/libcamera/meson.build
>> @@ -58,6 +58,7 @@ includes = [
>>   ]
>>
>>   libatomic = cc.find_library('atomic', required : false)
>> +libthreads = dependency('threads')
>>
>>   subdir('base')
>>   subdir('ipa')
>> diff --git a/test/meson.build b/test/meson.build
>> index 6cc778415dc8..9bf7bf34e796 100644
>> --- a/test/meson.build
>> +++ b/test/meson.build
>> @@ -51,7 +51,7 @@ internal_tests = [
>>       ['pixel-format',                    'pixel-format.cpp'],
>>       ['shared-fd',                       'shared-fd.cpp'],
>>       ['signal-threads',                  'signal-threads.cpp'],
>> -    ['threads',                         'threads.cpp'],
>> +    ['threads',                         'threads.cpp', [libthreads]],
>>       ['timer',                           'timer.cpp'],
>>       ['timer-thread',                    'timer-thread.cpp'],
>>       ['unique-fd',                       'unique-fd.cpp'],
>> @@ -65,8 +65,13 @@ internal_non_parallel_tests = [
>>   ]
>>
>>   foreach t : public_tests
>> +    deps = [libcamera_public]
>> +    if t.length() > 2
>> +        deps += t[2]
>> +    endif
>> +
> This seems very fragile.
> Can't we had libthreads to the list of test dependencies
> unconditionally ? Seems like it's something that should be easily
> available everywhere, isn't it ?

I tend with agree with Jacopo here. It would be better if libthreads is 
part of test dependencies.

I am not even sure if per-test dependency for the test-suite makes sense 
or not. Since, if someone is running the test suite, they are mostly 
interested in the entire suite run (for e.g. distros)  rather than 
individual tests?
>
>>       exe = executable(t[0], t[1],
>> -                     dependencies : libcamera_public,
>> +                     dependencies : deps,
>>                        link_with : test_libraries,
>>                        include_directories : test_includes_public)
>>
>> @@ -74,8 +79,13 @@ foreach t : public_tests
>>   endforeach
>>
>>   foreach t : internal_tests
>> +    deps = [libcamera_private]
>> +    if t.length() > 2
>> +        deps += t[2]
>> +    endif
>> +
>>       exe = executable(t[0], t[1],
>> -                     dependencies : libcamera_private,
>> +                     dependencies : deps,
>>                        link_with : test_libraries,
>>                        include_directories : test_includes_internal)
>>
>> @@ -83,8 +93,13 @@ foreach t : internal_tests
>>   endforeach
>>
>>   foreach t : internal_non_parallel_tests
>> +    deps = [libcamera_private]
>> +    if t.length() > 2
>> +        deps += t[2]
>> +    endif
>> +
>>       exe = executable(t[0], t[1],
>> -                     dependencies : libcamera_private,
>> +                     dependencies : deps,
>>                        link_with : test_libraries,
>>                        include_directories : test_includes_internal)
>>
>>
>> base-commit: 12f4708e35cde15ff9607d59eb053ece1b2bd081
>> --
>> Regards,
>>
>> Laurent Pinchart
>>