[libcamera-ci] [RFC PATCH v1 2/3] Separate the building and running of unit tests
Kieran Bingham
kieran.bingham at ideasonboard.com
Tue Dec 17 18:38:03 CET 2024
Quoting Laurent Pinchart (2024-12-17 17:24:59)
> Hi Barnabás,
>
> On Tue, Dec 17, 2024 at 05:09:45PM +0100, Barnabás Pőcze wrote:
> > 2024. 12. 16. 21:01 keltezéssel, Laurent Pinchart írta:
> > > On Mon, Dec 16, 2024 at 06:26:14PM +0000, Kieran Bingham wrote:
> > >> Quoting Barnabás Pőcze (2024-12-16 17:28:46)
> > >>> 2024. 12. 16. 12:04 keltezéssel, Laurent Pinchart írta:
> > >>>> On Mon, Dec 16, 2024 at 10:13:54AM +0100, Barnabás Pőcze wrote:
> > >>>>> 2024. 12. 15. 21:43 keltezéssel, Laurent Pinchart írta:
> > >>>>>> On Sun, Dec 15, 2024 at 09:43:20PM +0200, Laurent Pinchart wrote:
> > >>>>>>> On Sun, Dec 15, 2024 at 09:04:08PM +0200, Laurent Pinchart wrote:
> > >>>>>>>> On Thu, Dec 12, 2024 at 07:16:54PM +0100, Barnabás Pőcze wrote:
> > >>>>>>>>> The built artifacts will be reused in a later job, so split
> > >>>>>>>>> the "test-unit" into the "build-test" and "test-unit" jobs.
> > >>>>>>>>>
> > >>>>>>>>> The `libevent` development package cannot be installed in the container
> > >>>>>>>>
> > >>>>>>>> I've write `libevent-dev` here to avoid ambiguities.
> > >>>>>>>>
> > >>>>>>>>> directly because it is not multiarch compatible. It is, however, installed
> > >>>>>>>>> in the architecture specific build jobs, right before building. To ensure
> > >>>>>>>>> that the it is available for already built executables in different jobs,
> > >>>>>>>>
> > >>>>>>>> "that the it is" ?
> > >>>>>>>>
> > >>>>>>>>> install just the libraries in the container.
> > >>>>>>>>
> > >>>>>>>> And name here `libevent`.
> > >>>>>>>>
> > >>>>>>>>>
> > >>>>>>>>> Signed-off-by: Barnabás Pőcze <barnabas.pocze at ideasonboard.com>
> > >>>>>>>>> ---
> > >>>>>>>>> .gitlab-ci/setup-container.sh | 3 +++
> > >>>>>>>>> gitlab-ci.yml | 42 +++++++++++++++++++++++------------
> > >>>>>>>>> 2 files changed, 31 insertions(+), 14 deletions(-)
> > >>>>>>>>>
> > >>>>>>>>> diff --git a/.gitlab-ci/setup-container.sh b/.gitlab-ci/setup-container.sh
> > >>>>>>>>> index d2909c7..0658368 100755
> > >>>>>>>>> --- a/.gitlab-ci/setup-container.sh
> > >>>>>>>>> +++ b/.gitlab-ci/setup-container.sh
> > >>>>>>>>> @@ -103,6 +103,9 @@ case $FDO_DISTRIBUTION_VERSION in
> > >>>>>>>>> 'bookworm')
> > >>>>>>>>> # libclang-rt-dev for the clang ASan runtime.
> > >>>>>>>>> PKGS_LIBCAMERA_RUNTIME_MULTIARCH+=( libclang-rt-dev )
> > >>>>>>>>> + # For cam and lc-compliance
> > >>>>>>>>> + # libevent-dev cannot be used here, see build-libcamera-common.sh
> > >>>>>>>>> + PKGS_LIBCAMERA_RUNTIME_MULTIARCH+=( libevent-2.1-7 libevent-pthreads-2.1-7 )
> > >>>>>>>>> ;;
> > >>>>>>>>> 'trixie')
> > >>>>>>>>> # gcc 13 to expand compilation testing coverage.
> > >>>>>>>>> diff --git a/gitlab-ci.yml b/gitlab-ci.yml
> > >>>>>>>>> index 8bc8bc2..c7448b8 100644
> > >>>>>>>>> --- a/gitlab-ci.yml
> > >>>>>>>>> +++ b/gitlab-ci.yml
> > >>>>>>>>> @@ -64,7 +64,7 @@ include:
> > >>>>>>>>> .libcamera-ci.debian:12:
> > >>>>>>>>> variables:
> > >>>>>>>>> FDO_DISTRIBUTION_VERSION: 'bookworm'
> > >>>>>>>>> - FDO_DISTRIBUTION_TAG: '2024-12-12.1'
> > >>>>>>>>> + FDO_DISTRIBUTION_TAG: '2024-12-12.2'
> > >>>>>>>>>
> > >>>>>>>>> .libcamera-ci.debian:13:
> > >>>>>>>>> variables:
> > >>>>>>>>> @@ -363,28 +363,18 @@ test-soraka:
> > >>>>>>>>> script:
> > >>>>>>>>> - submit .gitlab-ci/lava/soraka-camera-test.yml
> > >>>>>>>>>
> > >>>>>>>>> -# Run the unit tests in a virtual machine. Enable only the options exercised by
> > >>>>>>>>> -# the unit tests.
> > >>>>>>>>> -test-unit:
> > >>>>>>>>> +# Enable only the options exercised by the unit tests.
> > >>>>>>>>> +build-test:debug:
> > >>>>>>>>
> > >>>>>>>> I'd call this build-package:amd64, as we have build-package:arm64 and
> > >>>>>>>> build-package:cros. I think it would also make sense to use the same
> > >>>>>>>> build options for the amd64 and arm64 packages (beside possibly the
> > >>>>>>>> selected pipeline handlers, although the 'auto' option may work for
> > >>>>>>>> both).
> > >>>>>>>>
> > >>>>>>>>> extends:
> > >>>>>>>>> - .fdo.distribution-image at debian
> > >>>>>>>>> - .libcamera-ci.debian:12
> > >>>>>>>>> - .libcamera-ci.scripts
> > >>>>>>>>> - stage: test
> > >>>>>>>>> + stage: build
> > >>>>>>>>> needs:
> > >>>>>>>>> - job: container-debian:12
> > >>>>>>>>> artifacts: false
> > >>>>>>>>> - tags:
> > >>>>>>>>> - - kvm
> > >>>>>>>>> script:
> > >>>>>>>>> - $CI_PROJECT_DIR/.gitlab-ci/build-libcamera.sh
> > >>>>>>>>> - - $CI_PROJECT_DIR/.gitlab-ci/test-libcamera-qemu.sh
> > >>>>>>>>> - artifacts:
> > >>>>>>>>> - name: libcamera-unit-tests-${CI_COMMIT_SHA}
> > >>>>>>>>> - when: always
> > >>>>>>>>> - expire_in: 1 week
> > >>>>>>>>> - paths:
> > >>>>>>>>> - - build/meson-logs/
> > >>>>>>>>> variables:
> > >>>>>>>>> BUILD_TYPE: debug
> > >>>>>>>>> MESON_OPTIONS: >-
> > >>>>>>>>> @@ -399,6 +389,30 @@ test-unit:
> > >>>>>>>>> -D qcam=disabled
> > >>>>>>>>> -D test=true
> > >>>>>>>>> -D v4l2=true
> > >>>>>>>>> + artifacts:
> > >>>>>>>>> + paths:
> > >>>>>>>>> + - build/
> > >>>>>>>>
> > >>>>>>>> The whole build directory can be very large. Can't we do the same as
> > >>>>>>>> build-package:arm64 and run package-libcamera.sh to only package what we
> > >>>>>>>> need ? We'll need probably need an unpackage script for the test-unit
> > >>>>>>>> job.
> > >>>>>>>
> > >>>>>>> But of course the unit test binaries don't get installed... Can we fix
> > >>>>>>> that and install them ? You can specify "install_tag : 'tests'" in
> > >>>>>>> meson.build so they won't be installed by default (an appropriate
> > >>>>>>> install_dir is also needed). This in turn requires bumping the minimum
> > >>>>>>> meson version from 0.63.0 to 0.64.0, which shouldn't be an issue.
> > >>>>>>
> > >>>>>> I've been told on IRC that the motivation for the "tests" install tag in
> > >>>>>> meson is https://gitlab.gnome.org/GNOME/gnome-desktop-testing. I don't
> > >>>>>> think we should switch to a separate runner for unit tests (the pain is
> > >>>>>> not worth the gain at this point in my opinion), but it could be useful
> > >>>>>> to tag lc-compliance with install_tag = 'tests'.
> > >>>>>>
> > >>>>>>> And now that I've said this, I realize we wouldn't be able to run "meson
> > >>>>>>> test" to run the tests :-/ I'm not sure there's an appropriate solution
> > >>>>>>> for this. If not, given the size of the build directory, and to avoid
> > >>>>>>> transferring a large amount of data between runners, we may need to keep
> > >>>>>>> building libcamera within the test-unit job :-(
> > >>>>>>>
> > >>>>>>> A separate build-package target for lc-compliance would still make
> > >>>>>>> sense.
> > >>>>>
> > >>>>> I think it would be unfortunate to give up the usage `meson test` as you
> > >>>>> mentioned.
> > >>>>
> > >>>> We could work on a replacement, but it would require a significant
> > >>>> amount of work and I think there are better things to do.
> > >>>>
> > >>>>> I have not noticed that these build artifacts would put any
> > >>>>> appreciable strain on the infrastructure. The compressed build directory
> > >>>>> comes out to around 167 MiB; I am not sure if I would consider that a
> > >>>>> large amount of data. It is definitely cheaper, in terms of time, than
> > >>>>> building libcamera twice. Clearing the object files could be another
> > >>>>> option. With `artifacts:exclude: build/**/*.o` we can seemingly
> > >>>>> remove more than half of the uncompressed size, and about 1/4 of
> > >>>>> the compressed size. Does this look acceptable?
> > >>>>
> > >>>> Possibly. We should probably ask the fdo sysadmins about what is
> > >>>> acceptable.
> > >>>>
> > >>>> I gave it a try locally though, and deleting all *.o files in the build
> > >>>> directory results in "meson test" rebuilding everything.
> > >>>
> > >>> As far as I can see that does not happen with the `--no-rebuild` option,
> > >>> which is already used in `test-libcamera-qemu.sh`.
> > >
> > > Ah, good point, I missed that.
> > >
> > >>>> For other uses of the artifacts (in particular deployment on real
> > >>>> devices), I would still prefer minimizing the bandwidth, creating a
> > >>>> package similarly to what build-package:arm64 does. How about keeping
> > >>>> test-unit as-is, at the cost of a recompilation, and creating a
> > >>>> build-package:amd64 that will be used by the lc-compliance test job ? We
> > >>>> can try to improve on top when/if needed.
> > >>>
> > >>> Couple observations:
> > >>>
> > >>> 1. The virtual pipeline handler configuration is not installed, so
> > >>> that needs to be addressed. (Was this omitted intentionally?)
> > >
> > > I don't know. We have to be careful here, we don't want to virtual
> > > pipeline handler to end up being enabled with a default valid
> > > configuration in distributions, otherwise people will all of a sudden
> > > see virtual cameras poluting their devices list.
> >
> > I am not sure what should be done. Maybe a separate meson `install_tag`.
> > Or I suppose the configuration file could be created right before
> > running it?
>
> Hmmmm... If this is a sample configuration file that we don't want to
> install by default in the location where it will be lookup up, how about
> installing it as an example in doc_install_dir (e.g.
> /usr/share/doc/libcamera/) and name it virtual.yaml.example ? That seems
> to be a fairly common pattern.
That sounds fairly reasonable.
> You can move the doc_install_dir definition from
> Documentation/meson.build to src/meson.build and rename it to
> libcamera_docdir.
I'm fine with that. It would only get installed there if the virtual
pipeline handler is enabled, and I suspect distro's won't include that ?
Will the CI define it's own 'virtual camera config' file to support it's
own virtual requirements?
--
Kieran
>
> > >>> 2. I am not a fan of the extra `tar` and `ldconfig` calls that
> > >>> need to be sprinkled in. I think this would be much better
> > >>> if the package was not a mere tar archive but a proper deb/etc.
> > >>> package. I imagine that is a prerequisite of deploying on real
> > >>> hardware in any case, correct?
> > >>
> > >> Having the server build a 'real' deb would be a real benefit IMO, and
> > >> indeed could help with installation/set up on real targets for testing
> > >> in defined environments.
> > >>
> > >> I've wanted to tackle that for a while but never got time.
> > >
> > > Same, I think it's probably worth it, but it will require some effort
> > > and I didn't have enough time when I implemented build-package:arm64.
>
> --
> Regards,
>
> Laurent Pinchart
More information about the libcamera-devel
mailing list