[libcamera-ci] [RFC PATCH v1 2/3] Separate the building and running of unit tests
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Wed Dec 18 02:07:20 CET 2024
On Tue, Dec 17, 2024 at 05:38:03PM +0000, Kieran Bingham wrote:
> Quoting Laurent Pinchart (2024-12-17 17:24:59)
> > 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 ?
Distros may, but as long as the example file is installed as an example
in the documentation directory, and not where the virtual pipeline
handler looks for it, it won't disturb users.
> Will the CI define it's own 'virtual camera config' file to support it's
> own virtual requirements?
I'm increasingly thinking that should be the way, in which case we don't
need to install the example. We still could though.
> > > >>> 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