[libcamera-ci,RFC,v2,4/4] Add job to run lc-compliance on the virtual pipeline handler
diff mbox series

Message ID 20241216172827.1055088-4-barnabas.pocze@ideasonboard.com
State Superseded
Headers show
Series
  • [libcamera-ci,RFC,v2,1/4] Enable `UDMABUF` in the kernel
Related show

Commit Message

Barnabás Pőcze Dec. 16, 2024, 5:28 p.m. UTC 
Add a new job named `lc-compliance:virtual` that uses the
build artifacts created by the `build-package:debug` job
on amd64 to run `lc-compliance` on the "Virtual0" camera
in a virtual machine.

The `force_fallback_for=gtest` option is needed because `cpp_debustl`
makes ABI incompatible changes in the STL containers, so googletest
also needs to be compiled with these changes.

Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---

Changes in v2:
  * publish lc-compliance results to gitlab
  * add and use script to install libcamera binary package

---
 .gitlab-ci/test-lc-compliance.sh  | 37 +++++++++++++++++++++++++++++++
 .gitlab-ci/unpackage-libcamera.sh | 17 ++++++++++++++
 gitlab-ci.yml                     | 24 +++++++++++++++++++-
 3 files changed, 77 insertions(+), 1 deletion(-)
 create mode 100755 .gitlab-ci/test-lc-compliance.sh
 create mode 100755 .gitlab-ci/unpackage-libcamera.sh

--
2.47.1

Comments

Laurent Pinchart Dec. 17, 2024, 12:17 a.m. UTC | #1 
Hi Barnabás,

Thank you for the patch.

On Mon, Dec 16, 2024 at 06:28:27PM +0100, Barnabás Pőcze wrote:
> Add a new job named `lc-compliance:virtual` that uses the
> build artifacts created by the `build-package:debug` job
> on amd64 to run `lc-compliance` on the "Virtual0" camera
> in a virtual machine.
> 
> The `force_fallback_for=gtest` option is needed because `cpp_debustl`

s/debustl/debugstl/

> makes ABI incompatible changes in the STL containers, so googletest
> also needs to be compiled with these changes.

Downloading sources at build time in CI isn't great, I'd like to avoid
it when possible. If it can't be avoided, so be it. There are
precedents, we download and install the packages that are not multi-arch
compatible at build time, and we also force a fallback for gtest when
compiling with clang and libc++.

On a side note, I'm wondering if we shouldn't just disable lc-compliance
when building with clang and libc++, and download and cache the .deb
packages at container build time. If anyone wants to give it a try...
:-)

Back to the topic, does cpp_debugstl give use significant benefits
compared to ASan and UBSan ?

> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> ---
> 
> Changes in v2:
>   * publish lc-compliance results to gitlab
>   * add and use script to install libcamera binary package
> 
> ---
>  .gitlab-ci/test-lc-compliance.sh  | 37 +++++++++++++++++++++++++++++++
>  .gitlab-ci/unpackage-libcamera.sh | 17 ++++++++++++++
>  gitlab-ci.yml                     | 24 +++++++++++++++++++-
>  3 files changed, 77 insertions(+), 1 deletion(-)
>  create mode 100755 .gitlab-ci/test-lc-compliance.sh
>  create mode 100755 .gitlab-ci/unpackage-libcamera.sh
> 
> diff --git a/.gitlab-ci/test-lc-compliance.sh b/.gitlab-ci/test-lc-compliance.sh
> new file mode 100755
> index 0000000..a927f1d
> --- /dev/null
> +++ b/.gitlab-ci/test-lc-compliance.sh
> @@ -0,0 +1,37 @@
> +#!/bin/bash
> +
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +# SPDX-FileCopyrightText: © 2024 Google Inc.
> +
> +set -e
> +
> +source "$(dirname "$0")/lib.sh"
> +
> +libcamera_compliance() {
> +	echo "Running libcamera compliance tests in a qemu VM"
> +
> +	virtme-ng \
> +		--verbose \
> +		--skip-modules \
> +		--force-9p \
> +		--rwdir "$PWD/build" \
> +		--run /opt/linux/bzImage \
> +		--exec "\

s/"/" /

> +			LIBCAMERA_LOG_LEVELS=*:DEBUG \
> +			ASAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1 \
> +			UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 \
> +			GTEST_OUTPUT=xml:./build/lc-compliance-report.xml \
> +			lc-compliance -c Virtual0; \
> +			echo \\\$? > ./build/.test-status \
> +		"
> +
> +	local status=$(cat build/.test-status)
> +	echo "Test result exit state: $status"
> +	rm build/.test-status
> +
> +	if [[ $status != 0 ]] ; then
> +		exit $status
> +	fi
> +}
> +
> +run libcamera_compliance
> diff --git a/.gitlab-ci/unpackage-libcamera.sh b/.gitlab-ci/unpackage-libcamera.sh
> new file mode 100755
> index 0000000..9eb3192
> --- /dev/null
> +++ b/.gitlab-ci/unpackage-libcamera.sh
> @@ -0,0 +1,17 @@
> +#!/bin/bash
> +
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +# SPDX-FileCopyrightText: © 2024 Google Inc.
> +
> +set -e
> +
> +source "$(dirname "$0")/lib.sh"
> +
> +libcamera_unpackage() {
> +	echo "Unpackage libcamera binaries"
> +
> +	tar -xvf libcamera-${CI_COMMIT_SHA}.tar.xz -C /
> +	ldconfig -v
> +}
> +
> +run libcamera_unpackage
> diff --git a/gitlab-ci.yml b/gitlab-ci.yml
> index 0a3eab3..8ea1d12 100644
> --- a/gitlab-ci.yml
> +++ b/gitlab-ci.yml
> @@ -328,11 +328,13 @@ build-package:debug:
>      BUILD_TYPE: debug
>      MESON_OPTIONS: >-
>        -D auto_features=disabled
> +      -D lc-compliance=enabled
>        -D test=false
>        -D v4l2=false
>        -D b_sanitize=address,undefined
>        -D cpp_debugstl=true
> -      -D pipelines=[]
> +      -D pipelines=['virtual']
> +      -D force_fallback_for=['gtest']
>    parallel:
>      matrix:
>        - ARCH: amd64
> @@ -440,3 +442,23 @@ test-unit:
>    # artifacts:
>    #   reports:
>    #     junit: build/meson-logs/testlog.junit.xml
> +
> +lc-compliance:virtual:

