Merge "Fix single value variable inheritance order"

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"])