Message ID | 20241212181655.112958-2-barnabas.pocze@ideasonboard.com |
---|---|
State | Superseded |
Headers | show |
Series |
|
Related | show |
Hi Barnabás, Thank you for the patch. 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@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@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. > + expire_in: 1 day > + > +# Run the unit tests in a virtual machine. > +test-unit: > + extends: > + - .fdo.distribution-image@debian > + - .libcamera-ci.debian:12 > + - .libcamera-ci.scripts > + stage: test > + needs: > + - job: build-test:debug > + tags: > + - kvm > + script: > + - $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/ > > # meson prior to 1.2.0 doesn't correctly escape non-printable characters > # when generating the testlog XML. This results in an unparseable file.
On Sun, Dec 15, 2024 at 09:04:08PM +0200, Laurent Pinchart wrote: > Hi Barnabás, > > Thank you for the patch. > > 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@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@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. 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. > > + expire_in: 1 day > > + > > +# Run the unit tests in a virtual machine. > > +test-unit: > > + extends: > > + - .fdo.distribution-image@debian > > + - .libcamera-ci.debian:12 > > + - .libcamera-ci.scripts > > + stage: test > > + needs: > > + - job: build-test:debug > > + tags: > > + - kvm > > + script: > > + - $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/ > > > > # meson prior to 1.2.0 doesn't correctly escape non-printable characters > > # when generating the testlog XML. This results in an unparseable file.
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: > > Hi Barnabás, > > > > Thank you for the patch. > > > > 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@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@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. > > > > + expire_in: 1 day > > > + > > > +# Run the unit tests in a virtual machine. > > > +test-unit: > > > + extends: > > > + - .fdo.distribution-image@debian > > > + - .libcamera-ci.debian:12 > > > + - .libcamera-ci.scripts > > > + stage: test > > > + needs: > > > + - job: build-test:debug > > > + tags: > > > + - kvm > > > + script: > > > + - $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/ > > > > > > # meson prior to 1.2.0 doesn't correctly escape non-printable characters > > > # when generating the testlog XML. This results in an unparseable file.
Hi 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: >>> Hi Barnabás, >>> >>> Thank you for the patch. >>> >>> 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@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@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. 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? Regards, Barnabás Pőcze >> >>>> + expire_in: 1 day >>>> + >>>> +# Run the unit tests in a virtual machine. >>>> +test-unit: >>>> + extends: >>>> + - .fdo.distribution-image@debian >>>> + - .libcamera-ci.debian:12 >>>> + - .libcamera-ci.scripts >>>> + stage: test >>>> + needs: >>>> + - job: build-test:debug >>>> + tags: >>>> + - kvm >>>> + script: >>>> + - $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/ >>>> >>>> # meson prior to 1.2.0 doesn't correctly escape non-printable characters >>>> # when generating the testlog XML. This results in an unparseable file. >
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: > >>> Hi Barnabás, > >>> > >>> Thank you for the patch. > >>> > >>> 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@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@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. 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. > >>>> + expire_in: 1 day > >>>> + > >>>> +# Run the unit tests in a virtual machine. > >>>> +test-unit: > >>>> + extends: > >>>> + - .fdo.distribution-image@debian > >>>> + - .libcamera-ci.debian:12 > >>>> + - .libcamera-ci.scripts > >>>> + stage: test > >>>> + needs: > >>>> + - job: build-test:debug > >>>> + tags: > >>>> + - kvm > >>>> + script: > >>>> + - $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/ > >>>> > >>>> # meson prior to 1.2.0 doesn't correctly escape non-printable characters > >>>> # when generating the testlog XML. This results in an unparseable file.
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: >>>>> Hi Barnabás, >>>>> >>>>> Thank you for the patch. >>>>> >>>>> 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@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@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`. > > 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?) 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? 3. Ignoring `*.o` makes the sizes a lot closer in my opinion (for one particular build: uncompressed 163M vs. 143M, compressed 28M vs. 15M [probably largely due to the different compression schemes]). In any case, please see the new version of these changes. Regards, Barnabás Pőcze > >>>>>> + expire_in: 1 day >>>>>> + >>>>>> +# Run the unit tests in a virtual machine. >>>>>> +test-unit: >>>>>> + extends: >>>>>> + - .fdo.distribution-image@debian >>>>>> + - .libcamera-ci.debian:12 >>>>>> + - .libcamera-ci.scripts >>>>>> + stage: test >>>>>> + needs: >>>>>> + - job: build-test:debug >>>>>> + tags: >>>>>> + - kvm >>>>>> + script: >>>>>> + - $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/ >>>>>> >>>>>> # meson prior to 1.2.0 doesn't correctly escape non-printable characters >>>>>> # when generating the testlog XML. This results in an unparseable file. >
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: > >>>>> Hi Barnabás, > >>>>> > >>>>> Thank you for the patch. > >>>>> > >>>>> 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@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@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`. > > > > > 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?) > > 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. -- Kieran
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: > > >>>>> Hi Barnabás, > > >>>>> > > >>>>> Thank you for the patch. > > >>>>> > > >>>>> 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@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@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. > > 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.
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: >>>>>>>> Hi Barnabás, >>>>>>>> >>>>>>>> Thank you for the patch. >>>>>>>> >>>>>>>> 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@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@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? > >>> 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. >
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@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@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. You can move the doc_install_dir definition from Documentation/meson.build to src/meson.build and rename it to libcamera_docdir. > >>> 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.
On Tue, Dec 17, 2024 at 07:25:00PM +0200, Laurent Pinchart wrote: > 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@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@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. And then in CI we would take that file and copy it to the expected location, renaming it to virtual.yaml. There's also the question of what install_tag to use. Other data files use 'runtime', but those are installed to their runtime location. 'doc' could be an option, we can use that even if we disable documentation build in CI, but it may feel a bit weird, and possibly later install other types of documentation that we way not want (although I suppose installation of future "real" documentation could be conditioned by the documentation meson option). Another option is to create it from scratch in CI (possibly with the file just added to the CI repository and copied in the test job). > You can move the doc_install_dir definition from > Documentation/meson.build to src/meson.build and rename it to > libcamera_docdir. > > > >>> 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.
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@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@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
Quoting Laurent Pinchart (2024-12-17 17:32:25) > On Tue, Dec 17, 2024 at 07:25:00PM +0200, Laurent Pinchart wrote: > > > >>> 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. > > And then in CI we would take that file and copy it to the expected > location, renaming it to virtual.yaml. > > There's also the question of what install_tag to use. Other data files > use 'runtime', but those are installed to their runtime location. 'doc' > could be an option, we can use that even if we disable documentation > build in CI, but it may feel a bit weird, and possibly later install > other types of documentation that we way not want (although I suppose > installation of future "real" documentation could be conditioned by the > documentation meson option). > > Another option is to create it from scratch in CI (possibly with the > file just added to the CI repository and copied in the test job). Ultimately that's perhaps the 'easiest' option. But it would be nice to keep the CI aligned with any updates to the example virtual configuration too. -- Kieran
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@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@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.
On Tue, Dec 17, 2024 at 05:39:28PM +0000, Kieran Bingham wrote: > Quoting Laurent Pinchart (2024-12-17 17:32:25) > > On Tue, Dec 17, 2024 at 07:25:00PM +0200, Laurent Pinchart wrote: > > > > >>> 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. > > > > And then in CI we would take that file and copy it to the expected > > location, renaming it to virtual.yaml. > > > > There's also the question of what install_tag to use. Other data files > > use 'runtime', but those are installed to their runtime location. 'doc' > > could be an option, we can use that even if we disable documentation > > build in CI, but it may feel a bit weird, and possibly later install > > other types of documentation that we way not want (although I suppose > > installation of future "real" documentation could be conditioned by the > > documentation meson option). > > > > Another option is to create it from scratch in CI (possibly with the > > file just added to the CI repository and copied in the test job). > > Ultimately that's perhaps the 'easiest' option. But it would be nice to > keep the CI aligned with any updates to the example virtual > configuration too. Yes, but if we consider that the virtual pipeline handler configuration we use in CI has to be written for the needs of CI, then it should be part of the libcamera-ci repository, and updated as the virtual pipeline handler evolves. On the other hand, the pipeline handler should pass lc-compliance with any valid configuration, so it makes sense to test the example configuration. Installing it to the documentation directory and then copying it from there in the CI lc-compliance job sounds like a good option (which contradicts what I wrote in a separate e-mail 2 minutes ago :-)). Barnabás, what do you think ?
2024. 12. 18. 2:10 keltezéssel, Laurent Pinchart írta: > On Tue, Dec 17, 2024 at 05:39:28PM +0000, Kieran Bingham wrote: >> Quoting Laurent Pinchart (2024-12-17 17:32:25) >>> On Tue, Dec 17, 2024 at 07:25:00PM +0200, Laurent Pinchart wrote: >>>>>>>> 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. >>> >>> And then in CI we would take that file and copy it to the expected >>> location, renaming it to virtual.yaml. >>> >>> There's also the question of what install_tag to use. Other data files >>> use 'runtime', but those are installed to their runtime location. 'doc' >>> could be an option, we can use that even if we disable documentation >>> build in CI, but it may feel a bit weird, and possibly later install >>> other types of documentation that we way not want (although I suppose >>> installation of future "real" documentation could be conditioned by the >>> documentation meson option). >>> >>> Another option is to create it from scratch in CI (possibly with the >>> file just added to the CI repository and copied in the test job). >> >> Ultimately that's perhaps the 'easiest' option. But it would be nice to >> keep the CI aligned with any updates to the example virtual >> configuration too. > > Yes, but if we consider that the virtual pipeline handler configuration > we use in CI has to be written for the needs of CI, then it should be > part of the libcamera-ci repository, and updated as the virtual pipeline > handler evolves. > > On the other hand, the pipeline handler should pass lc-compliance with > any valid configuration, so it makes sense to test the example > configuration. Installing it to the documentation directory and then > copying it from there in the CI lc-compliance job sounds like a good > option (which contradicts what I wrote in a separate e-mail 2 minutes > ago :-)). Barnabás, what do you think ? > Not sure. To be honest I am not a fan of either solution, but I don't really ave an alternative idea. I suppose we could have a "special" camera(s) named something like "Virtual-CI" in the example configuration, which would be the one used in the CI test. Between these two options, keeping the configuration in the main repository seems better to me. Regards, Barnabás Pőcze
Hi Barnabás, On Wed, Dec 18, 2024 at 10:24:50PM +0100, Barnabás Pőcze wrote: > 2024. 12. 18. 2:10 keltezéssel, Laurent Pinchart írta: > > On Tue, Dec 17, 2024 at 05:39:28PM +0000, Kieran Bingham wrote: > >> Quoting Laurent Pinchart (2024-12-17 17:32:25) > >>> On Tue, Dec 17, 2024 at 07:25:00PM +0200, Laurent Pinchart wrote: > >>>>>>>> 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. > >>> > >>> And then in CI we would take that file and copy it to the expected > >>> location, renaming it to virtual.yaml. > >>> > >>> There's also the question of what install_tag to use. Other data files > >>> use 'runtime', but those are installed to their runtime location. 'doc' > >>> could be an option, we can use that even if we disable documentation > >>> build in CI, but it may feel a bit weird, and possibly later install > >>> other types of documentation that we way not want (although I suppose > >>> installation of future "real" documentation could be conditioned by the > >>> documentation meson option). > >>> > >>> Another option is to create it from scratch in CI (possibly with the > >>> file just added to the CI repository and copied in the test job). > >> > >> Ultimately that's perhaps the 'easiest' option. But it would be nice to > >> keep the CI aligned with any updates to the example virtual > >> configuration too. > > > > Yes, but if we consider that the virtual pipeline handler configuration > > we use in CI has to be written for the needs of CI, then it should be > > part of the libcamera-ci repository, and updated as the virtual pipeline > > handler evolves. > > > > On the other hand, the pipeline handler should pass lc-compliance with > > any valid configuration, so it makes sense to test the example > > configuration. Installing it to the documentation directory and then > > copying it from there in the CI lc-compliance job sounds like a good > > option (which contradicts what I wrote in a separate e-mail 2 minutes > > ago :-)). Barnabás, what do you think ? > > Not sure. To be honest I am not a fan of either solution, but I don't > really ave an alternative idea. I suppose we could have a "special" > camera(s) named something like "Virtual-CI" in the example configuration, > which would be the one used in the CI test. Between these two options, > keeping the configuration in the main repository seems better to me. Agreed. Let's go for that, installing the configuration file as an example, and copying it as part of the CI job.
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: extends: - .fdo.distribution-image@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/ + expire_in: 1 day + +# Run the unit tests in a virtual machine. +test-unit: + extends: + - .fdo.distribution-image@debian + - .libcamera-ci.debian:12 + - .libcamera-ci.scripts + stage: test + needs: + - job: build-test:debug + tags: + - kvm + script: + - $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/ # meson prior to 1.2.0 doesn't correctly escape non-printable characters # when generating the testlog XML. This results in an unparseable file.
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 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, install just the libraries in the container. Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> --- .gitlab-ci/setup-container.sh | 3 +++ gitlab-ci.yml | 42 +++++++++++++++++++++++------------ 2 files changed, 31 insertions(+), 14 deletions(-) -- 2.47.1