From 50dbce9e74018bf41f93dbd65969f4a49e8335f5 Mon Sep 17 00:00:00 2001 From: Jimmy Brisson Date: Tue, 12 Jul 2016 12:54:44 +0300 Subject: [PATCH 1/3] Fixes to function caching in targets.py Now funnctions are looked up in the cache using a (function name, arguments) key, which makes it possible to cache different invocations of the functions (with different arguments). Also applied the @cached attribute to get_target. --- tools/targets.py | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/tools/targets.py b/tools/targets.py index 73a3c0a840..f6a8b8cb39 100644 --- a/tools/targets.py +++ b/tools/targets.py @@ -48,9 +48,9 @@ class HookError(Exception): caches = {} def cached(func): def wrapper(*args, **kwargs): - if not caches.has_key(func): - caches[func] = func(*args, **kwargs) - return caches[func] + if not caches.has_key((func.__name__, args)): + caches[(func.__name__, args)] = func(*args, **kwargs) + return caches[(func.__name__, args)] return wrapper class Target: @@ -58,9 +58,6 @@ class Target: # need to be computed differently than regular attributes __cumulative_attributes = ['extra_labels', 'macros', 'device_has', 'features'] - # {target_name: target_instance} map for all the targets in the system - __target_map = {} - # List of targets that were added dynamically using "add_py_targets" (see below) __py_targets = set() @@ -200,10 +197,9 @@ class Target: # Return the target instance starting from the target name @staticmethod + @cached def get_target(name): - if not Target.__target_map.has_key(name): - Target.__target_map[name] = Target(name) - return Target.__target_map[name] + return Target(name) def __init__(self, name): self.name = name From 117b21df8fc0079d53f32e9522c0aeeaf1634975 Mon Sep 17 00:00:00 2001 From: Bogdan Marinescu Date: Tue, 12 Jul 2016 12:58:53 +0300 Subject: [PATCH 2/3] It's now possible to specify the location of targets.json With this change, it becomes possible to use targets.py with any targets.json, not just the default one in ../hal/targets.json. targets.py will still be initialized with the default targets.json, but the code can then call "set_targets_json_location" to specify the new location of targets.json. set_targets_json_location modifies all the data "in place"; that is, it doesn't create a new TARGET_MAP, TARGET_NAMES or alike, but rather modified the existing ones. This is important, because code using this construct: from tools.targets import TARGET_MAP can work unmodified with this change. --- tools/targets.py | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/tools/targets.py b/tools/targets.py index f6a8b8cb39..4257da6e08 100644 --- a/tools/targets.py +++ b/tools/targets.py @@ -61,11 +61,21 @@ class Target: # 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') + # Load the description of JSON target data @staticmethod @cached def get_json_target_data(): - return json_file_to_dict(os.path.join(os.path.dirname(os.path.abspath(__file__)), '..', 'hal', 'targets.json')) + return json_file_to_dict(Target.__targets_json_location) + + # Set the location of the targets.json file + @staticmethod + def set_targets_json_location(location): + Target.__targets_json_location = location + # Invalidate caches, since the location of the JSON file changed + caches.clear() # Get the members of this module using Python's "inspect" module @staticmethod @@ -410,3 +420,15 @@ def get_target_detect_codes(): for detect_code in target.detect_code: result[detect_code] = target.name return result + +# Sets the location of the JSON file that contains the targets +def set_targets_json_location(location): + # First instruct Target about the new location + Target.set_targets_json_location(location) + # Then re-initialize TARGETS, TARGET_MAP and TARGET_NAMES + # The re-initialization does not create new variables, it keeps the old ones instead + # This ensures compatibility with code that does "from tools.targets import TARGET_NAMES" + TARGETS[:] = [Target.get_target(name) for name, value in Target.get_json_target_data().items() if value.get("public", True)] + TARGET_MAP.clear() + TARGET_MAP.update(dict([(t.name, t) for t in TARGETS])) + TARGET_NAMES[:] = TARGET_MAP.keys() From 9704edfca7f579b0f79489ed275c22d878843b01 Mon Sep 17 00:00:00 2001 From: Bogdan Marinescu Date: Tue, 12 Jul 2016 14:42:21 +0300 Subject: [PATCH 3/3] 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: