[Buildroot] [PATCH 1/1] New package: gRPC, Google's remote procedue calss protocol.

Thomas Petazzoni thomas.petazzoni at bootlin.com
Tue Sep 18 20:00:45 UTC 2018


Hello Mark,

Thanks for this contribution! I believe I should mention that there was
a previous attempt at adding a package for "grpc":

  https://patchwork.ozlabs.org/patch/917779/
  https://patchwork.ozlabs.org/patch/917778/

Perhaps there are some useful things to re-use from this previous
contribution.

On Mon, 17 Sep 2018 12:34:55 -0700, Mark Fine wrote:
> Introduces support for gRPC, Google's remote procedure call
> protocol. Includes a patch for supporting cross compilation.
> Also patches the c-ares package for host support.
> 
> Signed-off-by: Mark Fine <mark.fine at gmail.com>

The commit title should be just:

	grpc: new package

> ---
>  package/c-ares/c-ares.mk                    |  1 +

This change should be a separate patch, coming before the patch adding
grpc.

> diff --git a/package/grpc/0001-fixup-cross-compile.patch b/package/grpc/0001-fixup-cross-compile.patch
> new file mode 100644
> index 0000000000..eaf262a95a
> --- /dev/null
> +++ b/package/grpc/0001-fixup-cross-compile.patch

All patches need to have a description and Signed-off-by. Also, since
the upstream project is using Git, we want the patches to be
Git-formatted (i.e be generated by 'git format-patch').

> + # These are automatically computed variables.
> + # There shouldn't be any need to change anything from now on.
> diff --git a/package/grpc/Config.in b/package/grpc/Config.in
> new file mode 100644
> index 0000000000..237a9d5a15
> --- /dev/null
> +++ b/package/grpc/Config.in
> @@ -0,0 +1,19 @@
> +config BR2_PACKAGE_GRPC
> +	bool "grpc"
> +	depends on BR2_INSTALL_LIBSTDCPP
> +	depends on BR2_TOOLCHAIN_HAS_THREADS
> +	select BR2_PACKAGE_C_ARES
> +	select BR2_PACKAGE_GFLAGS
> +	select BR2_PACKAGE_GTEST

Due to this select, you also need to replicate the following
dependencies:

        depends on BR2_USE_WCHAR
        depends on BR2_USE_MMU

> +	select BR2_PACKAGE_GTEST_GMOCK
> +	select BR2_PACKAGE_OPENSSL
> +	select BR2_PACKAGE_PROTOBUF

Due to this select, you also need to replicate the following
dependencies:

        depends on BR2_PACKAGE_PROTOBUF_ARCH_SUPPORTS
        depends on BR2_HOST_GCC_AT_LEAST_4_8 # C++11
        depends on BR2_TOOLCHAIN_GCC_AT_LEAST_4_8 # C++11
        depends on !BR2_STATIC_LIBS

> +	select BR2_PACKAGE_ZLIB
> +	help
> +	  gRPC is Google's protocol buffer based implementation of a
> +	  remote procedure call protocol.
> +
> +	  http://www.grpc.io
> +
> +comment "grpc needs a toolchain w/ C++, threads"
> +	depends on !(BR2_INSTALL_LIBSTDCPP && BR2_TOOLCHAIN_HAS_THREADS)

Make sure to update this comment with the new dependencies. Also, I
personally prefer:

	depends on !A || !B || !C

instead of

	depends on !(A && B && C)

> \ No newline at end of file

Please make sure the file ends with a newline.

> diff --git a/package/grpc/grpc.mk b/package/grpc/grpc.mk
> new file mode 100644
> index 0000000000..a74dece713
> --- /dev/null
> +++ b/package/grpc/grpc.mk
> @@ -0,0 +1,85 @@
> +################################################################################
> +#
> +# grpc
> +#
> +################################################################################
> +
> +GRPC_VERSION = v1.14.0
> +GRPC_SITE = https://github.com/grpc/grpc.git
> +GRPC_SITE_METHOD = git

Please use the tarball at
https://github.com/grpc/grpc/archive/v1.15.0.tar.gz instead.

See
https://buildroot.org/downloads/manual/manual.html#github-download-url
for more details about this.

