[Buildroot] [PATCH 0/3] grpc-related fixes

[Buildroot] [PATCH 0/3] grpc-related fixes

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

Commit fedf3318e3fd3c9ba57389ed2b36472f1a772b9e removed an obsolete patch on
grpc, but this had the side effect that building collectd after grpc no longer
succeeded, because grpc_cpp_plugin from the target was executed.

During investigation of this issue, a number of related problems were found.
This series addresses these problems.

Thomas De Schampheleire (3):
  package/collectd: stop passing '--with-libgrpc++'
  package/grpc: fix cross-compilation with
    gRPC_BUILD_GRPC_CPP_PLUGIN=OFF
  package/grpc: disable unnecessary build plugins

 package/collectd/collectd.mk                  |  3 --
 ...ix-cross-compilation-with-gRPC_BUILD.patch | 43 +++++++++++++++++++
 package/grpc/grpc.mk                          |  9 +++-
 3 files changed, 51 insertions(+), 4 deletions(-)
 create mode 100644 package/grpc/0001-CMakeLists.txt-fix-cross-compilation-with-gRPC_BUILD.patch

-- 
2.26.2

[Buildroot] [PATCH 1/3] package/collectd: stop passing '--with-libgrpc++'

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

Commit 903de16f5fd586a32a7aed7a792a43dce16365eb added passing
'--with-libgrpc++' with the explanation:

    "Use --with-libgrpc++ option as otherwise collectd will try to find
    grpc++.pc which is not available."

At the time of above commit, grpc version in Buildroot was 1.23.0.
Since grpc 1.25.0, a grpc++.pc file _is_ generated from cmake builds.
Hence, remove passing --with-libgrpc++.

This change fixes a problem introduced by commit
fedf3318e3fd3c9ba57389ed2b36472f1a772b9e. As a side effect of that change, a
target version of 'grpc_cpp_plugin' was now created. When collectd was built
after grpc, even without grpc support in collectd enabled, the collectd
configure script would find this target grpc_cpp_plugin and try to use it
(which is not possible because it is built for target).
When not passing '--with-libgrpc++', collectd will instead find the host
version of grpc_cpp_plugin, which works fine.

There are still two underlying problems:
1. the target version of grpc_cpp_plugin is not actually needed. This will
   be disabled in a subsequent commit.

2. collectd should not execute any grpc-related action if grpc support for
   collectd is disabled. This problem has been reported upstream:
   https://github.com/collectd/collectd/issues/3836

Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
---
 package/collectd/collectd.mk | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/package/collectd/collectd.mk b/package/collectd/collectd.mk
index ccca9bf388..3fcca12a45 100644
--- a/package/collectd/collectd.mk
+++ b/package/collectd/collectd.mk
@@ -193,9 +193,6 @@ COLLECTD_DEPENDENCIES = \
 	$(if $(BR2_PACKAGE_COLLECTD_WRITEREDIS),hiredis)
 
 # include/library fixups
-ifeq ($(BR2_PACKAGE_GRPC),y)
-COLLECTD_CONF_OPTS += --with-libgrpc++=$(STAGING_DIR)/usr
-endif
 ifeq ($(BR2_PACKAGE_JANSSON),y)
 COLLECTD_CONF_OPTS += --with-libjansson=$(STAGING_DIR)/usr
 endif
-- 
2.26.2

[Buildroot] [PATCH 2/3] package/grpc: fix cross-compilation with gRPC_BUILD_GRPC_CPP_PLUGIN=OFF

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

In commit fedf3318e3fd3c9ba57389ed2b36472f1a772b9e, an obsolete patch to
support cross-compilation was removed, in favor of the upstream solution.

However, this caused a small change in behavior: for the target grpc, the
tool 'grpc_cpp_plugin' is now also built, while before it was not.

This tool is only really needed on development machines. Since Buildroot
does not support compilers and such on target itself, the tool is not
needed.

There exists an option gRPC_BUILD_GRPC_CPP_PLUGIN which can be set to 'OFF',
but disabling it in a cross-compilation context yields build failures.

Add a patch to fix that. This patch is intended to be upstreamed to grpc.

Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
---
 ...ix-cross-compilation-with-gRPC_BUILD.patch | 43 +++++++++++++++++++
 1 file changed, 43 insertions(+)
 create mode 100644 package/grpc/0001-CMakeLists.txt-fix-cross-compilation-with-gRPC_BUILD.patch

