[libcamera-ci,RFC,v2,3/4] Add `build-package:debug` job

22369 diff mbox series

Add a new job named `build-package:debug`, whose purpose is to create
a build of libcamera with debugging additions (ASAN, UBSAN, STL checks),
and then to publish the resulting package as an artifact for other
jobs to use.

Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
 gitlab-ci.yml | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

--
2.47.1

Hi Barnabás,

Thank you for the patch.

On Mon, Dec 16, 2024 at 06:28:26PM +0100, Barnabás Pőcze wrote:
> Add a new job named `build-package:debug`, whose purpose is to create
> a build of libcamera with debugging additions (ASAN, UBSAN, STL checks),
> and then to publish the resulting package as an artifact for other
> jobs to use.
> 
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> ---
>  gitlab-ci.yml | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/gitlab-ci.yml b/gitlab-ci.yml
> index f679d6e..0a3eab3 100644
> --- a/gitlab-ci.yml
> +++ b/gitlab-ci.yml
> @@ -305,6 +305,39 @@ build-package:cros:
>      reports:
>        dotenv: env
> 
> +build-package:debug:
> +  extends:
> +    - .fdo.distribution-image@debian
> +    - .libcamera-ci.debian:12
> +    - .libcamera-ci.scripts
> +  stage: build
> +  needs:
> +    - job: container-debian:12
> +      artifacts: false
> +  script:
> +    - $CI_PROJECT_DIR/.gitlab-ci/build-libcamera.sh
> +    - $CI_PROJECT_DIR/.gitlab-ci/package-libcamera.sh
> +  artifacts:
> +    name: libcamera-debug-${ARCH}-${CI_COMMIT_SHA}
> +    when: always
> +    expire_in: 1 week
> +    paths:
> +      - build/meson-logs/
> +      - libcamera-${CI_COMMIT_SHA}.tar.xz
> +  variables:
> +    BUILD_TYPE: debug
> +    MESON_OPTIONS: >-
> +      -D auto_features=disabled
> +      -D test=false
> +      -D v4l2=false
> +      -D b_sanitize=address,undefined
> +      -D cpp_debugstl=true
> +      -D pipelines=[]

Any reason not to enable lc-compliance and the virtual pipeline handler
here already (essentially moving the change of options from patch 4/4 to
this one) ?

I would also like to unify this with build-package:arm64, as it's very
similar. You can add arm64 to the matrix below, and set arch-specific
meson options when they need to differ. After possibly updating
meson.build in libcamera to support "-D pipelines=['auto', 'virtual']",
I think you could simply set pipelines to 'auto' on all architectures.

> +  parallel:
> +    matrix:
> +      - ARCH: amd64
> +
> +
>  # ------------------------------------------------------------------------------
>  # Lint stage - Run checkstyle.py and check merge suitability
>  # ------------------------------------------------------------------------------

2024. 12. 17. 1:06 keltezéssel, Laurent Pinchart írta:
> Hi Barnabás,
> 
> Thank you for the patch.
> 
> On Mon, Dec 16, 2024 at 06:28:26PM +0100, Barnabás Pőcze wrote:
>> Add a new job named `build-package:debug`, whose purpose is to create
>> a build of libcamera with debugging additions (ASAN, UBSAN, STL checks),
>> and then to publish the resulting package as an artifact for other
>> jobs to use.
>>
>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
>> ---
>>   gitlab-ci.yml | 33 +++++++++++++++++++++++++++++++++
>>   1 file changed, 33 insertions(+)
>>
>> diff --git a/gitlab-ci.yml b/gitlab-ci.yml
>> index f679d6e..0a3eab3 100644
>> --- a/gitlab-ci.yml
>> +++ b/gitlab-ci.yml
>> @@ -305,6 +305,39 @@ build-package:cros:
>>       reports:
>>         dotenv: env
>>
>> +build-package:debug:
>> +  extends:
>> +    - .fdo.distribution-image@debian
>> +    - .libcamera-ci.debian:12
>> +    - .libcamera-ci.scripts
>> +  stage: build
>> +  needs:
>> +    - job: container-debian:12
>> +      artifacts: false
>> +  script:
>> +    - $CI_PROJECT_DIR/.gitlab-ci/build-libcamera.sh
>> +    - $CI_PROJECT_DIR/.gitlab-ci/package-libcamera.sh
>> +  artifacts:
>> +    name: libcamera-debug-${ARCH}-${CI_COMMIT_SHA}
>> +    when: always
>> +    expire_in: 1 week
>> +    paths:
>> +      - build/meson-logs/
>> +      - libcamera-${CI_COMMIT_SHA}.tar.xz
>> +  variables:
>> +    BUILD_TYPE: debug
>> +    MESON_OPTIONS: >-
>> +      -D auto_features=disabled
>> +      -D test=false
>> +      -D v4l2=false
>> +      -D b_sanitize=address,undefined
>> +      -D cpp_debugstl=true
>> +      -D pipelines=[]
> 
> Any reason not to enable lc-compliance and the virtual pipeline handler
> here already (essentially moving the change of options from patch 4/4 to
> this one) ?

