From cebf1fd668a499cf247a31f7f1d2e5c05fbdc7be Mon Sep 17 00:00:00 2001 From: Gregory Becker Date: Mon, 15 Apr 2019 19:07:45 -0700 Subject: stacks: update view management for multiple/combinatorial views This adds notion of a default view, other views in environments --- lib/spack/spack/cmd/env.py | 6 +- lib/spack/spack/environment.py | 139 ++++++++++++------- lib/spack/spack/filesystem_view.py | 13 +- lib/spack/spack/schema/env.py | 47 +++---- lib/spack/spack/test/cmd/env.py | 267 ++++++++++++++++++++++++++++++++++++- lib/spack/spack/test/concretize.py | 1 + lib/spack/spack/test/conftest.py | 12 ++ 7 files changed, 404 insertions(+), 81 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/cmd/env.py b/lib/spack/spack/cmd/env.py index 1b85849e8f..62e9d4f90c 100644 --- a/lib/spack/spack/cmd/env.py +++ b/lib/spack/spack/cmd/env.py @@ -315,11 +315,11 @@ def env_view(args): if args.view_path: view_path = args.view_path else: - view_path = env.default_view_path - env.update_view(view_path) + view_path = env.create_view_path + env.update_default_view(view_path) env.write() elif args.action == ViewAction.disable: - env.update_view(None) + env.update_default_view(None) env.write() else: tty.msg("No active environment") diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py index c2296018cf..6320477100 100644 --- a/lib/spack/spack/environment.py +++ b/lib/spack/spack/environment.py @@ -141,7 +141,7 @@ def activate( cmds += 'export SPACK_OLD_PS1="${PS1}"; fi;\n' cmds += 'export PS1="%s ${PS1}";\n' % prompt - if add_view and env._view_path: + if add_view and env.default_view_path: cmds += env.add_view_to_shell(shell) return cmds @@ -183,7 +183,7 @@ def deactivate(shell='sh'): cmds += 'unset SPACK_OLD_PS1; export SPACK_OLD_PS1;\n' cmds += 'fi;\n' - if _active_environment._view_path: + if _active_environment.default_view_path: cmds += _active_environment.rm_view_from_shell(shell) tty.debug("Deactivated environmennt '%s'" % _active_environment.name) @@ -467,9 +467,9 @@ class Environment(object): self._read_manifest(default_manifest_yaml) if with_view is False: - self._view_path = None + self.views = None elif isinstance(with_view, six.string_types): - self._view_path = with_view + self.views = {'default': self.create_view_descriptor(with_view)} # If with_view is None, then defer to the view settings determined by # the manifest file @@ -499,27 +499,18 @@ class Environment(object): enable_view = config_dict(self.yaml).get('view') # enable_view can be boolean, string, or None if enable_view is True or enable_view is None: - self._view_path = self.default_view_path + self.views = {'default': self.create_view_descriptor()} elif isinstance(enable_view, six.string_types): - self._view_path = enable_view + self.views = {'default': self.create_view_descriptor(enable_view)} + elif enable_view: + self.views = enable_view else: - self._view_path = None + self.views = None @property def user_specs(self): return self.read_specs['specs'] - enable_view = config_dict(self.yaml).get('view') - # enable_view can be true/false, a string, or None (if the manifest did - # not specify it) - if enable_view is True or enable_view is None: - self._view_path = self.default_view_path - elif isinstance(enable_view, six.string_types): - self._view_path = enable_view - else: - # enable_view is False - self._view_path = None - def _set_user_specs_from_lockfile(self): """Copy user_specs from a read-in lockfile.""" self.read_specs = { @@ -582,8 +573,13 @@ class Environment(object): def log_path(self): return os.path.join(self.path, env_subdir_name, 'logs') + def create_view_descriptor(self, root=None): + if root is None: + root = self.create_view_path + return {'root': root, 'projections': {}} + @property - def default_view_path(self): + def create_view_path(self): return os.path.join(self.env_subdir_path, 'view') @property @@ -824,26 +820,62 @@ class Environment(object): os.remove(build_log_link) os.symlink(spec.package.build_log_path, build_log_link) - def view(self): - if not self._view_path: + @property + def all_views(self): + if not self.views: raise SpackEnvironmentError( "{0} does not have a view enabled".format(self.name)) - return YamlFilesystemView( - self._view_path, spack.store.layout, ignore_conflicts=True) + return [YamlFilesystemView(view['root'], spack.store.layout, + ignore_conflicts=True, + projections=view['projections']) + for view in self.views.values()] + + @property + def default_view(self): + if not self.views: + raise SpackEnvironmentError( + "{0} does not have a view enabled".format(self.name)) - def update_view(self, view_path): - if self._view_path and self._view_path != view_path: - shutil.rmtree(self._view_path) + if 'default' not in self.views: + raise SpackEnvironmentError( + "{0} does not have a default view enabled".format(self.name)) - self._view_path = view_path + return YamlFilesystemView(self.default_view_path, + spack.store.layout, ignore_confligs=True) - def regenerate_view(self): - if not self._view_path: + @property + def default_view_path(self): + if not self.views or 'default' not in self.views: + return None + return self.views['default']['root'] + + def update_default_view(self, viewpath): + if self.default_view_path and self.default_view_path != viewpath: + shutil.rmtree(self.default_view_path) + + if viewpath: + if self.default_view_path: + self.views['default']['root'] = viewpath + elif self.views: + self.views['default'] = self.create_view_descriptor(viewpath) + else: + self.views = {'default': self.create_view_descriptor(viewpath)} + else: + self.views.pop('default', None) + + def regenerate_views(self): + if not self.views: tty.debug("Skip view update, this environment does not" " maintain a view") return + for name, view in zip(self.views.keys(), self.all_views): + self.regenerate_view(name, view) + + def regenerate_view(self, name, view): + view_descriptor = self.views[name] + specs_for_view = [] for spec in self._get_environment_specs(): # The view does not store build deps, so if we want it to @@ -852,13 +884,23 @@ class Environment(object): specs_for_view.append(spack.spec.Spec.from_dict( spec.to_dict(all_deps=False) )) + + select = view_descriptor.get('select', []) + if select: + select_fn = lambda x: any(x.satisfies(s) for s in select) + specs_for_view = list(filter(select_fn, specs_for_view)) + + exclude = view_descriptor.get('exclude', []) + if exclude: + exclude_fn = lambda x: not any(x.satisfies(e) for e in exclude) + specs_for_view = list(filter(exclude_fn, specs_for_view)) + installed_specs_for_view = set(s for s in specs_for_view if s.package.installed) - view = self.view() view.clean() specs_in_view = set(view.get_all_specs()) - tty.msg("Updating view at {0}".format(self._view_path)) + tty.msg("Updating view at {0}".format(view_descriptor['root'])) rm_specs = specs_in_view - installed_specs_for_view view.remove_specs(*rm_specs, with_dependents=False) @@ -880,7 +922,7 @@ class Environment(object): path_updates = list() for var, subdirs in updates: paths = filter(lambda x: os.path.exists(x), - list(os.path.join(self._view_path, x) + list(os.path.join(self.default_view_path, x) for x in subdirs)) path_updates.append((var, paths)) return path_updates @@ -945,7 +987,7 @@ class Environment(object): os.remove(build_log_link) os.symlink(spec.package.build_log_path, build_log_link) - self.regenerate_view() + self.regenerate_views() def all_specs_by_hash(self): """Map of hashes to spec for all specs in this environment.""" @@ -1014,8 +1056,7 @@ class Environment(object): If these specs appear under different user_specs, only one copy is added to the list returned. """ - package_to_spec = {} - spec_list = list() + spec_set = set() for spec_hash in self.concretized_order: spec = self.specs_by_hash[spec_hash] @@ -1023,17 +1064,9 @@ class Environment(object): specs = (spec.traverse(deptype=('link', 'run')) if recurse_dependencies else (spec,)) - for dep in specs: - prior = package_to_spec.get(dep.name) - if prior and prior != dep: - tty.debug("{0} takes priority over {1}" - .format(package_to_spec[dep.name].format(), - dep.format())) - else: - package_to_spec[dep.name] = dep - spec_list.append(dep) + spec_set.update(specs) - return spec_list + return list(spec_set) def _to_lockfile_dict(self): """Create a dictionary to store a lockfile for this environment.""" @@ -1158,10 +1191,16 @@ class Environment(object): yaml_spec_list = config_dict(self.yaml).setdefault('specs', []) yaml_spec_list[:] = self.user_specs.yaml_list - if self._view_path == self.default_view_path: - view = True - elif self._view_path: - view = self._view_path + if self.views and len(self.views) == 1 and self.default_view_path: + path = self.default_view_path + if self.views['default'] == self.create_view_descriptor(): + view = True + elif self.views['default'] == self.create_view_descriptor(path): + view = path + else: + view = self.views + elif self.views: + view = self.views else: view = False @@ -1179,7 +1218,7 @@ class Environment(object): # TODO: for operations that just add to the env (install etc.) this # could just call update_view - self.regenerate_view() + self.regenerate_views() def __enter__(self): self._previous_active = _active_environment diff --git a/lib/spack/spack/filesystem_view.py b/lib/spack/spack/filesystem_view.py index 4a6e45172b..1b3c692958 100644 --- a/lib/spack/spack/filesystem_view.py +++ b/lib/spack/spack/filesystem_view.py @@ -210,9 +210,16 @@ class YamlFilesystemView(FilesystemView): with open(projections_path, 'w') as f: f.write(s_yaml.dump({'projections': self.projections})) else: - msg = 'View at %s has projections file' % self._root - msg += ' and was passed projections manually.' - raise ConflictingProjectionsError(msg) + # Ensure projections are the same from each source + # Read projections file from view + with open(projections_path, 'r') as f: + projections_data = s_yaml.load(f) + spack.config.validate(projections_data, + spack.schema.projections.schema) + if self.projections != projections_data['projections']: + msg = 'View at %s has projections file' % self._root + msg += ' which does not match projections passed manually.' + raise ConflictingProjectionsError(msg) self.extensions_layout = YamlViewExtensionsLayout(self, layout) diff --git a/lib/spack/spack/schema/env.py b/lib/spack/spack/schema/env.py index 3c5265bacb..435d8f6b3f 100644 --- a/lib/spack/spack/schema/env.py +++ b/lib/spack/spack/schema/env.py @@ -67,9 +67,6 @@ schema = { 'type': 'string' }, }, - 'view': { - 'type': ['boolean', 'string'] - }, 'definitions': { 'type': 'array', 'default': [], @@ -81,7 +78,7 @@ schema = { } }, 'patternProperties': { - '^(?!when$)\w*': spec_list_schema + r'^(?!when$)\w*': spec_list_schema } } }, @@ -91,25 +88,29 @@ schema = { {'type': 'boolean'}, {'type': 'string'}, {'type': 'object', - 'required': ['root'], - 'additionalProperties': False, - 'properties': { - 'root': { - 'type': 'string' - }, - 'select': { - 'type': 'array', - 'items': { - 'type': 'string' - } - }, - 'exclude': { - 'type': 'array', - 'items': { - 'type': 'string' - } - }, - 'projections': projections_scheme + 'patternProperties': { + r'\w+': { + 'required': ['root'], + 'additionalProperties': False, + 'properties': { + 'root': { + 'type': 'string' + }, + 'select': { + 'type': 'array', + 'items': { + 'type': 'string' + } + }, + 'exclude': { + 'type': 'array', + 'items': { + 'type': 'string' + } + }, + 'projections': projections_scheme + } + } } } ] diff --git a/lib/spack/spack/test/cmd/env.py b/lib/spack/spack/test/cmd/env.py index ef22393026..fef017bf74 100644 --- a/lib/spack/spack/test/cmd/env.py +++ b/lib/spack/spack/test/cmd/env.py @@ -45,6 +45,13 @@ def check_viewdir_removal(viewdir): os.listdir(str(viewdir.join('.spack'))) == ['projections.yaml']) +@pytest.fixture() +def env_deactivate(): + yield + spack.environment._active_environment = None + os.environ.pop('SPACK_ENV', None) + + def test_add(): e = ev.create('test') e.add('mpileaks') @@ -637,7 +644,7 @@ def test_env_without_view_install( test_env = ev.read('test') with pytest.raises(spack.environment.SpackEnvironmentError): - test_env.view() + test_env.default_view view_dir = tmpdir.mkdir('view') @@ -668,7 +675,7 @@ env: e = ev.read('test') # Try retrieving the view object - view = e.view() + view = e.default_view assert view.get_spec('mpileaks') @@ -1088,3 +1095,259 @@ env: assert 'callpath' in packages_lists[1]['packages'] assert 'zmpi' in packages_lists[0]['packages'] assert 'zmpi' not in packages_lists[1]['packages'] + + +def test_stack_combinatorial_view(tmpdir, mock_fetch, mock_packages, + mock_archive, install_mockery): + filename = str(tmpdir.join('spack.yaml')) + viewdir = str(tmpdir.join('view')) + with open(filename, 'w') as f: + f.write("""\ +env: + definitions: + - packages: [mpileaks, callpath] + - compilers: ['%%gcc', '%%clang'] + specs: + - matrix: + - [$packages] + - [$compilers] + + view: + combinatorial: + root: %s + projections: + 'all': '${package}/${version}-${compilername}'""" % viewdir) + with tmpdir.as_cwd(): + env('create', 'test', './spack.yaml') + with ev.read('test'): + install() + + test = ev.read('test') + for _, spec in test.concretized_specs(): + assert os.path.exists( + os.path.join(viewdir, spec.name, '%s-%s' % + (spec.version, spec.compiler.name))) + + +def test_stack_view_select(tmpdir, mock_fetch, mock_packages, + mock_archive, install_mockery): + filename = str(tmpdir.join('spack.yaml')) + viewdir = str(tmpdir.join('view')) + with open(filename, 'w') as f: + f.write("""\ +env: + definitions: + - packages: [mpileaks, callpath] + - compilers: ['%%gcc', '%%clang'] + specs: + - matrix: + - [$packages] + - [$compilers] + + view: + combinatorial: + root: %s + select: ['%%gcc'] + projections: + 'all': '${package}/${version}-${compilername}'""" % viewdir) + with tmpdir.as_cwd(): + env('create', 'test', './spack.yaml') + with ev.read('test'): + install() + + test = ev.read('test') + for _, spec in test.concretized_specs(): + if spec.satisfies('%gcc'): + assert os.path.exists( + os.path.join(viewdir, spec.name, '%s-%s' % + (spec.version, spec.compiler.name))) + else: + assert not os.path.exists( + os.path.join(viewdir, spec.name, '%s-%s' % + (spec.version, spec.compiler.name))) + + +def test_stack_view_exclude(tmpdir, mock_fetch, mock_packages, + mock_archive, install_mockery): + filename = str(tmpdir.join('spack.yaml')) + viewdir = str(tmpdir.join('view')) + with open(filename, 'w') as f: + f.write("""\ +env: + definitions: + - packages: [mpileaks, callpath] + - compilers: ['%%gcc', '%%clang'] + specs: + - matrix: + - [$packages] + - [$compilers] + + view: + combinatorial: + root: %s + exclude: [callpath] + projections: + 'all': '${package}/${version}-${compilername}'""" % viewdir) + with tmpdir.as_cwd(): + env('create', 'test', './spack.yaml') + with ev.read('test'): + install() + + test = ev.read('test') + for _, spec in test.concretized_specs(): + if not spec.satisfies('callpath'): + assert os.path.exists( + os.path.join(viewdir, spec.name, '%s-%s' % + (spec.version, spec.compiler.name))) + else: + assert not os.path.exists( + os.path.join(viewdir, spec.name, '%s-%s' % + (spec.version, spec.compiler.name))) + + +def test_stack_view_select_and_exclude(tmpdir, mock_fetch, mock_packages, + mock_archive, install_mockery): + filename = str(tmpdir.join('spack.yaml')) + viewdir = str(tmpdir.join('view')) + with open(filename, 'w') as f: + f.write("""\ +env: + definitions: + - packages: [mpileaks, callpath] + - compilers: ['%%gcc', '%%clang'] + specs: + - matrix: + - [$packages] + - [$compilers] + + view: + combinatorial: + root: %s + select: ['%%gcc'] + exclude: [callpath] + projections: + 'all': '${package}/${version}-${compilername}'""" % viewdir) + with tmpdir.as_cwd(): + env('create', 'test', './spack.yaml') + with ev.read('test'): + install() + + test = ev.read('test') + for _, spec in test.concretized_specs(): + if spec.satisfies('%gcc') and not spec.satisfies('callpath'): + assert os.path.exists( + os.path.join(viewdir, spec.name, '%s-%s' % + (spec.version, spec.compiler.name))) + else: + assert not os.path.exists( + os.path.join(viewdir, spec.name, '%s-%s' % + (spec.version, spec.compiler.name))) + + +def test_stack_view_activate_from_default(tmpdir, mock_fetch, mock_packages, + mock_archive, install_mockery, + env_deactivate): + filename = str(tmpdir.join('spack.yaml')) + viewdir = str(tmpdir.join('view')) + with open(filename, 'w') as f: + f.write("""\ +env: + definitions: + - packages: [mpileaks, cmake] + - compilers: ['%%gcc', '%%clang'] + specs: + - matrix: + - [$packages] + - [$compilers] + + view: + default: + root: %s + select: ['%%gcc']""" % viewdir) + with tmpdir.as_cwd(): + env('create', 'test', './spack.yaml') + with ev.read('test'): + install() + + shell = env('activate', '--sh', 'test') + assert 'PATH' in shell + assert os.path.join(viewdir, 'bin') in shell + + +def test_stack_view_no_activate_without_default(tmpdir, mock_fetch, + mock_packages, mock_archive, + install_mockery, + env_deactivate): + filename = str(tmpdir.join('spack.yaml')) + viewdir = str(tmpdir.join('view')) + with open(filename, 'w') as f: + f.write("""\ +env: + definitions: + - packages: [mpileaks, cmake] + - compilers: ['%%gcc', '%%clang'] + specs: + - matrix: + - [$packages] + - [$compilers] + + view: + not-default: + root: %s + select: ['%%gcc']""" % viewdir) + with tmpdir.as_cwd(): + env('create', 'test', './spack.yaml') + with ev.read('test'): + install() + + shell = env('activate', '--sh', 'test') + assert 'PATH' not in shell + assert viewdir not in shell + + +def test_stack_view_multiple_views(tmpdir, mock_fetch, mock_packages, + mock_archive, install_mockery, + env_deactivate): + filename = str(tmpdir.join('spack.yaml')) + default_viewdir = str(tmpdir.join('default-view')) + combin_viewdir = str(tmpdir.join('combinatorial-view')) + with open(filename, 'w') as f: + f.write("""\ +env: + definitions: + - packages: [mpileaks, cmake] + - compilers: ['%%gcc', '%%clang'] + specs: + - matrix: + - [$packages] + - [$compilers] + + view: + default: + root: %s + select: ['%%gcc'] + combinatorial: + root: %s + exclude: [callpath %%gcc] + projections: + 'all': '${package}/${version}-${compilername}'""" % (default_viewdir, + combin_viewdir)) + with tmpdir.as_cwd(): + env('create', 'test', './spack.yaml') + with ev.read('test'): + install() + + shell = env('activate', '--sh', 'test') + assert 'PATH' in shell + assert os.path.join(default_viewdir, 'bin') in shell + + test = ev.read('test') + for _, spec in test.concretized_specs(): + if not spec.satisfies('callpath%gcc'): + assert os.path.exists( + os.path.join(combin_viewdir, spec.name, '%s-%s' % + (spec.version, spec.compiler.name))) + else: + assert not os.path.exists( + os.path.join(combin_viewdir, spec.name, '%s-%s' % + (spec.version, spec.compiler.name))) diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py index d8b0783297..eee0199726 100644 --- a/lib/spack/spack/test/concretize.py +++ b/lib/spack/spack/test/concretize.py @@ -15,6 +15,7 @@ from spack.spec import Spec, CompilerSpec from spack.spec import ConflictsInSpecError, SpecError from spack.version import ver from spack.test.conftest import MockPackage, MockPackageMultiRepo +import spack.compilers def check_spec(abstract, concrete): diff --git a/lib/spack/spack/test/conftest.py b/lib/spack/spack/test/conftest.py index 8fa5ff0e0e..146c5f7d5b 100644 --- a/lib/spack/spack/test/conftest.py +++ b/lib/spack/spack/test/conftest.py @@ -109,6 +109,18 @@ def no_chdir(): assert os.getcwd() == original_wd +@pytest.fixture(scope='function', autouse=True) +def reset_compiler_cache(): + """Ensure that the compiler cache is not shared across Spack tests + + This cache can cause later tests to fail if left in a state incompatible + with the new configuration. Since tests can make almost unlimited changes + to their setup, default to not use the compiler cache across tests.""" + spack.compilers._compiler_cache = {} + yield + spack.compilers._compiler_cache = {} + + @pytest.fixture(scope='session', autouse=True) def mock_stage(tmpdir_factory): """Mocks up a fake stage directory for use by tests.""" -- cgit v1.2.3-60-g2f50