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

@@ -315,11 +315,11 @@ def env_view(args):
             if args.view_path:
                 view_path = args.view_path
             else:
-                view_path = env.default_view_path
+                view_path = env.create_view_path
-            env.update_view(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")

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 @@ def __init__(self, path, init_file=None, with_view=None):
                 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 @@ def _read_manifest(self, f):
         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 @@ def repos_path(self):
     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 @@ def _install(self, spec, **install_args):
                 os.remove(build_log_link)
             os.symlink(spec.package.build_log_path, build_log_link)
 
-    def view(self):
+    @property
-        if not self._view_path:
+    def all_views(self):
+        if not self.views:
             raise SpackEnvironmentError(
                 "{0} does not have a view enabled".format(self.name))
 
-        return YamlFilesystemView(
+        return [YamlFilesystemView(view['root'], spack.store.layout,
-            self._view_path, spack.store.layout, ignore_conflicts=True)
+                                   ignore_conflicts=True,
+                                   projections=view['projections'])
+                for view in self.views.values()]
 
-    def update_view(self, view_path):
+    @property
-        if self._view_path and self._view_path != view_path:
+    def default_view(self):
-            shutil.rmtree(self._view_path)
+        if not self.views:
+            raise SpackEnvironmentError(
+                "{0} does not have a view enabled".format(self.name))
 
-        self._view_path = view_path
+        if 'default' not in self.views:
+            raise SpackEnvironmentError(
+                "{0} does not have a default view enabled".format(self.name))
 
-    def regenerate_view(self):
+        return YamlFilesystemView(self.default_view_path,
-        if not self._view_path:
+                                  spack.store.layout, ignore_confligs=True)
+
+    @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 @@ def regenerate_view(self):
             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 @@ def _shell_vars(self):
         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 @@ def install_all(self, args=None):
                     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 @@ def _get_environment_specs(self, recurse_dependencies=True):
         If these specs appear under different user_specs, only one copy
         is added to the list returned.
         """
-        package_to_spec = {}
+        spec_set = set()
-        spec_list = list()
 
         for spec_hash in self.concretized_order:
             spec = self.specs_by_hash[spec_hash]
@@ -1023,17 +1064,9 @@ def _get_environment_specs(self, recurse_dependencies=True):
             specs = (spec.traverse(deptype=('link', 'run'))
                      if recurse_dependencies else (spec,))
 
-            for dep in specs:
+            spec_set.update(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)
 
-        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 @@ def write(self):
         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:
+        if self.views and len(self.views) == 1 and self.default_view_path:
-            view = True
+            path = self.default_view_path
-        elif self._view_path:
+            if self.views['default'] == self.create_view_descriptor():
-            view = self._view_path
+                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 @@ def write(self):
 
         # 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

lib/spack/spack/filesystem_view.py

@@ -210,9 +210,16 @@ def __init__(self, root, layout, **kwargs):
             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
+            # Ensure projections are the same from each source
-            msg += ' and was passed projections manually.'
+            # Read projections file from view
-            raise ConflictingProjectionsError(msg)
+            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)
 

lib/spack/spack/schema/env.py

@@ -67,9 +67,6 @@
                             'type': 'string'
                         },
                     },
-                    'view': {
-                        'type': ['boolean', 'string']
-                    },
                     'definitions': {
                         'type': 'array',
                         'default': [],
@@ -81,7 +78,7 @@
                                 }
                             },
                             'patternProperties': {
-                                '^(?!when$)\w*': spec_list_schema
+                               r'^(?!when$)\w*': spec_list_schema
                             }
                         }
                     },
@@ -91,25 +88,29 @@
                             {'type': 'boolean'},
                             {'type': 'string'},
                             {'type': 'object',
-                             'required': ['root'],
+                             'patternProperties': {
-                             'additionalProperties': False,
+                                  r'\w+': {
-                             'properties': {
+                                      'required': ['root'],
-                                 'root': {
+                                      'additionalProperties': False,
-                                     'type': 'string'
+                                      'properties': {
-                                 },
+                                          'root': {
-                                 'select': {
+                                              'type': 'string'
-                                     'type': 'array',
+                                          },
-                                     'items': {
+                                          'select': {
-                                         'type': 'string'
+                                              'type': 'array',
-                                     }
+                                              'items': {
-                                 },
+                                                  'type': 'string'
-                                 'exclude': {
+                                              }
-                                     'type': 'array',
+                                          },
-                                     'items': {
+                                          'exclude': {
-                                         'type': 'string'
+                                              'type': 'array',
-                                     }
+                                              'items': {
-                                 },
+                                                  'type': 'string'
-                                 'projections': projections_scheme
+                                              }
+                                        },
+                                          'projections': projections_scheme
+                                      }
+                                  }
                              }
                             }
                         ]

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 @@ def test_env_config_view_default(
 
     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 @@ def test_stack_definition_conditional_add_write(tmpdir):
         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)))

lib/spack/spack/test/concretize.py

@@ -15,6 +15,7 @@
 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):

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."""

var/spack/repos/builtin.mock/packages/mpich/package.py

@@ -28,4 +28,4 @@ class Mpich(Package):
     provides('mpi@:1', when='@:1')
 
     def install(self, spec, prefix):
-        pass
+        touch(prefix.mpich)

var/spack/repos/builtin.mock/packages/mpileaks/package.py

@@ -27,7 +27,7 @@ class Mpileaks(Package):
     libs = None
 
     def install(self, spec, prefix):
-        pass
+        touch(prefix.mpileaks)
 
     def setup_environment(self, senv, renv):
         renv.set('FOOBAR', self.name)