No particular reason apart from just thinking that those changes
belong to the next patch.


> 
> I would also like to unify this with build-package:arm64, as it's very
> similar. You can add arm64 to the matrix below, and set arch-specific
> meson options when they need to differ. After possibly updating
> meson.build in libcamera to support "-D pipelines=['auto', 'virtual']",
> I think you could simply set pipelines to 'auto' on all architectures.

The motivation for not unifying the two was that build-package:arm64
does not compile with the same debug instrumentation.


Regards,
Barnabás Pőcze

> 
>> +  parallel:
>> +    matrix:
>> +      - ARCH: amd64
>> +
>> +
>>   # ------------------------------------------------------------------------------
>>   # Lint stage - Run checkstyle.py and check merge suitability
>>   # ------------------------------------------------------------------------------
>

On Tue, Dec 17, 2024 at 11:40:13AM +0100, Barnabás Pőcze wrote:
> 2024. 12. 17. 1:06 keltezéssel, Laurent Pinchart írta:
> > Hi Barnabás,
> > 
> > Thank you for the patch.
> > 
> > On Mon, Dec 16, 2024 at 06:28:26PM +0100, Barnabás Pőcze wrote:
> >> Add a new job named `build-package:debug`, whose purpose is to create
> >> a build of libcamera with debugging additions (ASAN, UBSAN, STL checks),
> >> and then to publish the resulting package as an artifact for other
> >> jobs to use.
> >>
> >> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> >> ---
> >>   gitlab-ci.yml | 33 +++++++++++++++++++++++++++++++++
> >>   1 file changed, 33 insertions(+)
> >>
> >> diff --git a/gitlab-ci.yml b/gitlab-ci.yml
> >> index f679d6e..0a3eab3 100644
> >> --- a/gitlab-ci.yml
> >> +++ b/gitlab-ci.yml
> >> @@ -305,6 +305,39 @@ build-package:cros:
> >>       reports:
> >>         dotenv: env
> >>
> >> +build-package:debug:
> >> +  extends:
> >> +    - .fdo.distribution-image@debian
> >> +    - .libcamera-ci.debian:12
> >> +    - .libcamera-ci.scripts
> >> +  stage: build
> >> +  needs:
> >> +    - job: container-debian:12
> >> +      artifacts: false
> >> +  script:
> >> +    - $CI_PROJECT_DIR/.gitlab-ci/build-libcamera.sh
> >> +    - $CI_PROJECT_DIR/.gitlab-ci/package-libcamera.sh
> >> +  artifacts:
> >> +    name: libcamera-debug-${ARCH}-${CI_COMMIT_SHA}
> >> +    when: always
> >> +    expire_in: 1 week
> >> +    paths:
> >> +      - build/meson-logs/
> >> +      - libcamera-${CI_COMMIT_SHA}.tar.xz
> >> +  variables:
> >> +    BUILD_TYPE: debug
> >> +    MESON_OPTIONS: >-
> >> +      -D auto_features=disabled
> >> +      -D test=false
> >> +      -D v4l2=false
> >> +      -D b_sanitize=address,undefined
> >> +      -D cpp_debugstl=true
> >> +      -D pipelines=[]
> > 
> > Any reason not to enable lc-compliance and the virtual pipeline handler
> > here already (essentially moving the change of options from patch 4/4 to
> > this one) ?
> 
> No particular reason apart from just thinking that those changes
> belong to the next patch.
> 
> > I would also like to unify this with build-package:arm64, as it's very
> > similar. You can add arm64 to the matrix below, and set arch-specific
> > meson options when they need to differ. After possibly updating
> > meson.build in libcamera to support "-D pipelines=['auto', 'virtual']",
> > I think you could simply set pipelines to 'auto' on all architectures.
> 
> The motivation for not unifying the two was that build-package:arm64
> does not compile with the same debug instrumentation.

