| Message ID | 20190905165306.2178-1-fontaine.fabrice@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v2,1/1] package/libcamera: link with atomic when needed | expand |
Hi Fabrice, Thank you for the updated patch, This looks like a good workable upstream solution. On 05/09/2019 17:53, Fabrice Fontaine wrote: > Fixes: > - http://autobuild.buildroot.org/results/1f0b8338f5f39aa86b9d432598dae2f53c5f7c84 > > Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com> > --- > Changes v1 -> v2 (after review of Kieran Bingham): > - Use an upstreamable solution > > ...son.build-link-with-atomic-when-need.patch | 28 +++++++++++++++++++ > 1 file changed, 28 insertions(+) > create mode 100644 package/libcamera/0001-src-libcamera-meson.build-link-with-atomic-when-need.patch > > diff --git a/package/libcamera/0001-src-libcamera-meson.build-link-with-atomic-when-need.patch b/package/libcamera/0001-src-libcamera-meson.build-link-with-atomic-when-need.patch > new file mode 100644 > index 0000000000..8c34497db4 > --- /dev/null > +++ b/package/libcamera/0001-src-libcamera-meson.build-link-with-atomic-when-need.patch > @@ -0,0 +1,28 @@ > +From 85b7aeef3c7765c5ba7e525c63a4c0f807f0bfc8 Mon Sep 17 00:00:00 2001 > +From: Fabrice Fontaine <fontaine.fabrice@gmail.com> > +Date: Thu, 5 Sep 2019 10:24:31 +0200 > +Subject: [PATCH] src/libcamera/meson.build: link with atomic when needed > + How about something a bit more descriptive here, something like: "Sparc and MIPS compilers use an external library to provide atomic functionality. Detect and link against libatomic when needed." Then if you could instead post it to: libcamera-devel@lists.libcamera.org we can get it integrated upstream and update the version sha on this package instead. > +Fixes: > + - http://autobuild.buildroot.org/results/1f0b8338f5f39aa86b9d432598dae2f53c5f7c84 > + > +Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com> > +--- > + src/libcamera/meson.build | 1 + > + 1 file changed, 1 insertion(+) > + > +diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build > +index c5d8f11..0706a08 100644 > +--- a/src/libcamera/meson.build > ++++ b/src/libcamera/meson.build > +@@ -99,6 +99,7 @@ version_cpp = vcs_tag(command : [gen_version, meson.build_root()], > + libcamera_sources += version_cpp > + > + libcamera_deps = [ > ++ cc.find_library('atomic', required: false), Great - this works out to be quite elegant and simple. -- Kieran > + cc.find_library('dl'), > + libudev, > + dependency('threads'), > +-- > +2.23.0.rc1 > + >
On Thu, 5 Sep 2019 18:53:06 +0200 Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote: > Fixes: > - http://autobuild.buildroot.org/results/1f0b8338f5f39aa86b9d432598dae2f53c5f7c84 > > Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com> > --- > Changes v1 -> v2 (after review of Kieran Bingham): > - Use an upstreamable solution Applied to master, thanks. Could you send the patch upstream, after taking into account the comments from Kieran ? Thanks! Thomas
Hi Thomas, On 15/09/2019 21:18, Thomas Petazzoni wrote: > On Thu, 5 Sep 2019 18:53:06 +0200 > Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote: > >> Fixes: >> - http://autobuild.buildroot.org/results/1f0b8338f5f39aa86b9d432598dae2f53c5f7c84 >> >> Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com> >> --- >> Changes v1 -> v2 (after review of Kieran Bingham): >> - Use an upstreamable solution > > Applied to master, thanks. Great, thanks for collecting this. > Could you send the patch upstream, after > taking into account the comments from Kieran ? This fix has already been integrated into libcamera master : https://git.linuxtv.org/libcamera.git/commit/?id=5d05418d9b53e1838692f687a6dc373dad45355c I haven't sent a version-bump patch yet, because we've got usages of O_TMPFILE in our tests, which even with making our build depend on kernel headers > 3.11 - I still saw failures in some toolchains. Do you know of a 'failsafe' way to build on all (or skip) toolchains when we make use of O_TMPFILE? I've tried adding: + depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_4_10 + depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_4_0 # Still 6 failures + depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_11 But I still get failures related to the O_TMPFILE usage. (Yes, I know each HEADERS_AT_LEAST brings in the older dependencies as well, but I started at 3_11) Cheers Kieran > Thanks! > > Thomas >
Hi Kieran, Le dim. 15 sept. 2019 à 22:45, Kieran Bingham <kieran.bingham@ideasonboard.com> a écrit : > > Hi Thomas, > > On 15/09/2019 21:18, Thomas Petazzoni wrote: > > On Thu, 5 Sep 2019 18:53:06 +0200 > > Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote: > > > >> Fixes: > >> - http://autobuild.buildroot.org/results/1f0b8338f5f39aa86b9d432598dae2f53c5f7c84 > >> > >> Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com> > >> --- > >> Changes v1 -> v2 (after review of Kieran Bingham): > >> - Use an upstreamable solution > > > > Applied to master, thanks. > > Great, thanks for collecting this. > > > Could you send the patch upstream, after > > taking into account the comments from Kieran ? > > This fix has already been integrated into libcamera master : > > https://git.linuxtv.org/libcamera.git/commit/?id=5d05418d9b53e1838692f687a6dc373dad45355c > > > I haven't sent a version-bump patch yet, because we've got usages of > O_TMPFILE in our tests, which even with making our build depend on > kernel headers > 3.11 - I still saw failures in some toolchains. > > Do you know of a 'failsafe' way to build on all (or skip) toolchains > when we make use of O_TMPFILE? There is a pretty long thread about O_TMPFILE in the context of runc here: https://patchwork.ozlabs.org/patch/1044933. You can also find the commit log for runc's patch here: https://git.buildroot.net/buildroot/commit/package/runc/?id=905e976a6af224b3ed015c46fcea2d717c155f55 > > I've tried adding: > > + depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_4_10 > + depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_4_0 # Still 6 failures > + depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_11 > > But I still get failures related to the O_TMPFILE usage. > > (Yes, I know each HEADERS_AT_LEAST brings in the older dependencies as > well, but I started at 3_11) > > Cheers > > Kieran > > > > Thanks! > > > > Thomas > > > > -- > Regards > -- > Kieran Best Regards, Fabrice
Hi Fabrice, Thank you for the references, at least it wasn't me doing something distinctly wrong! On 16/09/2019 12:05, Fabrice Fontaine wrote: > Hi Kieran, > Le dim. 15 sept. 2019 à 22:45, Kieran Bingham > <kieran.bingham@ideasonboard.com> a écrit : >> >> Hi Thomas, >> >> On 15/09/2019 21:18, Thomas Petazzoni wrote: >>> On Thu, 5 Sep 2019 18:53:06 +0200 >>> Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote: >>> >>>> Fixes: >>>> - http://autobuild.buildroot.org/results/1f0b8338f5f39aa86b9d432598dae2f53c5f7c84 >>>> >>>> Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com> >>>> --- >>>> Changes v1 -> v2 (after review of Kieran Bingham): >>>> - Use an upstreamable solution >>> >>> Applied to master, thanks. >> >> Great, thanks for collecting this. >> >>> Could you send the patch upstream, after >>> taking into account the comments from Kieran ? >> >> This fix has already been integrated into libcamera master : >> >> https://git.linuxtv.org/libcamera.git/commit/?id=5d05418d9b53e1838692f687a6dc373dad45355c >> >> >> I haven't sent a version-bump patch yet, because we've got usages of >> O_TMPFILE in our tests, which even with making our build depend on >> kernel headers > 3.11 - I still saw failures in some toolchains. >> >> Do you know of a 'failsafe' way to build on all (or skip) toolchains >> when we make use of O_TMPFILE? > There is a pretty long thread about O_TMPFILE in the context of runc > here: https://patchwork.ozlabs.org/patch/1044933. > You can also find the commit log for runc's patch here: > https://git.buildroot.net/buildroot/commit/package/runc/?id=905e976a6af224b3ed015c46fcea2d717c155f55 I don't really see a resolution in there except for adding a whole bunch of architecture specific definitions (and a hacky workaround for non-support) of O_TMPFILE into libcamera. Is that the expected resolution here? I really doubt libcamera could be used on old toolchains without O_TMPFILE support, as it requires media specific features from new kernels anyway. Is there a clean way to mark libcamera as not supported on /really/ old toolchains? >> I've tried adding: >> >> + depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_4_10 >> + depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_4_0 # Still 6 failures >> + depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_11 >> >> But I still get failures related to the O_TMPFILE usage. >> >> (Yes, I know each HEADERS_AT_LEAST brings in the older dependencies as >> well, but I started at 3_11) >> >> Cheers >> >> Kieran >> >> >>> Thanks! >>> >>> Thomas >>> >> >> -- >> Regards >> -- >> Kieran > Best Regards, > > Fabrice >
Le lun. 16 sept. 2019 à 13:40, Kieran Bingham <kieran.bingham@ideasonboard.com> a écrit : > > Hi Fabrice, > > Thank you for the references, at least it wasn't me doing something > distinctly wrong! > > On 16/09/2019 12:05, Fabrice Fontaine wrote: > > Hi Kieran, > > Le dim. 15 sept. 2019 à 22:45, Kieran Bingham > > <kieran.bingham@ideasonboard.com> a écrit : > >> > >> Hi Thomas, > >> > >> On 15/09/2019 21:18, Thomas Petazzoni wrote: > >>> On Thu, 5 Sep 2019 18:53:06 +0200 > >>> Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote: > >>> > >>>> Fixes: > >>>> - http://autobuild.buildroot.org/results/1f0b8338f5f39aa86b9d432598dae2f53c5f7c84 > >>>> > >>>> Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com> > >>>> --- > >>>> Changes v1 -> v2 (after review of Kieran Bingham): > >>>> - Use an upstreamable solution > >>> > >>> Applied to master, thanks. > >> > >> Great, thanks for collecting this. > >> > >>> Could you send the patch upstream, after > >>> taking into account the comments from Kieran ? > >> > >> This fix has already been integrated into libcamera master : > >> > >> https://git.linuxtv.org/libcamera.git/commit/?id=5d05418d9b53e1838692f687a6dc373dad45355c > >> > >> > >> I haven't sent a version-bump patch yet, because we've got usages of > >> O_TMPFILE in our tests, which even with making our build depend on > >> kernel headers > 3.11 - I still saw failures in some toolchains. > >> > >> Do you know of a 'failsafe' way to build on all (or skip) toolchains > >> when we make use of O_TMPFILE? > > There is a pretty long thread about O_TMPFILE in the context of runc > > here: https://patchwork.ozlabs.org/patch/1044933. > > You can also find the commit log for runc's patch here: > > https://git.buildroot.net/buildroot/commit/package/runc/?id=905e976a6af224b3ed015c46fcea2d717c155f55 > > > I don't really see a resolution in there except for adding a whole bunch > of architecture specific definitions (and a hacky workaround for > non-support) of O_TMPFILE into libcamera. > > Is that the expected resolution here? > > I really doubt libcamera could be used on old toolchains without > O_TMPFILE support, as it requires media specific features from new > kernels anyway. > > Is there a clean way to mark libcamera as not supported on /really/ old > toolchains? From my understanding, there is no clean way to check for the availability of O_TMPFILE. However, O_TMPFILE is only used by tests and tests can be disabled through -Dtest=false. > > > >> I've tried adding: > >> > >> + depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_4_10 > >> + depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_4_0 # Still 6 failures > >> + depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_11 > >> > >> But I still get failures related to the O_TMPFILE usage. > >> > >> (Yes, I know each HEADERS_AT_LEAST brings in the older dependencies as > >> well, but I started at 3_11) > >> > >> Cheers > >> > >> Kieran > >> > >> > >>> Thanks! > >>> > >>> Thomas > >>> > >> > >> -- > >> Regards > >> -- > >> Kieran > > Best Regards, > > > > Fabrice > > > > -- > Regards > -- > Kieran Best Regards, Fabrice
Hi Fabrice, On 16/09/2019 12:54, Fabrice Fontaine wrote: > Le lun. 16 sept. 2019 à 13:40, Kieran Bingham > <kieran.bingham@ideasonboard.com> a écrit : >> >> Hi Fabrice, <snip> >> Is there a clean way to mark libcamera as not supported on /really/ old >> toolchains? > From my understanding, there is no clean way to check for the > availability of O_TMPFILE. > However, O_TMPFILE is only used by tests and tests can be disabled > through -Dtest=false. You're right. Disabling tests was renamed from -Dtests=false to -Dtest=false. The buildroot package was already disabling tests through -Dtests=false, so if we fix this up - then the tests will no longer be built again, and we won't have any issue with O_TMPFILE ... Problem (well two problems) solved :-D Thanks I should have remembered to fix up the -Dtests issue when I knew it was being changed! Do you want to update the buildroot package and bump the version? or shall I do it?
Le lun. 16 sept. 2019 à 14:01, Kieran Bingham <kieran.bingham@ideasonboard.com> a écrit : > > Hi Fabrice, > > On 16/09/2019 12:54, Fabrice Fontaine wrote: > > Le lun. 16 sept. 2019 à 13:40, Kieran Bingham > > <kieran.bingham@ideasonboard.com> a écrit : > >> > >> Hi Fabrice, > > <snip> > > >> Is there a clean way to mark libcamera as not supported on /really/ old > >> toolchains? > > From my understanding, there is no clean way to check for the > > availability of O_TMPFILE. > > However, O_TMPFILE is only used by tests and tests can be disabled > > through -Dtest=false. > > You're right. Disabling tests was renamed from -Dtests=false to > -Dtest=false. > > The buildroot package was already disabling tests through -Dtests=false, > so if we fix this up - then the tests will no longer be built again, and > we won't have any issue with O_TMPFILE ... > > Problem (well two problems) solved :-D > > Thanks I should have remembered to fix up the -Dtests issue when I knew > it was being changed! > > Do you want to update the buildroot package and bump the version? > or shall I do it? I think it's better if you do it. > -- > Regards > -- > Kieran Best Regards, Fabrice
>>>>> "Fabrice" == Fabrice Fontaine <fontaine.fabrice@gmail.com> writes: > Fixes: > - http://autobuild.buildroot.org/results/1f0b8338f5f39aa86b9d432598dae2f53c5f7c84 > Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com> > --- > Changes v1 -> v2 (after review of Kieran Bingham): > - Use an upstreamable solution Committed to 2019.05.x and 2019.08.x, thanks.
diff --git a/package/libcamera/0001-src-libcamera-meson.build-link-with-atomic-when-need.patch b/package/libcamera/0001-src-libcamera-meson.build-link-with-atomic-when-need.patch new file mode 100644 index 0000000000..8c34497db4 --- /dev/null +++ b/package/libcamera/0001-src-libcamera-meson.build-link-with-atomic-when-need.patch @@ -0,0 +1,28 @@ +From 85b7aeef3c7765c5ba7e525c63a4c0f807f0bfc8 Mon Sep 17 00:00:00 2001 +From: Fabrice Fontaine <fontaine.fabrice@gmail.com> +Date: Thu, 5 Sep 2019 10:24:31 +0200 +Subject: [PATCH] src/libcamera/meson.build: link with atomic when needed + +Fixes: + - http://autobuild.buildroot.org/results/1f0b8338f5f39aa86b9d432598dae2f53c5f7c84 + +Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com> +--- + src/libcamera/meson.build | 1 + + 1 file changed, 1 insertion(+) + +diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build +index c5d8f11..0706a08 100644 +--- a/src/libcamera/meson.build ++++ b/src/libcamera/meson.build +@@ -99,6 +99,7 @@ version_cpp = vcs_tag(command : [gen_version, meson.build_root()], + libcamera_sources += version_cpp + + libcamera_deps = [ ++ cc.find_library('atomic', required: false), + cc.find_library('dl'), + libudev, + dependency('threads'), +-- +2.23.0.rc1 +
Fixes: - http://autobuild.buildroot.org/results/1f0b8338f5f39aa86b9d432598dae2f53c5f7c84 Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com> --- Changes v1 -> v2 (after review of Kieran Bingham): - Use an upstreamable solution ...son.build-link-with-atomic-when-need.patch | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 package/libcamera/0001-src-libcamera-meson.build-link-with-atomic-when-need.patch