Nitpicking on the naming, we could name this test-lc-compliance:virtual
or test-compliance:virtual to have the same "test-" prefix as
"test-unit". Maybe "test:unit" and "test:compliance:virtual" could also
be an option ?

> +  extends:
> +    - .fdo.distribution-image@debian
> +    - .libcamera-ci.debian:12
> +    - .libcamera-ci.scripts
> +  stage: test
> +  needs:
> +    - job: build-package:debug
> +      parallel:
> +        matrix:
> +          - ARCH: amd64
> +  tags:
> +    - kvm
> +  script:
> +    - $CI_PROJECT_DIR/.gitlab-ci/unpackage-libcamera.sh
> +    - $CI_PROJECT_DIR/.gitlab-ci/test-lc-compliance.sh
> +  artifacts:
> +    reports:
> +      junit: build/lc-compliance-report.xml
Barnabás Pőcze Dec. 17, 2024, 10:41 a.m. UTC | #2 
2024. 12. 17. 1:17 keltezéssel, Laurent Pinchart írta:
> Hi Barnabás,
> 
> Thank you for the patch.
> 
> On Mon, Dec 16, 2024 at 06:28:27PM +0100, Barnabás Pőcze wrote:
>> Add a new job named `lc-compliance:virtual` that uses the
>> build artifacts created by the `build-package:debug` job
>> on amd64 to run `lc-compliance` on the "Virtual0" camera
>> in a virtual machine.
>>
>> The `force_fallback_for=gtest` option is needed because `cpp_debustl`
> 
> s/debustl/debugstl/
> 
>> makes ABI incompatible changes in the STL containers, so googletest
>> also needs to be compiled with these changes.
> 
> Downloading sources at build time in CI isn't great, I'd like to avoid
> it when possible. If it can't be avoided, so be it. There are
> precedents, we download and install the packages that are not multi-arch
> compatible at build time, and we also force a fallback for gtest when
> compiling with clang and libc++.

I believe libyuv is also downloaded.

> 
> On a side note, I'm wondering if we shouldn't just disable lc-compliance
> when building with clang and libc++, and download and cache the .deb
> packages at container build time. If anyone wants to give it a try...
> :-)
> 
> Back to the topic, does cpp_debugstl give use significant benefits
> compared to ASan and UBSan ?

Well, there are certainly things that it recognizes that ASAN/UBSAN
do not, for example:

   std::vector<int> v; ...
   auto it = /* into `v` */;
   v.erase(/* before `it` */);
   *it; // error


