| 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] > >>>>
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 >
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 >
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 > > > >
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 >> >
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 > > >
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(-)