Refactor Makefile #31
Conversation
Makefile
Outdated
| -I$(GRPC_SRC_PATH) -I./src/ | ||
| CXXFLAGS += -std=c++11 | ||
|
|
||
| HAS_PKG_CONFIG ?= true |
There was a problem hiding this comment.
It is the same technique used by the main gRPC Makefile to ease cross-compilation in the case where pkg-config is not available. To quote it:
# # The steps for cross-compiling are as follows: # First, clone and make install of grpc using the native compilers for the host. # Also, install protoc (e.g., from a package like apt-get) # Then clone a fresh grpc for the actual cross-compiled build # Set the environment variable GRPC_CROSS_COMPILE to true # Set CC, CXX, LD, LDXX, AR, and STRIP to the cross-compiling binaries # Also set PROTOBUF_CONFIG_OPTS to indicate cross-compilation to protobuf (e.g., # PROTOBUF_CONFIG_OPTS="--host=arm-linux --with-protoc=/usr/local/bin/protoc" ) # Set HAS_PKG_CONFIG=false # To build tests, go to third_party/gflags and follow its ccmake instructions # Make sure that you enable building shared libraries and set your prefix to # something useful like /usr/local/cross # You will also need to set GRPC_CROSS_LDOPTS and GRPC_CROSS_AROPTS to hold # additional required arguments for LD and AR (examples below) # Then you can do a make from the cross-compiling fresh clone! #
There was a problem hiding this comment.
I've now changed this a bit to make it like 100% similar to gRPC.
| ALSA_CFLAGS=`pkg-config --cflags alsa` | ||
| ALSA_LDFLAGS=`pkg-config --libs alsa` | ||
| else | ||
| GRPC_GRPCPP_CFLAGS ?= |
There was a problem hiding this comment.
Would we ever not want grpc to be compiled? ALSA is Linux-only, but grpc is a core part of the sample.
There was a problem hiding this comment.
If you don't have pkg-config, you need a way to pass the CFLAGS/LDFLAGS needed to use gRPC, e.g. in my actual build script:
<...>
local GRPC_GRPCPP_CFLAGS="-pthread -I${TARGET_INSTALL_PATH}/include"
local GRPC_GRPCPP_LDLAGS="-L${TARGET_INSTALL_PATH}/lib -lgpr -laddress_sorting -lgrpc -lgrpc++"
<...>
GRPC_SRC_PATH=${GRPC_SRC_PATH} \
GOOGLEAPIS_SRC_PATH=${GOOGLEAPIS_SRC_PATH} \
GOOGLEAPIS_GENS_PATH=${GOOGLEAPIS_GENS_PATH} \
HAS_PKG_CONFIG=false \
SYSTEM=${SYSTEM} \
GRPC_GRPCPP_CFLAGS=${GRPC_GRPCPP_CFLAGS} \
GRPC_GRPCPP_LDLAGS=${GRPC_GRPCPP_LDLAGS} \
CPPFLAGS=${CPPFLAGS} \
LDFLAGS=${LDFLAGS} \
CXX=${CXX} \
AR=${AR} \
PROTOC=${PROTOC} \
PROTOC_PLUGINS_PATH=${PROTOC_PLUGINS_PATH} \
make -C "${PROJECT_PATH}/assistant-sdk-cpp" clean run_assistant
It's a way to help people with custom build systems without changing anything for the other users.
| endif | ||
|
|
||
| .PHONY: all | ||
| all: run_assistant | ||
|
|
||
| googleapis.ar: $(GOOGLEAPIS_CCS:.cc=.o) | ||
| ar r $@ $? | ||
| $(AR) r $@ $? |
There was a problem hiding this comment.
$(AR) is a pre-defined variable in GNU make, like $(CXX), see here. By hard coding ar like this, there is no chance to properly cross-compile the SDK by passing its value from outside the Makefile.
Goal is to ease cross-compilation: - add PROTOC and PROTOC_PLUGINS_PATH variables - add GOOGLEAPIS_ASSISTANT_PATH variable with API path and version - remove references to /usr/local by using pkg-config - remove redefinition of default variable CXX - use default variable AR - fix broken target $(GOOGLEAPIS_ASSISTANT_CCS:.cc=.h) - add protobufs and clean to PHONY target - add a HAS_PKG_CONFIG variable to avoid it if not available
|
This looks good to me, thanks. |
Goal is to ease cross-compilation: