[v1] libcamera: pipeline: rpi: Set `werror=false` for libpisp
diff mbox series

Message ID 20251002145506.996443-1-barnabas.pocze@ideasonboard.com
State New
Headers show
Series
  • [v1] libcamera: pipeline: rpi: Set `werror=false` for libpisp
Related show

Commit Message

Barnabás Pőcze Oct. 2, 2025, 2:55 p.m. UTC
By default libcamera is configured with `werror=true`, and this option can
propagate into the subprojects. This can lead to compilation failures due to
warnings in those subprojects. This is the case with libpisp. Since these
issues are not directly fixable, set `werror=false` for libpisp.

Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
 src/libcamera/pipeline/rpi/pisp/meson.build | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Laurent Pinchart Oct. 2, 2025, 3:01 p.m. UTC | #1
Hi Barnabás,

On Thu, Oct 02, 2025 at 04:55:06PM +0200, Barnabás Pőcze wrote:
> By default libcamera is configured with `werror=true`, and this option can
> propagate into the subprojects. This can lead to compilation failures due to
> warnings in those subprojects. This is the case with libpisp. Since these
> issues are not directly fixable, set `werror=false` for libpisp.

Can't we fix those errors ?

> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> ---
>  src/libcamera/pipeline/rpi/pisp/meson.build | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/pipeline/rpi/pisp/meson.build b/src/libcamera/pipeline/rpi/pisp/meson.build
> index 178df94c2..642080014 100644
> --- a/src/libcamera/pipeline/rpi/pisp/meson.build
> +++ b/src/libcamera/pipeline/rpi/pisp/meson.build
> @@ -5,7 +5,9 @@ libcamera_internal_sources += files([
>  ])
>  
>  librt = cc.find_library('rt', required : true)
> -libpisp_dep = dependency('libpisp', fallback : ['libpisp', 'libpisp_dep'])
> +libpisp_dep = dependency('libpisp',
> +                         fallback : ['libpisp', 'libpisp_dep'],
> +                         default_options : ['werror=false'])
>  
>  libcamera_deps += [libpisp_dep, librt]
>
Naushir Patuck Oct. 2, 2025, 3:09 p.m. UTC | #2
Hi,


On Thu, 2 Oct 2025 at 16:01, Laurent Pinchart <
laurent.pinchart@ideasonboard.com> wrote:

> Hi Barnabás,
>
> On Thu, Oct 02, 2025 at 04:55:06PM +0200, Barnabás Pőcze wrote:
> > By default libcamera is configured with `werror=true`, and this option
> can
> > propagate into the subprojects. This can lead to compilation failures
> due to
> > warnings in those subprojects. This is the case with libpisp. Since these
> > issues are not directly fixable, set `werror=false` for libpisp.
>
> Can't we fix those errors ?
>

I've not noticed any compilation warnings over the few versions of gcc and
clang that we use in CI.  We'd be happy to fix any warnings if you can
share the compile logs.

Regards,
Naush


>
> > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> > ---
> >  src/libcamera/pipeline/rpi/pisp/meson.build | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/libcamera/pipeline/rpi/pisp/meson.build
> b/src/libcamera/pipeline/rpi/pisp/meson.build
> > index 178df94c2..642080014 100644
> > --- a/src/libcamera/pipeline/rpi/pisp/meson.build
> > +++ b/src/libcamera/pipeline/rpi/pisp/meson.build
> > @@ -5,7 +5,9 @@ libcamera_internal_sources += files([
> >  ])
> >
> >  librt = cc.find_library('rt', required : true)
> > -libpisp_dep = dependency('libpisp', fallback : ['libpisp',
> 'libpisp_dep'])
> > +libpisp_dep = dependency('libpisp',
> > +                         fallback : ['libpisp', 'libpisp_dep'],
> > +                         default_options : ['werror=false'])
> >
> >  libcamera_deps += [libpisp_dep, librt]
> >
>
> --
> Regards,
>
> Laurent Pinchart
>
Barnabás Pőcze Oct. 2, 2025, 3:19 p.m. UTC | #3
Hi

2025. 10. 02. 17:01 keltezéssel, Laurent Pinchart írta:
> Hi Barnabás,
> 
> On Thu, Oct 02, 2025 at 04:55:06PM +0200, Barnabás Pőcze wrote:
>> By default libcamera is configured with `werror=true`, and this option can
>> propagate into the subprojects. This can lead to compilation failures due to
>> warnings in those subprojects. This is the case with libpisp. Since these
>> issues are not directly fixable, set `werror=false` for libpisp.
> 
> Can't we fix those errors ?

I might be misunderstanding, but I think that should be done in the context of the
libpisp project. I would argue you cannot immediately act on the errors as if they were
inside the libcamera code base; short of adding patch files and having meson apply them.
But seems like a lot of effort.

The specific issue I have is already fixed: https://github.com/raspberrypi/libpisp/pull/53
but no release has been made, and the wrap file uses the last release, not the master branch.


Regards,
Barnabás Pőcze

> 
>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
>> ---
>>   src/libcamera/pipeline/rpi/pisp/meson.build | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/libcamera/pipeline/rpi/pisp/meson.build b/src/libcamera/pipeline/rpi/pisp/meson.build
>> index 178df94c2..642080014 100644
>> --- a/src/libcamera/pipeline/rpi/pisp/meson.build
>> +++ b/src/libcamera/pipeline/rpi/pisp/meson.build
>> @@ -5,7 +5,9 @@ libcamera_internal_sources += files([
>>   ])
>>
>>   librt = cc.find_library('rt', required : true)
>> -libpisp_dep = dependency('libpisp', fallback : ['libpisp', 'libpisp_dep'])
>> +libpisp_dep = dependency('libpisp',
>> +                         fallback : ['libpisp', 'libpisp_dep'],
>> +                         default_options : ['werror=false'])
>>
>>   libcamera_deps += [libpisp_dep, librt]
>>
> 
> --
> Regards,
> 
> Laurent Pinchart
Laurent Pinchart Oct. 2, 2025, 5:03 p.m. UTC | #4
Hi Barnabás,