diff --git a/package/grpc/0001-CMakeLists.txt-fix-cross-compilation-with-gRPC_BUILD.patch b/package/grpc/0001-CMakeLists.txt-fix-cross-compilation-with-gRPC_BUILD.patch
new file mode 100644
index 0000000000..553be8cb99
--- /dev/null
+++ b/package/grpc/0001-CMakeLists.txt-fix-cross-compilation-with-gRPC_BUILD.patch
@@ -0,0 +1,43 @@
+From 268815d74ea34856c686dce9e9c23106b51f2782 Mon Sep 17 00:00:00 2001
+From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
+Date: Tue, 2 Feb 2021 21:46:29 +0100
+Subject: [PATCH] CMakeLists.txt: fix cross-compilation with
+ gRPC_BUILD_GRPC_CPP_PLUGIN=OFF
+
+When cross-compiling gRPC, a native version of 'grpc_cpp_plugin' is searched
+in the environment. For most use cases, a cross version of this file is not
+needed.
+
+However, when cross-building with -DgRPC_BUILD_GRPC_CPP_PLUGIN=OFF, there
+are some build errors, for example:
+
+make[3]: *** No rule to make target 'grpc_cpp_plugin', needed by 'gens/src/proto/grpc/reflection/v1alpha/reflection.grpc.pb.cc'.  Stop.
+
+This is because there is still a hard dependency on 'grpc_cpp_plugin' for
+these targets, not taking into account the cross-compilation case.
+
+Fix by depending on the variable gRPC_CPP_PLUGIN, which is set correctly for
+either cross or native case.
+
+Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
+
+---
+ CMakeLists.txt | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/CMakeLists.txt b/CMakeLists.txt
+index 6bdae22083..08aea609f7 100644
+--- a/CMakeLists.txt
++++ b/CMakeLists.txt
+@@ -317,7 +317,7 @@ function(protobuf_generate_grpc_cpp)
+            --plugin=protoc-gen-grpc=${_gRPC_CPP_PLUGIN}
+            ${_protobuf_include_path}
+            ${REL_FIL}
+-      DEPENDS ${ABS_FIL} ${_gRPC_PROTOBUF_PROTOC} grpc_cpp_plugin
++      DEPENDS ${ABS_FIL} ${_gRPC_PROTOBUF_PROTOC} ${_gRPC_CPP_PLUGIN}
+       WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
+       COMMENT "Running gRPC C++ protocol buffer compiler on ${FIL}"
+       VERBATIM)
+-- 
+2.26.2
+
-- 
2.26.2

[Buildroot] [PATCH 3/3] package/grpc: disable unnecessary build plugins

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

grpc has plugins for multiple programming languages, which are needed on
development machines only. Examples are grpc_cpp_plugin, grpc_ruby_plugin,
etc.

Even though before commit fedf3318e3fd3c9ba57389ed2b36472f1a772b9e,
grpc_cpp_plugin was not installed for target, all other plugins still were.
This causes additional build time and rootfs space.

As Buildroot does not support building a development environment for target,
these tools can be disabled.

Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
---
 package/grpc/grpc.mk | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/package/grpc/grpc.mk b/package/grpc/grpc.mk
index a8a3af89ef..27b28deff3 100644
--- a/package/grpc/grpc.mk
+++ b/package/grpc/grpc.mk
@@ -27,7 +27,14 @@ GRPC_CONF_OPTS = \
 	-DgRPC_PROTOBUF_PROVIDER=package \
 	-DgRPC_RE2_PROVIDER=package \
 	-DgRPC_SSL_PROVIDER=package \
-	-DgRPC_ZLIB_PROVIDER=package
+	-DgRPC_ZLIB_PROVIDER=package \
+	-DgRPC_BUILD_GRPC_CPP_PLUGIN=OFF \
+	-DgRPC_BUILD_GRPC_CSHARP_PLUGIN=OFF \
+	-DgRPC_BUILD_GRPC_NODE_PLUGIN=OFF \
+	-DgRPC_BUILD_GRPC_OBJECTIVE_C_PLUGIN=OFF \
+	-DgRPC_BUILD_GRPC_PHP_PLUGIN=OFF \
+	-DgRPC_BUILD_GRPC_PYTHON_PLUGIN=OFF \
+	-DgRPC_BUILD_GRPC_RUBY_PLUGIN=OFF
 
 # grpc can use __atomic builtins, so we need to link with
 # libatomic when available
