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

@@ -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)

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):

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")