From be719a502ed27ca19b217848c3c7fc13169f30b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=AD=20Bol=C3=ADvar?= Date: Fri, 17 Jan 2020 14:12:06 -0800 Subject: [PATCH] manifest: support maps of imports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This completes the feature set for west manifest imports. We include a test of name whitelists from the documentation as a basic sanity check. Further testing is left to future work. To close out this feature after this, we should only need more error handling and testing. In particular, beyond general coverage issues, we need to try to reject any obvious import cycles, and add tests for the same. Signed-off-by: Martí Bolívar --- src/west/manifest.py | 117 ++++++++++++++++++++++++++++++++++++----- tests/test_manifest.py | 69 ++++++++++++++++++++++++ 2 files changed, 173 insertions(+), 13 deletions(-) diff --git a/src/west/manifest.py b/src/west/manifest.py index 7d9205f..688deb2 100644 --- a/src/west/manifest.py +++ b/src/west/manifest.py @@ -636,7 +636,7 @@ class Manifest: for subimp in imp: self._import_from_self(mp, subimp, projects) elif imptype == dict: - self._import_map_from_self(mp, imp, projects) + self._import_map(mp, imp, self.import_path_from_self, projects) else: self._malformed(f'{mp.abspath}: "self: import: {imp}" ' f'has invalid type {imptype}') @@ -677,9 +677,6 @@ class Manifest: self._malformed(f'{mp.abspath}: "self: import: {imp}": ' f'file {p} not found') - def _import_map_from_self(self, mproject, map, projects): # TODO - raise NotImplementedError('import: is not yet implemented') - def _import_pathobj_from_self(self, mp, pathobj, projects): # Import a Path object, which is a manifest file in the # manifest repository whose ManifestProject is mp. @@ -822,7 +819,8 @@ class Manifest: for subimp in imp: self._import_from_project(project, subimp, projects) elif imptype == dict: - self._import_map_from_project(project, imp, projects) + self._import_map(project, imp, self._import_path_from_project, + projects) else: self._malformed(f'{project.name_and_path}: invalid import {imp} ' f'type: {imptype}') @@ -892,8 +890,67 @@ class Manifest: return content - def _import_map_from_project(self, project, map, projects): # TODO - raise NotImplementedError('import: from project unimplemented') + def _import_map(self, p, imp, base_importer, projects): + # Helper routine used to handle imports from self and projects. + # We import everything, then filter out what's not desired. + + imap = self._load_imap(p, imp) + + all_imported = {} + base_importer(p, imap.file, all_imported) + + for name, project in all_imported.items(): + if _is_imap_ok(project, imap): + if name in imap.rename: + project.name = imap.rename[name] + self._add_project(project, projects) + + def _load_imap(self, project, imp): + # Convert a parsed self or project import value from YAML into + # an _import_map namedtuple. + + # Work on a copy in case the caller needs the full value. + copy = dict(imp) + ret = _import_map(copy.pop('file', _WEST_YML), + copy.pop('name-whitelist', []), + copy.pop('path-whitelist', []), + copy.pop('name-blacklist', []), + copy.pop('path-blacklist', []), + copy.pop('rename', {})) + + # Find a useful name for the project on error. + if isinstance(project, ManifestProject): + what = f'manifest file {project.abspath}' + else: + what = f'project {project.name}' + + # Check that the value is OK. + if copy: + # We popped out all of the valid keys already. + self._malformed(f'{what}: invalid import contents: {copy}') + elif not _is_imap_list(ret.name_whitelist): + self._malformed(f'{what}: bad import name-whitelist ' + f'{ret.name_whitelist}') + elif not _is_imap_list(ret.path_whitelist): + self._malformed(f'{what}: bad import path-whitelist ' + f'{ret.path_whitelist}') + elif not _is_imap_list(ret.name_blacklist): + self._malformed(f'{what}: bad import name-blacklist ' + f'{ret.name_blacklist}') + elif not _is_imap_list(ret.path_blacklist): + self._malformed(f'{what}: bad import path-blacklist ' + f'{ret.path_blacklist}') + elif not isinstance(ret.rename, dict): + self._malformed(f'{what}: rename: {ret.rename} ' + f'expected a map, {type(ret.rename)}') + else: + err = f"{what}: import map's rename includes " + for f, t in ret.rename.items(): + if 'manifest' in [f, t]: + self._malformed(err + f'{f}: {t}; ' + '"manifest" is a reserved name') + + return ret def _add_project(self, project, projects): # Add the project to our map if we don't already know about it. @@ -927,13 +984,8 @@ class Manifest: # Merge two west_commands attributes. Try to keep the result a # str if possible, but upgrade it to a list if both wc1 and # wc2 are truthy. - if wc1 and wc2: - if isinstance(wc1, str): - wc1 = [wc1] - if isinstance(wc2, str): - wc2 = [wc2] - return wc1 + wc2 + return _ensure_list(wc1) + _ensure_list(wc2) else: return wc1 or wc2 @@ -1390,6 +1442,11 @@ class ManifestProject(Project): return ret _defaults = collections.namedtuple('_defaults', 'remote revision') +_import_map = collections.namedtuple('_import_map', + 'file ' + 'name_whitelist path_whitelist ' + 'name_blacklist path_blacklist ' + 'rename') _YML_EXTS = ['yml', 'yaml'] _WEST_YML = 'west.yml' _SCHEMA_PATH = os.path.join(os.path.dirname(__file__), "manifest-schema.yml") @@ -1480,3 +1537,37 @@ def _load(data): return yaml.safe_load(data) except yaml.scanner.ScannerError as e: raise MalformedManifest(data) from e + +def _is_imap_list(value): + # Return True if the value is a valid import map 'blacklist' or + # 'whitelist'. Empty strings and lists are OK, and list nothing. + + return (isinstance(value, str) or + (isinstance(value, list) and + all(isinstance(item, str) for item in value))) + +def _is_imap_ok(project, imap): + # Return True if a project passes an import map's filters, + # and False otherwise. + + nwl, pwl, nbl, pbl = [_ensure_list(l) for l in + (imap.name_whitelist, imap.path_whitelist, + imap.name_blacklist, imap.path_blacklist)] + name = project.name + path = PurePath(project.path) + blacklisted = (name in nbl) or any(path.match(p) for p in pbl) + whitelisted = (name in nwl) or any(path.match(p) for p in pwl) + no_whitelists = not (nwl or pwl) + + if blacklisted: + return whitelisted + else: + return whitelisted or no_whitelists + +def _ensure_list(item): + # Converts item to a list containing it if item is a string, or + # returns item. + + if isinstance(item, str): + return [item] + return item diff --git a/tests/test_manifest.py b/tests/test_manifest.py index 9d00b97..70ef1a5 100644 --- a/tests/test_manifest.py +++ b/tests/test_manifest.py @@ -1920,6 +1920,75 @@ def test_import_flags_ignore(tmpdir): ''', import_flags=ImportFlag.IGNORE) assert m.get_projects(['foo']) +def test_import_name_whitelist(fs_topdir): + # This tests an example from the documentation which uses + # name-whitelist. + + manifest_repo = fs_topdir / 'mp' + with open(manifest_repo / 'west.yml', 'w') as f: + f.write(''' + manifest: + projects: + - name: mainline + url: https://git.example.com/mainline/manifest + import: + name-whitelist: + - app + - lib2 + rename: + app: mainline-app + - name: app + url: https://git.example.com/downstream/app + - name: lib3 + path: libraries/lib3 + url: https://git.example.com/downstream/lib3 + self: + path: mp + ''') + + mainline = fs_topdir / 'mainline' + create_repo(mainline) + create_branch(mainline, 'manifest-rev', checkout=True) + add_commit(mainline, 'mainline/west.yml', + files={'west.yml': + ''' + manifest: + projects: + - name: app + path: examples/app + url: https://git.example.com/mainline/app + - name: lib + path: libraries/lib + url: https://git.example.com/mainline/lib + - name: lib2 + path: libraries/lib2 + url: https://git.example.com/mainline/lib2 + '''}) + checkout_branch(mainline, 'master') + + actual = MF().projects + + expected = M('''\ + projects: + - name: mainline + url: https://git.example.com/mainline/manifest + - name: app + url: https://git.example.com/downstream/app + - name: lib3 + path: libraries/lib3 + url: https://git.example.com/downstream/lib3 + - name: mainline-app + path: examples/app + url: https://git.example.com/mainline/app + - name: lib2 + path: libraries/lib2 + url: https://git.example.com/mainline/lib2 + ''', + manifest_path='mp', + topdir=fs_topdir).projects + + for a, e in zip(actual, expected): + check_proj_consistency(a, e) ######################################### # Various invalid manifests