On Thu, Oct 02, 2025 at 05:19:41PM +0200, Barnabás Pőcze wrote:
> 2025. 10. 02. 17:01 keltezéssel, Laurent Pinchart írta:
> > On Thu, Oct 02, 2025 at 04:55:06PM +0200, Barnabás Pőcze wrote:
> >> By default libcamera is configured with `werror=true`, and this option can
> >> propagate into the subprojects. This can lead to compilation failures due to
> >> warnings in those subprojects. This is the case with libpisp. Since these
> >> issues are not directly fixable, set `werror=false` for libpisp.
> > 
> > Can't we fix those errors ?
> 
> I might be misunderstanding, but I think that should be done in the context of the
> libpisp project.

Yes you're right.

> I would argue you cannot immediately act on the errors as if they were
> inside the libcamera code base; short of adding patch files and having meson apply them.
> But seems like a lot of effort.

Agreed, I don't think we should do that.

> The specific issue I have is already fixed: https://github.com/raspberrypi/libpisp/pull/53
> but no release has been made, and the wrap file uses the last release, not the master branch.

Naush, could you make a release ?

If this issue occurs again we would set werror=false for libpisp, but if
it's easy to fix this time we can just update to a new release.

I see libpisp has werror=true in meson.build, and there's a CI pipeline
that compiles the project with both gcc and clang. Why wasn't this
detected, different gcc versions maybe ?

This being said, keeping werror=true for this dependency means things
could break again in the future, and we'll have then to deal with the
issue. I'm wondering if there's value in us noticing compiler warnings
in libpisp, maybe it's best to ignore them for all dependencies ?

> >> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> >> ---
> >>   src/libcamera/pipeline/rpi/pisp/meson.build | 4 +++-
> >>   1 file changed, 3 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/src/libcamera/pipeline/rpi/pisp/meson.build b/src/libcamera/pipeline/rpi/pisp/meson.build
> >> index 178df94c2..642080014 100644
> >> --- a/src/libcamera/pipeline/rpi/pisp/meson.build
> >> +++ b/src/libcamera/pipeline/rpi/pisp/meson.build
> >> @@ -5,7 +5,9 @@ libcamera_internal_sources += files([
> >>   ])
> >>
> >>   librt = cc.find_library('rt', required : true)
> >> -libpisp_dep = dependency('libpisp', fallback : ['libpisp', 'libpisp_dep'])
> >> +libpisp_dep = dependency('libpisp',
> >> +                         fallback : ['libpisp', 'libpisp_dep'],
> >> +                         default_options : ['werror=false'])
> >>
> >>   libcamera_deps += [libpisp_dep, librt]
> >>
Barnabás Pőcze Oct. 2, 2025, 5:30 p.m. UTC | #5
2025. 10. 02. 19:03 keltezéssel, Laurent Pinchart írta:
> Hi Barnabás,
> 
> On Thu, Oct 02, 2025 at 05:19:41PM +0200, Barnabás Pőcze wrote:
>> 2025. 10. 02. 17:01 keltezéssel, Laurent Pinchart írta:
>>> On Thu, Oct 02, 2025 at 04:55:06PM +0200, Barnabás Pőcze wrote:
>>>> By default libcamera is configured with `werror=true`, and this option can
>>>> propagate into the subprojects. This can lead to compilation failures due to
>>>> warnings in those subprojects. This is the case with libpisp. Since these
>>>> issues are not directly fixable, set `werror=false` for libpisp.
>>>
>>> Can't we fix those errors ?
>>
>> I might be misunderstanding, but I think that should be done in the context of the
>> libpisp project.
> 
> Yes you're right.
> 
>> I would argue you cannot immediately act on the errors as if they were
>> inside the libcamera code base; short of adding patch files and having meson apply them.
>> But seems like a lot of effort.
> 
> Agreed, I don't think we should do that.
> 
>> The specific issue I have is already fixed: https://github.com/raspberrypi/libpisp/pull/53
>> but no release has been made, and the wrap file uses the last release, not the master branch.
> 
> Naush, could you make a release ?
> 
> If this issue occurs again we would set werror=false for libpisp, but if
> it's easy to fix this time we can just update to a new release.
> 
> I see libpisp has werror=true in meson.build, and there's a CI pipeline
> that compiles the project with both gcc and clang. Why wasn't this
> detected, different gcc versions maybe ?

I think one issue with that CI pipeline is that, as far as I can see, it runs
only on x86 and compiles natively. So it does not pick up the slight idiosyncrasies
of the (glibc) headers of different architectures.

There are aarch64 runners available nowadays, I believe that would be an easy way
to add aarch64 compilation coverage: https://github.com/orgs/community/discussions/148648


Regards,
Barnabás Pőcze


