Merge pull request #1940 from geky/fix-features

[build tools] Added better support for features and recursive configs

Author: sg-

tools/build_api.py

@@ -105,8 +105,27 @@ def get_config(src_path, target, toolchain_name):
     for path in src_paths[1:]:
         resources.add(toolchain.scan_resources(path))
 
-    config.add_config_files(resources.json_files)
-    return config.get_config_data()
+    # Update configuration files until added features creates no changes
+    prev_features = set()
+    while True:
+        # Update the configuration with any .json files found while scanning
+        config.add_config_files(resources.json_files)
+
+        # Add features while we find new ones
+        features = config.get_features()
+        if features == prev_features:
+            break
+
+        for feature in features:
+            if feature in resources.features:
+                resources += resources.features[feature]
+
+        prev_features = features
+    config.validate_config()
+
+    cfg, macros = config.get_config_data()
+    features = config.get_features()
+    return cfg, macros, features
 
 def build_project(src_path, build_path, target, toolchain_name,
         libraries_paths=None, options=None, linker_script=None,
@@ -195,8 +214,24 @@ def build_project(src_path, build_path, target, toolchain_name,
             else:
                 resources.inc_dirs.append(inc_dirs)
 
-        # Update the configuration with any .json files found while scanning
-        config.add_config_files(resources.json_files)
+        # Update configuration files until added features creates no changes
+        prev_features = set()
+        while True:
+            # Update the configuration with any .json files found while scanning
+            config.add_config_files(resources.json_files)
+
+            # Add features while we find new ones
+            features = config.get_features()
+            if features == prev_features:
+                break
+
+            for feature in features:
+                if feature in resources.features:
+                    resources += resources.features[feature]
+
+            prev_features = features
+        config.validate_config()
+
         # And add the configuration macros to the toolchain
         toolchain.add_macros(config.get_config_data_macros())
 
@@ -237,7 +272,7 @@ def build_project(src_path, build_path, target, toolchain_name,
             add_result_to_report(report, cur_result)
 
         # Let Exception propagate
-        raise e
+        raise
 
 def build_library(src_paths, build_path, target, toolchain_name,
          dependencies_paths=None, options=None, name=None, clean=False, archive=True,
@@ -349,8 +384,25 @@ def build_library(src_paths, build_path, target, toolchain_name,
 
         # Handle configuration
         config = Config(target)
-        # Update the configuration with any .json files found while scanning
-        config.add_config_files(resources.json_files)
+
+        # Update configuration files until added features creates no changes
+        prev_features = set()
+        while True:
+            # Update the configuration with any .json files found while scanning
+            config.add_config_files(resources.json_files)
+
+            # Add features while we find new ones
+            features = config.get_features()
+            if features == prev_features:
+                break
+
+            for feature in features:
+                if feature in resources.features:
+                    resources += resources.features[feature]
+
+            prev_features = features
+        config.validate_config()
+
         # And add the configuration macros to the toolchain
         toolchain.add_macros(config.get_config_data_macros())
 

tools/config.py

@@ -40,6 +40,7 @@ def __init__(self, name, data, unit_name, unit_kind):
         self.value = data.get("value", None)
         self.required = data.get("required", False)
         self.macro_name = data.get("macro_name", "MBED_CONF_%s" % self.sanitize(self.name.upper()))
+        self.config_errors = []
 
     # Return the full (prefixed) name of a parameter.
     # If the parameter already has a prefix, check if it is valid
@@ -147,6 +148,11 @@ class Config:
         "application": set(["config", "custom_targets", "target_overrides", "macros", "__config_path"])
     }
 
+    # Allowed features in configurations
+    __allowed_features = [
+        "UVISOR", "BLE", "CLIENT", "IPV4", "IPV6"
+    ]
+
     # The initialization arguments for Config are:
     #     target: the name of the mbed target used for this configuration instance
     #     top_level_dirs: a list of top level source directories (where mbed_abb_config.json could be found)
@@ -176,7 +182,9 @@ def __init__(self, target, top_level_dirs = []):
         self.processed_configs = {}
         self.target = target if isinstance(target, str) else target.name
         self.target_labels = Target.get_target(self.target).get_labels()
-        self.target_instance = Target.get_target(self.target)
+        self.added_features = set()
+        self.removed_features = set()
+        self.removed_unecessary_features = False
 
     # Add one or more configuration files
     def add_config_files(self, flist):
@@ -212,44 +220,59 @@ def _process_config_parameters(self, data, params, unit_name, unit_kind):
             params[full_name] = ConfigParameter(name, v if isinstance(v, dict) else {"value": v}, unit_name, unit_kind)
         return params
 
+    # Add features to the available features
+    def remove_features(self, features):
+        for feature in features:
+            if feature in self.added_features:
+                raise ConfigException("Configuration conflict. Feature %s both added and removed." % feature)
+
+        self.removed_features |= set(features)
+
+    # Remove features from the available features
+    def add_features(self, features):
+        for feature in features:
+            if (feature in self.removed_features 
+                or (self.removed_unecessary_features and feature not in self.added_features)):
+                raise ConfigException("Configuration conflict. Feature %s both added and removed." % feature)
+
+        self.added_features |= set(features)
+
     # Helper function: process "config_parameters" and "target_config_overrides" in a given dictionary
     # data: the configuration data of the library/appliation
     # params: storage for the discovered configuration parameters
     # unit_name: the unit (library/application) that defines this parameter
     # unit_kind: the kind of the unit ("library" or "application")
     def _process_config_and_overrides(self, data, params, unit_name, unit_kind):
+        self.config_errors = []
         self._process_config_parameters(data.get("config", {}), params, unit_name, unit_kind)
         for label, overrides in data.get("target_overrides", {}).items():
             # If the label is defined by the target or it has the special value "*", process the overrides
             if (label == '*') or (label in self.target_labels):
-                # Parse out cumulative attributes
-                for attr in Target._Target__cumulative_attributes:
-                    attrs = getattr(self.target_instance, attr)
-
-                    if attr in overrides:
-                        del attrs[:]
-                        attrs.extend(overrides[attr])
-                        del overrides[attr]
-
-                    if attr+'_add' in overrides:
-                        attrs.extend(overrides[attr+'_add'])
-                        del overrides[attr+'_add']
-
-                    if attr+'_remove' in overrides:
-                        for a in overrides[attr+'_remove']:
-                            attrs.remove(a)
-                        del overrides[attr+'_remove']
-
-                    setattr(self.target_instance, attr, attrs)
+                # Parse out features
+                if 'target.features' in overrides:
+                    features = overrides['target.features']
+                    self.remove_features(self.added_features - set(features))
+                    self.add_features(features)
+                    self.removed_unecessary_features = True
+                    del overrides['target.features']
+
+                if 'target.features_add' in overrides:
+                    self.add_features(overrides['target.features_add'])
+                    del overrides['target.features_add']
+
+                if 'target.features_remove' in overrides:
+                    self.remove_features(overrides['target.features_remove'])
+                    del overrides['target.features_remove']
 
                 # Consider the others as overrides
                 for name, v in overrides.items():
                     # Get the full name of the parameter
                     full_name = ConfigParameter.get_full_name(name, unit_name, unit_kind, label)
-                    # If an attempt is made to override a parameter that isn't defined, raise an error
-                    if not full_name in params:
-                        raise ConfigException("Attempt to override undefined parameter '%s' in '%s'" % (full_name, ConfigParameter.get_display_name(unit_name, unit_kind, label)))
-                    params[full_name].set_value(v, unit_name, unit_kind, label)
+                    if full_name in params:
+                        params[full_name].set_value(v, unit_name, unit_kind, label)
+                    else:
+                        self.config_errors.append(ConfigException("Attempt to override undefined parameter '%s' in '%s'" 
+                            % (full_name, ConfigParameter.get_display_name(unit_name, unit_kind, label))))
         return params
 
     # Read and interpret configuration data defined by targets
@@ -345,3 +368,23 @@ def get_config_data_macros(self):
         params, macros = self.get_config_data()
         self._check_required_parameters(params)
         return macros + self.parameters_to_macros(params)
+
+    # Returns any features in the configuration data
+    def get_features(self):
+        params, _ = self.get_config_data()
+        self._check_required_parameters(params)
+        features = ((set(Target.get_target(self.target).features)
+            | self.added_features) - self.removed_features)
+
+        for feature in features:
+            if feature not in self.__allowed_features:
+                raise ConfigException("Feature '%s' is not a supported features" % feature)
+
+        return features
+
+    # Validate configuration settings. This either returns True or raises an exception
+    def validate_config(self):
+        if self.config_errors:
+            raise self.config_errors[0]
+        return True
+        

tools/test/config_test/config_test.py

@@ -28,7 +28,7 @@ def compare_config(cfg, expected):
     except KeyError:
         return "Unexpected key '%s' in configuration data" % k
     for k in expected:
-        if k != "desc" and k != "expected_macros" and not k in cfg:
+        if k not in ["desc", "expected_macros", "expected_features"] + cfg.keys():
             return "Expected key '%s' was not found in configuration data" % k
     return ""
 
@@ -43,7 +43,7 @@ def test_tree(full_name, name):
         sys.stdout.flush()
         err_msg = None
         try:
-            cfg, macros = get_config(full_name, target, "GCC_ARM")
+            cfg, macros, features = get_config(full_name, target, "GCC_ARM")
         except ConfigException as e:
             err_msg = e.message
         if err_msg:
@@ -63,23 +63,33 @@ def test_tree(full_name, name):
                 failed += 1
         else:
             res = compare_config(cfg, expected)
+            expected_macros = expected.get("expected_macros", None)
+            expected_features = expected.get("expected_features", None)
+
             if res:
                 print "FAILED!"
                 sys.stdout.write("    " + res + "\n")
                 failed += 1
-            else:
-                expected_macros = expected.get("expected_macros", None)
-                if expected_macros is not None:
-                    if sorted(expected_macros) != sorted(macros):
-                        print "FAILED!"
-                        sys.stderr.write("    List of macros doesn't match\n")
-                        sys.stderr.write("    Expected: '%s'\n" % ",".join(sorted(expected_macros)))
-                        sys.stderr.write("    Got: '%s'\n" % ",".join(sorted(expected_macros)))
-                        failed += 1
-                    else:
-                        print "OK"
+            elif expected_macros is not None:
+                if sorted(expected_macros) != sorted(macros):
+                    print "FAILED!"
+                    sys.stderr.write("    List of macros doesn't match\n")
+                    sys.stderr.write("    Expected: '%s'\n" % ",".join(sorted(expected_macros)))
+                    sys.stderr.write("    Got: '%s'\n" % ",".join(sorted(expected_macros)))
+                    failed += 1
                 else:
                     print "OK"
+            elif expected_features is not None:
+                if sorted(expected_features) != sorted(features):
+                    print "FAILED!"
+                    sys.stderr.write("    List of features doesn't match\n")
+                    sys.stderr.write("    Expected: '%s'\n" % ",".join(sorted(expected_features)))
+                    sys.stderr.write("    Got: '%s'\n" % ",".join(sorted(expected_features)))
+                    failed += 1
+                else:
+                    print "OK"
+            else:
+                print "OK"
     sys.path.remove(full_name)
     return failed
 

tools/test/config_test/test21/mbed_app.json

@@ -0,0 +1,7 @@
+{
+    "target_overrides": {
+        "*": {
+            "target.features": ["IPV4", "IPV6"]
+        }
+    }
+}

tools/test/config_test/test21/test_data.py

@@ -0,0 +1,8 @@
+# Testing basic features
+
+expected_results = {
+    "K64F": {
+        "desc": "test basic features",
+        "expected_features": ["IPV4", "IPV6"]
+    }
+}

tools/test/config_test/test22/lib1/mbed_lib.json

@@ -0,0 +1,8 @@
+{
+    "name": "lib1",
+    "target_overrides": {
+        "*": {
+            "target.features_add": ["IPV4"]
+        }
+    }
+}

tools/test/config_test/test22/mbed_app.json

@@ -0,0 +1,7 @@
+{
+    "target_overrides": {
+        "*": {
+            "target.features_add": ["IPV6"]
+        }
+    }
+}

tools/test/config_test/test22/test_data.py

@@ -0,0 +1,8 @@
+# Testing when adding two features
+
+expected_results = {
+    "K64F": {
+        "desc": "test composing features",
+        "expected_features": ["IPV4", "IPV6"]
+    }
+}

tools/test/config_test/test23/lib1/mbed_lib.json

@@ -0,0 +1,8 @@
+{
+    "name": "lib1",
+    "target_overrides": {
+        "*": {
+            "target.features_add": ["IPV4"]
+        }
+    }
+}

tools/test/config_test/test23/lib2/mbed_lib.json

@@ -0,0 +1,8 @@
+{
+    "name": "lib2",
+    "target_overrides": {
+        "*": {
+            "target.features_remove": ["IPV4"]
+        }
+    }
+}

tools/test/config_test/test23/mbed_app.json

@@ -0,0 +1,7 @@
+{
+    "target_overrides": {
+        "*": {
+            "target.features_add": ["IPV6"]
+        }
+    }
+}

tools/test/config_test/test23/test_data.py

@@ -0,0 +1,8 @@
+# Testing when two features collide
+
+expected_results = {
+    "K64F": {
+        "desc": "test feature collisions",
+        "exception_msg": "Configuration conflict. Feature IPV4 both added and removed." 
+    }
+}

tools/test/config_test/test24/FEATURE_IPV4/lib1/mbed_lib.json

@@ -0,0 +1,8 @@
+{
+    "name": "lib1",
+    "target_overrides": {
+        "*": {
+            "target.features_add": ["IPV6"]
+        }
+    }
+}

tools/test/config_test/test24/FEATURE_IPV6/lib2/mbed_lib.json

@@ -0,0 +1,8 @@
+{
+    "name": "lib2",
+    "target_overrides": {
+        "*": {
+            "target.features_add": ["UVISOR"]
+        }
+    }
+}

tools/test/config_test/test24/mbed_app.json

@@ -0,0 +1,7 @@
+{
+    "target_overrides": {
+        "*": {
+            "target.features_add": ["IPV4"]
+        }
+    }
+}