> 
>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
>> ---
>>
>> Changes in v2:
>>    * publish lc-compliance results to gitlab
>>    * add and use script to install libcamera binary package
>>
>> ---
>>   .gitlab-ci/test-lc-compliance.sh  | 37 +++++++++++++++++++++++++++++++
>>   .gitlab-ci/unpackage-libcamera.sh | 17 ++++++++++++++
>>   gitlab-ci.yml                     | 24 +++++++++++++++++++-
>>   3 files changed, 77 insertions(+), 1 deletion(-)
>>   create mode 100755 .gitlab-ci/test-lc-compliance.sh
>>   create mode 100755 .gitlab-ci/unpackage-libcamera.sh
>>
>> diff --git a/.gitlab-ci/test-lc-compliance.sh b/.gitlab-ci/test-lc-compliance.sh
>> new file mode 100755
>> index 0000000..a927f1d
>> --- /dev/null
>> +++ b/.gitlab-ci/test-lc-compliance.sh
>> @@ -0,0 +1,37 @@
>> +#!/bin/bash
>> +
>> +# SPDX-License-Identifier: GPL-2.0-or-later
>> +# SPDX-FileCopyrightText: © 2024 Google Inc.
>> +
>> +set -e
>> +
>> +source "$(dirname "$0")/lib.sh"
>> +
>> +libcamera_compliance() {
>> +	echo "Running libcamera compliance tests in a qemu VM"
>> +
>> +	virtme-ng \
>> +		--verbose \
>> +		--skip-modules \
>> +		--force-9p \
>> +		--rwdir "$PWD/build" \
>> +		--run /opt/linux/bzImage \
>> +		--exec "\
> 
> s/"/" /
> 
>> +			LIBCAMERA_LOG_LEVELS=*:DEBUG \
>> +			ASAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1 \
>> +			UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 \
>> +			GTEST_OUTPUT=xml:./build/lc-compliance-report.xml \
>> +			lc-compliance -c Virtual0; \
>> +			echo \\\$? > ./build/.test-status \
>> +		"
>> +
>> +	local status=$(cat build/.test-status)
>> +	echo "Test result exit state: $status"
>> +	rm build/.test-status
>> +
>> +	if [[ $status != 0 ]] ; then
>> +		exit $status
>> +	fi
>> +}
>> +
>> +run libcamera_compliance
>> diff --git a/.gitlab-ci/unpackage-libcamera.sh b/.gitlab-ci/unpackage-libcamera.sh
>> new file mode 100755
>> index 0000000..9eb3192
>> --- /dev/null
>> +++ b/.gitlab-ci/unpackage-libcamera.sh
>> @@ -0,0 +1,17 @@
>> +#!/bin/bash
>> +
>> +# SPDX-License-Identifier: GPL-2.0-or-later
>> +# SPDX-FileCopyrightText: © 2024 Google Inc.
>> +
>> +set -e
>> +
>> +source "$(dirname "$0")/lib.sh"
>> +
>> +libcamera_unpackage() {
>> +	echo "Unpackage libcamera binaries"
>> +
>> +	tar -xvf libcamera-${CI_COMMIT_SHA}.tar.xz -C /
>> +	ldconfig -v
>> +}
>> +
>> +run libcamera_unpackage
>> diff --git a/gitlab-ci.yml b/gitlab-ci.yml
>> index 0a3eab3..8ea1d12 100644
>> --- a/gitlab-ci.yml
>> +++ b/gitlab-ci.yml
>> @@ -328,11 +328,13 @@ build-package:debug:
>>       BUILD_TYPE: debug
>>       MESON_OPTIONS: >-
>>         -D auto_features=disabled
>> +      -D lc-compliance=enabled
>>         -D test=false
>>         -D v4l2=false
>>         -D b_sanitize=address,undefined
>>         -D cpp_debugstl=true
>> -      -D pipelines=[]
>> +      -D pipelines=['virtual']
>> +      -D force_fallback_for=['gtest']
>>     parallel:
>>       matrix:
>>         - ARCH: amd64
>> @@ -440,3 +442,23 @@ test-unit:
>>     # artifacts:
>>     #   reports:
>>     #     junit: build/meson-logs/testlog.junit.xml
>> +
>> +lc-compliance:virtual:
> 
> Nitpicking on the naming, we could name this test-lc-compliance:virtual
> or test-compliance:virtual to have the same "test-" prefix as
> "test-unit". Maybe "test:unit" and "test:compliance:virtual" could also
> be an option ?
> 
>> +  extends:
>> +    - .fdo.distribution-image@debian
>> +    - .libcamera-ci.debian:12
>> +    - .libcamera-ci.scripts
>> +  stage: test
>> +  needs:
>> +    - job: build-package:debug
>> +      parallel:
>> +        matrix:
>> +          - ARCH: amd64
>> +  tags:
>> +    - kvm
>> +  script:
>> +    - $CI_PROJECT_DIR/.gitlab-ci/unpackage-libcamera.sh
>> +    - $CI_PROJECT_DIR/.gitlab-ci/test-lc-compliance.sh
>> +  artifacts:
>> +    reports:
>> +      junit: build/lc-compliance-report.xml
>
Laurent Pinchart Dec. 17, 2024, noon UTC | #3 
On Tue, Dec 17, 2024 at 11:41:57AM +0100, Barnabás Pőcze wrote:
> 2024. 12. 17. 1:17 keltezéssel, Laurent Pinchart írta:
> > Hi Barnabás,
> > 
> > Thank you for the patch.
> > 
> > On Mon, Dec 16, 2024 at 06:28:27PM +0100, Barnabás Pőcze wrote:
> >> Add a new job named `lc-compliance:virtual` that uses the
> >> build artifacts created by the `build-package:debug` job
> >> on amd64 to run `lc-compliance` on the "Virtual0" camera
> >> in a virtual machine.
> >>
> >> The `force_fallback_for=gtest` option is needed because `cpp_debustl`
> > 
> > s/debustl/debugstl/
> > 
> >> makes ABI incompatible changes in the STL containers, so googletest
> >> also needs to be compiled with these changes.
> > 
> > Downloading sources at build time in CI isn't great, I'd like to avoid
> > it when possible. If it can't be avoided, so be it. There are
> > precedents, we download and install the packages that are not multi-arch
> > compatible at build time, and we also force a fallback for gtest when
> > compiling with clang and libc++.
> 
> I believe libyuv is also downloaded.

Aarrrghhhhh. Why can't google get their projects packaged by
distributions ? :-(

> > On a side note, I'm wondering if we shouldn't just disable lc-compliance
> > when building with clang and libc++, and download and cache the .deb
> > packages at container build time. If anyone wants to give it a try...
> > :-)
> > 
> > Back to the topic, does cpp_debugstl give use significant benefits
> > compared to ASan and UBSan ?
> 
> Well, there are certainly things that it recognizes that ASAN/UBSAN
> do not, for example:
> 
>    std::vector<int> v; ...
>    auto it = /* into `v` */;
>    v.erase(/* before `it` */);
>    *it; // error

That's good to catch indeed. Is the error caught at runtime or compile
time ?