> 
> This being said, keeping werror=true for this dependency means things
> could break again in the future, and we'll have then to deal with the
> issue. I'm wondering if there's value in us noticing compiler warnings
> in libpisp, maybe it's best to ignore them for all dependencies ?
> 
>>>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
>>>> ---
>>>>    src/libcamera/pipeline/rpi/pisp/meson.build | 4 +++-
>>>>    1 file changed, 3 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/src/libcamera/pipeline/rpi/pisp/meson.build b/src/libcamera/pipeline/rpi/pisp/meson.build
>>>> index 178df94c2..642080014 100644
>>>> --- a/src/libcamera/pipeline/rpi/pisp/meson.build
>>>> +++ b/src/libcamera/pipeline/rpi/pisp/meson.build
>>>> @@ -5,7 +5,9 @@ libcamera_internal_sources += files([
>>>>    ])
>>>>
>>>>    librt = cc.find_library('rt', required : true)
>>>> -libpisp_dep = dependency('libpisp', fallback : ['libpisp', 'libpisp_dep'])
>>>> +libpisp_dep = dependency('libpisp',
>>>> +                         fallback : ['libpisp', 'libpisp_dep'],
>>>> +                         default_options : ['werror=false'])
>>>>
>>>>    libcamera_deps += [libpisp_dep, librt]
>>>>
> 
> --
> Regards,
> 
> Laurent Pinchart
Laurent Pinchart Oct. 2, 2025, 6:11 p.m. UTC | #6
On Thu, Oct 02, 2025 at 07:30:45PM +0200, Barnabás Pőcze wrote:
> 2025. 10. 02. 19:03 keltezéssel, Laurent Pinchart írta:
> > On Thu, Oct 02, 2025 at 05:19:41PM +0200, Barnabás Pőcze wrote:
> >> 2025. 10. 02. 17:01 keltezéssel, Laurent Pinchart írta:
> >>> On Thu, Oct 02, 2025 at 04:55:06PM +0200, Barnabás Pőcze wrote:
> >>>> By default libcamera is configured with `werror=true`, and this option can
> >>>> propagate into the subprojects. This can lead to compilation failures due to
> >>>> warnings in those subprojects. This is the case with libpisp. Since these
> >>>> issues are not directly fixable, set `werror=false` for libpisp.
> >>>
> >>> Can't we fix those errors ?
> >>
> >> I might be misunderstanding, but I think that should be done in the context of the
> >> libpisp project.
> > 
> > Yes you're right.
> > 
> >> I would argue you cannot immediately act on the errors as if they were
> >> inside the libcamera code base; short of adding patch files and having meson apply them.
> >> But seems like a lot of effort.
> > 
> > Agreed, I don't think we should do that.
> > 
> >> The specific issue I have is already fixed: https://github.com/raspberrypi/libpisp/pull/53
> >> but no release has been made, and the wrap file uses the last release, not the master branch.
> > 
> > Naush, could you make a release ?
> > 
> > If this issue occurs again we would set werror=false for libpisp, but if
> > it's easy to fix this time we can just update to a new release.
> > 
> > I see libpisp has werror=true in meson.build, and there's a CI pipeline
> > that compiles the project with both gcc and clang. Why wasn't this
> > detected, different gcc versions maybe ?
> 
> I think one issue with that CI pipeline is that, as far as I can see, it runs
> only on x86 and compiles natively. So it does not pick up the slight idiosyncrasies
> of the (glibc) headers of different architectures.
> 
> There are aarch64 runners available nowadays, I believe that would be an easy way
> to add aarch64 compilation coverage: https://github.com/orgs/community/discussions/148648

Especially for libpisp compilation on aarch64 is more useful than
compilation on x86 :-)

If you would like to merge this patch,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> > This being said, keeping werror=true for this dependency means things
> > could break again in the future, and we'll have then to deal with the
> > issue. I'm wondering if there's value in us noticing compiler warnings
> > in libpisp, maybe it's best to ignore them for all dependencies ?
> > 
> >>>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> >>>> ---
> >>>>    src/libcamera/pipeline/rpi/pisp/meson.build | 4 +++-
> >>>>    1 file changed, 3 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/src/libcamera/pipeline/rpi/pisp/meson.build b/src/libcamera/pipeline/rpi/pisp/meson.build
> >>>> index 178df94c2..642080014 100644
> >>>> --- a/src/libcamera/pipeline/rpi/pisp/meson.build
> >>>> +++ b/src/libcamera/pipeline/rpi/pisp/meson.build
> >>>> @@ -5,7 +5,9 @@ libcamera_internal_sources += files([
> >>>>    ])
> >>>>
> >>>>    librt = cc.find_library('rt', required : true)
> >>>> -libpisp_dep = dependency('libpisp', fallback : ['libpisp', 'libpisp_dep'])
> >>>> +libpisp_dep = dependency('libpisp',
> >>>> +                         fallback : ['libpisp', 'libpisp_dep'],
> >>>> +                         default_options : ['werror=false'])
> >>>>
> >>>>    libcamera_deps += [libpisp_dep, librt]
> >>>>
Naushir Patuck Oct. 3, 2025, 6:50 a.m. UTC | #7
On Thu, 2 Oct 2025 at 19:11, Laurent Pinchart <
laurent.pinchart@ideasonboard.com> wrote:

> On Thu, Oct 02, 2025 at 07:30:45PM +0200, Barnabás Pőcze wrote:
> > 2025. 10. 02. 19:03 keltezéssel, Laurent Pinchart írta:
> > > On Thu, Oct 02, 2025 at 05:19:41PM +0200, Barnabás Pőcze wrote:
> > >> 2025. 10. 02. 17:01 keltezéssel, Laurent Pinchart írta:
> > >>> On Thu, Oct 02, 2025 at 04:55:06PM +0200, Barnabás Pőcze wrote:
> > >>>> By default libcamera is configured with `werror=true`, and this
> option can
> > >>>> propagate into the subprojects. This can lead to compilation
> failures due to
> > >>>> warnings in those subprojects. This is the case with libpisp. Since
> these
> > >>>> issues are not directly fixable, set `werror=false` for libpisp.
> > >>>
> > >>> Can't we fix those errors ?
> > >>
> > >> I might be misunderstanding, but I think that should be done in the
> context of the
> > >> libpisp project.
> > >
> > > Yes you're right.
> > >
> > >> I would argue you cannot immediately act on the errors as if they were
> > >> inside the libcamera code base; short of adding patch files and
> having meson apply them.
> > >> But seems like a lot of effort.
> > >
> > > Agreed, I don't think we should do that.
> > >
> > >> The specific issue I have is already fixed:
> https://github.com/raspberrypi/libpisp/pull/53
> > >> but no release has been made, and the wrap file uses the last
> release, not the master branch.
> > >
> > > Naush, could you make a release ?
>

