From 552a35e12eb212a6523b7a788d58aa52d5a2432e Mon Sep 17 00:00:00 2001 From: Todd Gamblin Date: Wed, 4 Sep 2024 11:33:35 +0200 Subject: [PATCH] bugfix: submodule callbacks need to be resolved in specs The submodules argument for git versions supports callbacks (which are used on some key packages to conditionally fetch certain submodules). Callbacks can't be serialized to JSON, so we need to ensure that these things are resolved (by calling them) at concretization time. - [x] ensure that submodule callbacks are called when creating spec JSON - [x] add tests that try serializing all types of git versions --- lib/spack/spack/fetch_strategy.py | 12 +++++++++++- lib/spack/spack/test/spec_yaml.py | 16 ++++++++++++++++ .../packages/git-url-top-level/package.py | 6 ++++++ 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/lib/spack/spack/fetch_strategy.py b/lib/spack/spack/fetch_strategy.py index f08316bc9d2..5e0dc4cfe02 100644 --- a/lib/spack/spack/fetch_strategy.py +++ b/lib/spack/spack/fetch_strategy.py @@ -270,7 +270,7 @@ def __init__(self, *, url: str, checksum: Optional[str] = None, **kwargs) -> Non self._effective_url: Optional[str] = None def spec_attrs(self): - attrs = super(URLFetchStrategy, self).spec_attrs() + attrs = super().spec_attrs() if self.digest: try: hash_type = spack.util.crypto.hash_algo_for_digest(self.digest) @@ -759,6 +759,16 @@ def __init__(self, **kwargs): self.get_full_repo = kwargs.get("get_full_repo", False) self.git_sparse_paths = kwargs.get("git_sparse_paths", None) + def spec_attrs(self): + attrs = super().spec_attrs() + + # need to fully resolve submodule callbacks for node dicts + submodules = attrs.get("submodules", None) + if submodules and callable(submodules): + attrs["submodules"] = submodules(self.package) + + return attrs + @property def git_version(self): return GitFetchStrategy.version_from_git(self.git) diff --git a/lib/spack/spack/test/spec_yaml.py b/lib/spack/spack/test/spec_yaml.py index 5b64822b382..ee685078b6c 100644 --- a/lib/spack/spack/test/spec_yaml.py +++ b/lib/spack/spack/test/spec_yaml.py @@ -96,6 +96,22 @@ def test_invalid_json_spec(invalid_json, error_message): # Virtuals on edges "callpath", "mpileaks", + # all types of git versions + # ensure that we try to serialize all the things that might be in the node dict, + # e.g., submodule callbacks can fail serialization if they're not fully resolved. + "git-url-top-level@develop", + "git-url-top-level@submodules", + "git-url-top-level@submodules_callback", + "git-url-top-level@3.4", + "git-url-top-level@3.3", + "git-url-top-level@3.2", + "git-url-top-level@3.1", + "git-url-top-level@3.0", + "git-url-top-level@2.3", + "git-url-top-level@2.2", + "git-url-top-level@2.1", + "git-url-top-level@2.0", + "git-url-top-level@2.3", ], ) def test_roundtrip_concrete_specs(abstract_spec, default_mock_concretization): diff --git a/var/spack/repos/builtin.mock/packages/git-url-top-level/package.py b/var/spack/repos/builtin.mock/packages/git-url-top-level/package.py index 7e3009d7dc9..ac5bdf669b2 100644 --- a/var/spack/repos/builtin.mock/packages/git-url-top-level/package.py +++ b/var/spack/repos/builtin.mock/packages/git-url-top-level/package.py @@ -6,6 +6,11 @@ from spack.package import * +def use_submodules(pkg): + """test example of a submodule callback""" + return ["a", "b"] + + class GitUrlTopLevel(Package): """Mock package that top-level git and url attributes. @@ -22,6 +27,7 @@ class GitUrlTopLevel(Package): # These resolve to git fetchers version("develop", branch="develop") version("submodules", submodules=True) + version("submodules_callback", submodules=use_submodules) version("3.4", commit="abc34") version("3.3", branch="releases/v3.3", commit="abc33") version("3.2", branch="releases/v3.2")