Merge "Evaluate intermediate products properly" am: 90814c0706 am: 5ada12d182 am: 0144f19122

Original change: https://android-review.googlesource.com/c/platform/build/+/2060370

Change-Id: I1089f71d09fa5c022d7f9312e4ad3a8c1cdaaffc
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

core/product_config.rbc

@@ -120,25 +120,19 @@ def _product_configuration(top_pcm_name, top_pcm, input_variables_init):
 
     globals, globals_base = _init_globals(input_variables_init)
 
-    config_postfix = []  # Configs in postfix order
-
     # Each PCM is represented by a quadruple of function, config, children names
     # and readyness (that is, the configurations from inherited PCMs have been
     # substituted).
     configs = {top_pcm_name: (top_pcm, None, [], False)}  # All known PCMs
 
-    stash = []  # Configs to push once their descendants are done
-
-    # Stack containing PCMs to be processed. An item in the stack
-    # is a pair of PCMs name and its height in the product inheritance tree.
-    pcm_stack = [(top_pcm_name, 0)]
-    pcm_count = 0
+    # Stack containing PCMs to be processed
+    pcm_stack = [top_pcm_name]
 
     # Run it until pcm_stack is exhausted, but no more than N times
     for n in range(1000):
         if not pcm_stack:
             break
-        (name, height) = pcm_stack.pop()
+        name = pcm_stack.pop()
         pcm, cfg, c, _ = configs[name]
 
         # cfg is set only after PCM has been called, leverage this
@@ -146,9 +140,6 @@ def _product_configuration(top_pcm_name, top_pcm, input_variables_init):
         if cfg != None:
             continue
 
-        # Push ancestors until we reach this node's height
-        config_postfix.extend([stash.pop() for i in range(len(stash) - height)])
-
         # Run this one, obtaining its configuration and child PCMs.
         if _options.trace_modules:
             print("#%d: %s" % (n, name))
@@ -175,34 +166,75 @@ def _product_configuration(top_pcm_name, top_pcm, input_variables_init):
         # Starlark dictionaries are guaranteed to iterate through in insertion order,
         # so children.keys() will be ordered by the inherit() calls
         configs[name] = (pcm, handle.cfg, children.keys(), False)
-        pcm_count = pcm_count + 1
 
-        if len(children) == 0:
-            # Leaf PCM goes straight to the config_postfix
-            config_postfix.append(name)
-            continue
-
-        # Stash this PCM, process children in the sorted order
-        stash.append(name)
         for child_name in sorted(children, reverse = True):
             if child_name not in configs:
                 configs[child_name] = (children[child_name], None, [], False)
-            pcm_stack.append((child_name, len(stash)))
+            pcm_stack.append(child_name)
     if pcm_stack:
         fail("Inheritance processing took too many iterations")
 
-    # Flush the stash
-    config_postfix.extend([stash.pop() for i in range(len(stash))])
-    if len(config_postfix) != pcm_count:
-        fail("Ran %d modules but postfix tree has only %d entries" % (pcm_count, len(config_postfix)))
+    for pcm_name in globals.get("ARTIFACT_PATH_REQUIREMENT_PRODUCTS", []):
+        for var, val in evaluate_finalized_product_variables(configs, pcm_name[:-3]).items():
+            globals["PRODUCTS."+pcm_name+"."+var] = val
 
-    if _options.trace_modules:
+    # Copy product config variables from the cfg dictionary to the
+    # PRODUCTS.<top_level_makefile_name>.<var_name> global variables.
+    for var, val in evaluate_finalized_product_variables(configs, top_pcm_name, _options.trace_modules).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 evaluate_finalized_product_variables(configs, top_level_pcm_name, trace=False):
+    configs_postfix = []
+    pcm_stack = [(top_level_pcm_name, True)]
+    for i in range(1000):
+        if not pcm_stack:
+            break
+
+        pcm_name, before = pcm_stack.pop()
+        if before:
+            pcm_stack.append((pcm_name, False))
+            for child in sorted(configs[pcm_name][2], reverse = True):
+                pcm_stack.append((child, True))
+        else:
+            configs_postfix.append(pcm_name)
+    if pcm_stack:
+        fail("Inheritance processing took too many iterations")
+
+    # clone the configs, because in the process of evaluating the
+    # final cfg dictionary we will remove values from the intermediate
+    # cfg dictionaries. We need to be able to call evaluate_finalized_product_variables()
+    # multiple times, so we can't change the origional configs object.
+    cloned_configs = {}
+    for pcm_name in configs:
+        # skip unneeded pcms
+        if pcm_name not in configs_postfix:
+            continue
+        pcm, cfg, children_names, ready = configs[pcm_name]
+        cloned_cfg = {}
+        for var, val in cfg.items():
+            if type(val) == 'list':
+                cloned_cfg[var] = list(val)
+            else:
+                cloned_cfg[var] = val
+        cloned_configs[pcm_name] = (pcm, cloned_cfg, children_names, ready)
+    configs = cloned_configs
+
+    if trace:
         print("\n#---Postfix---")
