env: currently activated environment cannot be destroyed

lib/spack/spack/cmd/env.py

@@ -70,6 +70,7 @@ def get_env(args, cmd_name, fail_on_error=True):
                 % cmd_name)
 
     environment = ev.disambiguate(env)
+
     if not environment:
         tty.die('no such environment: %s' % env)
     return environment
@@ -263,8 +264,15 @@ def env_destroy(args):
         if not answer:
             tty.die("Will not destroy any environments")
 
-    for env in args.env:
-        ev.destroy(env)
+    for env_name in args.env:
+        env = ev.disambiguate(env_name)
+        if not env:
+            tty.die('no such environment: %s' % env_name)
+
+        if ev.active and ev.active.path == env.path:
+            tty.die("Environment %s can't be destroyed while activated.")
+
+        env.destroy()
         tty.msg("Successfully destroyed environment '%s'" % env)
 
 

lib/spack/spack/environment.py

@@ -150,7 +150,6 @@ def disambiguate(env, env_dir=None):
             return Environment(env_dir)
         else:
             raise EnvError('no environment in %s' % env_dir)
-        return
 
     return None
 
@@ -190,17 +189,6 @@ def create(name, init_file=None):
     return Environment(root(name), init_file)
 
 
-def destroy(name):
-    """Destroy a named environment."""
-    validate_env_name(name)
-    if not exists(name):
-        raise EnvError("no such environment '%s'" % name)
-    if not os.access(root(name), os.W_OK):
-        raise EnvError(
-            "insufficient permissions to modify environment: '%s'" % name)
-    shutil.rmtree(root(name))
-
-
 def config_dict(yaml_data):
     """Get the configuration scope section out of an spack.yaml"""
     key = spack.config.first_existing(yaml_data, env_schema_keys)
@@ -317,7 +305,15 @@ def clear(self):
 
     @property
     def name(self):
-        return os.path.basename(self.path)
+        """Human-readable representation of the environment.
+
+        This is the path for directory environments, and just the name
+        for named environments.
+        """
+        if self.path.startswith(env_path):
+            return os.path.basename(self.path)
+        else:
+            return self.path
 
     @property
     def manifest_path(self):
@@ -566,12 +562,7 @@ def uninstall(self, args):
 
     def status(self, stream, **kwargs):
         """List the specs in an environment."""
-        if self.path.startswith(env_path):
-            name = os.path.basename(self.path)
-        else:
-            name = self.path
-
-        tty.msg('In environment %s' % name)
+        tty.msg('In environment %s' % self.name)
 
         concretized = [(spec, self.specs_by_hash[h])
                        for spec, h in zip(self.concretized_user_specs,

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

@@ -43,7 +43,7 @@ def test_env_list():
     assert 'baz' in out
 
 
-def test_env_destroy():
+def test_env_destroy(capfd):
     env('create', 'foo')
     env('create', 'bar')
 
@@ -51,6 +51,13 @@ def test_env_destroy():
     assert 'foo' in out
     assert 'bar' in out
 
+    foo = ev.read('foo')
+    with foo:
+        with pytest.raises(spack.main.SpackCommandError):
+            with capfd.disabled():
+                env('destroy', '-y', 'foo')
+        assert 'foo' in env('list')
+
     env('destroy', '-y', 'foo')
     out = env('list')
     assert 'foo' not in out
@@ -62,6 +69,20 @@ def test_env_destroy():
     assert 'bar' not in out
 
 
+def test_destroy_env_dir(capfd):
+    env('create', '-d', 'foo')
+    assert os.path.isdir('foo')
+
+    foo = ev.Environment('foo')
+    with foo:
+        with pytest.raises(spack.main.SpackCommandError):
+            with capfd.disabled():
+                env('destroy', '-y', 'foo')
+
+    env('destroy', '-y', './foo')
+    assert not os.path.isdir('foo')
+
+
 def test_concretize():
     e = ev.create('test')
     e.add('mpileaks')