Fix single value variable inheritance order

List variables needed to be percolated in the order of inherit() calls.
children.keys() was in that order, due to starlark dictionaries being
iterable in the order of insertion, but the previous cl broke that
behavior by sorting them. Instead, only sort the children for
single value variables.

Fixes: 226206409
Fixes: 228044099
Test: ./out/rbcrun ./build/make/tests/run.rbc and testing aosp_arm64
Change-Id: I5b91514e87b158b615e4d4ec7868fccb0248379b

core/product_config.rbc

@@ -168,6 +168,8 @@ def _product_configuration(top_pcm_name, top_pcm, input_variables_init):
         children = handle.inherited_modules
         if _options.trace_modules:
             print("#   ", "    ".join(children.keys()))
+        # 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
 
@@ -291,12 +293,6 @@ def _percolate_inherited(configs, cfg_name, cfg, children_names):
         child_cfg = configs[child_name][1]
         for attr, value in child_cfg.items():
             if type(value) != "list":
-                # Single value variables take the first value available from the leftmost
-                # branch of the tree. If we also had "or attr in percolated_attrs" in this
-                # if statement, it would take the value from the rightmost branch.
-                if cfg.get(attr, "") == "":
-                    cfg[attr] = value
-                    percolated_attrs[attr] = True
                 continue
             if attr in percolated_attrs:
                 # We already are percolating this one, just add this list
@@ -306,6 +302,19 @@ def _percolate_inherited(configs, cfg_name, cfg, children_names):
                 cfg[attr] = []
                 __move_items(cfg[attr], child_cfg, attr)
 
+    # single value variables need to be inherited in alphabetical order,
+    # not in the order of inherit() calls.
+    for child_name in sorted(children_names):
+        child_cfg = configs[child_name][1]
+        for attr, value in child_cfg.items():
+            if type(value) != "list":
+                # Single value variables take the first value available from the leftmost
+                # branch of the tree. If we also had "or attr in percolated_attrs" in this
+                # if statement, it would take the value from the rightmost branch.
+                if cfg.get(attr, "") == "":
+                    cfg[attr] = value
+                    percolated_attrs[attr] = True
+
     for attr in _options.trace_variables:
         if attr in percolated_attrs:
             print("%s: %s^=%s" % (cfg_name, attr, cfg[attr]))

tests/single_value_inheritance/inherit1.rbc

@@ -19,3 +19,5 @@ def init(g, handle):
 
   cfg["PRODUCT_CHARACTERISTICS"] = "tablet"
   cfg["PRODUCT_DEFAULT_DEV_CERTIFICATE"] = "vendor/myvendor/certs/devkeys/devkey"
+  cfg.setdefault("PRODUCT_PACKAGES", [])
+  cfg["PRODUCT_PACKAGES"] += ["bar"]

tests/single_value_inheritance/inherit2.rbc

@@ -18,3 +18,5 @@ def init(g, handle):
   cfg = rblf.cfg(handle)
 
   cfg["PRODUCT_CHARACTERISTICS"] = "nosdcard"
+  cfg.setdefault("PRODUCT_PACKAGES", [])
+  cfg["PRODUCT_PACKAGES"] += ["foo"]

tests/single_value_inheritance/product.rbc

@@ -18,7 +18,7 @@ load(":inherit2.rbc", _inherit2_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/inherit1", _inherit1_init)
 
   cfg["PRODUCT_DEFAULT_DEV_CERTIFICATE"] = ""

tests/single_value_inheritance/test.rbc

@@ -25,3 +25,4 @@ 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"])