-- 
2.26.2

[Buildroot] [PATCH 3/3] package/grpc: disable unnecessary build plugins

[Yann E. MORIN]

Thomas, All,

On 2021-02-03 13:43 +0100, Thomas De Schampheleire spake thusly:
> From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> 
> grpc has plugins for multiple programming languages, which are needed on
> development machines only. Examples are grpc_cpp_plugin, grpc_ruby_plugin,
> etc.
> 
> Even though before commit fedf3318e3fd3c9ba57389ed2b36472f1a772b9e,
> grpc_cpp_plugin was not installed for target, all other plugins still were.
> This causes additional build time and rootfs space.
> 
> As Buildroot does not support building a development environment for target,
> these tools can be disabled.
> 
> Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> ---
>  package/grpc/grpc.mk | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/package/grpc/grpc.mk b/package/grpc/grpc.mk
> index a8a3af89ef..27b28deff3 100644
> --- a/package/grpc/grpc.mk
> +++ b/package/grpc/grpc.mk
> @@ -27,7 +27,14 @@ GRPC_CONF_OPTS = \
>  	-DgRPC_PROTOBUF_PROVIDER=package \
>  	-DgRPC_RE2_PROVIDER=package \
>  	-DgRPC_SSL_PROVIDER=package \
> -	-DgRPC_ZLIB_PROVIDER=package
> +	-DgRPC_ZLIB_PROVIDER=package \
> +	-DgRPC_BUILD_GRPC_CPP_PLUGIN=OFF \
> +	-DgRPC_BUILD_GRPC_CSHARP_PLUGIN=OFF \
> +	-DgRPC_BUILD_GRPC_NODE_PLUGIN=OFF \
> +	-DgRPC_BUILD_GRPC_OBJECTIVE_C_PLUGIN=OFF \
> +	-DgRPC_BUILD_GRPC_PHP_PLUGIN=OFF \
> +	-DgRPC_BUILD_GRPC_PYTHON_PLUGIN=OFF \
> +	-DgRPC_BUILD_GRPC_RUBY_PLUGIN=OFF

So I am OK with applying this patch in bulk, because it makes sense.

IIUC, the grpc_cpp_plugin will generate C++ code, and so the
grpc_node_plugin will generate JS code, and so on... Right?

If so, would it make sense to also trim the host variant further, too?

I.e. if there is no nodejs on the target, then no need for the node
plugin. Ditto, if there is no C++, then no need to build the cpp plugin,
right?

Oh, except those may also be used to build host packages, too... And we
do not have a good way to know what languages the host packages would
need... So in the end, we do not want to disable any plugin for the host
variant...

