From bb35a98079b10674ed0cdf83872c863cf1978ed9 Mon Sep 17 00:00:00 2001 From: Greg Becker Date: Fri, 31 Jan 2025 19:41:18 -0600 Subject: [PATCH] env create: create copies of relative include files in envs created from manifest (#48689) Currently, environments created from manifest files with relative includes result in broken references to config files. This PR modifies `spack env create` to create local copies in the new environment of any local config files from relative paths in the environment manifest passed as an init file. This PR does not change the behavior if the include is an absolute path or if the include is from a relative path outside the environment directory, but it does warn about missing relative includes if they are inside the environment directory. Includes regression test and short blurb in docs. --- lib/spack/docs/environments.rst | 27 ++++++++--- lib/spack/spack/environment/environment.py | 26 +++++++++++ lib/spack/spack/test/cmd/env.py | 52 ++++++++++++++++++++++ 3 files changed, 98 insertions(+), 7 deletions(-) diff --git a/lib/spack/docs/environments.rst b/lib/spack/docs/environments.rst index 2647a297da7..6f8b1ac26e6 100644 --- a/lib/spack/docs/environments.rst +++ b/lib/spack/docs/environments.rst @@ -112,6 +112,19 @@ the original but may concretize differently in the presence of different explicit or default configuration settings (e.g., a different version of Spack or for a different user account). +Environments created from a manifest will copy any included configs +from relative paths inside the environment. Relative paths from +outside the environment will cause errors, and absolute paths will be +kept absolute. For example, if ``spack.yaml`` includes: + +.. code-block:: yaml + + spack: + include: [./config.yaml] + +then the created environment will have its own copy of the file +``config.yaml`` copied from the location in the original environment. + Create an environment from a ``spack.lock`` file using: .. code-block:: console @@ -160,7 +173,7 @@ accepts. If an environment already exists then spack will simply activate it and ignore the create-specific flags. .. code-block:: console - + $ spack env activate --create -p myenv # ... # [creates if myenv does not exist yet] @@ -424,8 +437,8 @@ Developing Packages in a Spack Environment The ``spack develop`` command allows one to develop Spack packages in an environment. It requires a spec containing a concrete version, and -will configure Spack to install the package from local source. -If a version is not provided from the command line interface then spack +will configure Spack to install the package from local source. +If a version is not provided from the command line interface then spack will automatically pick the highest version the package has defined. This means any infinity versions (``develop``, ``main``, ``stable``) will be preferred in this selection process. @@ -435,9 +448,9 @@ set, and Spack will ensure the package and its dependents are rebuilt any time the environment is installed if the package's local source code has been modified. Spack's native implementation to check for modifications is to check if ``mtime`` is newer than the installation. -A custom check can be created by overriding the ``detect_dev_src_change`` method -in your package class. This is particularly useful for projects using custom spack repo's -to drive development and want to optimize performance. +A custom check can be created by overriding the ``detect_dev_src_change`` method +in your package class. This is particularly useful for projects using custom spack repo's +to drive development and want to optimize performance. Spack ensures that all instances of a developed package in the environment are concretized to match the @@ -453,7 +466,7 @@ Further development on ``foo`` can be tested by re-installing the environment, and eventually committed and pushed to the upstream git repo. If the package being developed supports out-of-source builds then users can use the -``--build_directory`` flag to control the location and name of the build directory. +``--build_directory`` flag to control the location and name of the build directory. This is a shortcut to set the ``package_attributes:build_directory`` in the ``packages`` configuration (see :ref:`assigning-package-attributes`). The supplied location will become the build-directory for that package in all future builds. diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py index b32c7b3e5fb..92bdc6871fe 100644 --- a/lib/spack/spack/environment/environment.py +++ b/lib/spack/spack/environment/environment.py @@ -2634,6 +2634,32 @@ def _ensure_env_dir(): shutil.copy(envfile, target_manifest) + # Copy relative path includes that live inside the environment dir + try: + manifest = EnvironmentManifestFile(environment_dir) + except Exception: + # error handling for bad manifests is handled on other code paths + return + + includes = manifest[TOP_LEVEL_KEY].get("include", []) + for include in includes: + if os.path.isabs(include): + continue + + abspath = pathlib.Path(os.path.normpath(environment_dir / include)) + common_path = pathlib.Path(os.path.commonpath([environment_dir, abspath])) + if common_path != environment_dir: + tty.debug(f"Will not copy relative include from outside environment: {include}") + continue + + orig_abspath = os.path.normpath(envfile.parent / include) + if not os.path.exists(orig_abspath): + tty.warn(f"Included file does not exist; will not copy: '{include}'") + continue + + fs.touchp(abspath) + shutil.copy(orig_abspath, abspath) + class EnvironmentManifestFile(collections.abc.Mapping): """Manages the in-memory representation of a manifest file, and its synchronization diff --git a/lib/spack/spack/test/cmd/env.py b/lib/spack/spack/test/cmd/env.py index 661ba7a1995..9460b060207 100644 --- a/lib/spack/spack/test/cmd/env.py +++ b/lib/spack/spack/test/cmd/env.py @@ -1038,6 +1038,58 @@ def test_init_from_yaml(environment_from_manifest): assert not e2.specs_by_hash +def test_init_from_yaml_relative_includes(tmp_path): + files = [ + "relative_copied/packages.yaml", + "./relative_copied/compilers.yaml", + "repos.yaml", + "./config.yaml", + ] + + manifest = f""" +spack: + specs: [] + include: {files} +""" + + e1_path = tmp_path / "e1" + e1_manifest = e1_path / "spack.yaml" + fs.mkdirp(e1_path) + with open(e1_manifest, "w", encoding="utf-8") as f: + f.write(manifest) + + for f in files: + fs.touchp(e1_path / f) + + e2 = _env_create("test2", init_file=e1_manifest) + + for f in files: + assert os.path.exists(os.path.join(e2.path, f)) + + +def test_init_from_yaml_relative_includes_outside_env(tmp_path): + files = ["../outside_env_not_copied/repos.yaml"] + + manifest = f""" +spack: + specs: [] + include: {files} +""" + + # subdir to ensure parent of environment dir is not shared + e1_path = tmp_path / "e1_subdir" / "e1" + e1_manifest = e1_path / "spack.yaml" + fs.mkdirp(e1_path) + with open(e1_manifest, "w", encoding="utf-8") as f: + f.write(manifest) + + for f in files: + fs.touchp(e1_path / f) + + with pytest.raises(spack.config.ConfigFileError, match="Detected 1 missing include"): + _ = _env_create("test2", init_file=e1_manifest) + + def test_env_view_external_prefix(tmp_path, mutable_database, mock_packages): fake_prefix = tmp_path / "a-prefix" fake_bin = fake_prefix / "bin"