I can arrange a release shortly.  Also happy to update the wrap file to
point to master instead of a particular release if you prefer?


> > >
> > > If this issue occurs again we would set werror=false for libpisp, but
> if
> > > it's easy to fix this time we can just update to a new release.
> > >
> > > I see libpisp has werror=true in meson.build, and there's a CI pipeline
> > > that compiles the project with both gcc and clang. Why wasn't this
> > > detected, different gcc versions maybe ?
> >
> > I think one issue with that CI pipeline is that, as far as I can see, it
> runs
> > only on x86 and compiles natively. So it does not pick up the slight
> idiosyncrasies
> > of the (glibc) headers of different architectures.
> >
> > There are aarch64 runners available nowadays, I believe that would be an
> easy way
> > to add aarch64 compilation coverage:
> https://github.com/orgs/community/discussions/148648
>
> Especially for libpisp compilation on aarch64 is more useful than
> compilation on x86 :-)
>

The aarch64 build happens as part of the device run-tests, but it's a later
version of gcc which seems to complain.

Regards,
Naush


>
> If you would like to merge this patch,
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> > > This being said, keeping werror=true for this dependency means things
> > > could break again in the future, and we'll have then to deal with the
> > > issue. I'm wondering if there's value in us noticing compiler warnings
> > > in libpisp, maybe it's best to ignore them for all dependencies ?
> > >
> > >>>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> > >>>> ---
> > >>>>    src/libcamera/pipeline/rpi/pisp/meson.build | 4 +++-
> > >>>>    1 file changed, 3 insertions(+), 1 deletion(-)
> > >>>>
> > >>>> diff --git a/src/libcamera/pipeline/rpi/pisp/meson.build
> b/src/libcamera/pipeline/rpi/pisp/meson.build
> > >>>> index 178df94c2..642080014 100644
> > >>>> --- a/src/libcamera/pipeline/rpi/pisp/meson.build
> > >>>> +++ b/src/libcamera/pipeline/rpi/pisp/meson.build
> > >>>> @@ -5,7 +5,9 @@ libcamera_internal_sources += files([
> > >>>>    ])
> > >>>>
> > >>>>    librt = cc.find_library('rt', required : true)
> > >>>> -libpisp_dep = dependency('libpisp', fallback : ['libpisp',
> 'libpisp_dep'])
> > >>>> +libpisp_dep = dependency('libpisp',
> > >>>> +                         fallback : ['libpisp', 'libpisp_dep'],
> > >>>> +                         default_options : ['werror=false'])
> > >>>>
> > >>>>    libcamera_deps += [libpisp_dep, librt]
> > >>>>
>
> --
> Regards,
>
> Laurent Pinchart
>
Barnabás Pőcze Oct. 3, 2025, 7:32 a.m. UTC | #8
2025. 10. 03. 8:50 keltezéssel, Naushir Patuck írta:
> 
> 
> On Thu, 2 Oct 2025 at 19:11, Laurent Pinchart <laurent.pinchart@ideasonboard.com <mailto:laurent.pinchart@ideasonboard.com>> wrote:
> 
>     On Thu, Oct 02, 2025 at 07:30:45PM +0200, Barnabás Pőcze wrote:
>      > 2025. 10. 02. 19:03 keltezéssel, Laurent Pinchart írta:
>      > > On Thu, Oct 02, 2025 at 05:19:41PM +0200, Barnabás Pőcze wrote:
>      > >> 2025. 10. 02. 17:01 keltezéssel, Laurent Pinchart írta:
>      > >>> On Thu, Oct 02, 2025 at 04:55:06PM +0200, Barnabás Pőcze wrote:
>      > >>>> By default libcamera is configured with `werror=true`, and this option can
>      > >>>> propagate into the subprojects. This can lead to compilation failures due to
>      > >>>> warnings in those subprojects. This is the case with libpisp. Since these
>      > >>>> issues are not directly fixable, set `werror=false` for libpisp.
>      > >>>
>      > >>> Can't we fix those errors ?
>      > >>
>      > >> I might be misunderstanding, but I think that should be done in the context of the
>      > >> libpisp project.
>      > >
>      > > Yes you're right.
>      > >
>      > >> I would argue you cannot immediately act on the errors as if they were
>      > >> inside the libcamera code base; short of adding patch files and having meson apply them.
>      > >> But seems like a lot of effort.
>      > >
>      > > Agreed, I don't think we should do that.
>      > >
>      > >> The specific issue I have is already fixed: https://github.com/raspberrypi/libpisp/pull/53 <https://github.com/raspberrypi/libpisp/pull/53>
>      > >> but no release has been made, and the wrap file uses the last release, not the master branch.
>      > >
>      > > Naush, could you make a release ?
> 
> 
> I can arrange a release shortly.  Also happy to update the wrap file to point to master instead of a particular release if you prefer?
> 
>      > >
>      > > If this issue occurs again we would set werror=false for libpisp, but if
>      > > it's easy to fix this time we can just update to a new release.
>      > >
>      > > I see libpisp has werror=true in meson.build, and there's a CI pipeline
>      > > that compiles the project with both gcc and clang. Why wasn't this
>      > > detected, different gcc versions maybe ?
>      >
>      > I think one issue with that CI pipeline is that, as far as I can see, it runs
>      > only on x86 and compiles natively. So it does not pick up the slight idiosyncrasies
>      > of the (glibc) headers of different architectures.
>      >
>      > There are aarch64 runners available nowadays, I believe that would be an easy way
>      > to add aarch64 compilation coverage: https://github.com/orgs/community/discussions/148648 <https://github.com/orgs/community/discussions/148648>
> 
>     Especially for libpisp compilation on aarch64 is more useful than
>     compilation on x86 :-)
> 
> 
> The aarch64 build happens as part of the device run-tests, but it's a later version of gcc which seems to complain.

