From 9704edfca7f579b0f79489ed275c22d878843b01 Mon Sep 17 00:00:00 2001 From: Bogdan Marinescu Date: Tue, 12 Jul 2016 14:42:21 +0300 Subject: [PATCH] Fix add_py_targets and tests Previously, add_py_targets assumed that it is not an error to add an existing target if that target was previously added using add_py_targets. This was done to aid testing, but it was weird, error prone and broke with the latest changes to the caching mechanism. With this commit: - it is always an error to add a target that was previously added, which is much more consistent. - tests for the configuration mechanism use the newly added 'set_targets_json_location' function to clear the internal caches in targets.py (and thus all previously added custom targets). As a side effect, this commit also tests the 'set_targets_json_location' function itself. --- tools/targets.py | 32 ++++++--------------------- tools/test/config_test/config_test.py | 3 +++ 2 files changed, 10 insertions(+), 25 deletions(-) diff --git a/tools/targets.py b/tools/targets.py index 4257da6e08..3d4a3b380c 100644 --- a/tools/targets.py +++ b/tools/targets.py @@ -58,9 +58,6 @@ class Target: # need to be computed differently than regular attributes __cumulative_attributes = ['extra_labels', 'macros', 'device_has', 'features'] - # List of targets that were added dynamically using "add_py_targets" (see below) - __py_targets = set() - # Location of the 'targets.json' file __targets_json_location = os.path.join(os.path.dirname(os.path.abspath(__file__)), '..', 'hal', 'targets.json') @@ -175,35 +172,20 @@ class Target: return v # Add one or more new target(s) represented as a Python dictionary in 'new_targets' - # It it an error to add a target with a name that exists in "targets.json" - # However, it is OK to add a target that was previously added via "add_py_targets" - # (this makes testing easier without changing the regular semantics) + # It is an error to add a target with a name that already exists. @staticmethod def add_py_targets(new_targets): crt_data = Target.get_json_target_data() - # First add all elemnts to the internal dictionary for tk, tv in new_targets.items(): - if crt_data.has_key(tk) and (not tk in Target.__py_targets): + if crt_data.has_key(tk): raise Exception("Attempt to add target '%s' that already exists" % tk) + # Add target data to the internal target dictionary crt_data[tk] = tv - Target.__py_targets.add(tk) - # Then create the new instances and update global variables if needed - for tk, tv in new_targets.items(): - # Is the target already created? - old_target = Target.__target_map.get(tk, None) - # Instantiate this target. If it is public, update the data in - # in TARGETS, TARGET_MAP, TARGET_NAMES + # Create the new target and add it to the relevant data structures new_target = Target(tk) - if tv.get("public", True): - if old_target: # remove the old target from TARGETS and TARGET_NAMES - TARGETS.remove(old_target) - TARGET_NAMES.remove(tk) - # Add the new target - TARGETS.append(new_target) - TARGET_MAP[tk] = new_target - TARGET_NAMES.append(tk) - # Update the target cache - Target.__target_map[tk] = new_target + TARGETS.append(new_target) + TARGET_MAP[tk] = new_target + TARGET_NAMES.append(tk) # Return the target instance starting from the target name @staticmethod diff --git a/tools/test/config_test/config_test.py b/tools/test/config_test/config_test.py index 59c2b50c98..d9f692e5bf 100644 --- a/tools/test/config_test/config_test.py +++ b/tools/test/config_test/config_test.py @@ -16,6 +16,7 @@ limitations under the License. """ from tools.build_api import get_config +from tools.targets import set_targets_json_location, Target from tools.config import ConfigException, Config import os, sys @@ -43,6 +44,8 @@ def test_tree(full_name, name): sys.stdout.flush() err_msg = None try: + # Use 'set_targets_json_location' to remove the previous custom targets from the target list + set_targets_json_location(Target._Target__targets_json_location) cfg, macros, features = get_config(full_name, target, "GCC_ARM") macros = Config.config_macros_to_macros(macros) except ConfigException as e: