add test, minor refactor to make testing easier

lib/spack/spack/environment.py

@@ -453,15 +453,19 @@ def _eval_conditional(string):
     return eval(string, valid_variables)
 
 
+_default_keep = 5
+
+
 class ViewDescriptor(object):
     def __init__(self, base_path, root, projections={}, select=[], exclude=[],
-                 link=default_view_link):
+                 link=default_view_link, keep=_default_keep):
         self.base = base_path
         self.root = spack.util.path.canonicalize_path(root)
         self.projections = projections
         self.select = select
         self.exclude = exclude
         self.link = link
+        self.keep = keep
 
     def select_fn(self, spec):
         return any(spec.satisfies(s) for s in self.select)
@@ -474,7 +478,8 @@ def __eq__(self, other):
                     self.projections == other.projections,
                     self.select == other.select,
                     self.exclude == other.exclude,
-                    self.link == other.link])
+                    self.link == other.link,
+                    self.keep == other.keep])
 
     def to_dict(self):
         ret = syaml.syaml_dict([('root', self.root)])
@@ -491,6 +496,8 @@ def to_dict(self):
             ret['exclude'] = self.exclude
         if self.link != default_view_link:
             ret['link'] = self.link
+        if self.keep != _default_keep:
+            ret['keep'] = self.keep
         return ret
 
     @staticmethod
@@ -500,7 +507,13 @@ def from_dict(base_path, d):
                               d.get('projections', {}),
                               d.get('select', []),
                               d.get('exclude', []),
-                              d.get('link', default_view_link))
+                              d.get('link', default_view_link),
+                              d.get('keep', _default_keep))
+
+    @property
+    def _impl_dir(self):
+        root_dir, root_name = os.path.split(self.root)
+        return os.path.join(root_dir, '._%s' % root_name)
 
     @property
     def _current_root(self):
@@ -516,9 +529,7 @@ def _current_root(self):
 
     def _next_root(self, specs):
         content_hash = self.content_hash(specs)
-        root_dir = os.path.dirname(self.root)
-        root_name = os.path.basename(self.root)
-        return os.path.join(root_dir, '._%s' % root_name, content_hash)
+        return os.path.join(self._impl_dir, content_hash)
 
     def content_hash(self, specs):
         d = syaml.syaml_dict([
@@ -649,12 +660,25 @@ def regenerate(self, all_specs, roots):
                 raise SpackEnvironmentViewError(msg)
             os.rename(tmp_symlink_name, self.root)
 
-            # remove old_root
-            if old_root and os.path.exists(old_root):
+            # If we have more than the number of views to keep in the directory
+            # delete the oldest until the correct number remain. Do not count
+            # things that aren't named as a root would be.
+            roots = [os.path.join(self._impl_dir, entry)
+                     for entry in os.listdir(self._impl_dir)
+                     if re.match(r'[a-z0-9]{32}', entry)]
+
+            num_to_remove = len(roots) - self.keep
+            if num_to_remove <= 0:
+                return
+
+            # sorting by st_mtime means the oldest come first
+            oldest_roots = sorted(roots, key=lambda path: os.stat(path).st_mtime)
+            for root_to_delete in oldest_roots[:num_to_remove]:
+                # remove old_root
                 try:
-                    shutil.rmtree(old_root)
+                    shutil.rmtree(root_to_delete)
                 except (IOError, OSError) as e:
-                    msg = "Failed to remove old view at %s\n" % old_root
+                    msg = "Failed to remove old view at %s\n" % root_to_delete
                     msg += str(e)
                     tty.warn(msg)
 

lib/spack/spack/schema/env.py

@@ -126,6 +126,10 @@
                                                 'type': 'string',
                                                 'pattern': '(roots|all)',
                                             },
+                                            'keep': {
+                                                'type': 'integer',
+                                                'minimum': 1,
+                                            },
                                             'select': {
                                                 'type': 'array',
                                                 'items': {

lib/spack/spack/test/cmd/env.py

@@ -2581,6 +2581,36 @@ def test_multiple_modules_post_env_hook(tmpdir, install_mockery, mock_fetch):
     assert spec.prefix in full_contents
 
 
+def test_view_keep_number(tmpdir, install_mockery, mock_fetch):
+    spack_yaml = """
+spack:
+  specs:
+  - trivial-install-test-package
+  view:
+    default:
+      root: view
+      keep: 1
+"""
+    _env_create('test', StringIO(spack_yaml))
+
+    with ev.read('test') as e:
+        install()
+
+        # Add something that will affect the view hash
+        e.default_view.exclude = ['gcc']
+        e.regenerate_views()
+
+        view_path = e.default_view._impl_dir
+
+    assert len(os.listdir(view_path)) == 1
+
+    with ev.read('test') as e:
+        e.default_view.keep = 2
+        e.regenerate_views()
+
+    assert len(os.listdir(view_path)) == 2
+
+
 @pytest.mark.regression('24148')
 def test_virtual_spec_concretize_together(tmpdir):
     # An environment should permit to concretize "mpi"