Ahh, sorry, I see now that you have on device tests using self-hosted runners in
pisp-verification.test.yaml. I still think adding the github-provided aarch64
runners to libpisp-build-test.yaml should be simple and is still useful.


Regards,
Barnabás Pőcze

> 
> Regards,
> Naush
> 
> 
>     If you would like to merge this patch,
> 
>     Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com <mailto:laurent.pinchart@ideasonboard.com>>
> 
>      > > This being said, keeping werror=true for this dependency means things
>      > > could break again in the future, and we'll have then to deal with the
>      > > issue. I'm wondering if there's value in us noticing compiler warnings
>      > > in libpisp, maybe it's best to ignore them for all dependencies ?
>      > >
>      > >>>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com <mailto:barnabas.pocze@ideasonboard.com>>
>      > >>>> ---
>      > >>>>    src/libcamera/pipeline/rpi/pisp/meson.build | 4 +++-
>      > >>>>    1 file changed, 3 insertions(+), 1 deletion(-)
>      > >>>>
>      > >>>> diff --git a/src/libcamera/pipeline/rpi/pisp/meson.build b/src/libcamera/pipeline/rpi/pisp/meson.build
>      > >>>> index 178df94c2..642080014 100644
>      > >>>> --- a/src/libcamera/pipeline/rpi/pisp/meson.build
>      > >>>> +++ b/src/libcamera/pipeline/rpi/pisp/meson.build
>      > >>>> @@ -5,7 +5,9 @@ libcamera_internal_sources += files([
>      > >>>>    ])
>      > >>>>
>      > >>>>    librt = cc.find_library('rt', required : true)
>      > >>>> -libpisp_dep = dependency('libpisp', fallback : ['libpisp', 'libpisp_dep'])
>      > >>>> +libpisp_dep = dependency('libpisp',
>      > >>>> +                         fallback : ['libpisp', 'libpisp_dep'],
>      > >>>> +                         default_options : ['werror=false'])
>      > >>>>
>      > >>>>    libcamera_deps += [libpisp_dep, librt]
>      > >>>>
> 
>     -- 
>     Regards,
> 
>     Laurent Pinchart
>
Naushir Patuck Oct. 3, 2025, 7:38 a.m. UTC | #9
On Fri, 3 Oct 2025 at 08:32, Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
wrote:

> 2025. 10. 03. 8:50 keltezéssel, Naushir Patuck írta:
> >
> >
> > On Thu, 2 Oct 2025 at 19:11, Laurent Pinchart <
> laurent.pinchart@ideasonboard.com <mailto:
> laurent.pinchart@ideasonboard.com>> wrote:
> >
> >     On Thu, Oct 02, 2025 at 07:30:45PM +0200, Barnabás Pőcze wrote:
> >      > 2025. 10. 02. 19:03 keltezéssel, Laurent Pinchart írta:
> >      > > On Thu, Oct 02, 2025 at 05:19:41PM +0200, Barnabás Pőcze wrote:
> >      > >> 2025. 10. 02. 17:01 keltezéssel, Laurent Pinchart írta:
> >      > >>> On Thu, Oct 02, 2025 at 04:55:06PM +0200, Barnabás Pőcze
> wrote:
> >      > >>>> By default libcamera is configured with `werror=true`, and
> this option can
> >      > >>>> propagate into the subprojects. This can lead to compilation
> failures due to
> >      > >>>> warnings in those subprojects. This is the case with
> libpisp. Since these
> >      > >>>> issues are not directly fixable, set `werror=false` for
> libpisp.
> >      > >>>
> >      > >>> Can't we fix those errors ?
> >      > >>
> >      > >> I might be misunderstanding, but I think that should be done
> in the context of the
> >      > >> libpisp project.
> >      > >
> >      > > Yes you're right.
> >      > >
> >      > >> I would argue you cannot immediately act on the errors as if
> they were
> >      > >> inside the libcamera code base; short of adding patch files
> and having meson apply them.
> >      > >> But seems like a lot of effort.
> >      > >
> >      > > Agreed, I don't think we should do that.
> >      > >
> >      > >> The specific issue I have is already fixed:
> https://github.com/raspberrypi/libpisp/pull/53 <
> https://github.com/raspberrypi/libpisp/pull/53>
> >      > >> but no release has been made, and the wrap file uses the last
> release, not the master branch.
> >      > >
> >      > > Naush, could you make a release ?
> >
> >
> > I can arrange a release shortly.  Also happy to update the wrap file to
> point to master instead of a particular release if you prefer?
> >
> >      > >
> >      > > If this issue occurs again we would set werror=false for
> libpisp, but if
> >      > > it's easy to fix this time we can just update to a new release.
> >      > >
> >      > > I see libpisp has werror=true in meson.build, and there's a CI
> pipeline
> >      > > that compiles the project with both gcc and clang. Why wasn't
> this
> >      > > detected, different gcc versions maybe ?
> >      >
> >      > I think one issue with that CI pipeline is that, as far as I can
> see, it runs
> >      > only on x86 and compiles natively. So it does not pick up the
> slight idiosyncrasies
> >      > of the (glibc) headers of different architectures.
> >      >
> >      > There are aarch64 runners available nowadays, I believe that
> would be an easy way
> >      > to add aarch64 compilation coverage:
> https://github.com/orgs/community/discussions/148648 <
> https://github.com/orgs/community/discussions/148648>
> >
> >     Especially for libpisp compilation on aarch64 is more useful than
> >     compilation on x86 :-)
> >
> >
> > The aarch64 build happens as part of the device run-tests, but it's a
> later version of gcc which seems to complain.
>
> Ahh, sorry, I see now that you have on device tests using self-hosted
> runners in
> pisp-verification.test.yaml. I still think adding the github-provided
> aarch64
> runners to libpisp-build-test.yaml should be simple and is still useful.
>

