From d370a3f7bd4441dd492cb383da154c5214f21877 Mon Sep 17 00:00:00 2001 From: Cole Faust Date: Mon, 18 Apr 2022 17:18:08 -0700 Subject: [PATCH] Sort inherited products correctly We pass the filename without the extension to rblf.inherit(). Removing the extension changes the sort order when one file's name is a prefix of another: ``` >>> sorted(["base", "base-secondary"]) ['base', 'base-secondary'] >>> sorted(["base.mk", "base-secondary.mk"]) ['base-secondary.mk', 'base.mk'] ``` Correct the sort order so that global variables get their correct ordering. Bug: 229132189 Test: ./out/rbcrun ./build/make/tests/run.rbc Change-Id: I22367eb49b33956b71ac1b966fe78c1308b94257 --- core/product_config.rbc | 9 ++++-- tests/prefixed_sort_order/base-secondary.rbc | 21 ++++++++++++++ tests/prefixed_sort_order/base.rbc | 21 ++++++++++++++ tests/prefixed_sort_order/product.rbc | 29 ++++++++++++++++++++ tests/prefixed_sort_order/test.rbc | 26 ++++++++++++++++++ tests/run.rbc | 2 ++ 6 files changed, 106 insertions(+), 2 deletions(-) create mode 100644 tests/prefixed_sort_order/base-secondary.rbc create mode 100644 tests/prefixed_sort_order/base.rbc create mode 100644 tests/prefixed_sort_order/product.rbc create mode 100644 tests/prefixed_sort_order/test.rbc diff --git a/core/product_config.rbc b/core/product_config.rbc index 928cc557c0..ddf55c7a9d 100644 --- a/core/product_config.rbc +++ b/core/product_config.rbc @@ -104,6 +104,11 @@ def __printvars_rearrange_list(value_list): seen = {item: 0 for item in value_list} return sorted(seen.keys()) if _options.rearrange == "sort" else seen.keys() +def __sort_pcm_names(pcm_names): + # We have to add an extension back onto the pcm names when sorting, + # or else the sort order could be wrong when one is a prefix of another. + return [x[:-3] for x in sorted([y + ".mk" for y in pcm_names], reverse=True)] + def _product_configuration(top_pcm_name, top_pcm, input_variables_init): """Creates configuration.""" @@ -167,7 +172,7 @@ def _product_configuration(top_pcm_name, top_pcm, input_variables_init): # so children.keys() will be ordered by the inherit() calls configs[name] = (pcm, handle.cfg, children.keys(), False) - for child_name in sorted(children, reverse = True): + for child_name in __sort_pcm_names(children.keys()): if child_name not in configs: configs[child_name] = (children[child_name], None, [], False) pcm_stack.append(child_name) @@ -202,7 +207,7 @@ def evaluate_finalized_product_variables(configs, top_level_pcm_name, trace=Fals pcm_name, before = pcm_stack.pop() if before: pcm_stack.append((pcm_name, False)) - for child in sorted(configs[pcm_name][2], reverse = True): + for child in __sort_pcm_names(configs[pcm_name][2]): pcm_stack.append((child, True)) else: configs_postfix.append(pcm_name) diff --git a/tests/prefixed_sort_order/base-secondary.rbc b/tests/prefixed_sort_order/base-secondary.rbc new file mode 100644 index 0000000000..5446e8ff64 --- /dev/null +++ b/tests/prefixed_sort_order/base-secondary.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) + + g.setdefault("MY_VAR", []) + g["MY_VAR"] += ["foo"] diff --git a/tests/prefixed_sort_order/base.rbc b/tests/prefixed_sort_order/base.rbc new file mode 100644 index 0000000000..05b0d5d84e --- /dev/null +++ b/tests/prefixed_sort_order/base.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) + + g.setdefault("MY_VAR", []) + g["MY_VAR"] += ["bar"] diff --git a/tests/prefixed_sort_order/product.rbc b/tests/prefixed_sort_order/product.rbc new file mode 100644 index 0000000000..619b2c0ca3 --- /dev/null +++ b/tests/prefixed_sort_order/product.rbc @@ -0,0 +1,29 @@ +# 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(":base.rbc", _base_init = "init") +load(":base-secondary.rbc", _base_secondary_init = "init") + +def init(g, handle): + cfg = rblf.cfg(handle) + + # It's important that base-secondary uses a dash, an underscore won't expose the sort order issue: + # >>> sorted(["base", "base-secondary"]) + # ['base', 'base-secondary'] + # >>> sorted(["base.mk", "base-secondary.mk"]) + # ['base-secondary.mk', 'base.mk'] + + rblf.inherit(handle, "base", _base_init) + rblf.inherit(handle, "base-secondary", _base_secondary_init) diff --git a/tests/prefixed_sort_order/test.rbc b/tests/prefixed_sort_order/test.rbc new file mode 100644 index 0000000000..e59a509f45 --- /dev/null +++ b/tests/prefixed_sort_order/test.rbc @@ -0,0 +1,26 @@ +# 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/device", init, input_variables_init) + assert_eq(["foo", "bar"], globals["MY_VAR"]) diff --git a/tests/run.rbc b/tests/run.rbc index 56ba39413b..77b6e89fdb 100644 --- a/tests/run.rbc +++ b/tests/run.rbc @@ -27,6 +27,7 @@ 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") +load("//build/make/tests/prefixed_sort_order:test.rbc", test_prefixed_sort_order = "test") def assert_eq(expected, actual): if expected != actual: @@ -142,3 +143,4 @@ assert_eq({"A_LIST_VARIABLE": ["foo"]}, board_globals_base) test_single_value_inheritance() test_artifact_path_requirements() +test_prefixed_sort_order()