[v2,1/1] package/libcamera: link with atomic when needed
diff mbox series

Message ID 20190905165306.2178-1-fontaine.fabrice@gmail.com
State Accepted
Headers show
Series
  • [v2,1/1] package/libcamera: link with atomic when needed
Related show

Commit Message

Fabrice Fontaine Sept. 5, 2019, 4:53 p.m. UTC
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

Comments

Kieran Bingham Sept. 5, 2019, 6:54 p.m. UTC | #1
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
> +
>
Thomas Petazzoni Sept. 15, 2019, 8:18 p.m. UTC | #2
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
Kieran Bingham Sept. 15, 2019, 8:45 p.m. UTC | #3
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
>
Fabrice Fontaine Sept. 16, 2019, 11:05 a.m. UTC | #4
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
Kieran Bingham Sept. 16, 2019, 11:40 a.m. UTC | #5
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
>
Fabrice Fontaine Sept. 16, 2019, 11:54 a.m. UTC | #6
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
Kieran Bingham Sept. 16, 2019, 12:01 p.m. UTC | #7
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?
Fabrice Fontaine Sept. 16, 2019, 12:22 p.m. UTC | #8
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
Peter Korsgaard Sept. 26, 2019, 8:41 a.m. UTC | #9
>>>>> "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.

Patch
diff mbox series

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
+