Makes sense, I'll make the update to the CI.

Naush


>
> Regards,
> Barnabás Pőcze
>
> >
> > Regards,
> > Naush
> >
> >
> >     If you would like to merge this patch,
> >
> >     Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com
> <mailto:laurent.pinchart@ideasonboard.com>>
> >
> >      > > This being said, keeping werror=true for this dependency means
> things
> >      > > could break again in the future, and we'll have then to deal
> with the
> >      > > issue. I'm wondering if there's value in us noticing compiler
> warnings
> >      > > in libpisp, maybe it's best to ignore them for all dependencies
> ?
> >      > >
> >      > >>>> Signed-off-by: Barnabás Pőcze <
> barnabas.pocze@ideasonboard.com <mailto:barnabas.pocze@ideasonboard.com>>
> >      > >>>> ---
> >      > >>>>    src/libcamera/pipeline/rpi/pisp/meson.build | 4 +++-
> >      > >>>>    1 file changed, 3 insertions(+), 1 deletion(-)
> >      > >>>>
> >      > >>>> diff --git a/src/libcamera/pipeline/rpi/pisp/meson.build
> b/src/libcamera/pipeline/rpi/pisp/meson.build
> >      > >>>> index 178df94c2..642080014 100644
> >      > >>>> --- a/src/libcamera/pipeline/rpi/pisp/meson.build
> >      > >>>> +++ b/src/libcamera/pipeline/rpi/pisp/meson.build
> >      > >>>> @@ -5,7 +5,9 @@ libcamera_internal_sources += files([
> >      > >>>>    ])
> >      > >>>>
> >      > >>>>    librt = cc.find_library('rt', required : true)
> >      > >>>> -libpisp_dep = dependency('libpisp', fallback : ['libpisp',
> 'libpisp_dep'])
> >      > >>>> +libpisp_dep = dependency('libpisp',
> >      > >>>> +                         fallback : ['libpisp',
> 'libpisp_dep'],
> >      > >>>> +                         default_options : ['werror=false'])
> >      > >>>>
> >      > >>>>    libcamera_deps += [libpisp_dep, librt]
> >      > >>>>
> >
> >     --
> >     Regards,
> >
> >     Laurent Pinchart
> >
>
>
Barnabás Pőcze Oct. 3, 2025, 7:42 a.m. UTC | #10
2025. 10. 03. 9:32 keltezéssel, Barnabás Pőcze írta:
> 2025. 10. 03. 8:50 keltezéssel, Naushir Patuck írta:
>>
>>
>> On Thu, 2 Oct 2025 at 19:11, Laurent Pinchart <laurent.pinchart@ideasonboard.com <mailto:laurent.pinchart@ideasonboard.com>> wrote:
>>
>>     On Thu, Oct 02, 2025 at 07:30:45PM +0200, Barnabás Pőcze wrote:
>>      > 2025. 10. 02. 19:03 keltezéssel, Laurent Pinchart írta:
>>      > > On Thu, Oct 02, 2025 at 05:19:41PM +0200, Barnabás Pőcze wrote:
>>      > >> 2025. 10. 02. 17:01 keltezéssel, Laurent Pinchart írta:
>>      > >>> On Thu, Oct 02, 2025 at 04:55:06PM +0200, Barnabás Pőcze wrote:
>>      > >>>> By default libcamera is configured with `werror=true`, and this option can
>>      > >>>> propagate into the subprojects. This can lead to compilation failures due to
>>      > >>>> warnings in those subprojects. This is the case with libpisp. Since these
>>      > >>>> issues are not directly fixable, set `werror=false` for libpisp.
>>      > >>>
>>      > >>> Can't we fix those errors ?
>>      > >>
>>      > >> I might be misunderstanding, but I think that should be done in the context of the
>>      > >> libpisp project.
>>      > >
>>      > > Yes you're right.
>>      > >
>>      > >> I would argue you cannot immediately act on the errors as if they were
>>      > >> inside the libcamera code base; short of adding patch files and having meson apply them.
>>      > >> But seems like a lot of effort.
>>      > >
>>      > > Agreed, I don't think we should do that.
>>      > >
>>      > >> The specific issue I have is already fixed: https://github.com/raspberrypi/libpisp/pull/53 <https://github.com/raspberrypi/libpisp/pull/53>
>>      > >> but no release has been made, and the wrap file uses the last release, not the master branch.
>>      > >
>>      > > Naush, could you make a release ?
>>
>>
>> I can arrange a release shortly.  Also happy to update the wrap file to point to master instead of a particular release if you prefer?
>>
>>      > >
>>      > > If this issue occurs again we would set werror=false for libpisp, but if
>>      > > it's easy to fix this time we can just update to a new release.
>>      > >
>>      > > I see libpisp has werror=true in meson.build, and there's a CI pipeline
>>      > > that compiles the project with both gcc and clang. Why wasn't this
>>      > > detected, different gcc versions maybe ?
>>      >
>>      > I think one issue with that CI pipeline is that, as far as I can see, it runs
>>      > only on x86 and compiles natively. So it does not pick up the slight idiosyncrasies
>>      > of the (glibc) headers of different architectures.
>>      >
>>      > There are aarch64 runners available nowadays, I believe that would be an easy way
>>      > to add aarch64 compilation coverage: https://github.com/orgs/community/discussions/148648 <https://github.com/orgs/community/discussions/148648>
>>
>>     Especially for libpisp compilation on aarch64 is more useful than
>>     compilation on x86 :-)
>>
>>
>> The aarch64 build happens as part of the device run-tests, but it's a later version of gcc which seems to complain.
> 
> Ahh, sorry, I see now that you have on device tests using self-hosted runners in
> pisp-verification.test.yaml. I still think adding the github-provided aarch64
> runners to libpisp-build-test.yaml should be simple and is still useful.

