Commit b0b27850 authored by Christophe Dufaza's avatar Christophe Dufaza Committed by Mahesh Mahadevan
Browse files

Revert "edtlib: fix "last modified" semantic for included ... specs"



[1] was introduced to get more valuable answers from
the PropertySpec.path API, which is supposed to tell
in which file the property's specification was "last modfied".

Further work on related issues [2] showed that the
approach chosen in [1] is dead end: we need to first rethink
how bindings (and especially child-bindings) are initialized.

[1] edtlib: fix last modified semantic in included property specs
[2] edtlib: Preserve paths of properties from included child bindings

See also: #65221, #78095

This reverts commit b3b5ad81.

Signed-off-by: default avatarChristophe Dufaza <chris@openmarl.org>
parent c58d6761
Loading
Loading
Loading
Loading
+15 −116
Original line number Diff line number Diff line
@@ -163,9 +163,7 @@ class Binding:

    def __init__(self, path: Optional[str], fname2path: Dict[str, str],
                 raw: Any = None, require_compatible: bool = True,
                 require_description: bool = True,
                 inc_allowlist: Optional[List[str]] = None,
                 inc_blocklist: Optional[List[str]] = None):
                 require_description: bool = True):
        """
        Binding constructor.

@@ -193,36 +191,16 @@ class Binding:
          "description:" line. If False, a missing "description:" is
          not an error. Either way, "description:" must be a string
          if it is present in the binding.

        inc_allowlist:
          The property-allowlist filter set by including bindings.

        inc_blocklist:
          The property-blocklist filter set by including bindings.
        """
        self.path: Optional[str] = path
        self._fname2path: Dict[str, str] = fname2path

        self._inc_allowlist: Optional[List[str]] = inc_allowlist
        self._inc_blocklist: Optional[List[str]] = inc_blocklist

        if raw is None:
            if path is None:
                _err("you must provide either a 'path' or a 'raw' argument")
            with open(path, encoding="utf-8") as f:
                raw = yaml.load(f, Loader=_BindingLoader)

        # Get the properties this binding modifies
        # before we merge the included ones.
        last_modified_props = list(raw.get("properties", {}).keys())

        # Map property names to their specifications:
        # - first, _merge_includes() will recursively populate prop2specs with
        #   the properties specified by the included bindings
        # - eventually, we'll update prop2specs with the properties
        #   this binding itself defines or modifies
        self.prop2specs: Dict[str, 'PropertySpec'] = {}

        # Merge any included files into self.raw. This also pulls in
        # inherited child binding definitions, so it has to be done
        # before initializing those.
@@ -246,11 +224,10 @@ class Binding:
        # Make sure this is a well defined object.
        self._check(require_compatible, require_description)

        # Update specs with the properties this binding defines or modifies.
        for prop_name in last_modified_props:
            self.prop2specs[prop_name] = PropertySpec(prop_name, self)

        # Initialize look up tables.
        self.prop2specs: Dict[str, 'PropertySpec'] = {}
        for prop_name in self.raw.get("properties", {}).keys():
            self.prop2specs[prop_name] = PropertySpec(prop_name, self)
        self.specifier2cells: Dict[str, List[str]] = {}
        for key, val in self.raw.items():
            if key.endswith("-cells"):
