From 05f9f358363da66a51ebcba57409116dfa9e3a53 Mon Sep 17 00:00:00 2001 From: Ying Wang Date: Thu, 5 May 2016 20:02:08 -0700 Subject: [PATCH] Harden dependency on generated sources. Previously if a library has custom generated headers in LOCAL_GENERATED_SOURCES and export its include path with LOCAL_EXPORT_C_INCLUDE_DIRS, there is almost no way for the users of the library to set up dependency of their object files on the generated headers. This change makes the generated sources dependency of the library's export_includes, which is guaranteed generated before client code gets compiled. Also we added proto-generated cpp files to my_generated_sources so that we can deal solely with $(my_generated_sources). Because many Android.mks assume the generted .pb.hs are in $(generated_sources_dir) instead of $(intermediates), we have to generate the source files in $(generated_sources_dir) and make a copy in $(intermediates). Bug: 28622149 Change-Id: I73b21443fa706f3735faf16356ed8c08fbfecca6 --- core/binary.mk | 95 +++++++++++++++++++++------------------------ core/definitions.mk | 3 ++ 2 files changed, 47 insertions(+), 51 deletions(-) diff --git a/core/binary.mk b/core/binary.mk index c2e3069da..4736f0608 100644 --- a/core/binary.mk +++ b/core/binary.mk @@ -654,59 +654,57 @@ endif ## Compile the .proto files to .cc (or .c) and then to .o ########################################################### proto_sources := $(filter %.proto,$(my_src_files)) -proto_generated_objects := -proto_generated_headers := ifneq ($(proto_sources),) -proto_generated_sources_dir := $(generated_sources_dir)/proto -proto_generated_obj_dir := $(intermediates)/proto +proto_gen_dir := $(generated_sources_dir)/proto proto_sources_fullpath := $(addprefix $(LOCAL_PATH)/, $(proto_sources)) +my_rename_cpp_ext := ifneq (,$(filter nanopb-c nanopb-c-enable_malloc, $(LOCAL_PROTOC_OPTIMIZE_TYPE))) my_proto_source_suffix := .c my_proto_c_includes := external/nanopb-c -my_protoc_flags := --nanopb_out=$(proto_generated_sources_dir) \ +my_protoc_flags := --nanopb_out=$(proto_gen_dir) \ --plugin=external/nanopb-c/generator/protoc-gen-nanopb my_protoc_deps := $(NANOPB_SRCS) $(proto_sources_fullpath:%.proto=%.options) else -my_proto_source_suffix := .cc +my_proto_source_suffix := $(LOCAL_CPP_EXTENSION) +ifneq ($(my_proto_source_suffix),.cc) +# aprotoc is hardcoded to write out only .cc file. +# We need to rename the extension to $(LOCAL_CPP_EXTENSION) if it's not .cc. +my_rename_cpp_ext := true +endif my_proto_c_includes := external/protobuf/src my_cflags += -DGOOGLE_PROTOBUF_NO_RTTI -my_protoc_flags := --cpp_out=$(proto_generated_sources_dir) +my_protoc_flags := --cpp_out=$(proto_gen_dir) my_protoc_deps := endif -my_proto_c_includes += $(proto_generated_sources_dir) +my_proto_c_includes += $(proto_gen_dir) -proto_generated_sources := $(addprefix $(proto_generated_sources_dir)/, \ +proto_generated_cpps := $(addprefix $(proto_gen_dir)/, \ $(patsubst %.proto,%.pb$(my_proto_source_suffix),$(proto_sources_fullpath))) -proto_generated_headers := $(patsubst %.pb$(my_proto_source_suffix),%.pb.h, $(proto_generated_sources)) -proto_generated_objects := $(addprefix $(proto_generated_obj_dir)/, \ - $(patsubst %.proto,%.pb.o,$(proto_sources_fullpath))) -$(call track-src-file-obj,$(proto_sources),$(proto_generated_objects)) # Ensure the transform-proto-to-cc rule is only defined once in multilib build. -ifndef $(my_prefix)_$(LOCAL_MODULE_CLASS)_$(LOCAL_MODULE)_proto_defined -$(proto_generated_sources): PRIVATE_PROTO_INCLUDES := $(TOP) -$(proto_generated_sources): PRIVATE_PROTOC_FLAGS := $(LOCAL_PROTOC_FLAGS) $(my_protoc_flags) -$(proto_generated_sources): $(proto_generated_sources_dir)/%.pb$(my_proto_source_suffix): %.proto $(my_protoc_deps) $(PROTOC) +ifndef $(my_host)$(LOCAL_MODULE_CLASS)_$(LOCAL_MODULE)_proto_defined +$(proto_generated_cpps): PRIVATE_PROTO_INCLUDES := $(TOP) +$(proto_generated_cpps): PRIVATE_PROTOC_FLAGS := $(LOCAL_PROTOC_FLAGS) $(my_protoc_flags) +$(proto_generated_cpps): PRIVATE_RENAME_CPP_EXT := $(my_rename_cpp_ext) +$(proto_generated_cpps): $(proto_gen_dir)/%.pb$(my_proto_source_suffix): %.proto $(my_protoc_deps) $(PROTOC) $(transform-proto-to-cc) -# This is just a dummy rule to make sure gmake doesn't skip updating the dependents. -$(proto_generated_headers): $(proto_generated_sources_dir)/%.pb.h: $(proto_generated_sources_dir)/%.pb$(my_proto_source_suffix) - @echo "Updated header file $@." - $(hide) touch $@ - -$(my_prefix)_$(LOCAL_MODULE_CLASS)_$(LOCAL_MODULE)_proto_defined := true -endif # transform-proto-to-cc rule included only once - -$(proto_generated_objects): PRIVATE_ARM_MODE := $(normal_objects_mode) -$(proto_generated_objects): PRIVATE_ARM_CFLAGS := $(normal_objects_cflags) -$(proto_generated_objects): $(proto_generated_obj_dir)/%.o: $(proto_generated_sources_dir)/%$(my_proto_source_suffix) $(proto_generated_headers) -ifeq ($(my_proto_source_suffix),.c) - $(transform-$(PRIVATE_HOST)c-to-o) -else - $(transform-$(PRIVATE_HOST)cpp-to-o) +$(my_host)$(LOCAL_MODULE_CLASS)_$(LOCAL_MODULE)_proto_defined := true endif -$(call include-depfiles-for-objs, $(proto_generated_objects)) +# Ideally we can generate the source directly into $(intermediates). +# But many Android.mks assume the .pb.hs are in $(generated_sources_dir). +# As a workaround, we make a copy in the $(intermediates). +proto_intermediate_dir := $(intermediates)/proto +proto_intermediate_cpps := $(patsubst $(proto_gen_dir)/%,$(proto_intermediate_dir)/%,\ + $(proto_generated_cpps)) +$(proto_intermediate_cpps) : $(proto_intermediate_dir)/% : $(proto_gen_dir)/% + @echo "Copy: $@" + $(copy-file-to-target) + $(hide) cp $(basename $<).h $(basename $@).h +$(call track-src-file-gen,$(proto_sources),$(proto_intermediate_cpps)) + +my_generated_sources += $(proto_intermediate_cpps) my_c_includes += $(my_proto_c_includes) # Auto-export the generated proto source dir. @@ -892,7 +890,7 @@ $(call track-src-file-obj,$(patsubst %,%.arm,$(cpp_arm_sources)),$(cpp_arm_objec dotdot_arm_objects := $(foreach s,$(dotdot_arm_sources),\ $(eval $(call compile-dotdot-cpp-file,$(s),\ - $(yacc_cpps) $(proto_generated_headers) $(my_additional_dependencies),\ + $(my_additional_dependencies),\ dotdot_arm_objects))) $(call track-src-file-obj,$(patsubst %,%.arm,$(dotdot_arm_sources)),$(dotdot_arm_objects)) @@ -900,7 +898,7 @@ dotdot_sources := $(filter ../%$(LOCAL_CPP_EXTENSION),$(my_src_files)) dotdot_objects := $(foreach s,$(dotdot_sources),\ $(eval $(call compile-dotdot-cpp-file,$(s),\ - $(yacc_cpps) $(proto_generated_headers) $(my_additional_dependencies),\ + $(my_additional_dependencies),\ dotdot_objects))) $(call track-src-file-obj,$(dotdot_sources),$(dotdot_objects)) @@ -918,7 +916,6 @@ cpp_objects := $(cpp_arm_objects) $(cpp_normal_objects) ifneq ($(strip $(cpp_objects)),) $(cpp_objects): $(intermediates)/%.o: \ $(TOPDIR)$(LOCAL_PATH)/%$(LOCAL_CPP_EXTENSION) \ - $(yacc_cpps) $(proto_generated_headers) \ $(my_additional_dependencies) $(transform-$(PRIVATE_HOST)cpp-to-o) $(call include-depfiles-for-objs, $(cpp_objects)) @@ -940,8 +937,7 @@ ifneq ($(strip $(gen_cpp_objects)),) $(gen_cpp_objects): PRIVATE_ARM_MODE := $(normal_objects_mode) $(gen_cpp_objects): PRIVATE_ARM_CFLAGS := $(normal_objects_cflags) $(gen_cpp_objects): $(intermediates)/%.o: \ - $(intermediates)/%$(LOCAL_CPP_EXTENSION) $(yacc_cpps) \ - $(proto_generated_headers) \ + $(intermediates)/%$(LOCAL_CPP_EXTENSION) \ $(my_additional_dependencies) $(transform-$(PRIVATE_HOST)cpp-to-o) $(call include-depfiles-for-objs, $(gen_cpp_objects)) @@ -996,7 +992,7 @@ $(call track-src-file-obj,$(patsubst %,%.arm,$(c_arm_sources)),$(c_arm_objects)) dotdot_arm_objects := $(foreach s,$(dotdot_arm_sources),\ $(eval $(call compile-dotdot-c-file,$(s),\ - $(yacc_cpps) $(proto_generated_headers) $(my_additional_dependencies),\ + $(my_additional_dependencies),\ dotdot_arm_objects))) $(call track-src-file-obj,$(patsubst %,%.arm,$(dotdot_arm_sources)),$(dotdot_arm_objects)) @@ -1004,7 +1000,7 @@ dotdot_sources := $(filter ../%.c, $(my_src_files)) dotdot_objects := $(foreach s, $(dotdot_sources),\ $(eval $(call compile-dotdot-c-file,$(s),\ - $(yacc_cpps) $(proto_generated_headers) $(my_additional_dependencies),\ + $(my_additional_dependencies),\ dotdot_objects))) $(call track-src-file-obj,$(dotdot_sources),$(dotdot_objects)) @@ -1020,7 +1016,7 @@ $(dotdot_objects) $(c_normal_objects): PRIVATE_ARM_CFLAGS := $(normal_objects_cf c_objects := $(c_arm_objects) $(c_normal_objects) ifneq ($(strip $(c_objects)),) -$(c_objects): $(intermediates)/%.o: $(TOPDIR)$(LOCAL_PATH)/%.c $(yacc_cpps) $(proto_generated_headers) \ +$(c_objects): $(intermediates)/%.o: $(TOPDIR)$(LOCAL_PATH)/%.c \ $(my_additional_dependencies) $(transform-$(PRIVATE_HOST)c-to-o) $(call include-depfiles-for-objs, $(c_objects)) @@ -1041,7 +1037,7 @@ ifneq ($(strip $(gen_c_objects)),) # TODO: support compiling certain generated files as arm. $(gen_c_objects): PRIVATE_ARM_MODE := $(normal_objects_mode) $(gen_c_objects): PRIVATE_ARM_CFLAGS := $(normal_objects_cflags) -$(gen_c_objects): $(intermediates)/%.o: $(intermediates)/%.c $(yacc_cpps) $(proto_generated_headers) \ +$(gen_c_objects): $(intermediates)/%.o: $(intermediates)/%.c \ $(my_additional_dependencies) $(transform-$(PRIVATE_HOST)c-to-o) $(call include-depfiles-for-objs, $(gen_c_objects)) @@ -1056,7 +1052,7 @@ objc_objects := $(addprefix $(intermediates)/,$(objc_sources:.m=.o)) $(call track-src-file-obj,$(objc_sources),$(objc_objects)) ifneq ($(strip $(objc_objects)),) -$(objc_objects): $(intermediates)/%.o: $(TOPDIR)$(LOCAL_PATH)/%.m $(yacc_cpps) $(proto_generated_headers) \ +$(objc_objects): $(intermediates)/%.o: $(TOPDIR)$(LOCAL_PATH)/%.m \ $(my_additional_dependencies) $(transform-$(PRIVATE_HOST)m-to-o) $(call include-depfiles-for-objs, $(objc_objects)) @@ -1071,7 +1067,7 @@ objcpp_objects := $(addprefix $(intermediates)/,$(objcpp_sources:.mm=.o)) $(call track-src-file-obj,$(objcpp_sources),$(objcpp_objects)) ifneq ($(strip $(objcpp_objects)),) -$(objcpp_objects): $(intermediates)/%.o: $(TOPDIR)$(LOCAL_PATH)/%.mm $(yacc_cpps) $(proto_generated_headers) \ +$(objcpp_objects): $(intermediates)/%.o: $(TOPDIR)$(LOCAL_PATH)/%.mm \ $(my_additional_dependencies) $(transform-$(PRIVATE_HOST)mm-to-o) $(call include-depfiles-for-objs, $(objcpp_objects)) @@ -1201,8 +1197,7 @@ normal_objects := \ $(c_objects) \ $(gen_c_objects) \ $(objc_objects) \ - $(objcpp_objects) \ - $(proto_generated_objects) + $(objcpp_objects) new_order_normal_objects := $(foreach f,$(my_src_files),$(my_src_file_obj_$(f))) new_order_normal_objects += $(foreach f,$(my_gen_src_files),$(my_src_file_obj_$(f))) @@ -1483,11 +1478,9 @@ export_include_deps += $(strip \ $(foreach l,$(LOCAL_EXPORT_STATIC_LIBRARY_HEADERS), \ $(call intermediates-dir-for,STATIC_LIBRARIES,$(l),$(LOCAL_IS_HOST_MODULE),,$(LOCAL_2ND_ARCH_VAR_PREFIX),$(my_host_cross))/export_includes)) $(export_includes): PRIVATE_REEXPORTED_INCLUDES := $(export_include_deps) -# Make sure .pb.h are already generated before any dependent source files get compiled. -# Similarly, the generated DBus headers need to exist before we export their location. -# People are not going to consume the aidl generated cpp file, but the cpp file is -# generated after the headers, so this is a convenient way to ensure the headers exist. -$(export_includes) : $(proto_generated_headers) $(dbus_generated_headers) $(aidl_gen_cpp) $(export_include_deps) +# By adding $(my_generated_sources) it makes sure the headers get generated +# before any dependent source files get compiled. +$(export_includes) : $(my_generated_sources) $(export_include_deps) @echo Export includes file: $< -- $@ $(hide) mkdir -p $(dir $@) && rm -f $@.tmp && touch $@.tmp ifdef my_export_c_include_dirs diff --git a/core/definitions.mk b/core/definitions.mk index 664d86bce..420691dba 100644 --- a/core/definitions.mk +++ b/core/definitions.mk @@ -1109,6 +1109,9 @@ $(hide) $(PROTOC) \ $(addprefix --proto_path=, $(PRIVATE_PROTO_INCLUDES)) \ $(PRIVATE_PROTOC_FLAGS) \ $< +@# aprotoc outputs only .cc. Rename it to .cpp if necessary. +$(if $(PRIVATE_RENAME_CPP_EXT),\ + $(hide) mv $(basename $@).cc $@) endef