[libcamera-devel] [PATCH] test: v4l2_videodevice: meson: Fix race with generated IPA headers

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Wed, May 26, 2021 at 10:23:52AM +0200, Niklas Söderlund wrote:
> The header v4l2_videodevice_test.h includes
> libcamera/internal/camera_sensor.h which in turn depends on the auto
> generated IPA header core_ipa_interface.h. This dependency is not
> described in meson and building in an empty build directory can trigger
> a build race.
> 
>   [52/349] Compiling C++ object test/v4l2_videodevice/v4l2_m2mdevice.p/v4l2_videodevice_test.cpp.o
>   FAILED: test/v4l2_videodevice/v4l2_m2mdevice.p/v4l2_videodevice_test.cpp.o
>   ccache c++ -Itest/v4l2_videodevice/v4l2_m2mdevice.p -Itest/v4l2_videodevice -I../../test/v4l2_videodevice -Itest/libtest -I../../test/libtest -Iinclude -I../../include -Iinclude/libcamera -fdiagnostics-color=always -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wnon-virtual-dtor -Wextra -Werror -std=c++17 -g -Wshadow -include config.h -MD -MQ test/v4l2_videodevice/v4l2_m2mdevice.p/v4l2_videodevice_test.cpp.o -MF test/v4l2_videodevice/v4l2_m2mdevice.p/v4l2_videodevice_test.cpp.o.d -o test/v4l2_videodevice/v4l2_m2mdevice.p/v4l2_videodevice_test.cpp.o -c ../../test/v4l2_videodevice/v4l2_videodevice_test.cpp
>   In file included from ../../test/v4l2_videodevice/v4l2_videodevice_test.h:14,
>                    from ../../test/v4l2_videodevice/v4l2_videodevice_test.cpp:15:
>   ../../include/libcamera/internal/camera_sensor.h:17:10: fatal error: libcamera/ipa/core_ipa_interface.h: No such file or directory
>      17 | #include <libcamera/ipa/core_ipa_interface.h>
>         |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   compilation terminated.
> 

If the generated IPA headers become a dependency for anything that uses
the internal headers, we'll play a game of whack-a-mole trying to
address the issue in executable individually. There's another similar
race condition with test/camera-sensor.cpp as it includes
camera_sensor.h, and other tests also include it indirectly through
ipa_data_serializer.h

One option would be to add libcamera_generated_ipa_headers to the
sources of libcamera_dep. This isn't ideal, as conceptually speaking,
libcamera_dep is supposed to model the dependencies required for usage
of the public API, not the internal API. Another option would be to
include the generated headers in libcamera_ipa_headers, but that's also
not ideal. libcamera_generated_ipa_headers groups the IPA interface,
serializer and proxy headers, and that's too much for
libcamera_ipa_headers (which is a bit ill-defined itself).

As we'll have to reorganize this when splitting some utilities from the
libcamera core library into a separate library, I'd go for the simple
solution of adding libcamera_generated_ipa_headers to libcamera_dep,
with a todo comment. What do you think ?

> Fix this by describing the dependency in meson the same way it is done
> for the IPA test cases.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  test/v4l2_videodevice/meson.build | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/test/v4l2_videodevice/meson.build b/test/v4l2_videodevice/meson.build
> index e733518c0185fda5..ad43748f560908ce 100644
> --- a/test/v4l2_videodevice/meson.build
> +++ b/test/v4l2_videodevice/meson.build
> @@ -15,7 +15,7 @@ v4l2_videodevice_tests = [
>  ]
>  
>  foreach t : v4l2_videodevice_tests
> -    exe = executable(t[0], [t[1], 'v4l2_videodevice_test.cpp'],
> +    exe = executable(t[0], [t[1], 'v4l2_videodevice_test.cpp', libcamera_generated_ipa_headers],
>                       dependencies : libcamera_dep,
>                       link_with : test_libraries,
>                       include_directories : test_includes_internal)

-- 
Regards,

Laurent Pinchart