-        for x in config_postfix:
+        for x in configs_postfix:
             print("#   ", x)
 
     # Traverse the tree from the bottom, evaluating inherited values
-    for pcm_name in config_postfix:
+    for pcm_name in configs_postfix:
         pcm, cfg, children_names, ready = configs[pcm_name]
 
         # Should run
@@ -221,25 +253,7 @@ def _product_configuration(top_pcm_name, top_pcm, input_variables_init):
         _substitute_inherited(configs, pcm_name, cfg)
         _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.<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)
-
+    return configs[top_level_pcm_name][1]
 
 def _dictionary_difference(a, b):
     result = {}

tests/artifact_path_requirements/inherit1.rbc

@@ -0,0 +1,21 @@
+# Copyright 2022 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+load("//build/make/core:product_config.rbc", "rblf")
+load(":inherit3.rbc", _inherit3_init = "init")
+
+def init(g, handle):
+  cfg = rblf.cfg(handle)
+
+  rblf.inherit(handle, "test/inherit3", _inherit3_init)

tests/artifact_path_requirements/inherit2.rbc

@@ -0,0 +1,22 @@
+# Copyright 2022 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+load("//build/make/core:product_config.rbc", "rblf")
+load(":inherit4.rbc", _inherit4_init = "init")
+
+def init(g, handle):
+  cfg = rblf.cfg(handle)
+
+  rblf.inherit(handle, "test/inherit4", _inherit4_init)
+  rblf.require_artifacts_in_path(handle, "vendor/", "")

tests/artifact_path_requirements/inherit3.rbc

@@ -0,0 +1,22 @@
+# Copyright 2022 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+load("//build/make/core:product_config.rbc", "rblf")
+load(":inherit4.rbc", _inherit4_init = "init")
+
+def init(g, handle):
+  cfg = rblf.cfg(handle)
+
+  rblf.inherit(handle, "test/inherit4", _inherit4_init)
+  rblf.require_artifacts_in_path(handle, "vendor/", "")

tests/artifact_path_requirements/inherit4.rbc

@@ -0,0 +1,21 @@
+# Copyright 2022 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+load("//build/make/core:product_config.rbc", "rblf")
+
+def init(g, handle):
+  cfg = rblf.cfg(handle)
+
+  rblf.setdefault(handle, "PRODUCT_COPY_FILES")
+  cfg["PRODUCT_COPY_FILES"] += ["foo/bar/baz.txt:vendor/etc/baz.txt"]

tests/artifact_path_requirements/product.rbc

@@ -0,0 +1,24 @@
+# Copyright 2022 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+load("//build/make/core:product_config.rbc", "rblf")
+load(":inherit1.rbc", _inherit1_init = "init")
+load(":inherit2.rbc", _inherit2_init = "init")
+load(":inherit3.rbc", _inherit3_init = "init")
+
+def init(g, handle):
+  cfg = rblf.cfg(handle)
+  rblf.inherit(handle, "test/inherit1", _inherit1_init)
+  rblf.inherit(handle, "test/inherit2", _inherit2_init)
+  rblf.inherit(handle, "test/inherit3", _inherit3_init)

tests/artifact_path_requirements/test.rbc

@@ -0,0 +1,27 @@
+# Copyright 2022 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+load("//build/make/core:product_config.rbc", "rblf")
+load("//build/make/tests/input_variables.rbc", input_variables_init = "init")
+load(":product.rbc", "init")
+
+def assert_eq(expected, actual):
+    if expected != actual:
+        fail("Expected '%s', got '%s'" % (expected, actual))
+
+def test():
+    (globals, globals_base) = rblf.product_configuration("test/product", init, input_variables_init)
+    assert_eq(["foo/bar/baz.txt:vendor/etc/baz.txt"], globals["PRODUCTS.test/product.mk.PRODUCT_COPY_FILES"])
+    assert_eq(["foo/bar/baz.txt:vendor/etc/baz.txt"], globals["PRODUCTS.test/inherit2.mk.PRODUCT_COPY_FILES"])
+    assert_eq(["foo/bar/baz.txt:vendor/etc/baz.txt"], globals["PRODUCTS.test/inherit3.mk.PRODUCT_COPY_FILES"])

tests/run.rbc

@@ -26,11 +26,16 @@ load(":product.rbc", "init")
 load(":board.rbc", board_init = "init")
 load(":board_input_vars.rbc", board_input_vars_init = "init")
 load("//build/make/tests/single_value_inheritance:test.rbc", test_single_value_inheritance = "test")
