Relax environment manifest filename requirements and lockfile identification criteria (#37413)

* Relax filename requirements and lockfile identification criteria

* Tests

* Update function docs and help text

* Update function documentation

* Update Sphinx documentation

* Adjustments per https://github.com/spack/spack/pull/37413#pullrequestreview-1413540132

* Further tweaks per https://github.com/spack/spack/pull/37413#pullrequestreview-1413971254

* Doc fixes per https://github.com/spack/spack/pull/37413#issuecomment-1535976068

lib/spack/docs/configuration.rst

@@ -20,8 +20,9 @@ case you want to skip directly to specific docs:
 * :ref:`packages.yaml <build-settings>`
 * :ref:`repos.yaml <repositories>`
 
-You can also add any of these as inline configuration in ``spack.yaml``
+You can also add any of these as inline configuration in the YAML
-in an :ref:`environment <environment-configuration>`.
+manifest file (``spack.yaml``) describing an :ref:`environment
+<environment-configuration>`.
 
 -----------
 YAML Format

lib/spack/docs/environments.rst

@@ -94,9 +94,9 @@ an Environment, the ``.spack-env`` directory also contains:
   * ``logs/``: A directory containing the build logs for the packages
     in this Environment.
 
-Spack Environments can also be created from either a ``spack.yaml``
+Spack Environments can also be created from either a manifest file
-manifest or a ``spack.lock`` lockfile. To create an Environment from a
+(usually but not necessarily named, ``spack.yaml``) or a lockfile.
-``spack.yaml`` manifest:
+To create an Environment from a manifest:
 
 .. code-block:: console
 
@@ -174,7 +174,7 @@ Anonymous specs can be created in place using the command:
 
    $ spack env create -d .
 
-In this case Spack simply creates a spack.yaml file in the requested
+In this case Spack simply creates a ``spack.yaml`` file in the requested
 directory.
 
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

lib/spack/docs/mirrors.rst

@@ -163,7 +163,7 @@ your site.
 Mirror environment
 ^^^^^^^^^^^^^^^^^^
 
-To create a mirror of all packages required by a concerte environment, activate the environment and call ``spack mirror create -a``.
+To create a mirror of all packages required by a concrete environment, activate the environment and call ``spack mirror create -a``.
 This is especially useful to create a mirror of an environment concretized on another machine.
 
 .. code-block:: console

lib/spack/spack/cmd/env.py

@@ -283,7 +283,7 @@ def env_create_setup_parser(subparser):
         "envfile",
         nargs="?",
         default=None,
-        help="optional init file; can be spack.yaml or spack.lock",
+        help="either a lockfile (must end with '.json' or '.lock') or a manifest file.",
     )
 
 
@@ -292,7 +292,7 @@ def env_create(args):
         # Expand relative paths provided on the command line to the current working directory
         # This way we interpret `spack env create --with-view ./view --dir ./env` as
         # a view in $PWD/view, not $PWD/env/view. This is different from specifying a relative
-        # path in spack.yaml, which is resolved relative to the environment file.
+        # path in the manifest, which is resolved relative to the manifest file's location.
         with_view = os.path.abspath(args.with_view)
     elif args.without_view:
         with_view = False