> +GRPC_LICENSE = BSD-3-Clause
> +GRPC_LICENSE_FILES = LICENSE
> +
> +GRPC_DEPENDENCIES = host-grpc gflags gtest c-ares openssl protobuf zlib
> +HOST_GRPC_DEPENDENCIES = host-c-ares host-protobuf host-openssl
> +
> +GRPC_INSTALL_STAGING = YES
> +
> +GRPC_MAKE_ENV = \
> +	CC="$(TARGET_CC)" \
> +	CXX="$(TARGET_CXX)" \
> +	LD="$(TARGET_CC)" \
> +	LDXX="$(TARGET_CXX)" \
> +	STRIP="$(TARGET_STRIP)" \
> +	PROTOC="$(HOST_DIR)/bin/protoc" \
> +	PATH="$(HOST_DIR)/bin:$(PATH)" \

Use BR_PATH instead.

> +	GRPC_CROSS_COMPILE=true \
> +	GRPC_CROSS_LDOPTS="$(TARGET_LDFLAGS)" \
> +	GRPC_CROSS_AROPTS="$(LTO_PLUGIN)" \
> +	HAS_PKG_CONFIG=false \

Why ?

> +	PROTOBUF_CONFIG_OPTS="--host=$(GNU_TARGET_NAME) --with-protoc=$(HOST_DIR)/bin/protoc" \

Seems weird, is grpc going to build its own protobuf ?

> +	HOST_CC="$(HOSTCC)" \
> +	HOST_CXX="$(HOSTCXX)" \
> +	HOST_LD="$(HOSTCC)" \
> +	HOST_LDXX="$(HOSTCXX)" \
> +	HOST_CPPFLAGS="$(HOST_CPPFLAGS)" \
> +	HOST_CFLAGS="$(HOST_CFLAGS)" \
> +	HOST_CXXFLAGS="$(HOST_CXXFLAGS)" \
> +	HOST_LDFLAGS="$(HOST_LDFLAGS)"
> +
> +GRPC_MAKE_OPTS = \
> +	LD_LIBRARY_PATH="$(STAGING_DIR)/usr/lib" \

This looks very bad. LD_LIBRARY_PATH will be used by the dynamic loader
on your build machine to look for libraries... and you point it to
libraries built for the target. Not good at all.

> +	PROTOC="$(HOST_DIR)/bin/protoc"
> +
> +GRPC_INSTALL_TARGET_OPTS = \
> +	prefix="$(TARGET_DIR)/usr"
> +
> +GRPC_INSTALL_STAGING_OPTS = \
> +	prefix="$(STAGING_DIR)/usr"

These two variables are not needed, just use the value directly in the
install staging/target commands.

> +
> +define GRPC_BUILD_CMDS
> +	$(GRPC_MAKE_ENV) $(MAKE) $(GRPC_MAKE_OPTS) -C $(@D) \
> +		shared
> +endef
> +
> +define GRPC_INSTALL_STAGING_CMDS
> +	$(GRPC_MAKE_ENV) $(MAKE) $(GRPC_INSTALL_STAGING_OPTS) -C $(@D) \
> +		install-headers install-shared_c install-shared_cxx
> +endef
> +
> +define GRPC_INSTALL_TARGET_CMDS
> +	$(GRPC_MAKE_ENV) $(MAKE) $(GRPC_INSTALL_TARGET_OPTS) -C $(@D) \
> +		install-shared_c install-shared_cxx
> +endef
> +
> +HOST_GRPC_MAKE_ENV = \
> +	$(HOST_MAKE_ENV) \
> +	CFLAGS="$(HOST_CFLAGS)" \
> +	LDFLAGS="$(HOST_LDFLAGS)"
> +
> +HOST_GRPC_MAKE_OPTS = \
> +	LD_LIBRARY_PATH="$(HOST_DIR)/usr/lib" \

This also shouldn't be needed.

> +	prefix="$(HOST_DIR)/usr"

Use just $(HOST_DIR), not $(HOST_DIR)/usr

Could you rework your patch according to those comments? Perhaps the
previous attempt at adding grpc will provide some hints/help on how to
improve things.

Do not hesitate to let us know if you need more explanation about those
comments. You are the second person trying to add grpc to Buildroot, so
we definitely want to add it :-)

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


More information about the buildroot mailing list