> >> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> >> ---
> >>
> >> Changes in v2:
> >>    * publish lc-compliance results to gitlab
> >>    * add and use script to install libcamera binary package
> >>
> >> ---
> >>   .gitlab-ci/test-lc-compliance.sh  | 37 +++++++++++++++++++++++++++++++
> >>   .gitlab-ci/unpackage-libcamera.sh | 17 ++++++++++++++
> >>   gitlab-ci.yml                     | 24 +++++++++++++++++++-
> >>   3 files changed, 77 insertions(+), 1 deletion(-)
> >>   create mode 100755 .gitlab-ci/test-lc-compliance.sh
> >>   create mode 100755 .gitlab-ci/unpackage-libcamera.sh
> >>
> >> diff --git a/.gitlab-ci/test-lc-compliance.sh b/.gitlab-ci/test-lc-compliance.sh
> >> new file mode 100755
> >> index 0000000..a927f1d
> >> --- /dev/null
> >> +++ b/.gitlab-ci/test-lc-compliance.sh
> >> @@ -0,0 +1,37 @@
> >> +#!/bin/bash
> >> +
> >> +# SPDX-License-Identifier: GPL-2.0-or-later
> >> +# SPDX-FileCopyrightText: © 2024 Google Inc.
> >> +
> >> +set -e
> >> +
> >> +source "$(dirname "$0")/lib.sh"
> >> +
> >> +libcamera_compliance() {
> >> +	echo "Running libcamera compliance tests in a qemu VM"
> >> +
> >> +	virtme-ng \
> >> +		--verbose \
> >> +		--skip-modules \
> >> +		--force-9p \
> >> +		--rwdir "$PWD/build" \
> >> +		--run /opt/linux/bzImage \
> >> +		--exec "\
> > 
> > s/"/" /
> > 
> >> +			LIBCAMERA_LOG_LEVELS=*:DEBUG \
> >> +			ASAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1 \
> >> +			UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 \
> >> +			GTEST_OUTPUT=xml:./build/lc-compliance-report.xml \
> >> +			lc-compliance -c Virtual0; \
> >> +			echo \\\$? > ./build/.test-status \
> >> +		"
> >> +
> >> +	local status=$(cat build/.test-status)
> >> +	echo "Test result exit state: $status"
> >> +	rm build/.test-status
> >> +
> >> +	if [[ $status != 0 ]] ; then
> >> +		exit $status
> >> +	fi
> >> +}
> >> +
> >> +run libcamera_compliance
> >> diff --git a/.gitlab-ci/unpackage-libcamera.sh b/.gitlab-ci/unpackage-libcamera.sh
> >> new file mode 100755
> >> index 0000000..9eb3192
> >> --- /dev/null
> >> +++ b/.gitlab-ci/unpackage-libcamera.sh
> >> @@ -0,0 +1,17 @@
> >> +#!/bin/bash
> >> +
> >> +# SPDX-License-Identifier: GPL-2.0-or-later
> >> +# SPDX-FileCopyrightText: © 2024 Google Inc.
> >> +
> >> +set -e
> >> +
> >> +source "$(dirname "$0")/lib.sh"
> >> +
> >> +libcamera_unpackage() {
> >> +	echo "Unpackage libcamera binaries"
> >> +
> >> +	tar -xvf libcamera-${CI_COMMIT_SHA}.tar.xz -C /
> >> +	ldconfig -v
> >> +}
> >> +
> >> +run libcamera_unpackage
> >> diff --git a/gitlab-ci.yml b/gitlab-ci.yml
> >> index 0a3eab3..8ea1d12 100644
> >> --- a/gitlab-ci.yml
> >> +++ b/gitlab-ci.yml
> >> @@ -328,11 +328,13 @@ build-package:debug:
> >>       BUILD_TYPE: debug
> >>       MESON_OPTIONS: >-
> >>         -D auto_features=disabled
> >> +      -D lc-compliance=enabled
> >>         -D test=false
> >>         -D v4l2=false
> >>         -D b_sanitize=address,undefined
> >>         -D cpp_debugstl=true
> >> -      -D pipelines=[]
> >> +      -D pipelines=['virtual']
> >> +      -D force_fallback_for=['gtest']
> >>     parallel:
> >>       matrix:
> >>         - ARCH: amd64
> >> @@ -440,3 +442,23 @@ test-unit:
> >>     # artifacts:
> >>     #   reports:
> >>     #     junit: build/meson-logs/testlog.junit.xml
> >> +
> >> +lc-compliance:virtual:
> > 
> > Nitpicking on the naming, we could name this test-lc-compliance:virtual
> > or test-compliance:virtual to have the same "test-" prefix as
> > "test-unit". Maybe "test:unit" and "test:compliance:virtual" could also
> > be an option ?
> > 
> >> +  extends:
> >> +    - .fdo.distribution-image@debian
> >> +    - .libcamera-ci.debian:12
> >> +    - .libcamera-ci.scripts
> >> +  stage: test
> >> +  needs:
> >> +    - job: build-package:debug
> >> +      parallel:
> >> +        matrix:
> >> +          - ARCH: amd64
> >> +  tags:
> >> +    - kvm
> >> +  script:
> >> +    - $CI_PROJECT_DIR/.gitlab-ci/unpackage-libcamera.sh
> >> +    - $CI_PROJECT_DIR/.gitlab-ci/test-lc-compliance.sh
> >> +  artifacts:
> >> +    reports:
> >> +      junit: build/lc-compliance-report.xml
Barnabás Pőcze Dec. 17, 2024, 12:34 p.m. UTC | #4 
2024. 12. 17. 13:00 keltezéssel, Laurent Pinchart írta:
> On Tue, Dec 17, 2024 at 11:41:57AM +0100, Barnabás Pőcze wrote:
>> 2024. 12. 17. 1:17 keltezéssel, Laurent Pinchart írta:
>>> Hi Barnabás,
>>>
>>> Thank you for the patch.
>>>
>>> On Mon, Dec 16, 2024 at 06:28:27PM +0100, Barnabás Pőcze wrote:
>>>> Add a new job named `lc-compliance:virtual` that uses the
>>>> build artifacts created by the `build-package:debug` job
>>>> on amd64 to run `lc-compliance` on the "Virtual0" camera
>>>> in a virtual machine.
>>>>
>>>> The `force_fallback_for=gtest` option is needed because `cpp_debustl`
>>>
>>> s/debustl/debugstl/
>>>
>>>> makes ABI incompatible changes in the STL containers, so googletest
>>>> also needs to be compiled with these changes.
>>>
>>> Downloading sources at build time in CI isn't great, I'd like to avoid
>>> it when possible. If it can't be avoided, so be it. There are
>>> precedents, we download and install the packages that are not multi-arch
>>> compatible at build time, and we also force a fallback for gtest when
>>> compiling with clang and libc++.
>>
>> I believe libyuv is also downloaded.
> 
> Aarrrghhhhh. Why can't google get their projects packaged by
> distributions ? :-(

It seems to be packaged. The main issue is that no pkg-config or
cmake files are supplied, so meson cannot really discover it.
I suppose one could do something like this:

diff --git a/src/meson.build b/src/meson.build
index 76198e953..a62dc2c30 100644
--- a/src/meson.build
+++ b/src/meson.build
@@ -32,6 +32,14 @@ endif
  # by distributions.
  libyuv_dep = dependency('libyuv', required : false)

+if not libyuv_dep.found()
+    libyuv_dep = cxx.find_library(
+        'yuv',
+        required : false,
+        has_headers : 'libyuv.h',
+    )
+endif
+
  if (pipelines.contains('virtual') or get_option('android').allowed()) 
and \
     not libyuv_dep.found()
      cmake = import('cmake')



> 
>>> On a side note, I'm wondering if we shouldn't just disable lc-compliance
>>> when building with clang and libc++, and download and cache the .deb
>>> packages at container build time. If anyone wants to give it a try...
>>> :-)
>>>
>>> Back to the topic, does cpp_debugstl give use significant benefits
>>> compared to ASan and UBSan ?
>>
>> Well, there are certainly things that it recognizes that ASAN/UBSAN
>> do not, for example:
>>
>>     std::vector<int> v; ...
>>     auto it = /* into `v` */;
>>     v.erase(/* before `it` */);
>>     *it; // error
> 
> That's good to catch indeed. Is the error caught at runtime or compile
> time ?

