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

[Laurent Pinchart]

Hi Jacopo,

On Wed, Oct 05, 2022 at 03:02:16PM +0200, Jacopo Mondi wrote:
> 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 ?

The library will be there, but there's no need to link each test to it.
This patch also prepares for adding other dependencies to individual
tests in the future. Of course one could argue that we shouldn't prepare
for something that is not guaranteed to be needed.

> >      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