+load("//build/make/tests/artifact_path_requirements:test.rbc", test_artifact_path_requirements = "test")
 
 def assert_eq(expected, actual):
     if expected != actual:
         fail("Expected '%s', got '%s'" % (expected, actual))
 
+def assert_dict_subset(expected, actual):
+    for key, val in expected.items():
+        assert_eq(val, actual[key])
+
 # Unit tests for non-trivial runtime functions
 assert_eq(["a", "b", "c"], rblf.mksort("b a    c c"))
 assert_eq(["a", "b", "c"], rblf.mksort(["b", "a", "c", "c"]))
@@ -80,31 +85,28 @@ assert_eq(
     rblf.expand_wildcard("build/make/tests/run.rbc build/make/tests/nonexistent.rbc")
 )
 
-(globals, config, globals_base) = rblf.product_configuration("test/device", init, input_variables_init)
-assert_eq(
-    {
-      "PRODUCT_COPY_FILES": [
-          "part_from:part_to",
-          "device_from:device_to",
-          "device/google/redfin/audio/audio_platform_info_noextcodec_snd.xml:||VENDOR-PATH-PH||/etc/audio/audio_platform_info_noextcodec_snd.xml",
-          "xyz:/etc/xyz",
-          "x.xml:/etc/x.xml",
-          "y.xml:/etc/y.xml",
-          "from/sub/x:to/x",
-          "from/sub/y:to/y",
-      ],
-      "PRODUCT_HOST_PACKAGES": ["host"],
-      "PRODUCT_PACKAGES": [
-          "dev",
-          "inc",
-          "dev_after",
-          "board1_in",
-          "board1_is",
-      ],
-      "PRODUCT_PRODUCT_PROPERTIES": ["part_properties"]
-    },
-    { k:v for k, v in sorted(config.items()) }
-)
+(globals, globals_base) = rblf.product_configuration("test/device", init, input_variables_init)
+assert_dict_subset({
+    "PRODUCTS.test/device.mk.PRODUCT_COPY_FILES": [
+        "part_from:part_to",
+        "device_from:device_to",
+        "device/google/redfin/audio/audio_platform_info_noextcodec_snd.xml:||VENDOR-PATH-PH||/etc/audio/audio_platform_info_noextcodec_snd.xml",
+        "xyz:/etc/xyz",
+        "x.xml:/etc/x.xml",
+        "y.xml:/etc/y.xml",
+        "from/sub/x:to/x",
+        "from/sub/y:to/y",
+    ],
+    "PRODUCTS.test/device.mk.PRODUCT_HOST_PACKAGES": ["host"],
+    "PRODUCTS.test/device.mk.PRODUCT_PACKAGES": [
+        "dev",
+        "inc",
+        "dev_after",
+        "board1_in",
+        "board1_is",
+    ],
+    "PRODUCTS.test/device.mk.PRODUCT_PRODUCT_PROPERTIES": ["part_properties"]
+}, globals)
 
 ns = globals["$SOONG_CONFIG_NAMESPACES"]
 assert_eq(
@@ -134,8 +136,9 @@ assert_eq(
     { k:v for k,v in sorted(goals.items()) }
 )
 
-(board_globals, board_config, board_globals_base) = rblf.board_configuration(board_init, board_input_vars_init)
+(board_globals, board_globals_base) = rblf.board_configuration(board_init, board_input_vars_init)
 assert_eq({"A_LIST_VARIABLE": ["foo", "bar"]}, board_globals)
 assert_eq({"A_LIST_VARIABLE": ["foo"]}, board_globals_base)
 
 test_single_value_inheritance()
+test_artifact_path_requirements()

tests/single_value_inheritance/test.rbc

@@ -22,7 +22,7 @@ def assert_eq(expected, actual):
         fail("Expected '%s', got '%s'" % (expected, actual))
 
 def test():
-    (globals, config, globals_base) = rblf.product_configuration("test/device", init, input_variables_init)
-    assert_eq("tablet", config["PRODUCT_CHARACTERISTICS"])
-    assert_eq("vendor/myvendor/certs/devkeys/devkey", config["PRODUCT_DEFAULT_DEV_CERTIFICATE"])
-    assert_eq(["foo", "bar"], config["PRODUCT_PACKAGES"])
+    (globals, globals_base) = rblf.product_configuration("test/device", init, input_variables_init)
+    assert_eq("tablet", globals["PRODUCTS.test/device.mk.PRODUCT_CHARACTERISTICS"])
+    assert_eq("vendor/myvendor/certs/devkeys/devkey", globals["PRODUCTS.test/device.mk.PRODUCT_DEFAULT_DEV_CERTIFICATE"])
+    assert_eq(["foo", "bar"], globals["PRODUCTS.test/device.mk.PRODUCT_PACKAGES"])