According to
   * https://github.com/actions/runner-images/blob/main/images/ubuntu/Ubuntu2404-Readme.md
   * https://github.com/actions/partner-runner-images/blob/main/images/arm-ubuntu-24-image.md
the images have multiple versions of gcc and clang by default, so should be possible to extend
the coverage even more by just adding `ubuntu-24.04-arm` and the available compiler versions
in the "strategy matrix", I think. (Although arguably hard-coding the compiler version when
using the "dynamic" `ubuntu-latest` may not be a good idea.)


> 
> 
> Regards,
> Barnabás Pőcze
> 
>>
>> Regards,
>> Naush
>>
>>
>>     If you would like to merge this patch,
>>
>>     Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com <mailto:laurent.pinchart@ideasonboard.com>>
>>
>>      > > This being said, keeping werror=true for this dependency means things
>>      > > could break again in the future, and we'll have then to deal with the
>>      > > issue. I'm wondering if there's value in us noticing compiler warnings
>>      > > in libpisp, maybe it's best to ignore them for all dependencies ?
>>      > >
>>      > >>>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com <mailto:barnabas.pocze@ideasonboard.com>>
>>      > >>>> ---
>>      > >>>>    src/libcamera/pipeline/rpi/pisp/meson.build | 4 +++-
>>      > >>>>    1 file changed, 3 insertions(+), 1 deletion(-)
>>      > >>>>
>>      > >>>> diff --git a/src/libcamera/pipeline/rpi/pisp/meson.build b/src/libcamera/pipeline/rpi/pisp/meson.build
>>      > >>>> index 178df94c2..642080014 100644
>>      > >>>> --- a/src/libcamera/pipeline/rpi/pisp/meson.build
>>      > >>>> +++ b/src/libcamera/pipeline/rpi/pisp/meson.build
>>      > >>>> @@ -5,7 +5,9 @@ libcamera_internal_sources += files([
>>      > >>>>    ])
>>      > >>>>
>>      > >>>>    librt = cc.find_library('rt', required : true)
>>      > >>>> -libpisp_dep = dependency('libpisp', fallback : ['libpisp', 'libpisp_dep'])
>>      > >>>> +libpisp_dep = dependency('libpisp',
>>      > >>>> +                         fallback : ['libpisp', 'libpisp_dep'],
>>      > >>>> +                         default_options : ['werror=false'])
>>      > >>>>
>>      > >>>>    libcamera_deps += [libpisp_dep, librt]
>>      > >>>>
>>
>>     --     Regards,
>>
>>     Laurent Pinchart
>>
>
Barnabás Pőcze Oct. 3, 2025, 7:42 a.m. UTC | #11
2025. 10. 03. 9:38 keltezéssel, Naushir Patuck írta:
> 
> 
> On Fri, 3 Oct 2025 at 08:32, Barnabás Pőcze <barnabas.pocze@ideasonboard.com <mailto:barnabas.pocze@ideasonboard.com>> wrote:
> 
>     2025. 10. 03. 8:50 keltezéssel, Naushir Patuck írta:
>      >
>      >
>      > On Thu, 2 Oct 2025 at 19:11, Laurent Pinchart <laurent.pinchart@ideasonboard.com <mailto:laurent.pinchart@ideasonboard.com> <mailto:laurent.pinchart@ideasonboard.com <mailto:laurent.pinchart@ideasonboard.com>>> wrote:
>      >
>      >     On Thu, Oct 02, 2025 at 07:30:45PM +0200, Barnabás Pőcze wrote:
>      >      > 2025. 10. 02. 19:03 keltezéssel, Laurent Pinchart írta:
>      >      > > On Thu, Oct 02, 2025 at 05:19:41PM +0200, Barnabás Pőcze wrote:
>      >      > >> 2025. 10. 02. 17:01 keltezéssel, Laurent Pinchart írta:
>      >      > >>> On Thu, Oct 02, 2025 at 04:55:06PM +0200, Barnabás Pőcze wrote:
>      >      > >>>> By default libcamera is configured with `werror=true`, and this option can
>      >      > >>>> propagate into the subprojects. This can lead to compilation failures due to
>      >      > >>>> warnings in those subprojects. This is the case with libpisp. Since these
>      >      > >>>> issues are not directly fixable, set `werror=false` for libpisp.
>      >      > >>>
>      >      > >>> Can't we fix those errors ?
>      >      > >>
>      >      > >> I might be misunderstanding, but I think that should be done in the context of the
>      >      > >> libpisp project.
>      >      > >
>      >      > > Yes you're right.
>      >      > >
>      >      > >> I would argue you cannot immediately act on the errors as if they were
>      >      > >> inside the libcamera code base; short of adding patch files and having meson apply them.
>      >      > >> But seems like a lot of effort.
>      >      > >
>      >      > > Agreed, I don't think we should do that.
>      >      > >
>      >      > >> The specific issue I have is already fixed: https://github.com/raspberrypi/libpisp/pull/53 <https://github.com/raspberrypi/libpisp/pull/53> <https://github.com/raspberrypi/libpisp/pull/53 <https://github.com/raspberrypi/libpisp/pull/53>>
>      >      > >> but no release has been made, and the wrap file uses the last release, not the master branch.
>      >      > >
>      >      > > Naush, could you make a release ?
>      >
>      >
>      > I can arrange a release shortly.  Also happy to update the wrap file to point to master instead of a particular release if you prefer?
>      >
>      >      > >
>      >      > > If this issue occurs again we would set werror=false for libpisp, but if
>      >      > > it's easy to fix this time we can just update to a new release.
>      >      > >
>      >      > > I see libpisp has werror=true in meson.build, and there's a CI pipeline
>      >      > > that compiles the project with both gcc and clang. Why wasn't this
>      >      > > detected, different gcc versions maybe ?
>      >      >
>      >      > I think one issue with that CI pipeline is that, as far as I can see, it runs
>      >      > only on x86 and compiles natively. So it does not pick up the slight idiosyncrasies
>      >      > of the (glibc) headers of different architectures.
>      >      >
>      >      > There are aarch64 runners available nowadays, I believe that would be an easy way
>      >      > to add aarch64 compilation coverage: https://github.com/orgs/community/discussions/148648 <https://github.com/orgs/community/discussions/148648> <https://github.com/orgs/community/discussions/148648 <https://github.com/orgs/community/discussions/148648>>
>      >
>      >     Especially for libpisp compilation on aarch64 is more useful than
>      >     compilation on x86 :-)
>      >
>      >
>      > The aarch64 build happens as part of the device run-tests, but it's a later version of gcc which seems to complain.
> 
>     Ahh, sorry, I see now that you have on device tests using self-hosted runners in
>     pisp-verification.test.yaml. I still think adding the github-provided aarch64
>     runners to libpisp-build-test.yaml should be simple and is still useful.
> 
> 
> Makes sense, I'll make the update to the CI.

