From 1c1d439a01924886a1b52d7f1b789198c3bed8fa Mon Sep 17 00:00:00 2001 From: Tamara Dahlgren <35777542+tldahlgren@users.noreply.github.com> Date: Fri, 13 Dec 2024 09:44:08 -0800 Subject: [PATCH] Circular import fix: spack.config -> spack.environment (#48057) Fix by moving setup to spack.main --- lib/spack/spack/config.py | 24 ------- lib/spack/spack/environment/__init__.py | 2 + lib/spack/spack/environment/environment.py | 23 +++++++ lib/spack/spack/main.py | 32 +++++++++- lib/spack/spack/test/config.py | 67 -------------------- lib/spack/spack/test/main.py | 74 +++++++++++++++++++++- 6 files changed, 126 insertions(+), 96 deletions(-) diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py index 471638e9ea2..9bf7675ac20 100644 --- a/lib/spack/spack/config.py +++ b/lib/spack/spack/config.py @@ -790,30 +790,6 @@ def config_paths_from_entry_points() -> List[Tuple[str, str]]: return config_paths -def _add_command_line_scopes(cfg: Configuration, command_line_scopes: List[str]) -> None: - """Add additional scopes from the --config-scope argument, either envs or dirs.""" - import spack.environment.environment as env # circular import - - for i, path in enumerate(command_line_scopes): - name = f"cmd_scope_{i}" - - if env.exists(path): # managed environment - manifest = env.EnvironmentManifestFile(env.root(path)) - elif env.is_env_dir(path): # anonymous environment - manifest = env.EnvironmentManifestFile(path) - elif os.path.isdir(path): # directory with config files - cfg.push_scope(DirectoryConfigScope(name, path, writable=False)) - _add_platform_scope(cfg, name, path, writable=False) - continue - else: - raise spack.error.ConfigError(f"Invalid configuration scope: {path}") - - for scope in manifest.env_config_scopes: - scope.name = f"{name}:{scope.name}" - scope.writable = False - cfg.push_scope(scope) - - def create() -> Configuration: """Singleton Configuration instance. diff --git a/lib/spack/spack/environment/__init__.py b/lib/spack/spack/environment/__init__.py index 62445e04379..24d23e5ab51 100644 --- a/lib/spack/spack/environment/__init__.py +++ b/lib/spack/spack/environment/__init__.py @@ -482,6 +482,7 @@ display_specs, environment_dir_from_name, environment_from_name_or_dir, + environment_path_scopes, exists, initialize_environment_dir, installed_specs, @@ -518,6 +519,7 @@ "display_specs", "environment_dir_from_name", "environment_from_name_or_dir", + "environment_path_scopes", "exists", "initialize_environment_dir", "installed_specs", diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py index a5c7314ed91..75d2e3d1f30 100644 --- a/lib/spack/spack/environment/environment.py +++ b/lib/spack/spack/environment/environment.py @@ -3058,6 +3058,29 @@ def use_config(self): self.deactivate_config_scope() +def environment_path_scopes(name: str, path: str) -> Optional[List[spack.config.ConfigScope]]: + """Retrieve the suitably named environment path scopes + + Arguments: + name: configuration scope name + path: path to configuration file(s) + + Returns: list of environment scopes, if any, or None + """ + if exists(path): # managed environment + manifest = EnvironmentManifestFile(root(path)) + elif is_env_dir(path): # anonymous environment + manifest = EnvironmentManifestFile(path) + else: + return None + + for scope in manifest.env_config_scopes: + scope.name = f"{name}:{scope.name}" + scope.writable = False + + return manifest.env_config_scopes + + class SpackEnvironmentError(spack.error.SpackError): """Superclass for all errors to do with Spack environments.""" diff --git a/lib/spack/spack/main.py b/lib/spack/spack/main.py index 7cab47d77f7..8756a019402 100644 --- a/lib/spack/spack/main.py +++ b/lib/spack/spack/main.py @@ -48,7 +48,6 @@ import spack.util.debug import spack.util.environment import spack.util.lock -from spack.error import SpackError #: names of profile statistics stat_names = pstats.Stats.sort_arg_dict_default @@ -858,6 +857,33 @@ def resolve_alias(cmd_name: str, cmd: List[str]) -> Tuple[str, List[str]]: return cmd_name, cmd +def add_command_line_scopes( + cfg: spack.config.Configuration, command_line_scopes: List[str] +) -> None: + """Add additional scopes from the --config-scope argument, either envs or dirs. + + Args: + cfg: configuration instance + command_line_scopes: list of configuration scope paths + + Raises: + spack.error.ConfigError: if the path is an invalid configuration scope + """ + for i, path in enumerate(command_line_scopes): + name = f"cmd_scope_{i}" + scopes = ev.environment_path_scopes(name, path) + if scopes is None: + if os.path.isdir(path): # directory with config files + cfg.push_scope(spack.config.DirectoryConfigScope(name, path, writable=False)) + spack.config._add_platform_scope(cfg, name, path, writable=False) + continue + else: + raise spack.error.ConfigError(f"Invalid configuration scope: {path}") + + for scope in scopes: + cfg.push_scope(scope) + + def _main(argv=None): """Logic for the main entry point for the Spack command. @@ -926,7 +952,7 @@ def _main(argv=None): # Push scopes from the command line last if args.config_scopes: - spack.config._add_command_line_scopes(spack.config.CONFIG, args.config_scopes) + add_command_line_scopes(spack.config.CONFIG, args.config_scopes) spack.config.CONFIG.push_scope(spack.config.InternalConfigScope("command_line")) setup_main_options(args) @@ -1012,7 +1038,7 @@ def main(argv=None): try: return _main(argv) - except SpackError as e: + except spack.error.SpackError as e: tty.debug(e) e.die() # gracefully die on any SpackErrors diff --git a/lib/spack/spack/test/config.py b/lib/spack/spack/test/config.py index 79149e2e038..f0d4b0b7b0b 100644 --- a/lib/spack/spack/test/config.py +++ b/lib/spack/spack/test/config.py @@ -848,73 +848,6 @@ def test_bad_config_section(mock_low_high_config): spack.config.get("foobar") -def test_bad_command_line_scopes(tmp_path, config): - cfg = spack.config.Configuration() - file_path = tmp_path / "file_instead_of_dir" - non_existing_path = tmp_path / "non_existing_dir" - - file_path.write_text("") - - with pytest.raises(spack.error.ConfigError): - spack.config._add_command_line_scopes(cfg, [str(file_path)]) - - with pytest.raises(spack.error.ConfigError): - spack.config._add_command_line_scopes(cfg, [str(non_existing_path)]) - - -def test_add_command_line_scopes(tmpdir, mutable_config): - config_yaml = str(tmpdir.join("config.yaml")) - with open(config_yaml, "w", encoding="utf-8") as f: - f.write( - """\ -config: - verify_ssl: False - dirty: False -""" - ) - - spack.config._add_command_line_scopes(mutable_config, [str(tmpdir)]) - assert mutable_config.get("config:verify_ssl") is False - assert mutable_config.get("config:dirty") is False - - -def test_add_command_line_scope_env(tmp_path, mutable_mock_env_path): - """Test whether --config-scope works, either by name or path.""" - managed_env = ev.create("example").manifest_path - - with open(managed_env, "w", encoding="utf-8") as f: - f.write( - """\ -spack: - config: - install_tree: - root: /tmp/first -""" - ) - - with open(tmp_path / "spack.yaml", "w", encoding="utf-8") as f: - f.write( - """\ -spack: - config: - install_tree: - root: /tmp/second -""" - ) - - config = spack.config.Configuration() - spack.config._add_command_line_scopes(config, ["example", str(tmp_path)]) - assert len(config.scopes) == 2 - assert config.get("config:install_tree:root") == "/tmp/second" - - config = spack.config.Configuration() - spack.config._add_command_line_scopes(config, [str(tmp_path), "example"]) - assert len(config.scopes) == 2 - assert config.get("config:install_tree:root") == "/tmp/first" - - assert ev.active_environment() is None # shouldn't cause an environment to be activated - - def test_nested_override(): """Ensure proper scope naming of nested overrides.""" base_name = spack.config._OVERRIDES_BASE_NAME diff --git a/lib/spack/spack/test/main.py b/lib/spack/spack/test/main.py index f4ef09986eb..af6a728291c 100644 --- a/lib/spack/spack/test/main.py +++ b/lib/spack/spack/test/main.py @@ -9,10 +9,13 @@ import llnl.util.filesystem as fs import spack +import spack.config +import spack.environment as ev +import spack.error +import spack.main import spack.paths import spack.util.executable as exe import spack.util.git -from spack.main import main pytestmark = pytest.mark.not_on_windows( "Test functionality supported but tests are failing on Win" @@ -81,7 +84,7 @@ def test_main_calls_get_version(tmpdir, capsys, working_env, monkeypatch): monkeypatch.setattr(spack.util.git, "git", lambda: None) # make sure we get a bare version (without commit) when this happens - main(["-V"]) + spack.main.main(["-V"]) out, err = capsys.readouterr() assert spack.spack_version == out.strip() @@ -98,3 +101,70 @@ def test_get_version_bad_git(tmpdir, working_env, monkeypatch): monkeypatch.setattr(spack.util.git, "git", lambda: exe.which(bad_git)) assert spack.spack_version == spack.get_version() + + +def test_bad_command_line_scopes(tmp_path, config): + cfg = spack.config.Configuration() + file_path = tmp_path / "file_instead_of_dir" + non_existing_path = tmp_path / "non_existing_dir" + + file_path.write_text("") + + with pytest.raises(spack.error.ConfigError): + spack.main.add_command_line_scopes(cfg, [str(file_path)]) + + with pytest.raises(spack.error.ConfigError): + spack.main.add_command_line_scopes(cfg, [str(non_existing_path)]) + + +def test_add_command_line_scopes(tmpdir, mutable_config): + config_yaml = str(tmpdir.join("config.yaml")) + with open(config_yaml, "w") as f: + f.write( + """\ +config: + verify_ssl: False + dirty: False +""" + ) + + spack.main.add_command_line_scopes(mutable_config, [str(tmpdir)]) + assert mutable_config.get("config:verify_ssl") is False + assert mutable_config.get("config:dirty") is False + + +def test_add_command_line_scope_env(tmp_path, mutable_mock_env_path): + """Test whether --config-scope works, either by name or path.""" + managed_env = ev.create("example").manifest_path + + with open(managed_env, "w") as f: + f.write( + """\ +spack: + config: + install_tree: + root: /tmp/first +""" + ) + + with open(tmp_path / "spack.yaml", "w") as f: + f.write( + """\ +spack: + config: + install_tree: + root: /tmp/second +""" + ) + + config = spack.config.Configuration() + spack.main.add_command_line_scopes(config, ["example", str(tmp_path)]) + assert len(config.scopes) == 2 + assert config.get("config:install_tree:root") == "/tmp/second" + + config = spack.config.Configuration() + spack.main.add_command_line_scopes(config, [str(tmp_path), "example"]) + assert len(config.scopes) == 2 + assert config.get("config:install_tree:root") == "/tmp/first" + + assert ev.active_environment() is None # shouldn't cause an environment to be activated