From e5cebb6b6ff487500028a87a98051fda13c0a077 Mon Sep 17 00:00:00 2001
From: Luisa Burini <luisa.burini@gmail.com>
Date: Tue, 31 Oct 2023 19:39:42 -0300
Subject: fix create/remove env with invalid spack.yaml (#39898)

* fix create/remove env with invalid spack.yaml
* fix isort error
* fix env ident unittests
* Fix pull request points
---
 lib/spack/spack/cmd/env.py                 | 49 +++++++++++++++++-------------
 lib/spack/spack/environment/environment.py | 19 +++++++-----
 lib/spack/spack/test/cmd/env.py            | 22 ++++++++++++--
 3 files changed, 60 insertions(+), 30 deletions(-)

diff --git a/lib/spack/spack/cmd/env.py b/lib/spack/spack/cmd/env.py
index cf5671aafa..4ca98dcdcc 100644
--- a/lib/spack/spack/cmd/env.py
+++ b/lib/spack/spack/cmd/env.py
@@ -380,28 +380,35 @@ def env_remove(args):
     and manifests embedded in repositories should be removed manually.
     """
     read_envs = []
+    bad_envs = []
     for env_name in args.rm_env:
-        env = ev.read(env_name)
-        read_envs.append(env)
-
-    if not args.yes_to_all:
-        answer = tty.get_yes_or_no(
-            "Really remove %s %s?"
-            % (
-                string.plural(len(args.rm_env), "environment", show_n=False),
-                string.comma_and(args.rm_env),
-            ),
-            default=False,
-        )
-        if not answer:
-            tty.die("Will not remove any environments")
-
-    for env in read_envs:
-        if env.active:
-            tty.die("Environment %s can't be removed while activated." % env.name)
-
-        env.destroy()
-        tty.msg("Successfully removed environment '%s'" % env.name)
+        try:
+            env = ev.read(env_name)
+            read_envs.append(env)
+        except spack.config.ConfigFormatError:
+            bad_envs.append(env_name)
+
+        if not args.yes_to_all:
+            environments = string.plural(len(args.rm_env), "environment", show_n=False)
+            envs = string.comma_and(args.rm_env)
+            answer = tty.get_yes_or_no(f"Really remove  {environments} {envs}?", default=False)
+            if not answer:
+                tty.die("Will not remove any environments")
+
+        for env in read_envs:
+            name = env.name
+            if env.active:
+                tty.die(f"Environment {name} can't be removed while activated.")
+            env.destroy()
+            tty.msg(f"Successfully removed environment {name}")
+
+        for bad_env_name in bad_envs:
+            shutil.rmtree(
+                spack.environment.environment.environment_dir_from_name(
+                    bad_env_name, exists_ok=True
+                )
+            )
+            tty.msg(f"Successfully removed environment '{bad_env_name}'")
 
 
 #
diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py
index cd2a5a7533..3fd75f3d70 100644
--- a/lib/spack/spack/environment/environment.py
+++ b/lib/spack/spack/environment/environment.py
@@ -330,16 +330,21 @@ def create_in_dir(
     if with_view is None and keep_relative:
         return Environment(manifest_dir)
 
-    manifest = EnvironmentManifestFile(manifest_dir)
+    try:
+        manifest = EnvironmentManifestFile(manifest_dir)
 
-    if with_view is not None:
-        manifest.set_default_view(with_view)
+        if with_view is not None:
+            manifest.set_default_view(with_view)
 
-    if not keep_relative and init_file is not None and str(init_file).endswith(manifest_name):
-        init_file = pathlib.Path(init_file)
-        manifest.absolutify_dev_paths(init_file.parent)
+        if not keep_relative and init_file is not None and str(init_file).endswith(manifest_name):
+            init_file = pathlib.Path(init_file)
+            manifest.absolutify_dev_paths(init_file.parent)
+
+        manifest.flush()
 
-    manifest.flush()
+    except spack.config.ConfigFormatError as e:
+        shutil.rmtree(manifest_dir)
+        raise e
 
     return Environment(manifest_dir)
 
diff --git a/lib/spack/spack/test/cmd/env.py b/lib/spack/spack/test/cmd/env.py
index 7d0eb37951..24657c30f9 100644
--- a/lib/spack/spack/test/cmd/env.py
+++ b/lib/spack/spack/test/cmd/env.py
@@ -991,8 +991,26 @@ spack:
     with tmpdir.as_cwd():
         with pytest.raises(spack.config.ConfigFormatError) as e:
             env("create", "test", "./spack.yaml")
-        assert "spack.yaml:2" in str(e)
-        assert "'spacks' was unexpected" in str(e)
+            assert "'spacks' was unexpected" in str(e)
+
+    assert "test" not in env("list")
+
+
+def test_bad_env_yaml_format_remove():
+    badenv = "badenv"
+    env("create", badenv)
+    tmpdir = spack.environment.environment.environment_dir_from_name(badenv, exists_ok=True)
+    filename = os.path.join(tmpdir, "spack.yaml")
+    with open(filename, "w") as f:
+        f.write(
+            """\
+    - mpileaks
+"""
+        )
+
+    assert badenv in env("list")
+    env("remove", "-y", badenv)
+    assert badenv not in env("list")
 
 
 def test_env_loads(install_mockery, mock_fetch):
-- 
cgit v1.2.3-60-g2f50