Merge "Fix single value variable inheritance order"
diff --git a/core/product_config.rbc b/core/product_config.rbc
index 81f9689..0187251 100644
--- a/core/product_config.rbc
+++ b/core/product_config.rbc
@@ -168,6 +168,8 @@
         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 @@
         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 @@
                 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]))
diff --git a/tests/single_value_inheritance/inherit1.rbc b/tests/single_value_inheritance/inherit1.rbc
index b71ffb3..0cc98a9 100644
--- a/tests/single_value_inheritance/inherit1.rbc
+++ b/tests/single_value_inheritance/inherit1.rbc
@@ -19,3 +19,5 @@
 
   cfg["PRODUCT_CHARACTERISTICS"] = "tablet"
   cfg["PRODUCT_DEFAULT_DEV_CERTIFICATE"] = "vendor/myvendor/certs/devkeys/devkey"
+  cfg.setdefault("PRODUCT_PACKAGES", [])
+  cfg["PRODUCT_PACKAGES"] += ["bar"]
diff --git a/tests/single_value_inheritance/inherit2.rbc b/tests/single_value_inheritance/inherit2.rbc
index be85866..ed5e569 100644
--- a/tests/single_value_inheritance/inherit2.rbc
+++ b/tests/single_value_inheritance/inherit2.rbc
@@ -18,3 +18,5 @@
   cfg = rblf.cfg(handle)
 
   cfg["PRODUCT_CHARACTERISTICS"] = "nosdcard"
+  cfg.setdefault("PRODUCT_PACKAGES", [])
+  cfg["PRODUCT_PACKAGES"] += ["foo"]
diff --git a/tests/single_value_inheritance/product.rbc b/tests/single_value_inheritance/product.rbc
index 3327043..d090af6 100644
--- a/tests/single_value_inheritance/product.rbc
+++ b/tests/single_value_inheritance/product.rbc
@@ -18,7 +18,7 @@
 
 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"] = ""
diff --git a/tests/single_value_inheritance/test.rbc b/tests/single_value_inheritance/test.rbc
index 07a5e65..dcde7e0 100644
--- a/tests/single_value_inheritance/test.rbc
+++ b/tests/single_value_inheritance/test.rbc
@@ -25,3 +25,4 @@
     (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"])