From 0472730380dcbe9500ab14feab29f052baa5b386 Mon Sep 17 00:00:00 2001 From: Cole Faust Date: Mon, 28 Feb 2022 10:49:33 -0800 Subject: [PATCH 1/2] Support `m product-graph` in Starlark product config Bug: 221312856 Test: Manually Change-Id: Ia3a78df2de356801c83b1ba5c17601acfc026d06 --- core/board_config.mk | 2 +- core/dumpconfig.mk | 1 - core/main.mk | 24 ------------------------ core/product-graph.mk | 14 +------------- core/product.mk | 6 ++---- core/product_config.mk | 7 ------- core/product_config.rbc | 37 ++++++++++++++++++++++++------------- 7 files changed, 28 insertions(+), 63 deletions(-) diff --git a/core/board_config.mk b/core/board_config.mk index 72a8044882..dc50a689ad 100644 --- a/core/board_config.mk +++ b/core/board_config.mk @@ -254,7 +254,7 @@ else endif $(shell build/soong/scripts/update_out $(OUT_DIR)/rbc/rbc_board_config_results.mk \ - $(OUT_DIR)/rbcrun RBC_OUT="make,global" $(OUT_DIR)/rbc/boardlauncher.rbc) + $(OUT_DIR)/rbcrun RBC_OUT="make" $(OUT_DIR)/rbc/boardlauncher.rbc) ifneq ($(.SHELLSTATUS),0) $(error board configuration runner failed: $(.SHELLSTATUS)) endif diff --git a/core/dumpconfig.mk b/core/dumpconfig.mk index 9b1f2c29e5..640fe10f9c 100644 --- a/core/dumpconfig.mk +++ b/core/dumpconfig.mk @@ -117,7 +117,6 @@ DUMPCONFIG_SKIP_VARS := \ 9 \ LOCAL_PATH \ MAKEFILE_LIST \ - PARENT_PRODUCT_FILES \ current_mk \ _eiv_ev \ _eiv_i \ diff --git a/core/main.mk b/core/main.mk index 72958dab2d..a9628dc723 100644 --- a/core/main.mk +++ b/core/main.mk @@ -1234,33 +1234,9 @@ endef # 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 := \ diff --git a/core/product-graph.mk b/core/product-graph.mk index 6d51db17a9..63e904086a 100644 --- a/core/product-graph.mk +++ b/core/product-graph.mk @@ -17,7 +17,7 @@ # the sort also acts as a strip to remove the single space entries that creep in because of the evals define gather-all-products $(eval _all_products_visited := )\ -$(sort $(call all-products-inner, $(PARENT_PRODUCT_FILES))) +$(sort $(call all-products-inner, $(PRODUCTS))) endef define all-products-inner @@ -78,7 +78,6 @@ $(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)$(RBC_BOARD_CONFIG)) @echo Product graph DOT: $@ for $(PRIVATE_PRODUCTS_FILTER) $(hide) echo 'digraph {' > $@.in $(hide) echo 'graph [ ratio=.5 ];' >> $@.in @@ -87,20 +86,9 @@ ifeq (,$(RBC_PRODUCT_CONFIG)$(RBC_NO_PRODUCT_GRAPH)$(RBC_BOARD_CONFIG)) $(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 - -ifeq (,$(RBC_PRODUCT_CONFIG)$(RBC_NO_PRODUCT_GRAPH)$(RBC_BOARD_CONFIG)) .PHONY: product-graph 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 diff --git a/core/product.mk b/core/product.mk index 1f304cd278..f31611443c 100644 --- a/core/product.mk +++ b/core/product.mk @@ -396,12 +396,11 @@ endef # $(1): product to inherit # # To be called from product makefiles, and is later evaluated during the import-nodes -# call below. It does three things: +# call below. It does the following: # 1. Inherits all of the variables from $1. # 2. Records the inheritance in the .INHERITS_FROM variable -# 3. Records the calling makefile in PARENT_PRODUCT_FILES # -# (2) and (3) can be used together to reconstruct the include hierarchy +# (2) and the PRODUCTS variable can be used together to reconstruct the include hierarchy # See e.g. product-graph.mk for an example of this. # define inherit-product @@ -416,7 +415,6 @@ define inherit-product $(eval current_mk := $(strip $(word 1,$(_include_stack)))) \ $(eval inherit_var := PRODUCTS.$(current_mk).INHERITS_FROM) \ $(eval $(inherit_var) := $(sort $($(inherit_var)) $(np))) \ - $(eval PARENT_PRODUCT_FILES := $(sort $(PARENT_PRODUCT_FILES) $(current_mk))) \ $(call dump-inherit,$(strip $(word 1,$(_include_stack))),$(1)) \ $(call dump-config-vals,$(current_mk),inherit)) endef diff --git a/core/product_config.mk b/core/product_config.mk index be4aded376..95b390f059 100644 --- a/core/product_config.mk +++ b/core/product_config.mk @@ -257,14 +257,11 @@ else $(error product configuration converter failed: $(.SHELLSTATUS)) endif include $(OUT_DIR)/rbc/rbc_product_config_results.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 @@ -284,7 +281,6 @@ 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. # @@ -297,9 +293,6 @@ endif ############################################################################ # Strip and assign the PRODUCT_ variables. $(call strip-product-vars) -else -INTERNAL_PRODUCT := $(current_product_makefile) -endif current_product_makefile := all_product_makefiles := diff --git a/core/product_config.rbc b/core/product_config.rbc index 018725186a..888ff501d4 100644 --- a/core/product_config.rbc +++ b/core/product_config.rbc @@ -70,14 +70,7 @@ def __print_attr(attr, value): def _printvars(state): """Prints configuration and global variables.""" - (globals, cfg, globals_base) = state - for attr, val in sorted(cfg.items()): - __print_attr(attr, val) - if _options.print_globals: - print() - _printglobals(globals, globals_base) - -def _printglobals(globals, globals_base): + (globals, globals_base) = state for attr, val in sorted(globals.items()): if attr == _soong_config_namespaces_key: __print_attr("SOONG_CONFIG_NAMESPACES", val.keys()) @@ -218,7 +211,19 @@ def _product_configuration(top_pcm_name, top_pcm, input_variables_init): _percolate_inherited(configs, pcm_name, cfg, children_names) configs[pcm_name] = pcm, cfg, children_names, True - return (globals, configs[top_pcm_name][1], globals_base) + # Copy product config variables from the cfg dictionary to the + # PRODUCTS.. global variables. + for var, val in configs[top_pcm_name][1].items(): + globals["PRODUCTS."+top_pcm_name+".mk."+var] = val + + # Record inheritance hierarchy in PRODUCTS..INHERITS_FROM variables. + # This is required for m product-graph. + for config in configs: + if len(configs[config][2]) > 0: + globals["PRODUCTS."+config+".mk.INHERITS_FROM"] = sorted([x + ".mk" for x in configs[config][2]]) + globals["PRODUCTS"] = __words(globals.get("PRODUCTS", [])) + [top_pcm_name + ".mk"] + + return (globals, globals_base) def _dictionary_difference(a, b): @@ -237,7 +242,14 @@ def _board_configuration(board_config_init, input_variables_init): input_variables_init(globals_base, h_base) input_variables_init(globals, h) board_config_init(globals, h) - return (globals, _dictionary_difference(h.cfg, h_base.cfg), globals_base) + + # Board configuration files aren't really supposed to change + # product configuration variables, but some do. You lose the + # inheritance features of the product config variables if you do. + for var, value in _dictionary_difference(h.cfg, h_base.cfg).items(): + globals[var] = value + + return (globals, globals_base) def _substitute_inherited(configs, pcm_name, cfg): @@ -726,7 +738,6 @@ def __get_options(): """Returns struct containing runtime global settings.""" settings = dict( format = "pretty", - print_globals = False, rearrange = "", trace_modules = False, trace_variables = [], @@ -740,7 +751,8 @@ def __get_options(): elif x == "pretty" or x == "make": settings["format"] = x elif x == "global": - settings["print_globals"] = True + # TODO: Remove this, kept for backwards compatibility + pass elif x != "": fail("RBC_OUT: got %s, should be one of: [pretty|make] [sort|unique]" % x) for x in getattr(rblf_cli, "RBC_DEBUG", "").split(","): @@ -789,7 +801,6 @@ rblf = struct( mksubst = _mksubst, notdir = _notdir, printvars = _printvars, - printglobals = _printglobals, product_configuration = _product_configuration, board_configuration = _board_configuration, product_copy_files_by_pattern = _product_copy_files_by_pattern, From 2e8bb7989edbb0998cd8bc70a51c06b857d54bd4 Mon Sep 17 00:00:00 2001 From: Cole Faust Date: Mon, 21 Mar 2022 16:43:49 -0700 Subject: [PATCH 2/2] Add artifact path requirement support to Starlark Artifact path requirements requires setting some new variables, and also dumping the state of the product config variables after evaluating partial products that set artifact path requirements. Bug: 188079133 Test: Removed the code that disabled artifacts path requirements from the rbc dashboard script, and verified that it was still green for aosp_arm64 Change-Id: I228e35285d788f4c83aa695c0f28b7c7db02544c --- core/main.mk | 2 +- core/product_config.mk | 5 ++++- core/product_config.rbc | 32 +++++++++++++++++++++++++------- 3 files changed, 30 insertions(+), 9 deletions(-) diff --git a/core/main.mk b/core/main.mk index a9628dc723..f6da679ef7 100644 --- a/core/main.mk +++ b/core/main.mk @@ -1351,7 +1351,7 @@ else ifdef FULL_BUILD # Verify the artifact path requirements made by included products. is_asan := $(if $(filter address,$(SANITIZE_TARGET)),true) - ifeq (,$(or $(is_asan),$(DISABLE_ARTIFACT_PATH_REQUIREMENTS),$(RBC_PRODUCT_CONFIG),$(RBC_BOARD_CONFIG))) + ifeq (,$(or $(is_asan),$(DISABLE_ARTIFACT_PATH_REQUIREMENTS))) include $(BUILD_SYSTEM)/artifact_path_requirements.mk endif else diff --git a/core/product_config.mk b/core/product_config.mk index 95b390f059..939a02206a 100644 --- a/core/product_config.mk +++ b/core/product_config.mk @@ -263,7 +263,10 @@ endif # Import all or just the current product makefile # Quick check $(check-all-products) -ifeq ($(SKIP_ARTIFACT_PATH_REQUIREMENT_PRODUCTS_CHECK),) +# This step was already handled in the RBC product configuration. +# Since the equivalent starlark code will not add the partial products to +# the PRODUCTS variable, it's ok for them to be set before check-all-products +ifeq ($(RBC_PRODUCT_CONFIG)$(SKIP_ARTIFACT_PATH_REQUIREMENT_PRODUCTS_CHECK),) # Import all the products that have made artifact path requirements, so that we can verify # the artifacts they produce. # These are imported after check-all-products because some of them might not be real products. diff --git a/core/product_config.rbc b/core/product_config.rbc index 888ff501d4..60105d444b 100644 --- a/core/product_config.rbc +++ b/core/product_config.rbc @@ -157,6 +157,13 @@ def _product_configuration(top_pcm_name, top_pcm, input_variables_init): handle = __h_new() pcm(globals, handle) + if handle.artifact_path_requirements: + globals["PRODUCTS."+name+".mk.ARTIFACT_PATH_REQUIREMENTS"] = handle.artifact_path_requirements + globals["PRODUCTS."+name+".mk.ARTIFACT_PATH_ALLOWED_LIST"] = handle.artifact_path_allowed_list + globals["PRODUCTS."+name+".mk.ARTIFACT_PATH_REQUIREMENT_IS_RELAXED"] = "true" if handle.artifact_path_requirement_is_relaxed[0] else "" + globals.setdefault("ARTIFACT_PATH_REQUIREMENT_PRODUCTS", []) + globals["ARTIFACT_PATH_REQUIREMENT_PRODUCTS"] += [name+".mk"] + # Now we know everything about this PCM, record it in 'configs'. children = handle.inherited_modules if _options.trace_modules: @@ -211,6 +218,10 @@ def _product_configuration(top_pcm_name, top_pcm, input_variables_init): _percolate_inherited(configs, pcm_name, cfg, children_names) configs[pcm_name] = pcm, cfg, children_names, True + if (pcm_name + ".mk") in globals.get("ARTIFACT_PATH_REQUIREMENT_PRODUCTS", []): + for var, val in cfg.items(): + globals["PRODUCTS."+pcm_name+".mk."+var] = val + # Copy product config variables from the cfg dictionary to the # PRODUCTS.. global variables. for var, val in configs[top_pcm_name][1].items(): @@ -419,7 +430,10 @@ def __h_new(): return struct( cfg = dict(), inherited_modules = dict(), - default_list_value = list() + default_list_value = list(), + artifact_path_requirements = list(), + artifact_path_allowed_list = list(), + artifact_path_requirement_is_relaxed = [False], # as a list so that we can reassign it ) def __h_cfg(handle): @@ -564,13 +578,17 @@ def _notdir(paths): """ return " ".join([__base(w) for w in __words(paths)]) -def _require_artifacts_in_path(paths, allowed_paths): - """TODO.""" - pass +def _require_artifacts_in_path(handle, paths, allowed_paths): + """Equivalent to require-artifacts-in-path in Make.""" + handle.artifact_path_requirements.clear() + handle.artifact_path_requirements.extend(__words(paths)) + handle.artifact_path_allowed_list.clear() + handle.artifact_path_allowed_list.extend(__words(allowed_paths)) -def _require_artifacts_in_path_relaxed(paths, allowed_paths): - """TODO.""" - pass +def _require_artifacts_in_path_relaxed(handle, paths, allowed_paths): + """Equivalent to require-artifacts-in-path-relaxed in Make.""" + _require_artifacts_in_path(handle, paths, allowed_paths) + handle.artifact_path_requirement_is_relaxed[0] = True def _expand_wildcard(pattern): """Expands shell wildcard pattern."""