@@ -314,41 +291,18 @@ class Binding:

        if isinstance(include, str):
            # Simple scalar string case
            # Load YAML file and register property specs into prop2specs.
            inc_raw = self._load_raw(include, self._inc_allowlist,
                                     self._inc_blocklist)

            _merge_props(merged, inc_raw, None, binding_path,  False)
            _merge_props(merged, self._load_raw(include), None, binding_path,
                         False)
        elif isinstance(include, list):
            # List of strings and maps. These types may be intermixed.
            for elem in include:
                if isinstance(elem, str):
                    # Load YAML file and register property specs into prop2specs.
                    inc_raw = self._load_raw(elem, self._inc_allowlist,
                                             self._inc_blocklist)

                    _merge_props(merged, inc_raw, None, binding_path, False)
                    _merge_props(merged, self._load_raw(elem), None,
                                 binding_path, False)
                elif isinstance(elem, dict):
                    name = elem.pop('name', None)

                    # Merge this include property-allowlist filter
                    # with filters from including bindings.
                    allowlist = elem.pop('property-allowlist', None)
                    if allowlist is not None:
                        if self._inc_allowlist:
                            allowlist.extend(self._inc_allowlist)
                    else:
                        allowlist = self._inc_allowlist

                    # Merge this include property-blocklist filter
                    # with filters from including bindings.
                    blocklist = elem.pop('property-blocklist', None)
                    if blocklist is not None:
                        if self._inc_blocklist:
                            blocklist.extend(self._inc_blocklist)
                    else:
                        blocklist = self._inc_blocklist

                    child_filter = elem.pop('child-binding', None)

                    if elem:
@@ -359,12 +313,10 @@ class Binding:
                    _check_include_dict(name, allowlist, blocklist,
                                        child_filter, binding_path)

                    # Load YAML file, and register (filtered) property specs
                    # into prop2specs.
                    contents = self._load_raw(name,
                                              allowlist, blocklist,
                                              child_filter)
                    contents = self._load_raw(name)

                    _filter_properties(contents, allowlist, blocklist,
                                       child_filter, binding_path)
                    _merge_props(merged, contents, None, binding_path, False)
                else:
                    _err(f"all elements in 'include:' in {binding_path} "
@@ -384,17 +336,11 @@ class Binding:

        return raw


    def _load_raw(self, fname: str,
                  allowlist: Optional[List[str]] = None,
                  blocklist: Optional[List[str]] = None,
                  child_filter: Optional[dict] = None) -> dict:
    def _load_raw(self, fname: str) -> dict:
        # Returns the contents of the binding given by 'fname' after merging
        # any bindings it lists in 'include:' into it, according to the given
        # property filters.
        #
        # Will also register the (filtered) included property specs
        # into prop2specs.
        # any bindings it lists in 'include:' into it. 'fname' is just the
        # basename of the file, so we check that there aren't multiple
        # candidates.

        path = self._fname2path.get(fname)

@@ -406,55 +352,8 @@ class Binding:
            if not isinstance(contents, dict):
                _err(f'{path}: invalid contents, expected a mapping')

        # Apply constraints to included YAML contents.
        _filter_properties(contents,
                           allowlist, blocklist,
                           child_filter, self.path)

        # Register included property specs.
        self._add_included_prop2specs(fname, contents, allowlist, blocklist)

        return self._merge_includes(contents, path)

    def _add_included_prop2specs(self, fname: str, contents: dict,
                                 allowlist: Optional[List[str]] = None,
                                 blocklist: Optional[List[str]] = None) -> None:
        # Registers the properties specified by an included binding file
        # into the properties this binding supports/requires (aka prop2specs).
        #
        # Consider "this" binding B includes I1 which itself includes I2.
        #
        # We assume to be called in that order:
        # 1) _add_included_prop2spec(B, I1)
        # 2) _add_included_prop2spec(B, I2)
        #
        # Where we don't want I2 "taking ownership" for properties
        # modified by I1.
        #
        # So we:
        # - first create a binding that represents the included file
        # - then add the property specs defined by this binding to prop2specs,
        #   without overriding the specs modified by an including binding
        #
        # Note: Unfortunately, we can't cache these base bindings,
        # as a same YAML file may be included with different filters
        # (property-allowlist and such), leading to different contents.

        inc_binding = Binding(
            self._fname2path[fname],
            self._fname2path,
            contents,
            require_compatible=False,
            require_description=False,
            # Recursively pass filters to included bindings.
            inc_allowlist=allowlist,
            inc_blocklist=blocklist,
        )

        for prop, spec in inc_binding.prop2specs.items():
            if prop not in self.prop2specs:
                self.prop2specs[prop] = spec

    def _check(self, require_compatible: bool, require_description: bool):
        # Does sanity checking on the binding.