As both packages are meant to run test on devices (with lc-compliance
being the main test), shouldn't we unify the debug instrumentation ? Or
are there reasons to keep them different between arm64 and amd64 ? The
amd64 package needs to run in a VM on a gitlab runner so that could make
it a bit special, but it would be nice to make it suitable to run on
real hardware too, to test the ipu3 or simple (for IPU6) pipeline
handlers.

> >> +  parallel:
> >> +    matrix:
> >> +      - ARCH: amd64
> >> +
> >> +
> >>   # ------------------------------------------------------------------------------
> >>   # Lint stage - Run checkstyle.py and check merge suitability
> >>   # ------------------------------------------------------------------------------

2024. 12. 17. 12:58 keltezéssel, Laurent Pinchart írta:
> On Tue, Dec 17, 2024 at 11:40:13AM +0100, Barnabás Pőcze wrote:
>> 2024. 12. 17. 1:06 keltezéssel, Laurent Pinchart írta:
>>> Hi Barnabás,
>>>
>>> Thank you for the patch.
>>>
>>> On Mon, Dec 16, 2024 at 06:28:26PM +0100, Barnabás Pőcze wrote:
>>>> Add a new job named `build-package:debug`, whose purpose is to create
>>>> a build of libcamera with debugging additions (ASAN, UBSAN, STL checks),
>>>> and then to publish the resulting package as an artifact for other
>>>> jobs to use.
>>>>
>>>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
>>>> ---
>>>>    gitlab-ci.yml | 33 +++++++++++++++++++++++++++++++++
>>>>    1 file changed, 33 insertions(+)
>>>>
>>>> diff --git a/gitlab-ci.yml b/gitlab-ci.yml
>>>> index f679d6e..0a3eab3 100644
>>>> --- a/gitlab-ci.yml
>>>> +++ b/gitlab-ci.yml
>>>> @@ -305,6 +305,39 @@ build-package:cros:
>>>>        reports:
>>>>          dotenv: env
>>>>
>>>> +build-package:debug:
>>>> +  extends:
>>>> +    - .fdo.distribution-image@debian
>>>> +    - .libcamera-ci.debian:12
>>>> +    - .libcamera-ci.scripts
>>>> +  stage: build
>>>> +  needs:
>>>> +    - job: container-debian:12
>>>> +      artifacts: false
>>>> +  script:
>>>> +    - $CI_PROJECT_DIR/.gitlab-ci/build-libcamera.sh
>>>> +    - $CI_PROJECT_DIR/.gitlab-ci/package-libcamera.sh
>>>> +  artifacts:
>>>> +    name: libcamera-debug-${ARCH}-${CI_COMMIT_SHA}
>>>> +    when: always
>>>> +    expire_in: 1 week
>>>> +    paths:
>>>> +      - build/meson-logs/
>>>> +      - libcamera-${CI_COMMIT_SHA}.tar.xz
>>>> +  variables:
>>>> +    BUILD_TYPE: debug
>>>> +    MESON_OPTIONS: >-
>>>> +      -D auto_features=disabled
>>>> +      -D test=false
>>>> +      -D v4l2=false
>>>> +      -D b_sanitize=address,undefined
>>>> +      -D cpp_debugstl=true
>>>> +      -D pipelines=[]
>>>
>>> Any reason not to enable lc-compliance and the virtual pipeline handler
>>> here already (essentially moving the change of options from patch 4/4 to
>>> this one) ?
>>
>> No particular reason apart from just thinking that those changes
>> belong to the next patch.
>>
>>> I would also like to unify this with build-package:arm64, as it's very
>>> similar. You can add arm64 to the matrix below, and set arch-specific
>>> meson options when they need to differ. After possibly updating
>>> meson.build in libcamera to support "-D pipelines=['auto', 'virtual']",
>>> I think you could simply set pipelines to 'auto' on all architectures.
>>
>> The motivation for not unifying the two was that build-package:arm64
>> does not compile with the same debug instrumentation.
> 
> As both packages are meant to run test on devices (with lc-compliance
> being the main test), shouldn't we unify the debug instrumentation ? Or
> are there reasons to keep them different between arm64 and amd64 ? The
> amd64 package needs to run in a VM on a gitlab runner so that could make
> it a bit special, but it would be nice to make it suitable to run on
> real hardware too, to test the ipu3 or simple (for IPU6) pipeline
> handlers.

I think it is definitely possible to unify them.


> 
>>>> +  parallel:
>>>> +    matrix:
>>>> +      - ARCH: amd64
>>>> +
>>>> +
>>>>    # ------------------------------------------------------------------------------
>>>>    # Lint stage - Run checkstyle.py and check merge suitability
>>>>    # ------------------------------------------------------------------------------
>