diff mbox series

[1/1] package/grpc: add explicitly link with libatomic

Message ID 20181207171241.27832-1-ryan.barnett@rockwellcollins.com
State Accepted
Headers show
Series [1/1] package/grpc: add explicitly link with libatomic | expand

Commit Message

Ryan Barnett Dec. 7, 2018, 5:12 p.m. UTC
On some architectures, you must link with libatomic, on some other
architectures, they are available built-in, but in all cases, linking
against libatomic does not harm.

Fixes:
  http://autobuild.buildroot.org/results/2f2/2f208fbfe4e9da94be5b9c030dbd278cb8ba053c/

Signed-off-by: Ryan Barnett <ryan.barnett@rockwellcollins.com>
---
 package/grpc/grpc.mk | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

robert rose Dec. 7, 2018, 6:03 p.m. UTC | #1
Reviewed-by: Robert Rose <robertroyrose@gmail.com>

On Fri, Dec 7, 2018 at 9:12 AM Ryan Barnett <
ryan.barnett@rockwellcollins.com> wrote:

> On some architectures, you must link with libatomic, on some other
> architectures, they are available built-in, but in all cases, linking
> against libatomic does not harm.
>
> Fixes:
>
> http://autobuild.buildroot.org/results/2f2/2f208fbfe4e9da94be5b9c030dbd278cb8ba053c/
>
> Signed-off-by: Ryan Barnett <ryan.barnett@rockwellcollins.com>
> ---
>  package/grpc/grpc.mk | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/package/grpc/grpc.mk b/package/grpc/grpc.mk
> index 6ca6536fff..527c7fa7e2 100644
> --- a/package/grpc/grpc.mk
> +++ b/package/grpc/grpc.mk
> @@ -23,6 +23,12 @@ GRPC_CONF_OPTS = \
>         -DgRPC_ZLIB_PROVIDER=package \
>         -DgRPC_NATIVE_CPP_PLUGIN=$(HOST_DIR)/bin/grpc_cpp_plugin
>
> +# grpc can uses __atomic builtins, so we need to link with
> +# libatomic for the architectures who need libatomic.
> +ifeq ($(BR2_TOOLCHAIN_HAS_LIBATOMIC),y)
> +GRPC_CONF_OPTS += -DCMAKE_C_STANDARD_LIBRARIES="-latomic"
> +endif
> +
>  HOST_GRPC_CONF_OPTS = \
>         -D_gRPC_CARES_LIBRARIES=cares \
>         -DgRPC_CARES_PROVIDER=none \
> --
> 2.17.1
>
>
<div dir="ltr"><div dir="ltr">Reviewed-by: Robert Rose &lt;<a href="mailto:robertroyrose@gmail.com">robertroyrose@gmail.com</a>&gt;<br></div></div><br><div class="gmail_quote"><div dir="ltr">On Fri, Dec 7, 2018 at 9:12 AM Ryan Barnett &lt;<a href="mailto:ryan.barnett@rockwellcollins.com">ryan.barnett@rockwellcollins.com</a>&gt; wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On some architectures, you must link with libatomic, on some other<br>
architectures, they are available built-in, but in all cases, linking<br>
against libatomic does not harm.<br>
<br>
Fixes:<br>
  <a href="http://autobuild.buildroot.org/results/2f2/2f208fbfe4e9da94be5b9c030dbd278cb8ba053c/" rel="noreferrer" target="_blank">http://autobuild.buildroot.org/results/2f2/2f208fbfe4e9da94be5b9c030dbd278cb8ba053c/</a><br>
<br>
Signed-off-by: Ryan Barnett &lt;<a href="mailto:ryan.barnett@rockwellcollins.com" target="_blank">ryan.barnett@rockwellcollins.com</a>&gt;<br>
---<br>
 package/grpc/<a href="http://grpc.mk" rel="noreferrer" target="_blank">grpc.mk</a> | 6 ++++++<br>
 1 file changed, 6 insertions(+)<br>
<br>
diff --git a/package/grpc/<a href="http://grpc.mk" rel="noreferrer" target="_blank">grpc.mk</a> b/package/grpc/<a href="http://grpc.mk" rel="noreferrer" target="_blank">grpc.mk</a><br>
index 6ca6536fff..527c7fa7e2 100644<br>
--- a/package/grpc/<a href="http://grpc.mk" rel="noreferrer" target="_blank">grpc.mk</a><br>
+++ b/package/grpc/<a href="http://grpc.mk" rel="noreferrer" target="_blank">grpc.mk</a><br>
@@ -23,6 +23,12 @@ GRPC_CONF_OPTS = \<br>
        -DgRPC_ZLIB_PROVIDER=package \<br>
        -DgRPC_NATIVE_CPP_PLUGIN=$(HOST_DIR)/bin/grpc_cpp_plugin<br>
<br>
+# grpc can uses __atomic builtins, so we need to link with<br>
+# libatomic for the architectures who need libatomic.<br>
+ifeq ($(BR2_TOOLCHAIN_HAS_LIBATOMIC),y)<br>
+GRPC_CONF_OPTS += -DCMAKE_C_STANDARD_LIBRARIES=&quot;-latomic&quot;<br>
+endif<br>
+<br>
 HOST_GRPC_CONF_OPTS = \<br>
        -D_gRPC_CARES_LIBRARIES=cares \<br>
        -DgRPC_CARES_PROVIDER=none \<br>
-- <br>
2.17.1<br>
<br>
</blockquote></div>
Thomas Petazzoni Dec. 31, 2018, 2:54 p.m. UTC | #2
Hello,

On Fri,  7 Dec 2018 11:12:41 -0600, Ryan Barnett wrote:
> On some architectures, you must link with libatomic, on some other
> architectures, they are available built-in, but in all cases, linking
> against libatomic does not harm.
> 
> Fixes:
>   http://autobuild.buildroot.org/results/2f2/2f208fbfe4e9da94be5b9c030dbd278cb8ba053c/
> 
> Signed-off-by: Ryan Barnett <ryan.barnett@rockwellcollins.com>

It's a bit more complicated than that:

 - If it uses atomic built-ins, one needs to "depends on
   BR2_TOOLCHAIN_HAS_ATOMIC"

 - It can also use sync builtins.

So I've fixed that up, and applied your patch with the following
changes:

    [Thomas:
     - add the dependency on sync or atomic builtins in Config.in
     - use -DCMAKE_EXE_LINKER_FLAGS instead of
       -DCMAKE_C_STANDARD_LIBRARIES, to be consistent with what we do in
       some other cmake-based packages
     - tweak commit log]

Best regards,

Thomas
diff mbox series

Patch

diff --git a/package/grpc/grpc.mk b/package/grpc/grpc.mk
index 6ca6536fff..527c7fa7e2 100644
--- a/package/grpc/grpc.mk
+++ b/package/grpc/grpc.mk
@@ -23,6 +23,12 @@  GRPC_CONF_OPTS = \
 	-DgRPC_ZLIB_PROVIDER=package \
 	-DgRPC_NATIVE_CPP_PLUGIN=$(HOST_DIR)/bin/grpc_cpp_plugin
 
+# grpc can uses __atomic builtins, so we need to link with
+# libatomic for the architectures who need libatomic.
+ifeq ($(BR2_TOOLCHAIN_HAS_LIBATOMIC),y)
+GRPC_CONF_OPTS += -DCMAKE_C_STANDARD_LIBRARIES="-latomic"
+endif
+
 HOST_GRPC_CONF_OPTS = \
 	-D_gRPC_CARES_LIBRARIES=cares \
 	-DgRPC_CARES_PROVIDER=none \