Runtime.


> 
>>>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
>>>> ---
>>>>
>>>> Changes in v2:
>>>>     * publish lc-compliance results to gitlab
>>>>     * add and use script to install libcamera binary package
>>>>
>>>> ---
>>>>    .gitlab-ci/test-lc-compliance.sh  | 37 +++++++++++++++++++++++++++++++
>>>>    .gitlab-ci/unpackage-libcamera.sh | 17 ++++++++++++++
>>>>    gitlab-ci.yml                     | 24 +++++++++++++++++++-
>>>>    3 files changed, 77 insertions(+), 1 deletion(-)
>>>>    create mode 100755 .gitlab-ci/test-lc-compliance.sh
>>>>    create mode 100755 .gitlab-ci/unpackage-libcamera.sh
>>>>
>>>> diff --git a/.gitlab-ci/test-lc-compliance.sh b/.gitlab-ci/test-lc-compliance.sh
>>>> new file mode 100755
>>>> index 0000000..a927f1d
>>>> --- /dev/null
>>>> +++ b/.gitlab-ci/test-lc-compliance.sh
>>>> @@ -0,0 +1,37 @@
>>>> +#!/bin/bash
>>>> +
>>>> +# SPDX-License-Identifier: GPL-2.0-or-later
>>>> +# SPDX-FileCopyrightText: © 2024 Google Inc.
>>>> +
>>>> +set -e
>>>> +
>>>> +source "$(dirname "$0")/lib.sh"
>>>> +
>>>> +libcamera_compliance() {
>>>> +	echo "Running libcamera compliance tests in a qemu VM"
>>>> +
>>>> +	virtme-ng \
>>>> +		--verbose \
>>>> +		--skip-modules \
>>>> +		--force-9p \
>>>> +		--rwdir "$PWD/build" \
>>>> +		--run /opt/linux/bzImage \
>>>> +		--exec "\
>>>
>>> s/"/" /
>>>
>>>> +			LIBCAMERA_LOG_LEVELS=*:DEBUG \
>>>> +			ASAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1 \
>>>> +			UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 \
>>>> +			GTEST_OUTPUT=xml:./build/lc-compliance-report.xml \
>>>> +			lc-compliance -c Virtual0; \
>>>> +			echo \\\$? > ./build/.test-status \
>>>> +		"
>>>> +
>>>> +	local status=$(cat build/.test-status)
>>>> +	echo "Test result exit state: $status"
>>>> +	rm build/.test-status
>>>> +
>>>> +	if [[ $status != 0 ]] ; then
>>>> +		exit $status
>>>> +	fi
>>>> +}
>>>> +
>>>> +run libcamera_compliance
>>>> diff --git a/.gitlab-ci/unpackage-libcamera.sh b/.gitlab-ci/unpackage-libcamera.sh
>>>> new file mode 100755
>>>> index 0000000..9eb3192
>>>> --- /dev/null
>>>> +++ b/.gitlab-ci/unpackage-libcamera.sh
>>>> @@ -0,0 +1,17 @@
>>>> +#!/bin/bash
>>>> +
>>>> +# SPDX-License-Identifier: GPL-2.0-or-later
>>>> +# SPDX-FileCopyrightText: © 2024 Google Inc.
>>>> +
>>>> +set -e
>>>> +
>>>> +source "$(dirname "$0")/lib.sh"
>>>> +
>>>> +libcamera_unpackage() {
>>>> +	echo "Unpackage libcamera binaries"
>>>> +
>>>> +	tar -xvf libcamera-${CI_COMMIT_SHA}.tar.xz -C /
>>>> +	ldconfig -v
>>>> +}
>>>> +
>>>> +run libcamera_unpackage
>>>> diff --git a/gitlab-ci.yml b/gitlab-ci.yml
>>>> index 0a3eab3..8ea1d12 100644
>>>> --- a/gitlab-ci.yml
>>>> +++ b/gitlab-ci.yml
>>>> @@ -328,11 +328,13 @@ build-package:debug:
>>>>        BUILD_TYPE: debug
>>>>        MESON_OPTIONS: >-
>>>>          -D auto_features=disabled
>>>> +      -D lc-compliance=enabled
>>>>          -D test=false
>>>>          -D v4l2=false
>>>>          -D b_sanitize=address,undefined
>>>>          -D cpp_debugstl=true
>>>> -      -D pipelines=[]
>>>> +      -D pipelines=['virtual']
>>>> +      -D force_fallback_for=['gtest']
>>>>      parallel:
>>>>        matrix:
>>>>          - ARCH: amd64
>>>> @@ -440,3 +442,23 @@ test-unit:
>>>>      # artifacts:
>>>>      #   reports:
>>>>      #     junit: build/meson-logs/testlog.junit.xml
>>>> +
>>>> +lc-compliance:virtual:
>>>
>>> Nitpicking on the naming, we could name this test-lc-compliance:virtual
>>> or test-compliance:virtual to have the same "test-" prefix as
>>> "test-unit". Maybe "test:unit" and "test:compliance:virtual" could also
>>> be an option ?
>>>
>>>> +  extends:
>>>> +    - .fdo.distribution-image@debian
>>>> +    - .libcamera-ci.debian:12
>>>> +    - .libcamera-ci.scripts
>>>> +  stage: test
>>>> +  needs:
>>>> +    - job: build-package:debug
>>>> +      parallel:
>>>> +        matrix:
>>>> +          - ARCH: amd64
>>>> +  tags:
>>>> +    - kvm
>>>> +  script:
>>>> +    - $CI_PROJECT_DIR/.gitlab-ci/unpackage-libcamera.sh
>>>> +    - $CI_PROJECT_DIR/.gitlab-ci/test-lc-compliance.sh
>>>> +  artifacts:
>>>> +    reports:
>>>> +      junit: build/lc-compliance-report.xml
>
Laurent Pinchart Dec. 17, 2024, 2:23 p.m. UTC | #5 
On Tue, Dec 17, 2024 at 01:34:17PM +0100, Barnabás Pőcze wrote:
> 2024. 12. 17. 13:00 keltezéssel, Laurent Pinchart írta:
> > On Tue, Dec 17, 2024 at 11:41:57AM +0100, Barnabás Pőcze wrote:
> >> 2024. 12. 17. 1:17 keltezéssel, Laurent Pinchart írta:
> >>> On Mon, Dec 16, 2024 at 06:28:27PM +0100, Barnabás Pőcze wrote:
> >>>> Add a new job named `lc-compliance:virtual` that uses the
> >>>> build artifacts created by the `build-package:debug` job
> >>>> on amd64 to run `lc-compliance` on the "Virtual0" camera
> >>>> in a virtual machine.
> >>>>
> >>>> The `force_fallback_for=gtest` option is needed because `cpp_debustl`
> >>>
> >>> s/debustl/debugstl/
> >>>
> >>>> makes ABI incompatible changes in the STL containers, so googletest
> >>>> also needs to be compiled with these changes.
> >>>
> >>> Downloading sources at build time in CI isn't great, I'd like to avoid
> >>> it when possible. If it can't be avoided, so be it. There are
> >>> precedents, we download and install the packages that are not multi-arch
> >>> compatible at build time, and we also force a fallback for gtest when
> >>> compiling with clang and libc++.
> >>
> >> I believe libyuv is also downloaded.
> > 
> > Aarrrghhhhh. Why can't google get their projects packaged by
> > distributions ? :-(
> 
> It seems to be packaged. The main issue is that no pkg-config or
> cmake files are supplied, so meson cannot really discover it.

Ah, I didn't know it was packaged in Debian. Thanks for the information.

> I suppose one could do something like this:
> 
> diff --git a/src/meson.build b/src/meson.build
> index 76198e953..a62dc2c30 100644
> --- a/src/meson.build
> +++ b/src/meson.build
> @@ -32,6 +32,14 @@ endif
>   # by distributions.
>   libyuv_dep = dependency('libyuv', required : false)
> 
> +if not libyuv_dep.found()
> +    libyuv_dep = cxx.find_library(
> +        'yuv',
> +        required : false,
> +        has_headers : 'libyuv.h',
> +    )
> +endif
> +

Or we could create a pkgconfig file manually, as we do for pybind11 in
setup-container.sh. Handling it in meson.build makes it usable
everywhere, so maybe that's a better approach.

Anyway, that's unrelated to this patch series. We can force the fallback
for now. I'd like to see if we could download and cache the subproject
in the container instead of doing it during every build, but that's not
urgent.

https://mesonbuild.com/Wrap-dependency-system-manual.html#specific-to-wrapfile
states

  Since 0.49.0 if source_filename or patch_filename is found in the
  project's subprojects/packagecache directory, it will be used instead
  of downloading the file, even if --wrap-mode option is set to
  nodownload.  The file's hash will be checked.

  Since 1.3.0 if the MESON_PACKAGE_CACHE_DIR environment variable is
  set, it is used instead of the project's subprojects/packagecache.
  This allows sharing the cache across multiple projects. In addition it
  can contain an already extracted source tree as long as it has the
  same directory name as the directory field in the wrap file. In that
  case, the directory will be copied into subprojects/ before applying
  patches.

which seems useful. We install meson from pip when needed, so we can use
features from version 1.3.0.

>   if (pipelines.contains('virtual') or get_option('android').allowed()) 
> and \
>      not libyuv_dep.found()
>       cmake = import('cmake')
> 
> >>> On a side note, I'm wondering if we shouldn't just disable lc-compliance
> >>> when building with clang and libc++, and download and cache the .deb
> >>> packages at container build time. If anyone wants to give it a try...
> >>> :-)
> >>>
> >>> Back to the topic, does cpp_debugstl give use significant benefits
> >>> compared to ASan and UBSan ?
> >>
> >> Well, there are certainly things that it recognizes that ASAN/UBSAN
> >> do not, for example:
> >>
> >>     std::vector<int> v; ...
> >>     auto it = /* into `v` */;
> >>     v.erase(/* before `it` */);
> >>     *it; // error
> > 
> > That's good to catch indeed. Is the error caught at runtime or compile
> > time ?
> 
> Runtime.
> 
> >>>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> >>>> ---
> >>>>
> >>>> Changes in v2:
> >>>>     * publish lc-compliance results to gitlab
> >>>>     * add and use script to install libcamera binary package
> >>>>
> >>>> ---
> >>>>    .gitlab-ci/test-lc-compliance.sh  | 37 +++++++++++++++++++++++++++++++
> >>>>    .gitlab-ci/unpackage-libcamera.sh | 17 ++++++++++++++
> >>>>    gitlab-ci.yml                     | 24 +++++++++++++++++++-
> >>>>    3 files changed, 77 insertions(+), 1 deletion(-)
> >>>>    create mode 100755 .gitlab-ci/test-lc-compliance.sh
> >>>>    create mode 100755 .gitlab-ci/unpackage-libcamera.sh
> >>>>
> >>>> diff --git a/.gitlab-ci/test-lc-compliance.sh b/.gitlab-ci/test-lc-compliance.sh
> >>>> new file mode 100755
> >>>> index 0000000..a927f1d
> >>>> --- /dev/null
> >>>> +++ b/.gitlab-ci/test-lc-compliance.sh
> >>>> @@ -0,0 +1,37 @@
> >>>> +#!/bin/bash
> >>>> +
> >>>> +# SPDX-License-Identifier: GPL-2.0-or-later
> >>>> +# SPDX-FileCopyrightText: © 2024 Google Inc.
> >>>> +
> >>>> +set -e
> >>>> +
> >>>> +source "$(dirname "$0")/lib.sh"
> >>>> +
> >>>> +libcamera_compliance() {
> >>>> +	echo "Running libcamera compliance tests in a qemu VM"
> >>>> +
> >>>> +	virtme-ng \
> >>>> +		--verbose \
> >>>> +		--skip-modules \
> >>>> +		--force-9p \
> >>>> +		--rwdir "$PWD/build" \
> >>>> +		--run /opt/linux/bzImage \
> >>>> +		--exec "\
> >>>
> >>> s/"/" /
> >>>
> >>>> +			LIBCAMERA_LOG_LEVELS=*:DEBUG \
> >>>> +			ASAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1 \
> >>>> +			UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 \
> >>>> +			GTEST_OUTPUT=xml:./build/lc-compliance-report.xml \
> >>>> +			lc-compliance -c Virtual0; \
> >>>> +			echo \\\$? > ./build/.test-status \
> >>>> +		"
> >>>> +
> >>>> +	local status=$(cat build/.test-status)
> >>>> +	echo "Test result exit state: $status"
> >>>> +	rm build/.test-status
> >>>> +
> >>>> +	if [[ $status != 0 ]] ; then
> >>>> +		exit $status
> >>>> +	fi
> >>>> +}
> >>>> +
> >>>> +run libcamera_compliance
> >>>> diff --git a/.gitlab-ci/unpackage-libcamera.sh b/.gitlab-ci/unpackage-libcamera.sh
> >>>> new file mode 100755
> >>>> index 0000000..9eb3192
> >>>> --- /dev/null
> >>>> +++ b/.gitlab-ci/unpackage-libcamera.sh
> >>>> @@ -0,0 +1,17 @@
> >>>> +#!/bin/bash
> >>>> +
> >>>> +# SPDX-License-Identifier: GPL-2.0-or-later
> >>>> +# SPDX-FileCopyrightText: © 2024 Google Inc.
> >>>> +
> >>>> +set -e
> >>>> +
> >>>> +source "$(dirname "$0")/lib.sh"
> >>>> +
> >>>> +libcamera_unpackage() {
> >>>> +	echo "Unpackage libcamera binaries"
> >>>> +
> >>>> +	tar -xvf libcamera-${CI_COMMIT_SHA}.tar.xz -C /
> >>>> +	ldconfig -v
> >>>> +}
> >>>> +
> >>>> +run libcamera_unpackage
> >>>> diff --git a/gitlab-ci.yml b/gitlab-ci.yml
> >>>> index 0a3eab3..8ea1d12 100644
> >>>> --- a/gitlab-ci.yml
> >>>> +++ b/gitlab-ci.yml
> >>>> @@ -328,11 +328,13 @@ build-package:debug:
> >>>>        BUILD_TYPE: debug
> >>>>        MESON_OPTIONS: >-
> >>>>          -D auto_features=disabled
> >>>> +      -D lc-compliance=enabled
> >>>>          -D test=false
> >>>>          -D v4l2=false
> >>>>          -D b_sanitize=address,undefined
> >>>>          -D cpp_debugstl=true
> >>>> -      -D pipelines=[]
> >>>> +      -D pipelines=['virtual']
> >>>> +      -D force_fallback_for=['gtest']
> >>>>      parallel:
> >>>>        matrix:
> >>>>          - ARCH: amd64
> >>>> @@ -440,3 +442,23 @@ test-unit:
> >>>>      # artifacts:
> >>>>      #   reports:
> >>>>      #     junit: build/meson-logs/testlog.junit.xml
> >>>> +
> >>>> +lc-compliance:virtual:
> >>>
> >>> Nitpicking on the naming, we could name this test-lc-compliance:virtual
> >>> or test-compliance:virtual to have the same "test-" prefix as
> >>> "test-unit". Maybe "test:unit" and "test:compliance:virtual" could also
> >>> be an option ?
> >>>
> >>>> +  extends:
> >>>> +    - .fdo.distribution-image@debian
> >>>> +    - .libcamera-ci.debian:12
> >>>> +    - .libcamera-ci.scripts
> >>>> +  stage: test
> >>>> +  needs:
> >>>> +    - job: build-package:debug
> >>>> +      parallel:
> >>>> +        matrix:
> >>>> +          - ARCH: amd64
> >>>> +  tags:
> >>>> +    - kvm
> >>>> +  script:
> >>>> +    - $CI_PROJECT_DIR/.gitlab-ci/unpackage-libcamera.sh
> >>>> +    - $CI_PROJECT_DIR/.gitlab-ci/test-lc-compliance.sh
> >>>> +  artifacts:
> >>>> +    reports:
> >>>> +      junit: build/lc-compliance-report.xml

