| Message ID | 20241209095150.164960-1-barnabas.pocze@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Sorry, of course I forgot to specify that this is for libcamera-ci. 2024. 12. 09. 10:51 keltezéssel, Barnabás Pőcze írta: > Meson's `cpp_debugstl` built-in option enables extra checks > in libstdc++ and libc++, such as iterator invalidation tests, > bounds checking in `operator[]` of multiple types, etc by > setting `GLIBCXX_DEBUG` and `_LIBCPP_HARDENING_MODE`. > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > --- > gitlab-ci.yml | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/gitlab-ci.yml b/gitlab-ci.yml > index ea038ec..ec96330 100644 > --- a/gitlab-ci.yml > +++ b/gitlab-ci.yml > @@ -15,6 +15,7 @@ variables: > -D android=enabled > -D b_sanitize=address > -D cam=enabled > + -D cpp_debugstl=true > -D documentation=enabled > -D gstreamer=enabled > -D lc-compliance=enabled > @@ -58,17 +59,17 @@ include: > .libcamera-ci.debian:11: > variables: > FDO_DISTRIBUTION_VERSION: 'bullseye' > - FDO_DISTRIBUTION_TAG: '2024-11-05.1' > + FDO_DISTRIBUTION_TAG: '2024-12-06.1' > > .libcamera-ci.debian:12: > variables: > FDO_DISTRIBUTION_VERSION: 'bookworm' > - FDO_DISTRIBUTION_TAG: '2024-11-05.1' > + FDO_DISTRIBUTION_TAG: '2024-12-06.1' > > .libcamera-ci.debian:13: > variables: > FDO_DISTRIBUTION_VERSION: 'trixie' > - FDO_DISTRIBUTION_TAG: '2024-11-05.1' > + FDO_DISTRIBUTION_TAG: '2024-12-06.1' > > .container-debian: > extends: > @@ -389,6 +390,7 @@ test-unit: > MESON_OPTIONS: >- > -D b_sanitize=address > -D cam=disabled > + -D cpp_debugstl=true > -D documentation=disabled > -D gstreamer=enabled > -D lc-compliance=disabled
Hi Barnabás, Thank you for the patch. On Mon, Dec 09, 2024 at 10:51:50AM +0100, Barnabás Pőcze wrote: > Meson's `cpp_debugstl` built-in option enables extra checks > in libstdc++ and libc++, such as iterator invalidation tests, > bounds checking in `operator[]` of multiple types, etc by > setting `GLIBCXX_DEBUG` and `_LIBCPP_HARDENING_MODE`. Could you point to a successful pipeline run with this enabled ? > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > --- > gitlab-ci.yml | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/gitlab-ci.yml b/gitlab-ci.yml > index ea038ec..ec96330 100644 > --- a/gitlab-ci.yml > +++ b/gitlab-ci.yml > @@ -15,6 +15,7 @@ variables: > -D android=enabled > -D b_sanitize=address > -D cam=enabled > + -D cpp_debugstl=true > -D documentation=enabled > -D gstreamer=enabled > -D lc-compliance=enabled > @@ -58,17 +59,17 @@ include: > .libcamera-ci.debian:11: > variables: > FDO_DISTRIBUTION_VERSION: 'bullseye' > - FDO_DISTRIBUTION_TAG: '2024-11-05.1' > + FDO_DISTRIBUTION_TAG: '2024-12-06.1' You only need to bump the container tag when you make changes to the container itself. To enable cpp_debugstl you don't need to rebuild containers. > > .libcamera-ci.debian:12: > variables: > FDO_DISTRIBUTION_VERSION: 'bookworm' > - FDO_DISTRIBUTION_TAG: '2024-11-05.1' > + FDO_DISTRIBUTION_TAG: '2024-12-06.1' > > .libcamera-ci.debian:13: > variables: > FDO_DISTRIBUTION_VERSION: 'trixie' > - FDO_DISTRIBUTION_TAG: '2024-11-05.1' > + FDO_DISTRIBUTION_TAG: '2024-12-06.1' > > .container-debian: > extends: > @@ -389,6 +390,7 @@ test-unit: > MESON_OPTIONS: >- > -D b_sanitize=address > -D cam=disabled > + -D cpp_debugstl=true > -D documentation=disabled > -D gstreamer=enabled > -D lc-compliance=disabled
Hi 2024. 12. 09. 11:12 keltezéssel, Laurent Pinchart írta: > Hi Barnabás, > > Thank you for the patch. > > On Mon, Dec 09, 2024 at 10:51:50AM +0100, Barnabás Pőcze wrote: >> Meson's `cpp_debugstl` built-in option enables extra checks >> in libstdc++ and libc++, such as iterator invalidation tests, >> bounds checking in `operator[]` of multiple types, etc by >> setting `GLIBCXX_DEBUG` and `_LIBCPP_HARDENING_MODE`. > > Could you point to a successful pipeline run with this enabled ? Please see here: https://gitlab.freedesktop.org/pobrn/libcamera/-/pipelines/1324797 Should I put it in the commit message? > >> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> >> --- >> gitlab-ci.yml | 8 +++++--- >> 1 file changed, 5 insertions(+), 3 deletions(-) >> >> diff --git a/gitlab-ci.yml b/gitlab-ci.yml >> index ea038ec..ec96330 100644 >> --- a/gitlab-ci.yml >> +++ b/gitlab-ci.yml >> @@ -15,6 +15,7 @@ variables: >> -D android=enabled >> -D b_sanitize=address >> -D cam=enabled >> + -D cpp_debugstl=true >> -D documentation=enabled >> -D gstreamer=enabled >> -D lc-compliance=enabled >> @@ -58,17 +59,17 @@ include: >> .libcamera-ci.debian:11: >> variables: >> FDO_DISTRIBUTION_VERSION: 'bullseye' >> - FDO_DISTRIBUTION_TAG: '2024-11-05.1' >> + FDO_DISTRIBUTION_TAG: '2024-12-06.1' > > You only need to bump the container tag when you make changes to the > container itself. To enable cpp_debugstl you don't need to rebuild > containers. > >> >> .libcamera-ci.debian:12: >> variables: >> FDO_DISTRIBUTION_VERSION: 'bookworm' >> - FDO_DISTRIBUTION_TAG: '2024-11-05.1' >> + FDO_DISTRIBUTION_TAG: '2024-12-06.1' >> >> .libcamera-ci.debian:13: >> variables: >> FDO_DISTRIBUTION_VERSION: 'trixie' >> - FDO_DISTRIBUTION_TAG: '2024-11-05.1' >> + FDO_DISTRIBUTION_TAG: '2024-12-06.1' >> >> .container-debian: >> extends: >> @@ -389,6 +390,7 @@ test-unit: >> MESON_OPTIONS: >- >> -D b_sanitize=address >> -D cam=disabled >> + -D cpp_debugstl=true >> -D documentation=disabled >> -D gstreamer=enabled >> -D lc-compliance=disabled > Regards, Barnabás Pőcze
On Mon, Dec 09, 2024 at 11:20:05AM +0100, Barnabás Pőcze wrote: > 2024. 12. 09. 11:12 keltezéssel, Laurent Pinchart írta: > > On Mon, Dec 09, 2024 at 10:51:50AM +0100, Barnabás Pőcze wrote: > >> Meson's `cpp_debugstl` built-in option enables extra checks > >> in libstdc++ and libc++, such as iterator invalidation tests, > >> bounds checking in `operator[]` of multiple types, etc by > >> setting `GLIBCXX_DEBUG` and `_LIBCPP_HARDENING_MODE`. > > > > Could you point to a successful pipeline run with this enabled ? > > Please see here: > https://gitlab.freedesktop.org/pobrn/libcamera/-/pipelines/1324797 > > Should I put it in the commit message? No that's fine, it was only for my information. For next time, information that you don't think belongs to the commit message but can be useful for review can be mentioned either in a cover letter, or below a --- line in the commit message of the patch. > >> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > >> --- > >> gitlab-ci.yml | 8 +++++--- > >> 1 file changed, 5 insertions(+), 3 deletions(-) > >> > >> diff --git a/gitlab-ci.yml b/gitlab-ci.yml > >> index ea038ec..ec96330 100644 > >> --- a/gitlab-ci.yml > >> +++ b/gitlab-ci.yml > >> @@ -15,6 +15,7 @@ variables: > >> -D android=enabled > >> -D b_sanitize=address > >> -D cam=enabled > >> + -D cpp_debugstl=true One additional question, should this be done for all builds, or for debug builds only ? > >> -D documentation=enabled > >> -D gstreamer=enabled > >> -D lc-compliance=enabled > >> @@ -58,17 +59,17 @@ include: > >> .libcamera-ci.debian:11: > >> variables: > >> FDO_DISTRIBUTION_VERSION: 'bullseye' > >> - FDO_DISTRIBUTION_TAG: '2024-11-05.1' > >> + FDO_DISTRIBUTION_TAG: '2024-12-06.1' > > > > You only need to bump the container tag when you make changes to the > > container itself. To enable cpp_debugstl you don't need to rebuild > > containers. > > > >> > >> .libcamera-ci.debian:12: > >> variables: > >> FDO_DISTRIBUTION_VERSION: 'bookworm' > >> - FDO_DISTRIBUTION_TAG: '2024-11-05.1' > >> + FDO_DISTRIBUTION_TAG: '2024-12-06.1' > >> > >> .libcamera-ci.debian:13: > >> variables: > >> FDO_DISTRIBUTION_VERSION: 'trixie' > >> - FDO_DISTRIBUTION_TAG: '2024-11-05.1' > >> + FDO_DISTRIBUTION_TAG: '2024-12-06.1' > >> > >> .container-debian: > >> extends: > >> @@ -389,6 +390,7 @@ test-unit: > >> MESON_OPTIONS: >- > >> -D b_sanitize=address > >> -D cam=disabled > >> + -D cpp_debugstl=true > >> -D documentation=disabled > >> -D gstreamer=enabled > >> -D lc-compliance=disabled
Hi 2024. 12. 09. 11:26 keltezéssel, Laurent Pinchart írta: > On Mon, Dec 09, 2024 at 11:20:05AM +0100, Barnabás Pőcze wrote: >> 2024. 12. 09. 11:12 keltezéssel, Laurent Pinchart írta: >>> On Mon, Dec 09, 2024 at 10:51:50AM +0100, Barnabás Pőcze wrote: >>>> Meson's `cpp_debugstl` built-in option enables extra checks >>>> in libstdc++ and libc++, such as iterator invalidation tests, >>>> bounds checking in `operator[]` of multiple types, etc by >>>> setting `GLIBCXX_DEBUG` and `_LIBCPP_HARDENING_MODE`. >>> >>> Could you point to a successful pipeline run with this enabled ? >> >> Please see here: >> https://gitlab.freedesktop.org/pobrn/libcamera/-/pipelines/1324797 >> >> Should I put it in the commit message? > > No that's fine, it was only for my information. > > For next time, information that you don't think belongs to the commit > message but can be useful for review can be mentioned either in a cover > letter, or below a --- line in the commit message of the patch. > >>>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> >>>> --- >>>> gitlab-ci.yml | 8 +++++--- >>>> 1 file changed, 5 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/gitlab-ci.yml b/gitlab-ci.yml >>>> index ea038ec..ec96330 100644 >>>> --- a/gitlab-ci.yml >>>> +++ b/gitlab-ci.yml >>>> @@ -15,6 +15,7 @@ variables: >>>> -D android=enabled >>>> -D b_sanitize=address >>>> -D cam=enabled >>>> + -D cpp_debugstl=true > > One additional question, should this be done for all builds, or for > debug builds only ? After some deliberation I decided to enable it everywhere where AddressSanitizer is enabled, as I don't believe it to be meaningfully different from AddressSanitizer in purpose. But I think not enabling it for release builds also makes sense, so I could do that too. Regards, Barnabás Pőcze > >>>> -D documentation=enabled >>>> -D gstreamer=enabled >>>> -D lc-compliance=enabled >>>> @@ -58,17 +59,17 @@ include: >>>> .libcamera-ci.debian:11: >>>> variables: >>>> FDO_DISTRIBUTION_VERSION: 'bullseye' >>>> - FDO_DISTRIBUTION_TAG: '2024-11-05.1' >>>> + FDO_DISTRIBUTION_TAG: '2024-12-06.1' >>> >>> You only need to bump the container tag when you make changes to the >>> container itself. To enable cpp_debugstl you don't need to rebuild >>> containers. >>> >>>> >>>> .libcamera-ci.debian:12: >>>> variables: >>>> FDO_DISTRIBUTION_VERSION: 'bookworm' >>>> - FDO_DISTRIBUTION_TAG: '2024-11-05.1' >>>> + FDO_DISTRIBUTION_TAG: '2024-12-06.1' >>>> >>>> .libcamera-ci.debian:13: >>>> variables: >>>> FDO_DISTRIBUTION_VERSION: 'trixie' >>>> - FDO_DISTRIBUTION_TAG: '2024-11-05.1' >>>> + FDO_DISTRIBUTION_TAG: '2024-12-06.1' >>>> >>>> .container-debian: >>>> extends: >>>> @@ -389,6 +390,7 @@ test-unit: >>>> MESON_OPTIONS: >- >>>> -D b_sanitize=address >>>> -D cam=disabled >>>> + -D cpp_debugstl=true >>>> -D documentation=disabled >>>> -D gstreamer=enabled >>>> -D lc-compliance=disabled >
On Mon, Dec 09, 2024 at 11:38:31AM +0100, Barnabás Pőcze wrote: > 2024. 12. 09. 11:26 keltezéssel, Laurent Pinchart írta: > > On Mon, Dec 09, 2024 at 11:20:05AM +0100, Barnabás Pőcze wrote: > >> 2024. 12. 09. 11:12 keltezéssel, Laurent Pinchart írta: > >>> On Mon, Dec 09, 2024 at 10:51:50AM +0100, Barnabás Pőcze wrote: > >>>> Meson's `cpp_debugstl` built-in option enables extra checks > >>>> in libstdc++ and libc++, such as iterator invalidation tests, > >>>> bounds checking in `operator[]` of multiple types, etc by > >>>> setting `GLIBCXX_DEBUG` and `_LIBCPP_HARDENING_MODE`. > >>> > >>> Could you point to a successful pipeline run with this enabled ? > >> > >> Please see here: > >> https://gitlab.freedesktop.org/pobrn/libcamera/-/pipelines/1324797 > >> > >> Should I put it in the commit message? > > > > No that's fine, it was only for my information. > > > > For next time, information that you don't think belongs to the commit > > message but can be useful for review can be mentioned either in a cover > > letter, or below a --- line in the commit message of the patch. > > > >>>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > >>>> --- > >>>> gitlab-ci.yml | 8 +++++--- > >>>> 1 file changed, 5 insertions(+), 3 deletions(-) > >>>> > >>>> diff --git a/gitlab-ci.yml b/gitlab-ci.yml > >>>> index ea038ec..ec96330 100644 > >>>> --- a/gitlab-ci.yml > >>>> +++ b/gitlab-ci.yml > >>>> @@ -15,6 +15,7 @@ variables: > >>>> -D android=enabled > >>>> -D b_sanitize=address > >>>> -D cam=enabled > >>>> + -D cpp_debugstl=true > > > > One additional question, should this be done for all builds, or for > > debug builds only ? > > After some deliberation I decided to enable it everywhere where > AddressSanitizer is enabled, as I don't believe it to be meaningfully > different from AddressSanitizer in purpose. But I think not enabling it > for release builds also makes sense, so I could do that too. Tying it to ASan makes sense. Please mention that in the commit message for the next version. > >>>> -D documentation=enabled > >>>> -D gstreamer=enabled > >>>> -D lc-compliance=enabled > >>>> @@ -58,17 +59,17 @@ include: > >>>> .libcamera-ci.debian:11: > >>>> variables: > >>>> FDO_DISTRIBUTION_VERSION: 'bullseye' > >>>> - FDO_DISTRIBUTION_TAG: '2024-11-05.1' > >>>> + FDO_DISTRIBUTION_TAG: '2024-12-06.1' > >>> > >>> You only need to bump the container tag when you make changes to the > >>> container itself. To enable cpp_debugstl you don't need to rebuild > >>> containers. > >>> > >>>> > >>>> .libcamera-ci.debian:12: > >>>> variables: > >>>> FDO_DISTRIBUTION_VERSION: 'bookworm' > >>>> - FDO_DISTRIBUTION_TAG: '2024-11-05.1' > >>>> + FDO_DISTRIBUTION_TAG: '2024-12-06.1' > >>>> > >>>> .libcamera-ci.debian:13: > >>>> variables: > >>>> FDO_DISTRIBUTION_VERSION: 'trixie' > >>>> - FDO_DISTRIBUTION_TAG: '2024-11-05.1' > >>>> + FDO_DISTRIBUTION_TAG: '2024-12-06.1' > >>>> > >>>> .container-debian: > >>>> extends: > >>>> @@ -389,6 +390,7 @@ test-unit: > >>>> MESON_OPTIONS: >- > >>>> -D b_sanitize=address > >>>> -D cam=disabled > >>>> + -D cpp_debugstl=true > >>>> -D documentation=disabled > >>>> -D gstreamer=enabled > >>>> -D lc-compliance=disabled
diff --git a/gitlab-ci.yml b/gitlab-ci.yml index ea038ec..ec96330 100644 --- a/gitlab-ci.yml +++ b/gitlab-ci.yml @@ -15,6 +15,7 @@ variables: -D android=enabled -D b_sanitize=address -D cam=enabled + -D cpp_debugstl=true -D documentation=enabled -D gstreamer=enabled -D lc-compliance=enabled @@ -58,17 +59,17 @@ include: .libcamera-ci.debian:11: variables: FDO_DISTRIBUTION_VERSION: 'bullseye' - FDO_DISTRIBUTION_TAG: '2024-11-05.1' + FDO_DISTRIBUTION_TAG: '2024-12-06.1' .libcamera-ci.debian:12: variables: FDO_DISTRIBUTION_VERSION: 'bookworm' - FDO_DISTRIBUTION_TAG: '2024-11-05.1' + FDO_DISTRIBUTION_TAG: '2024-12-06.1' .libcamera-ci.debian:13: variables: FDO_DISTRIBUTION_VERSION: 'trixie' - FDO_DISTRIBUTION_TAG: '2024-11-05.1' + FDO_DISTRIBUTION_TAG: '2024-12-06.1' .container-debian: extends: @@ -389,6 +390,7 @@ test-unit: MESON_OPTIONS: >- -D b_sanitize=address -D cam=disabled + -D cpp_debugstl=true -D documentation=disabled -D gstreamer=enabled -D lc-compliance=disabled
Meson's `cpp_debugstl` built-in option enables extra checks in libstdc++ and libc++, such as iterator invalidation tests, bounds checking in `operator[]` of multiple types, etc by setting `GLIBCXX_DEBUG` and `_LIBCPP_HARDENING_MODE`. Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> --- gitlab-ci.yml | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)