From 8d97bf5327e58588dc749f21eb8584bbc977d70d Mon Sep 17 00:00:00 2001 From: Sasha Smundak Date: Tue, 4 May 2021 09:58:38 -0700 Subject: [PATCH 1/2] Integrate Starlark-based configuration runner If RBC_PRODUCT_CONFIG variable is set, obtain product configuration variables by converting product configuration makefiles to Starlark files and then executing them. Also, introduce RBC_NO_PRODUCT_GRAPH variable to suppress product graph generation. We cannot generate product graph with Starlark, so this option allows to verify that the rest of the contents of the generated Ninja files remains the same when Starlark-based converter is used. This allows to perform the regression testing, i.e. running `RBC_NO_PRODUCT_GRAPH=t DISABLE_ARTIFACT_PATH_REQUIREMENTS=t m nothing` and `RBC_PRODUCT_CONFIG=t m nothing` should generate identical *.ninja files. Bug: 181797530 Test: Manual Change-Id: Ic6173a9640f32766b71c02a2b1833ce7a278e4cc --- core/main.mk | 47 ++++++++++++++++++++++++++++++++++-------- core/product-graph.mk | 11 ++++++++++ core/product_config.mk | 17 +++++++++++++++ 3 files changed, 66 insertions(+), 9 deletions(-) diff --git a/core/main.mk b/core/main.mk index 8b759312c6..e0fb58d824 100644 --- a/core/main.mk +++ b/core/main.mk @@ -1242,14 +1242,43 @@ endef # Name resolution for LOCAL_REQUIRED_MODULES: # See the select-bitness-of-required-modules definition. # $(1): product makefile + +# TODO(asmundak): +# `product-installed-files` and `host-installed-files` macros below used to +# call `get-product-var` directly to obtain per-file configuration variable +# values (the value of variable FOO is fetched from PRODUCT..FOO). +# Starlark-based configuration does not maintain per-file variable variable +# values. To work around this problem, we utilize the fact that +# `product-installed-files` and `host-installed-files` are called only in +# two places: +# 1. For the top-level product makefile (in this file). In this case +# $(call get-product-var , FOO) is the same as $(FOO) as the +# product configuration has been run already. Therefore we define +# _product-var macro to pick the values directly from product config +# variables when using Starlark-based configuration. +# 2. To check the path requirements (in artifact_path_requirements.mk). +# Starlark-based configuration does not perform this check at the moment. +# In the longer run most of the logic of this file will be moved to the +# Starlark. + +ifndef RBC_PRODUCT_CONFIG +define _product-var + $(call get-product-var,$(1),$(2)) +endef +else +define _product-var + $(call $(2)) +endef +endif + define product-installed-files $(eval _pif_modules := \ - $(call get-product-var,$(1),PRODUCT_PACKAGES) \ - $(if $(filter eng,$(tags_to_install)),$(call get-product-var,$(1),PRODUCT_PACKAGES_ENG)) \ - $(if $(filter debug,$(tags_to_install)),$(call get-product-var,$(1),PRODUCT_PACKAGES_DEBUG)) \ - $(if $(filter tests,$(tags_to_install)),$(call get-product-var,$(1),PRODUCT_PACKAGES_TESTS)) \ - $(if $(filter asan,$(tags_to_install)),$(call get-product-var,$(1),PRODUCT_PACKAGES_DEBUG_ASAN)) \ - $(if $(filter java_coverage,$(tags_to_install)),$(call get-product-var,$(1),PRODUCT_PACKAGES_DEBUG_JAVA_COVERAGE)) \ + $(call _product-var,$(1),PRODUCT_PACKAGES) \ + $(if $(filter eng,$(tags_to_install)),$(call _product-var,$(1),PRODUCT_PACKAGES_ENG)) \ + $(if $(filter debug,$(tags_to_install)),$(call _product-var,$(1),PRODUCT_PACKAGES_DEBUG)) \ + $(if $(filter tests,$(tags_to_install)),$(call _product-var,$(1),PRODUCT_PACKAGES_TESTS)) \ + $(if $(filter asan,$(tags_to_install)),$(call _product-var,$(1),PRODUCT_PACKAGES_DEBUG_ASAN)) \ + $(if $(filter java_coverage,$(tags_to_install)),$(call _product-var,$(1),PRODUCT_PACKAGES_DEBUG_JAVA_COVERAGE)) \ $(call auto-included-modules) \ ) \ $(eval ### Filter out the overridden packages and executables before doing expansion) \ @@ -1260,13 +1289,13 @@ define product-installed-files $(call expand-required-modules,_pif_modules,$(_pif_modules),$(_pif_overrides)) \ $(filter-out $(HOST_OUT_ROOT)/%,$(call module-installed-files, $(_pif_modules))) \ $(call resolve-product-relative-paths,\ - $(foreach cf,$(call get-product-var,$(1),PRODUCT_COPY_FILES),$(call word-colon,2,$(cf)))) + $(foreach cf,$(call _product-var,$(1),PRODUCT_COPY_FILES),$(call word-colon,2,$(cf)))) endef # Similar to product-installed-files above, but handles PRODUCT_HOST_PACKAGES instead # This does support the :32 / :64 syntax, but does not support module overrides. define host-installed-files - $(eval _hif_modules := $(call get-product-var,$(1),PRODUCT_HOST_PACKAGES)) \ + $(eval _hif_modules := $(call _product-var,$(1),PRODUCT_HOST_PACKAGES)) \ $(eval ### Split host vs host cross modules) \ $(eval _hcif_modules := $(filter host_cross_%,$(_hif_modules))) \ $(eval _hif_modules := $(filter-out host_cross_%,$(_hif_modules))) \ @@ -1351,7 +1380,7 @@ ifdef FULL_BUILD # Verify the artifact path requirements made by included products. is_asan := $(if $(filter address,$(SANITIZE_TARGET)),true) - ifneq (true,$(or $(is_asan),$(DISABLE_ARTIFACT_PATH_REQUIREMENTS))) + ifeq (,$(or $(is_asan),$(DISABLE_ARTIFACT_PATH_REQUIREMENTS),$(RBC_PRODUCT_CONFIG))) include $(BUILD_SYSTEM)/artifact_path_requirements.mk endif else diff --git a/core/product-graph.mk b/core/product-graph.mk index 968d01be0f..de4e581009 100644 --- a/core/product-graph.mk +++ b/core/product-graph.mk @@ -81,6 +81,7 @@ $(products_graph): PRIVATE_PRODUCTS := $(all_products) $(products_graph): PRIVATE_PRODUCTS_FILTER := $(products_list) $(products_graph): $(this_makefile) +ifeq (,$(RBC_PRODUCT_CONFIG)$(RBC_NO_PRODUCT_GRAPH)) @echo Product graph DOT: $@ for $(PRIVATE_PRODUCTS_FILTER) $(hide) echo 'digraph {' > $@.in $(hide) echo 'graph [ ratio=.5 ];' >> $@.in @@ -89,6 +90,10 @@ $(products_graph): $(this_makefile) $(foreach p,$(PRIVATE_PRODUCTS),$(call emit-product-node-props,$(p),$@.in)) $(hide) echo '}' >> $@.in $(hide) build/make/tools/filter-product-graph.py $(PRIVATE_PRODUCTS_FILTER) < $@.in > $@ +else + @echo RBC_PRODUCT_CONFIG and RBC_NO_PRODUCT_GRAPH should be unset to generate product graph + false +endif # Evaluates to the name of the product file # $(1) product file @@ -143,6 +148,7 @@ $(call product-debug-filename, $(p)): \ $(hide) cat $$< | build/make/tools/product_debug.py > $$@ endef +ifeq (,$(RBC_PRODUCT_CONFIG)$(RBC_NO_PRODUCT_GRAPH)) product_debug_files:= $(foreach p,$(all_products), \ $(eval $(call transform-product-debug, $(p))) \ @@ -154,3 +160,8 @@ product-graph: $(products_graph) @echo Product graph .dot file: $(products_graph) @echo Command to convert to pdf: dot -Tpdf -Nshape=box -o $(OUT_DIR)/products.pdf $(products_graph) @echo Command to convert to svg: dot -Tsvg -Nshape=box -o $(OUT_DIR)/products.svg $(products_graph) +else +.PHONY: product-graph + @echo RBC_PRODUCT_CONFIG and RBC_NO_PRODUCT_GRAPH should be unset to generate product graph + false +endif \ No newline at end of file diff --git a/core/product_config.mk b/core/product_config.mk index 99ff8aad62..403c6be130 100644 --- a/core/product_config.mk +++ b/core/product_config.mk @@ -172,11 +172,24 @@ endif ifneq (1,$(words $(current_product_makefile))) $(error Product "$(TARGET_PRODUCT)" ambiguous: matches $(current_product_makefile)) endif + +ifndef RBC_PRODUCT_CONFIG $(call import-products, $(current_product_makefile)) +else + rbcscript=build/soong/scripts/rbc-run + rc := $(shell $(rbcscript) $(TARGET_PRODUCT)-$(TARGET_BUILD_VARIANT) >$(OUT_DIR)/rbctemp.mk || echo $$?) + ifneq (,$(rc)) + $(error product configuration converter failed: $(rc)) + endif + include $(OUT_DIR)/rbctemp.mk + PRODUCTS += $(current_product_makefile) +endif endif # Import all or just the current product makefile +ifndef RBC_PRODUCT_CONFIG # Quick check $(check-all-products) +endif ifeq ($(SKIP_ARTIFACT_PATH_REQUIREMENT_PRODUCTS_CHECK),) # Import all the products that have made artifact path requirements, so that we can verify @@ -196,6 +209,7 @@ ifneq ($(filter dump-products, $(MAKECMDGOALS)),) $(dump-products) endif +ifndef RBC_PRODUCT_CONFIG # Convert a short name like "sooner" into the path to the product # file defining that product. # @@ -208,6 +222,9 @@ endif ############################################################################ # Strip and assign the PRODUCT_ variables. $(call strip-product-vars) +else +INTERNAL_PRODUCT := $(current_product_makefile) +endif current_product_makefile := all_product_makefiles := From 596874af42a8f96a3a2ffd3260e19eb2af9915a4 Mon Sep 17 00:00:00 2001 From: Sasha Smundak Date: Wed, 4 Aug 2021 17:19:06 -0700 Subject: [PATCH 2/2] Soong namespaces should be initialized Bug: 193540681 Test: rbcrun build/make/tests/run.rbc Change-Id: I0ecafa41b462998e0bb386680683a798ae5c46e7 --- core/product_config.rbc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/product_config.rbc b/core/product_config.rbc index d9a2a72ffc..3714448590 100644 --- a/core/product_config.rbc +++ b/core/product_config.rbc @@ -79,6 +79,10 @@ def _printvars(globals, cfg): if attr == _soong_config_namespaces_key: __print_attr("SOONG_CONFIG_NAMESPACES", val.keys()) for nsname, nsvars in sorted(val.items()): + # Define SOONG_CONFIG_ for Make, othewise + # it cannot be added to .KATI_READONLY list + if _options.format == "make": + print("SOONG_CONFIG_" + nsname, ":=") for var, val in sorted(nsvars.items()): __print_attr("SOONG_CONFIG_%s_%s" % (nsname, var), val) elif attr not in _globals_base: