From 2f2673e89ba8647fa3b08a761e1247528515c685 Mon Sep 17 00:00:00 2001 From: Douglas Jacobsen Date: Wed, 26 Aug 2026 08:22:51 -0600 Subject: [PATCH 1/3] Allow explicit commits in utility requirements This commit adds support to the requires_utility directive for objects to define explicit versions / commits they need for a utility instead of an allowable range. Signed-off-by: Douglas Jacobsen --- lib/ramble/ramble/cmd/workspace.py | 12 +- .../application-base/base_class.py | 127 +++++++++--- .../base_classes/utility-base/base_class.py | 183 +++++++++++++++--- 3 files changed, 263 insertions(+), 59 deletions(-) diff --git a/lib/ramble/ramble/cmd/workspace.py b/lib/ramble/ramble/cmd/workspace.py index 8bd36e1f3f..a5e4b77484 100644 --- a/lib/ramble/ramble/cmd/workspace.py +++ b/lib/ramble/ramble/cmd/workspace.py @@ -850,6 +850,15 @@ def workspace_info_setup_parser(subparser): def workspace_info(args): + def _hashable_val(val): + if isinstance(val, (list, tuple)): + return tuple(_hashable_val(x) for x in val) + elif isinstance(val, dict): + return tuple( + (k, _hashable_val(v)) for k, v in sorted(val.items(), key=lambda x: str(x[0])) + ) + return val + ws = ramble.cmd.require_active_workspace("workspace info", args.dry_run) args.where = ramble.filters.resolve_and_apply_filter_groups(args, args.where) @@ -1128,9 +1137,10 @@ def workspace_info(args): for utility_name, utility_conf in utilities.items(): if utility_name not in all_utilities: all_utilities[utility_name] = set() + conf_tuple = tuple( sorted( - (k, tuple(v) if isinstance(v, list) else v) + (k, _hashable_val(v)) for k, v in utility_conf.items() if k != "when" ) diff --git a/var/ramble/repos/builtin/base_classes/application-base/base_class.py b/var/ramble/repos/builtin/base_classes/application-base/base_class.py index 1a8f4e13d4..d1defe5e54 100644 --- a/var/ramble/repos/builtin/base_classes/application-base/base_class.py +++ b/var/ramble/repos/builtin/base_classes/application-base/base_class.py @@ -2621,10 +2621,33 @@ def _bootstrap_utilities(self, workspace, app_inst=None): fetch_kwargs[var_name] = var_info.default - if ext_dep_name in ws_ext_deps: - fetch_kwargs.update(ws_ext_deps[ext_dep_name]) + conf_to_merge = ( + ws_ext_deps[ext_dep_name] + if ext_dep_name in ws_ext_deps + else ext_dep_conf + ) or {} + + if hasattr(ext_dep_inst, "map_fetch_kwargs"): + mapped_conf = ext_dep_inst.map_fetch_kwargs( + conf_to_merge + ) else: - fetch_kwargs.update(ext_dep_conf) + mapped_conf = conf_to_merge + + explicit_version_requested = any( + k in mapped_conf + for k in [ + "commit", + "hash", + "sha", + "version", + "tag", + "branch", + "revision", + ] + ) + + fetch_kwargs.update(conf_to_merge) if hasattr(ext_dep_inst, "map_fetch_kwargs"): fetch_kwargs = ext_dep_inst.map_fetch_kwargs( @@ -2637,16 +2660,6 @@ def _bootstrap_utilities(self, workspace, app_inst=None): min_version = fetch_kwargs.pop("min_version", None) max_version = fetch_kwargs.pop("max_version", None) - origin_name = obj.name - origin_type = getattr(obj, "origin_type", "object") - - ext_dep_versions[ext_dep_name] = { - "min_version": min_version, - "max_version": max_version, - "origin_name": origin_name, - "origin_type": origin_type, - } - for k, v in fetch_kwargs.items(): if isinstance(v, str): fetch_kwargs[k] = self.expander.expand_var(v) @@ -2659,6 +2672,27 @@ def _bootstrap_utilities(self, workspace, app_inst=None): or fetch_kwargs.get("revision") or "latest" ) + + origin_name = obj.name + origin_type = getattr(obj, "origin_type", "object") + + ext_dep_versions[ext_dep_name] = { + "min_version": min_version, + "max_version": max_version, + "exact_version": ( + version_str + if explicit_version_requested + else None + ), + "origin_name": origin_name, + "origin_type": origin_type, + "explicit_version": explicit_version_requested, + } + + if getattr(obj, "variables", None) is None: + obj.variables = {} + for k, v in fetch_kwargs.items(): + obj.variables[f"utility::{ext_dep_name}::{k}"] = v ext_dep_dir = os.path.join( workspace.shared_dir, "bootstrapped_utilities", @@ -2675,8 +2709,6 @@ def _bootstrap_utilities(self, workspace, app_inst=None): sorted_kwargs_str, ) - if not hasattr(obj, "variables"): - obj.variables = {} obj.variables[f"utility::{ext_dep_name}::path"] = ( ext_dep_paths[ext_dep_name] ) @@ -2701,13 +2733,27 @@ def _bootstrap_utilities(self, workspace, app_inst=None): allow_external = bool(allow_external) if allow_external: - if hasattr( - ext_dep_inst, "is_available" - ) and ext_dep_inst.is_available( - workspace, - min_version=min_version, - max_version=max_version, - ): + exact_version_to_check = ( + version_str + if explicit_version_requested + else None + ) + is_avail = False + if hasattr(ext_dep_inst, "is_available"): + try: + is_avail = ext_dep_inst.is_available( + workspace, + min_version=min_version, + max_version=max_version, + exact_version=exact_version_to_check, + ) + except TypeError: + is_avail = ext_dep_inst.is_available( + workspace, + min_version=min_version, + max_version=max_version, + ) + if is_avail: logger.debug( f"External dependency {ext_dep_name} is already available in the environment, skipping fetch." ) @@ -2827,7 +2873,7 @@ def _bootstrap_utilities(self, workspace, app_inst=None): ) ) - if not hasattr(obj, "variables"): + if getattr(obj, "variables", None) is None: obj.variables = {} if source_scripts_commands: obj.variables["source_scripts_command"] = "\n".join( @@ -2865,20 +2911,37 @@ def _bootstrap_utilities(self, workspace, app_inst=None): versions = ext_dep_versions.get(ext_dep_name, {}) min_v = versions.get("min_version") max_v = versions.get("max_version") + exact_v = versions.get("exact_version") origin_name = versions.get("origin_name") origin_type = versions.get("origin_type") if not workspace.dry_run: - if not ext_dep_inst.validate_versions( - min_version=min_v, - max_version=max_v, - env=exp_env, - origin_name=origin_name, - origin_type=origin_type, - ): - logger.die( - f"Version validation failed for {ext_dep_name} after bootstrap:\n{ext_dep_inst.availability_error}" + try: + validated = ext_dep_inst.validate_versions( + min_version=min_v, + max_version=max_v, + exact_version=exact_v, + env=exp_env, + origin_name=origin_name, + origin_type=origin_type, ) + except TypeError: + validated = ext_dep_inst.validate_versions( + min_version=min_v, + max_version=max_v, + env=exp_env, + origin_name=origin_name, + origin_type=origin_type, + ) + if not validated: + if versions.get("explicit_version"): + logger.warn( + f"Version validation failed for {ext_dep_name} after bootstrap, but proceeding due to explicit version request:\n{ext_dep_inst.availability_error}" + ) + else: + logger.die( + f"Version validation failed for {ext_dep_name} after bootstrap:\n{ext_dep_inst.availability_error}" + ) if hasattr(obj, "bootstrap_utility"): obj.bootstrap_utility(workspace, ext_dep_paths) diff --git a/var/ramble/repos/builtin/base_classes/utility-base/base_class.py b/var/ramble/repos/builtin/base_classes/utility-base/base_class.py index 3eab428ba2..1cfc629cb1 100644 --- a/var/ramble/repos/builtin/base_classes/utility-base/base_class.py +++ b/var/ramble/repos/builtin/base_classes/utility-base/base_class.py @@ -60,13 +60,86 @@ def __init__(self, file_path): description="Name of external dependency for an experiment", ) + def _check_exact_match_via_vcs(self, exec_path, exact_version): + """Check if the provided executable matches the exact version via VCS history.""" + import os + import shutil + import subprocess + + if not exec_path or not exact_version: + return False + + # Resolve symlinks to find the actual repository directory + real_exec_path = os.path.realpath(exec_path) + exec_dir = os.path.dirname(real_exec_path) + if not exec_dir: + return False + + # Git check + if shutil.which("git"): + try: + is_git = subprocess.run( + [ + "git", + "-C", + exec_dir, + "rev-parse", + "--is-inside-work-tree", + ], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + universal_newlines=True, + check=False, + ) + if is_git.returncode == 0 and is_git.stdout.strip() == "true": + head_hash_res = subprocess.run( + ["git", "-C", exec_dir, "rev-parse", "HEAD"], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + universal_newlines=True, + check=False, + ) + exact_hash_res = subprocess.run( + [ + "git", + "-C", + exec_dir, + "rev-parse", + f"{exact_version}^{{commit}}", + ], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + universal_newlines=True, + check=False, + ) + if ( + head_hash_res.returncode == 0 + and exact_hash_res.returncode == 0 + ): + head_hash = head_hash_res.stdout.strip() + exact_hash = exact_hash_res.stdout.strip() + if ( + head_hash + and exact_hash + and head_hash == exact_hash + ): + return True + except Exception: + pass + + # Future VCS checks can be added here (e.g., hg, svn) + + return False + def validate_versions( self, min_version=None, max_version=None, + exact_version=None, env=None, origin_name=None, origin_type=None, + path=None, ): """Check if the provided executables are available and satisfy version constraints. Uses the provided environment or the system environment if None. @@ -82,7 +155,11 @@ def validate_versions( # When using a custom environment, we need to extract its PATH for shutil.which # Default to the current process PATH if not in the custom environment - search_path = check_env.get("PATH", os.environ.get("PATH", "")) + search_path = ( + path + if path is not None + else check_env.get("PATH", os.environ.get("PATH", "")) + ) # If the utility provides executables, check if they are in PATH if hasattr(self, "provided_executables") and self.provided_executables: @@ -91,7 +168,8 @@ def validate_versions( # we just check all provided executables. In the future this could be conditionally checked. for exec_info in exec_list: exec_name = exec_info["executable"] - if not shutil.which(exec_name, path=search_path): + exec_path = shutil.which(exec_name, path=search_path) + if not exec_path: self.availability_error = ( f"Executable '{exec_name}' not found in PATH." ) @@ -100,10 +178,28 @@ def validate_versions( version_cmd = exec_info.get("version_cmd") version_regex = exec_info.get("version_regex") + origin_str = ( + f" (required by {origin_type} '{origin_name}')" + if origin_name and origin_type + else "" + ) + + # Check exact version via VCS if available + exact_match_via_vcs = self._check_exact_match_via_vcs( + exec_path, exact_version + ) + + if ( + exact_match_via_vcs + and not min_version + and not max_version + ): + continue + if ( version_cmd and version_regex - and (min_version or max_version) + and (min_version or max_version or exact_version) ): try: import shlex @@ -112,8 +208,9 @@ def validate_versions( result = subprocess.run( shlex.split(version_cmd), env=check_env, - capture_output=True, - text=True, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + universal_newlines=True, check=True, ) output = result.stdout + result.stderr @@ -121,32 +218,56 @@ def validate_versions( # Extract the version using the regex match = re.search(version_regex, output) if not match: - self.availability_error = f"Could not determine version for '{exec_name}' using regex '{version_regex}'." - return False - - current_version = match.group(1) - - origin_str = ( - f" (required by {origin_type} '{origin_name}')" - if origin_name and origin_type - else "" - ) + if exact_version and exact_match_via_vcs: + # Git confirmed it, so lack of regex match is fine + current_version = None + else: + self.availability_error = f"Could not determine version for '{exec_name}' using regex '{version_regex}'." + return False + else: + current_version = match.group(1) # Compare versions - if min_version and Version( - current_version - ) < Version(min_version): + if ( + min_version + and current_version + and Version(current_version) + < Version(min_version) + ): self.availability_error = f"Version {current_version} for '{exec_name}' is less than required minimum {min_version}{origin_str}." return False - if max_version and Version( - current_version - ) > Version(max_version): + if ( + max_version + and current_version + and Version(current_version) + > Version(max_version) + ): self.availability_error = f"Version {current_version} for '{exec_name}' is greater than required maximum {max_version}{origin_str}." return False + if exact_version and not exact_match_via_vcs: + exact_version_str = str(exact_version) + if not current_version or ( + current_version != exact_version_str + and not re.search( + r"(? Date: Fri, 28 Aug 2026 11:04:45 -0600 Subject: [PATCH 2/3] Add unit tests and fix utility path validation Signed-off-by: Douglas Jacobsen --- .../ramble/test/end_to_end/setup_analyze.py | 22 +- .../ramble/test/test_base_classes_extra.py | 177 +++++++ lib/ramble/ramble/test/test_utility_base.py | 471 ++++++++++++++++++ .../base_classes/utility-base/base_class.py | 4 +- 4 files changed, 660 insertions(+), 14 deletions(-) create mode 100644 lib/ramble/ramble/test/test_utility_base.py diff --git a/lib/ramble/ramble/test/end_to_end/setup_analyze.py b/lib/ramble/ramble/test/end_to_end/setup_analyze.py index 2b3c294e1f..c155dd4cdd 100644 --- a/lib/ramble/ramble/test/end_to_end/setup_analyze.py +++ b/lib/ramble/ramble/test/end_to_end/setup_analyze.py @@ -35,19 +35,17 @@ def test_setup_analyze(test_case_path, workspace_name): """test_setup_analyze tests ramble objects that contain a `test_cases` directory. - Specifically, it assumes the following structure for the `test_cases` directory: + Specifically, it assumes the following structure for the `test_cases` directory:: - ``` - test_cases/ - └── test_scenario_1 (can have multiple scenarios) - ├── artifacts - │ └── ____ - │ └── .out (can have other artifacts) - ├── expected_analyze.out - ├── setup.yaml (contains workspace commands for setting up the ramble config) - └── configs (either this or the setup.yaml must be present) - └── ramble.yaml (can contain more config files) - ``` + test_cases/ + └── test_scenario_1 (can have multiple scenarios) + ├── artifacts + │ └── ____ + │ └── .out (can have other artifacts) + ├── expected_analyze.out + ├── setup.yaml (contains workspace commands for setting up the ramble config) + └── configs (either this or the setup.yaml must be present) + └── ramble.yaml (can contain more config files) When writing a Ramble object, if a `test_cases` directory is included, then the test case will be run to verify the output of analyze. diff --git a/lib/ramble/ramble/test/test_base_classes_extra.py b/lib/ramble/ramble/test/test_base_classes_extra.py index 4af055595c..53b2773f55 100644 --- a/lib/ramble/ramble/test/test_base_classes_extra.py +++ b/lib/ramble/ramble/test/test_base_classes_extra.py @@ -430,3 +430,180 @@ def mock_is_available(*args, **kwargs): ws.dry_run = False app_inst._bootstrap_utilities(ws) + + +def test_application_base_bootstrap_utilities_empty_conf_and_none_variables( + mutable_config, mutable_mock_workspace_path, monkeypatch, mock_applications, mock_utilities +): + import ramble.workspace + + ws = ramble.workspace.create("test_app_bootstrap") + + # Just need a mocked application instance + app_inst = ramble.repository.get("basic") + app_inst.variables = None # Explicitly set to None to test the getattr fallback + app_inst.utilities = {} # No utilities, just bypasses + + app_inst._bootstrap_utilities(ws) + + +def test_application_base_is_available_typeerror_fallback( + mutable_config, mutable_mock_workspace_path, monkeypatch +): + import ramble.workspace + from ramble.app.builtin.gromacs.application import Gromacs + + workspace = ramble.workspace.create("test_fallback_workspace") + workspace.dry_run = False + app = Gromacs("/tmp/dummy") + + class MockExpander: + def expand_var_name(self, name): + return name + + def satisfies(self, when_key, variant_set): + return True + + def expand_var(self, name): + return name + + class MockAppInst: + def __init__(self): + self.variables = {"gromacs_version": "1.2"} + self.expander = MockExpander() + + app_inst = MockAppInst() + app._app_inst = app_inst + app.expander = MockExpander() + app._is_experiment = True + + class MockUtilityType: + def __init__(self): + # self.bootstrappable removed + self.object_variables = {} + + def is_available(self, workspace, min_version=None, max_version=None): + return True + + class MockUtilityInst: + def get(self, *args, **kwargs): + return MockUtilityType() + + app.required_utilities = { + frozenset(): { + "spack": { + "require_utility": True, + "utility_name": "spack", + "allow_external": "True", + "min_version": "1.0", + "version": "1.0", + "url": "http://foo", + } + } + } + import ramble.repository + + monkeypatch.setitem( + ramble.repository.paths, ramble.repository.ObjectTypes.utilities, MockUtilityInst() + ) + + def mock_bootstrap(workspace, ext_dep_paths): + pass + + app.bootstrap_utility = mock_bootstrap + + import ramble.config + + monkeypatch.setattr(ramble.config, "get", lambda *args, **kwargs: True) + + app._bootstrap_utilities(workspace) + assert app._bootstrapped_utility_paths["spack"] == "system" + + +def test_application_base_validate_versions_typeerror_fallback( + mutable_config, mutable_mock_workspace_path, monkeypatch +): + import ramble.workspace + from ramble.app.builtin.gromacs.application import Gromacs + from ramble.util.logger import logger + + workspace = ramble.workspace.create("test_fallback_val_workspace") + workspace.dry_run = False + app = Gromacs("/tmp/dummy") + + class MockExpander: + def expand_var_name(self, name): + return name + + def satisfies(self, when_key, variant_set): + return True + + def expand_var(self, name): + return name + + class MockAppInst: + def __init__(self): + self.variables = {"gromacs_version": "1.2"} + self.expander = MockExpander() + + app_inst = MockAppInst() + app._app_inst = app_inst + app.expander = MockExpander() + app._is_experiment = True + + class MockUtilityType: + def __init__(self): + # self.bootstrappable removed + self.object_variables = {} + self.availability_error = "Mock error" + + def is_available(self, workspace, min_version=None, max_version=None, exact_version=None): + return False + + def setup_runner_environment(self, workspace, app_inst): + return None + + def validate_versions( + self, min_version=None, max_version=None, env=None, origin_name=None, origin_type=None + ): + return False + + class MockUtilityInst: + def get(self, *args, **kwargs): + return MockUtilityType() + + app.required_utilities = { + frozenset(): { + "spack": { + "require_utility": True, + "utility_name": "spack", + "allow_external": "True", + "version": "1.0", + "url": "http://foo", + } + } + } + import ramble.repository + + monkeypatch.setitem( + ramble.repository.paths, ramble.repository.ObjectTypes.utilities, MockUtilityInst() + ) + + def mock_bootstrap(workspace, ext_dep_paths): + pass + + app.bootstrap_utility = mock_bootstrap + + import ramble.config + + monkeypatch.setattr(ramble.config, "get", lambda *args, **kwargs: True) + + warn_called = [] + monkeypatch.setattr(logger, "warn", lambda msg: warn_called.append(msg)) + import ramble.stage + + monkeypatch.setattr(ramble.stage.InputStage, "fetch", lambda *args, **kwargs: None) + monkeypatch.setattr(ramble.stage.InputStage, "expand_archive", lambda *args, **kwargs: None) + + app._bootstrap_utilities(workspace) + assert any("proceeding due to explicit version request" in msg for msg in warn_called) diff --git a/lib/ramble/ramble/test/test_utility_base.py b/lib/ramble/ramble/test/test_utility_base.py new file mode 100644 index 0000000000..09d561e528 --- /dev/null +++ b/lib/ramble/ramble/test/test_utility_base.py @@ -0,0 +1,471 @@ +# Copyright 2022-2026 The Ramble Authors +# +# Licensed under the Apache License, Version 2.0 or the MIT license +# , at your +# option. This file may not be copied, modified, or distributed +# except according to those terms. + +from ramble.repository import ObjectTypes, get + + +def test_utility_base_validate_exact_version_via_vcs(monkeypatch): + """Test that an exact version request passes if VCS check passes, ignoring version output.""" + SpackClass = type(get("spack", ObjectTypes.utilities)) + spack_inst = SpackClass("/path/to/spack") + + def mock_shutil_which(cmd, *args, **kwargs): + if cmd == "spack": + return "/usr/local/bin/spack" + elif cmd == "git": + return "/usr/bin/git" + return None + + monkeypatch.setattr("shutil.which", mock_shutil_which) + + def mock_check_vcs(self, exec_path, exact_version): # noqa: E501 + # Mock VCS check returning True + return True + + monkeypatch.setattr(SpackClass, "_check_exact_match_via_vcs", mock_check_vcs) + + def mock_subprocess_run(cmd, *args, **kwargs): + class MockResult: + returncode = 0 + stdout = "Spack 0.19.0\n" + stderr = "" + + return MockResult() + + monkeypatch.setattr("subprocess.run", mock_subprocess_run) + + # exact_version='commit-hash' which is not in output, but vcs says True + assert spack_inst.validate_versions(exact_version="commit-hash") is True + assert spack_inst.availability_error is None + + +def test_utility_base_validate_exact_version_via_fallback_output(monkeypatch): + """Test that an exact version request passes if VCS fails but + output contains the exact version.""" + SpackClass = type(get("spack", ObjectTypes.utilities)) + spack_inst = SpackClass("/path/to/spack") + + def mock_shutil_which(cmd, *args, **kwargs): + if cmd == "spack": + return "/usr/local/bin/spack" + return None + + monkeypatch.setattr("shutil.which", mock_shutil_which) + + def mock_check_vcs(self, exec_path, exact_version): # noqa: E501 + # Mock VCS check returning False + return False + + monkeypatch.setattr(SpackClass, "_check_exact_match_via_vcs", mock_check_vcs) + + def mock_subprocess_run(cmd, *args, **kwargs): + class MockResult: + returncode = 0 + stdout = "Spack 0.19.0 (commit-hash)\n" + stderr = "" + + return MockResult() + + monkeypatch.setattr("subprocess.run", mock_subprocess_run) + + # exact_version='commit-hash' which is in output + assert spack_inst.validate_versions(exact_version="commit-hash") is True + assert spack_inst.availability_error is None + + +def test_utility_base_validate_exact_version_failure(monkeypatch): + """Test that an exact version request fails if both VCS + and output do not contain the exact version.""" + SpackClass = type(get("spack", ObjectTypes.utilities)) + spack_inst = SpackClass("/path/to/spack") + + def mock_shutil_which(cmd, *args, **kwargs): + if cmd == "spack": + return "/usr/local/bin/spack" + return None + + monkeypatch.setattr("shutil.which", mock_shutil_which) + + def mock_check_vcs(self, exec_path, exact_version): # noqa: E501 + # Mock VCS check returning False + return False + + monkeypatch.setattr(SpackClass, "_check_exact_match_via_vcs", mock_check_vcs) + + def mock_subprocess_run(cmd, *args, **kwargs): + class MockResult: + returncode = 0 + stdout = "Spack 0.19.0\n" + stderr = "" + + return MockResult() + + monkeypatch.setattr("subprocess.run", mock_subprocess_run) + + # exact_version='commit-hash' not in output, vcs says False + assert spack_inst.validate_versions(exact_version="commit-hash") is False + assert spack_inst.availability_error is not None + assert "does not match required exact version" in spack_inst.availability_error + + +def test_utility_base_vcs_check_logic(monkeypatch): + """Test the VCS check logic specifically.""" + SpackClass = type(get("spack", ObjectTypes.utilities)) + spack_inst = SpackClass("/path/to/spack") + + def mock_shutil_which(cmd, *args, **kwargs): + if cmd == "git": + return "/usr/bin/git" + return None + + monkeypatch.setattr("shutil.which", mock_shutil_which) + + def mock_subprocess_run(cmd, *args, **kwargs): + class MockResult: + returncode = 0 + stdout = "" + stderr = "" + + res = MockResult() + if "--is-inside-work-tree" in cmd: + res.stdout = "true" + elif "HEAD" in cmd: + res.stdout = "123456" + elif "789abc^{commit}" in cmd: + # Different commit + res.stdout = "789abc" + elif "123456^{commit}" in cmd: + # Same commit + res.stdout = "123456" + return res + + monkeypatch.setattr("subprocess.run", mock_subprocess_run) + + # Test exact match via vcs (fails) + assert spack_inst._check_exact_match_via_vcs("/path/to/spack/bin/spack", "789abc") is False + + # Test exact match via vcs (succeeds) + assert spack_inst._check_exact_match_via_vcs("/path/to/spack/bin/spack", "123456") is True + + +def test_utility_base_vcs_early_returns(): + """Test early return paths in _check_exact_match_via_vcs.""" + SpackClass = type(get("spack", ObjectTypes.utilities)) + spack_inst = SpackClass("/path/to/spack") + + assert spack_inst._check_exact_match_via_vcs("", "123") is False + assert spack_inst._check_exact_match_via_vcs("/some/path", "") is False + assert spack_inst._check_exact_match_via_vcs(None, None) is False + # Path without a directory component + assert spack_inst._check_exact_match_via_vcs("spack", "123") is False + + +def test_utility_base_vcs_exception(monkeypatch): + """Test exception handling in _check_exact_match_via_vcs.""" + SpackClass = type(get("spack", ObjectTypes.utilities)) + spack_inst = SpackClass("/path/to/spack") + + def mock_shutil_which(cmd, *args, **kwargs): + if cmd == "git": + return "/usr/bin/git" + return None + + monkeypatch.setattr("shutil.which", mock_shutil_which) + + def mock_subprocess_run(cmd, *args, **kwargs): + raise Exception("Some subprocess error") + + monkeypatch.setattr("subprocess.run", mock_subprocess_run) + + assert spack_inst._check_exact_match_via_vcs("/path/to/spack", "123") is False + + +def test_utility_base_validate_versions_exceptions(monkeypatch): + """Test exception handling during version string validation.""" + SpackClass = type(get("spack", ObjectTypes.utilities)) + spack_inst = SpackClass("/path/to/spack") + + def mock_shutil_which(cmd, *args, **kwargs): + if cmd == "spack": + return "/usr/local/bin/spack" + return None + + monkeypatch.setattr("shutil.which", mock_shutil_which) + + def mock_subprocess_run(*args, **kwargs): + raise Exception("Command failed") + + monkeypatch.setattr("subprocess.run", mock_subprocess_run) + + # Test exact match via vcs True, exception swallowed + def mock_check_vcs_true(*args, **kwargs): + return True + + monkeypatch.setattr(SpackClass, "_check_exact_match_via_vcs", mock_check_vcs_true) + assert spack_inst.validate_versions(exact_version="123") is True + + # Test exact match via vcs False, exception fails validation + def mock_check_vcs_false(*args, **kwargs): + return False + + monkeypatch.setattr(SpackClass, "_check_exact_match_via_vcs", mock_check_vcs_false) + assert spack_inst.validate_versions(exact_version="123") is False + assert spack_inst.availability_error is not None + + +def test_utility_base_validate_versions_with_path(monkeypatch): + """Test that validate_versions correctly uses the path parameter if provided.""" + SpackClass = type(get("spack", ObjectTypes.utilities)) + spack_inst = SpackClass("/path/to/spack") + + def mock_shutil_which(cmd, path=None, **kwargs): + if cmd == "spack" and path == "/custom/path/to/spack/bin": + return "/custom/path/to/spack/bin/spack" + return None + + monkeypatch.setattr("shutil.which", mock_shutil_which) + + def mock_subprocess_run(cmd, *args, **kwargs): + class MockResult: + returncode = 0 + stdout = "Spack 0.19.0\n" + stderr = "" + + return MockResult() + + monkeypatch.setattr("subprocess.run", mock_subprocess_run) + + # Validation should fail without the path parameter + assert spack_inst.validate_versions(min_version="0.18.0") is False + assert "not found in PATH" in spack_inst.availability_error + + # Validation should succeed with the path parameter + assert ( + spack_inst.validate_versions(min_version="0.18.0", path="/custom/path/to/spack/bin") + is True + ) + assert spack_inst.availability_error is None + + +class _MockUtility(get("spack", ObjectTypes.utilities).__class__): # type: ignore + name = "mock_util" + class_variants = { + "dummy_variant": {"name": "dummy_variant", "default": "True", "description": "dummy"} + } + env_prepends = {"default": [{"var": "PATH", "value": "/mock/path"}]} + env_appends = {"default": [{"var": "LD_LIBRARY_PATH", "value": "/mock/lib"}]} + provided_executables = { + "mock_exe_no_ver": [{"executable": "mock_exe_no_ver"}], + "mock_exe_with_ver": [ + { + "executable": "mock_exe_with_ver", + "version_cmd": "mock_exe_with_ver --version", + "version_regex": r"Version (.*)", + } + ], + } + + +def test_utility_base_variants(): + """Test line 39: class_variants loop inside __init__""" + inst = _MockUtility("/mock/path") + assert inst.object_variants is not None + + +def test_utility_base_validate_versions_no_version_cmd(monkeypatch): + """Test lines 262-263: exact_version requested but no version_cmd.""" + inst = _MockUtility("/mock/path") + + def mock_shutil_which(cmd, *args, **kwargs): + return "/mock/path/mock_exe_no_ver" + + monkeypatch.setattr("shutil.which", mock_shutil_which) + + def mock_check_vcs(self, exec_path, exact_version): # noqa: E501 + return False + + monkeypatch.setattr(_MockUtility, "_check_exact_match_via_vcs", mock_check_vcs) + + original = inst.provided_executables + inst.provided_executables = {"mock_exe_no_ver": original["mock_exe_no_ver"]} + + res = inst.validate_versions(exact_version="1.0.0") + assert res is False + assert "but no version command is defined" in inst.availability_error + + inst.provided_executables = original + + +def test_utility_base_validate_versions_regex_fails_but_vcs_true(monkeypatch): + """Test lines 212-217: regex fails but exact_match_via_vcs is True.""" + inst = _MockUtility("/mock/path") + + def mock_shutil_which(cmd, *args, **kwargs): + return "/mock/path/mock_exe_with_ver" + + monkeypatch.setattr("shutil.which", mock_shutil_which) + + def mock_check_vcs(self, exec_path, exact_version): # noqa: E501 + return True + + monkeypatch.setattr(_MockUtility, "_check_exact_match_via_vcs", mock_check_vcs) + + def mock_subprocess_run(cmd, *args, **kwargs): + class MockResult: + returncode = 0 + stdout = "Some random output without version\n" + stderr = "" + + return MockResult() + + monkeypatch.setattr("subprocess.run", mock_subprocess_run) + + original = inst.provided_executables + inst.provided_executables = {"mock_exe_with_ver": original["mock_exe_with_ver"]} + + res = inst.validate_versions(exact_version="1.0.0") + assert res is True + inst.provided_executables = original + + +def test_utility_base_get_env_workspace_modifications(): + """Test env_prepends and env_appends in setup_runner_environment.""" + inst = _MockUtility("/mock/path") + import ramble.expander + + class MockAppInst: + def __init__(self): + self.variables = {"foo": "bar", "utility::mock_util::path": "/mock/path"} + self.expander = ramble.expander.Expander(self.variables, None) + + def satisfy_when(self, *args, **kwargs): + return True + + class MockWorkspace: + dry_run = True + + app_inst = MockAppInst() + env_mod = inst.setup_runner_environment(MockWorkspace(), app_inst) + assert env_mod is not None + + +def test_utility_base_get_experiment_activation_command(): + """Test lines 363-365, 383-387, 390-394: env_prepends and env_appends, and expander init.""" + inst = _MockUtility("/mock/path") + + class MockAppInst: + def __init__(self): + self.variables = {"utility::mock_util::path": "/custom/path"} + + def satisfy_when(self, *args, **kwargs): + return True + + app_inst = MockAppInst() + cmd = inst.get_experiment_activation_command(None, app_inst) + + assert "export PATH=/mock/path:$PATH" in cmd + assert "export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:/mock/lib" in cmd + + +def test_utility_base_validate_versions_subprocess_execution(monkeypatch, tmpdir): + """Test subprocess execution for validate_versions.""" + import os + import stat + + script_path = str(tmpdir.join("fake_spack.sh")) + with open(script_path, "w", encoding="utf-8") as f: + f.write("#!/bin/bash\n") + f.write('if [ "$1" = "--version" ]; then\n') + f.write(' echo "0.19.0"\n') + f.write("else\n") + f.write(' echo "Error"\n') + f.write(" exit 1\n") + f.write("fi\n") + + os.chmod(script_path, os.stat(script_path).st_mode | stat.S_IEXEC) + + SpackClass = type(get("spack", ObjectTypes.utilities)) + spack_inst = SpackClass("/path/to/spack") + + # Overwrite the command to use our fake script + spack_inst.provided_executables[frozenset()][0]["version_cmd"] = f"{script_path} --version" + + def mock_shutil_which(cmd, *args, **kwargs): + if cmd == "spack": + return script_path + return None + + monkeypatch.setattr("shutil.which", mock_shutil_which) + + def mock_check_vcs(*args, **kwargs): + return False + + monkeypatch.setattr(SpackClass, "_check_exact_match_via_vcs", mock_check_vcs) + + # Test successful max_version match + assert spack_inst.validate_versions(max_version="0.20.0") is True + + # Test failed max_version match + assert spack_inst.validate_versions(max_version="0.18.0") is False + assert "greater than required maximum" in spack_inst.availability_error + + # Test failed min_version match + assert spack_inst.validate_versions(min_version="0.20.0") is False + assert "less than required minimum" in spack_inst.availability_error + + # Test failed exact_version match + assert spack_inst.validate_versions(exact_version="0.18.0") is False + assert "does not match required exact version" in spack_inst.availability_error + + +def test_utility_base_validate_versions_subprocess_fails(monkeypatch, tmpdir): + """Test subprocess execution failure.""" + import os + import stat + + script_path = str(tmpdir.join("fake_fail.sh")) + with open(script_path, "w", encoding="utf-8") as f: + f.write("#!/bin/bash\n") + f.write("exit 1\n") + + os.chmod(script_path, os.stat(script_path).st_mode | stat.S_IEXEC) + + SpackClass = type(get("spack", ObjectTypes.utilities)) + spack_inst = SpackClass("/path/to/spack") + + spack_inst.provided_executables[frozenset()][0]["version_cmd"] = script_path + + def mock_shutil_which(cmd, *args, **kwargs): + if cmd == "spack": + return script_path + return None + + monkeypatch.setattr("shutil.which", mock_shutil_which) + + def mock_check_vcs(*args, **kwargs): + return False + + monkeypatch.setattr(SpackClass, "_check_exact_match_via_vcs", mock_check_vcs) + + assert spack_inst.validate_versions(exact_version="0.18.0") is False + assert "Error checking version" in spack_inst.availability_error + + +def test_utility_base_class_variants(): + """Test line 39: class_variants loop inside __init__ with proper directive""" + from ramble.toolkit import variant + + SpackClass = type(get("spack", ObjectTypes.utilities)) + + class TestVariantUtility(SpackClass): + name = "test_var_util" + __module__ = "ramble.app" + variant("test_variant", default="True", description="test") + + inst = TestVariantUtility("/mock/path") + assert len(inst.class_variants) > 0 diff --git a/var/ramble/repos/builtin/base_classes/utility-base/base_class.py b/var/ramble/repos/builtin/base_classes/utility-base/base_class.py index 1cfc629cb1..6caf185cc9 100644 --- a/var/ramble/repos/builtin/base_classes/utility-base/base_class.py +++ b/var/ramble/repos/builtin/base_classes/utility-base/base_class.py @@ -124,8 +124,8 @@ def _check_exact_match_via_vcs(self, exec_path, exact_version): and head_hash == exact_hash ): return True - except Exception: - pass + except Exception as e: + logger.debug(f"VCS check for {exec_path} failed: {e}") # Future VCS checks can be added here (e.g., hg, svn) From ada572d1f9bf58fc234f6ed79de44b374adfb558 Mon Sep 17 00:00:00 2001 From: Douglas Jacobsen Date: Sat, 29 Aug 2026 12:28:57 -0600 Subject: [PATCH 3/3] Fix sphinx build issues Signed-off-by: Douglas Jacobsen --- .../tutorials/Workspace_config_command.rst | 4 +-- lib/ramble/docs/workspace_config.rst | 3 +- .../language/workflow_manager_language.py | 1 + lib/ramble/ramble/reports.py | 29 ++++++++++--------- 4 files changed, 20 insertions(+), 17 deletions(-) diff --git a/lib/ramble/docs/tutorials/Workspace_config_command.rst b/lib/ramble/docs/tutorials/Workspace_config_command.rst index b6fe1130c6..f84ec21a03 100644 --- a/lib/ramble/docs/tutorials/Workspace_config_command.rst +++ b/lib/ramble/docs/tutorials/Workspace_config_command.rst @@ -33,9 +33,9 @@ around configuring a workspace. Configuring experiments within a workspace will not be covered in this tutorial, however we will use pre-configured workspaces to illustrate the utility of the ``workspace config`` command. ---------------------- +------------------------ Create Complex Workspace ---------------------- +------------------------ To begin, we will construct a complex workspace to serve as an example of something we want to share with other users. Before configuring the workspace, diff --git a/lib/ramble/docs/workspace_config.rst b/lib/ramble/docs/workspace_config.rst index 5061c7b392..56704e23be 100644 --- a/lib/ramble/docs/workspace_config.rst +++ b/lib/ramble/docs/workspace_config.rst @@ -506,8 +506,9 @@ Variant Expansion Variants can be expanded like variables into a Spack-like syntax by using the syntax ``{{object_type}::variant::{variant_name}``. For example, a boolean variant with a value of ``True`` formats to ``+bool``, whereas ``False`` formats to ``~bool``. A value-based variant formats to ``key=value``. +^^^^^^^^^^^^^^^^^^^^^^^^^ Variant Expansion Example -~~~~~~~~~~~~~~~~~~~~~~~~~ +^^^^^^^^^^^^^^^^^^^^^^^^^ Suppose multiple applications in a workspace use the variant ``openmp`` (boolean) to parameterize their software specs for Spack. We can define it under the workspace ``variants:`` section: diff --git a/lib/ramble/ramble/language/workflow_manager_language.py b/lib/ramble/ramble/language/workflow_manager_language.py index 0988a7f0d9..7373fac897 100644 --- a/lib/ramble/ramble/language/workflow_manager_language.py +++ b/lib/ramble/ramble/language/workflow_manager_language.py @@ -31,6 +31,7 @@ def workflow_manager_variable( **kwargs, ): """Define a variable for this wm + Args: name: Name of variable default: Default value if the variable is not defined diff --git a/lib/ramble/ramble/reports.py b/lib/ramble/ramble/reports.py index d25cd56289..37f661bd97 100644 --- a/lib/ramble/ramble/reports.py +++ b/lib/ramble/ramble/reports.py @@ -339,24 +339,25 @@ def filter_exp_results(experiments: list): def generate_result_index(experiments: list, all_vars=False, where_query=None): """Creates an index from the results in the list of experiments - Index format is: - { - "applications": { - application_name: { - workload: { + Index format is:: + + { + "applications": { + application_name: { + workload: { + "Contexts": set(), + "FOMs": set(), + "Template Variables": set(), + } + } + } + "modifiers": { + modifier_name: { "Contexts": set(), "FOMs": set(), - "Template Variables": set(), } - } + (all other object types) } - "modifiers": { - modifier_name: { - "Contexts": set(), - "FOMs": set(), - } - (all other object types) - } """ result_index: Dict[str, dict] = {} for obj_name in OBJECT_NAMES.values():