From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Petazzoni Date: Tue, 18 Sep 2018 22:00:45 +0200 Subject: [Buildroot] [PATCH 1/1] New package: gRPC, Google's remote procedue calss protocol. In-Reply-To: <20180917193455.14329-1-mark.fine@gmail.com> References: <20180917193455.14329-1-mark.fine@gmail.com> Message-ID: <20180918220046.32289eab@windsurf> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: buildroot@busybox.net 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 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