@@ -317,7 +317,7 @@ def _env_create(name_or_path, *, init_file=None, dir=False, with_view=None, keep
     Arguments:
         name_or_path (str): name of the environment to create, or path to it
         init_file (str or file): optional initialization file -- can be
-            spack.yaml or spack.lock
+            a JSON lockfile (*.lock, *.json) or YAML manifest file
         dir (bool): if True, create an environment in a directory instead
             of a named environment
         keep_relative (bool): if True, develop paths are copied verbatim into
@@ -355,8 +355,7 @@ def env_remove(args):
     """Remove a *named* environment.
 
     This removes an environment managed by Spack. Directory environments
-    and `spack.yaml` files embedded in repositories should be removed
+    and manifests embedded in repositories should be removed manually.
-    manually.
     """
     read_envs = []
     for env_name in args.rm_env:
@@ -568,7 +567,7 @@ def env_revert(args):
     # Check that both the spack.yaml and the backup exist, the inform user
     # on what is going to happen and ask for confirmation
     if not os.path.exists(manifest_file):
-        msg = "cannot fine the manifest file of the environment [file={0}]"
+        msg = "cannot find the manifest file of the environment [file={0}]"
         tty.die(msg.format(manifest_file))
     if not os.path.exists(backup_file):
         msg = "cannot find the old manifest file to be restored [file={0}]"

lib/spack/spack/environment/environment.py

@@ -281,9 +281,12 @@ def create(
     A managed environment is created in a root directory managed by this Spack instance, so that
     Spack can keep track of them.
 
+    Files with suffix ``.json`` or ``.lock`` are considered lockfiles. Files with any other name
+    are considered manifest files.
+
     Args:
         name: name of the managed environment
-        init_file: either a "spack.yaml" or a "spack.lock" file or None
+        init_file: either a lockfile, a manifest file, or None
         with_view: whether a view should be maintained for the environment. If the value is a
             string, it specifies the path to the view
         keep_relative: if True, develop paths are copied verbatim into the new environment file,
@@ -303,9 +306,12 @@ def create_in_dir(
 ) -> "Environment":
     """Create an environment in the directory passed as input and returns it.
 
+    Files with suffix ``.json`` or ``.lock`` are considered lockfiles. Files with any other name
+    are considered manifest files.
+
     Args:
         manifest_dir: directory where to create the environment.
-        init_file: either a "spack.yaml" or a "spack.lock" file or None
+        init_file: either a lockfile, a manifest file, or None
         with_view: whether a view should be maintained for the environment. If the value is a
             string, it specifies the path to the view
         keep_relative: if True, develop paths are copied verbatim into the new environment file,
@@ -2496,7 +2502,8 @@ def initialize_environment_dir(
 ) -> None:
     """Initialize an environment directory starting from an envfile.
 
-    The envfile can be either a "spack.yaml" manifest file, or a "spack.lock" file.
+    Files with suffix .json or .lock are considered lockfiles. Files with any other name
+    are considered manifest files.
 
     Args:
         environment_dir: directory where the environment should be placed
@@ -2533,21 +2540,18 @@ def _ensure_env_dir():
         msg = f"cannot initialize environment, {envfile} is not a valid file"
         raise SpackEnvironmentError(msg)
 
-    if not str(envfile).endswith(manifest_name) and not str(envfile).endswith(lockfile_name):
-        msg = (
-            f"cannot initialize environment from '{envfile}', either a '{manifest_name}'"
-            f" or a '{lockfile_name}' file is needed"
-        )
-        raise SpackEnvironmentError(msg)
-
     _ensure_env_dir()
 
     # When we have a lockfile we should copy that and produce a consistent default manifest
-    if str(envfile).endswith(lockfile_name):
+    if str(envfile).endswith(".lock") or str(envfile).endswith(".json"):
         shutil.copy(envfile, target_lockfile)
         # This constructor writes a spack.yaml which is consistent with the root
         # specs in the spack.lock
-        EnvironmentManifestFile.from_lockfile(environment_dir)
+        try:
+            EnvironmentManifestFile.from_lockfile(environment_dir)
+        except Exception as e:
+            msg = f"cannot initialize environment, '{environment_dir}' from lockfile"
+            raise SpackEnvironmentError(msg) from e
         return
 
     shutil.copy(envfile, target_manifest)

lib/spack/spack/test/env.py

@@ -3,6 +3,7 @@
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 """Test environment internals without CLI"""
+import filecmp
 import os
 import pickle
 import sys
@@ -316,20 +317,13 @@ def test_cannot_initiliaze_if_dirname_exists_as_a_file(tmp_path):
         ev.create_in_dir(dir_name)
 
 
-def test_cannot_initiliaze_if_init_file_does_not_exist(tmp_path):
+def test_cannot_initialize_if_init_file_does_not_exist(tmp_path):
     """Tests that initializing an environment passing a non-existing init file raises an error."""
     init_file = tmp_path / ev.manifest_name
     with pytest.raises(ev.SpackEnvironmentError, match="cannot initialize"):
         ev.create_in_dir(tmp_path, init_file=init_file)
 
 
-def test_cannot_initialize_from_random_file(tmp_path):
-    init_file = tmp_path / "foo.txt"
-    init_file.touch()
-    with pytest.raises(ev.SpackEnvironmentError, match="cannot initialize"):
-        ev.create_in_dir(tmp_path, init_file=init_file)
-
-
 def test_environment_pickle(tmp_path):
     env1 = ev.create_in_dir(tmp_path)
     obj = pickle.dumps(env1)
@@ -419,3 +413,64 @@ def test_preserving_comments_when_adding_specs(
 
     content = spack_yaml.read_text()
     assert content == expected_yaml
+
+
+@pytest.mark.parametrize("filename", [ev.lockfile_name, "as9582g54.lock", "m3ia54s.json"])
+@pytest.mark.regression("37410")
+def test_initialize_from_lockfile(tmp_path, filename):
+    """Some users have workflows where they store multiple lockfiles in the
+    same directory, and pick one of them to create an environment depending
+    on external parameters e.g. while running CI jobs. This test ensures that
+    Spack can create environments from lockfiles that are not necessarily named
+    'spack.lock' and can thus coexist in the same directory.
+    """
+
+    init_file = tmp_path / filename
+    env_dir = tmp_path / "env_dir"
+    init_file.write_text('{ "roots": [] }\n')
+    ev.initialize_environment_dir(env_dir, init_file)
+
+    assert os.path.exists(env_dir / ev.lockfile_name)
+    assert filecmp.cmp(env_dir / ev.lockfile_name, init_file, shallow=False)
+
+
+def test_cannot_initialize_from_bad_lockfile(tmp_path):
+    """Test that we fail on an incorrectly constructed lockfile"""
+
+    init_file = tmp_path / ev.lockfile_name
+    env_dir = tmp_path / "env_dir"
+
+    init_file.write_text("Not a legal JSON file\n")
+
+    with pytest.raises(ev.SpackEnvironmentError, match="from lockfile"):
+        ev.initialize_environment_dir(env_dir, init_file)
+
+
+@pytest.mark.parametrize("filename", ["random.txt", "random.yaml", ev.manifest_name])
+@pytest.mark.regression("37410")
+def test_initialize_from_random_file_as_manifest(tmp_path, filename):
+    """Some users have workflows where they store multiple lockfiles in the
+    same directory, and pick one of them to create an environment depending
+    on external parameters e.g. while running CI jobs. This test ensures that
+    Spack can create environments from manifest that are not necessarily named
+    'spack.yaml' and can thus coexist in the same directory.
+    """
+
+    init_file = tmp_path / filename
+    env_dir = tmp_path / "env_dir"
+
+    init_file.write_text(
+        """\
+spack:
+  view: true
+  concretizer:
+    unify: true
+  specs: []
+"""
+    )
+
+    ev.create_in_dir(env_dir, init_file)
+
+    assert not os.path.exists(env_dir / ev.lockfile_name)
+    assert os.path.exists(env_dir / ev.manifest_name)
+    assert filecmp.cmp(env_dir / ev.manifest_name, init_file, shallow=False)