Merge changes from topic "colefaust_artifact_path_requirements"

* changes: Add artifact path requirement support to Starlark Support `m product-graph` in Starlark product config
2022-04-08 17:32:25 +00:00
parent 722d0a602a 2e8bb7989e
commit 7b51e8cbdf
7 changed files with 58 additions and 72 deletions
--- 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
--- 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 \
--- 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.<product-makefile>.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 <product>, 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 := \
@@ -1375,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
--- 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
--- 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
--- a/core/product_config.mk
+++ b/core/product_config.mk
@@ -257,16 +257,16 @@ 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),)
+# 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.
@@ -284,7 +284,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 +296,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 :=
--- 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())
@@ -164,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:
@@ -218,7 +218,23 @@ 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)
+        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.<top_level_makefile_name>.<var_name> 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.<file>.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 +253,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):
@@ -407,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):
@@ -552,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."""
@@ -740,7 +770,6 @@ def __get_options():
    """Returns struct containing runtime global settings."""
    settings = dict(
        format = "pretty",
-        print_globals = False,
        rearrange = "",
        trace_modules = False,
        trace_variables = [],
@@ -754,7 +783,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(","):
@@ -804,7 +834,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,