Patch
diff mbox series 

diff --git a/.gitlab-ci/test-lc-compliance.sh b/.gitlab-ci/test-lc-compliance.sh
new file mode 100755
index 0000000..a927f1d
--- /dev/null
+++ b/.gitlab-ci/test-lc-compliance.sh
@@ -0,0 +1,37 @@ 
+#!/bin/bash
+
+# SPDX-License-Identifier: GPL-2.0-or-later
+# SPDX-FileCopyrightText: © 2024 Google Inc.
+
+set -e
+
+source "$(dirname "$0")/lib.sh"
+
+libcamera_compliance() {
+	echo "Running libcamera compliance tests in a qemu VM"
+
+	virtme-ng \
+		--verbose \
+		--skip-modules \
+		--force-9p \
+		--rwdir "$PWD/build" \
+		--run /opt/linux/bzImage \
+		--exec "\
+			LIBCAMERA_LOG_LEVELS=*:DEBUG \
+			ASAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1 \
+			UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 \
+			GTEST_OUTPUT=xml:./build/lc-compliance-report.xml \
+			lc-compliance -c Virtual0; \
+			echo \\\$? > ./build/.test-status \
+		"
+
+	local status=$(cat build/.test-status)
+	echo "Test result exit state: $status"
+	rm build/.test-status
+
+	if [[ $status != 0 ]] ; then
+		exit $status
+	fi
+}
+
+run libcamera_compliance
diff --git a/.gitlab-ci/unpackage-libcamera.sh b/.gitlab-ci/unpackage-libcamera.sh
new file mode 100755
index 0000000..9eb3192
--- /dev/null
+++ b/.gitlab-ci/unpackage-libcamera.sh
@@ -0,0 +1,17 @@ 
+#!/bin/bash
+
+# SPDX-License-Identifier: GPL-2.0-or-later
+# SPDX-FileCopyrightText: © 2024 Google Inc.
+
+set -e
+
+source "$(dirname "$0")/lib.sh"
+
+libcamera_unpackage() {
+	echo "Unpackage libcamera binaries"
+
+	tar -xvf libcamera-${CI_COMMIT_SHA}.tar.xz -C /
+	ldconfig -v
+}
+
+run libcamera_unpackage
diff --git a/gitlab-ci.yml b/gitlab-ci.yml
index 0a3eab3..8ea1d12 100644
--- a/gitlab-ci.yml
+++ b/gitlab-ci.yml
@@ -328,11 +328,13 @@  build-package:debug:
     BUILD_TYPE: debug
     MESON_OPTIONS: >-
       -D auto_features=disabled
+      -D lc-compliance=enabled
       -D test=false
       -D v4l2=false
       -D b_sanitize=address,undefined
       -D cpp_debugstl=true
-      -D pipelines=[]
+      -D pipelines=['virtual']
+      -D force_fallback_for=['gtest']
   parallel:
     matrix:
       - ARCH: amd64
@@ -440,3 +442,23 @@  test-unit:
   # artifacts:
   #   reports:
   #     junit: build/meson-logs/testlog.junit.xml
+
+lc-compliance:virtual:
+  extends:
+    - .fdo.distribution-image@debian
+    - .libcamera-ci.debian:12
+    - .libcamera-ci.scripts
+  stage: test
+  needs:
+    - job: build-package:debug
+      parallel:
+        matrix:
+          - ARCH: amd64
+  tags:
+    - kvm
+  script:
+    - $CI_PROJECT_DIR/.gitlab-ci/unpackage-libcamera.sh
+    - $CI_PROJECT_DIR/.gitlab-ci/test-lc-compliance.sh
+  artifacts:
+    reports:
+      junit: build/lc-compliance-report.xml