| Message ID | 20251002145506.996443-1-barnabas.pocze@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
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] >
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 >
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
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] > >>
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
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] > >>>>
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]
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(-)