(Yeah, I'm thinking aloud, sorry for the noise...))

Damn, I already pushed, or I'd have added as a comment and to the
commit.

`nyway... Thanks for the fixes! :-)

Regards,
Yann E. MORIN.

>  # grpc can use __atomic builtins, so we need to link with
>  # libatomic when available
> -- 
> 2.26.2
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 3/3] package/grpc: disable unnecessary build plugins

[Thomas De Schampheleire]

Hi Yann,

El mi?, 3 feb 2021 a las 23:09, Yann E. MORIN
(<yann.morin.1998@free.fr>) escribi?:
>
> Thomas, All,
>
> On 2021-02-03 13:43 +0100, Thomas De Schampheleire spake thusly:
> > From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> >
> > grpc has plugins for multiple programming languages, which are needed on
> > development machines only. Examples are grpc_cpp_plugin, grpc_ruby_plugin,
> > etc.
> >
> > Even though before commit fedf3318e3fd3c9ba57389ed2b36472f1a772b9e,
> > grpc_cpp_plugin was not installed for target, all other plugins still were.
> > This causes additional build time and rootfs space.
> >
> > As Buildroot does not support building a development environment for target,
> > these tools can be disabled.
> >
> > Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> > ---
> >  package/grpc/grpc.mk | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/package/grpc/grpc.mk b/package/grpc/grpc.mk
> > index a8a3af89ef..27b28deff3 100644
> > --- a/package/grpc/grpc.mk
> > +++ b/package/grpc/grpc.mk
> > @@ -27,7 +27,14 @@ GRPC_CONF_OPTS = \
> >       -DgRPC_PROTOBUF_PROVIDER=package \
> >       -DgRPC_RE2_PROVIDER=package \
> >       -DgRPC_SSL_PROVIDER=package \
> > -     -DgRPC_ZLIB_PROVIDER=package
> > +     -DgRPC_ZLIB_PROVIDER=package \
> > +     -DgRPC_BUILD_GRPC_CPP_PLUGIN=OFF \
> > +     -DgRPC_BUILD_GRPC_CSHARP_PLUGIN=OFF \
> > +     -DgRPC_BUILD_GRPC_NODE_PLUGIN=OFF \
> > +     -DgRPC_BUILD_GRPC_OBJECTIVE_C_PLUGIN=OFF \
> > +     -DgRPC_BUILD_GRPC_PHP_PLUGIN=OFF \
> > +     -DgRPC_BUILD_GRPC_PYTHON_PLUGIN=OFF \
> > +     -DgRPC_BUILD_GRPC_RUBY_PLUGIN=OFF
>
> So I am OK with applying this patch in bulk, because it makes sense.
>
> IIUC, the grpc_cpp_plugin will generate C++ code, and so the
> grpc_node_plugin will generate JS code, and so on... Right?

Yes, this is my understanding.

>
> If so, would it make sense to also trim the host variant further, too?
>
> I.e. if there is no nodejs on the target, then no need for the node
> plugin. Ditto, if there is no C++, then no need to build the cpp plugin,
> right?
>
> Oh, except those may also be used to build host packages, too... And we
> do not have a good way to know what languages the host packages would
> need... So in the end, we do not want to disable any plugin for the host
> variant...

At this moment, the grpc target package in buildroot depends on the
full host-grpc, which has (IMHO) excessive dependencies: it depends on
host-libabseil-cpp, host-re2, etc.
Since libabseil-cpp requires a gcc >= 4.9, it cannot be built on
CentOS 7 which uses 4.8.5.
But, the dependency grpc <- host-grpc is actualy only there to provide
a native version of grpc_cpp_plugin. And that plugin does not need
libabseil-cpp, re2, etc. Only host-protobuf.

So locally, I have prepared a patch to grpc CMakeLists.txt to add an
option to only build grpc_cpp_plugin, and not all the rest, so that
the Buildroot host-grpc can remove its dependencies on
host-libabseil-cpp and host-re2.
I have opened an issue on the grpc project to discuss this:
https://github.com/grpc/grpc/issues/25322

In these changes, I disabled all but grpc_cpp_plugin. In Buildroot
itself, there is no other package that currently has grpc support,
except collectd, which only needs grpc_cpp_plugin.
While there could indeed be external uses for these additional
language plugins like grpc_python_plugin, I'm not sure whether there
is an immediate need to support them.
But as far as I can see, they don't include on libabseil-cpp and the
others either, so it wouldn't be a big problem to retain them in
host-grpc I think.

I am waiting a bit on feedback on the aformentioned issue before
submitting these changes to buildroot.

Best regards,
Thomas

[Buildroot] [PATCH 0/3] grpc-related fixes

[Yann E. MORIN]

Thomas, All,

On 2021-02-03 13:43 +0100, Thomas De Schampheleire spake thusly:
> From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> 
> Commit fedf3318e3fd3c9ba57389ed2b36472f1a772b9e removed an obsolete patch on
> grpc, but this had the side effect that building collectd after grpc no longer
> succeeded, because grpc_cpp_plugin from the target was executed.
> 
> During investigation of this issue, a number of related problems were found.
> This series addresses these problems.

Series applied to master, thanks.

> Thomas De Schampheleire (3):
>   package/collectd: stop passing '--with-libgrpc++'
>   package/grpc: fix cross-compilation with
>     gRPC_BUILD_GRPC_CPP_PLUGIN=OFF
>   package/grpc: disable unnecessary build plugins

I'll further reply to this last patch, though, because I have some
questions and suggestions.

Regards,
Yann E. MORIN.

>  package/collectd/collectd.mk                  |  3 --
>  ...ix-cross-compilation-with-gRPC_BUILD.patch | 43 +++++++++++++++++++
>  package/grpc/grpc.mk                          |  9 +++-
>  3 files changed, 51 insertions(+), 4 deletions(-)
>  create mode 100644 package/grpc/0001-CMakeLists.txt-fix-cross-compilation-with-gRPC_BUILD.patch
> 
> -- 
> 2.26.2
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'