Thanks, I hope it is as simple as it seems.


> 
> Naush
> 
> 
> 
>     Regards,
>     Barnabás Pőcze
> 
>      >
>      > Regards,
>      > Naush
>      >
>      >
>      >     If you would like to merge this patch,
>      >
>      >     Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com <mailto:laurent.pinchart@ideasonboard.com> <mailto:laurent.pinchart@ideasonboard.com <mailto:laurent.pinchart@ideasonboard.com>>>
>      >
>      >      > > This being said, keeping werror=true for this dependency means things
>      >      > > could break again in the future, and we'll have then to deal with the
>      >      > > issue. I'm wondering if there's value in us noticing compiler warnings
>      >      > > in libpisp, maybe it's best to ignore them for all dependencies ?
>      >      > >
>      >      > >>>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com <mailto:barnabas.pocze@ideasonboard.com> <mailto:barnabas.pocze@ideasonboard.com <mailto:barnabas.pocze@ideasonboard.com>>>
>      >      > >>>> ---
>      >      > >>>>    src/libcamera/pipeline/rpi/pisp/meson.build | 4 +++-
>      >      > >>>>    1 file changed, 3 insertions(+), 1 deletion(-)
>      >      > >>>>
>      >      > >>>> diff --git a/src/libcamera/pipeline/rpi/pisp/meson.build b/src/libcamera/pipeline/rpi/pisp/meson.build
>      >      > >>>> index 178df94c2..642080014 100644
>      >      > >>>> --- a/src/libcamera/pipeline/rpi/pisp/meson.build
>      >      > >>>> +++ b/src/libcamera/pipeline/rpi/pisp/meson.build
>      >      > >>>> @@ -5,7 +5,9 @@ libcamera_internal_sources += files([
>      >      > >>>>    ])
>      >      > >>>>
>      >      > >>>>    librt = cc.find_library('rt', required : true)
>      >      > >>>> -libpisp_dep = dependency('libpisp', fallback : ['libpisp', 'libpisp_dep'])
>      >      > >>>> +libpisp_dep = dependency('libpisp',
>      >      > >>>> +                         fallback : ['libpisp', 'libpisp_dep'],
>      >      > >>>> +                         default_options : ['werror=false'])
>      >      > >>>>
>      >      > >>>>    libcamera_deps += [libpisp_dep, librt]
>      >      > >>>>
>      >
>      >     --
>      >     Regards,
>      >
>      >     Laurent Pinchart
>      >
>

Patch
diff mbox series

diff --git a/src/libcamera/pipeline/rpi/pisp/meson.build b/src/libcamera/pipeline/rpi/pisp/meson.build
index 178df94c2..642080014 100644
--- a/src/libcamera/pipeline/rpi/pisp/meson.build
+++ b/src/libcamera/pipeline/rpi/pisp/meson.build
@@ -5,7 +5,9 @@  libcamera_internal_sources += files([
 ])
 
 librt = cc.find_library('rt', required : true)
-libpisp_dep = dependency('libpisp', fallback : ['libpisp', 'libpisp_dep'])
+libpisp_dep = dependency('libpisp',
+                         fallback : ['libpisp', 'libpisp_dep'],
+                         default_options : ['werror=false'])
 
 libcamera_deps += [libpisp_dep, librt]