Package-level submodule attribute: support explicit versions (#30085)

lib/spack/spack/fetch_strategy.py

@@ -1575,6 +1575,10 @@ def _from_merged_attrs(fetcher, pkg, version):
 
     attrs['fetch_options'] = pkg.fetch_options
     attrs.update(pkg.versions[version])
+
+    if fetcher.url_attr == 'git' and hasattr(pkg, 'submodules'):
+        attrs.setdefault('submodules', pkg.submodules)
+
     return fetcher(**attrs)
 
 

lib/spack/spack/test/conftest.py

@@ -1364,6 +1364,12 @@ def mock_git_repository(tmpdir_factory):
         ),
         'commit': Bunch(
             revision=r1, file=r1_file, args={'git': url, 'commit': r1}
+        ),
+        # In this case, the version() args do not include a 'git' key:
+        # this is the norm for packages, so this tests how the fetching logic
+        # would most-commonly assemble a Git fetcher
+        'master-no-per-version-git': Bunch(
+            revision='master', file=r0_file, args={'branch': 'master'}
         )
     }
 

lib/spack/spack/test/git_fetch.py

@@ -82,7 +82,8 @@ def test_bad_git(tmpdir, mock_bad_git):
             fetcher.fetch()
 
 
-@pytest.mark.parametrize("type_of_test", ['master', 'branch', 'tag', 'commit'])
+@pytest.mark.parametrize("type_of_test",
+                         ['master', 'branch', 'tag', 'commit'])
 @pytest.mark.parametrize("secure", [True, False])
 def test_fetch(type_of_test,
                secure,
@@ -104,6 +105,11 @@ def test_fetch(type_of_test,
     t = mock_git_repository.checks[type_of_test]
     h = mock_git_repository.hash
 
+    pkg_class = spack.repo.path.get_pkg_class('git-test')
+    # This would fail using the master-no-per-version-git check but that
+    # isn't included in this test
+    monkeypatch.delattr(pkg_class, 'git')
+
     # Construct the package under test
     spec = Spec('git-test')
     spec.concretize()
@@ -137,6 +143,40 @@ def test_fetch(type_of_test,
             assert h('HEAD') == h(t.revision)
 
 
+@pytest.mark.disable_clean_stage_check
+def test_fetch_pkg_attr_submodule_init(
+        mock_git_repository,
+        config,
+        mutable_mock_repo,
+        monkeypatch,
+        mock_stage):
+    """In this case the version() args do not contain a 'git' URL, so
+    the fetcher must be assembled using the Package-level 'git' attribute.
+    This test ensures that the submodules are properly initialized and the
+    expected branch file is present.
+    """
+
+    t = mock_git_repository.checks['master-no-per-version-git']
+    pkg_class = spack.repo.path.get_pkg_class('git-test')
+    # For this test, the version args don't specify 'git' (which is
+    # the majority of version specifications)
+    monkeypatch.setattr(pkg_class, 'git', mock_git_repository.url)
+
+    # Construct the package under test
+    spec = Spec('git-test')
+    spec.concretize()
+    pkg = spack.repo.get(spec)
+    monkeypatch.setitem(pkg.versions, ver('git'), t.args)
+
+    spec.package.do_stage()
+
+    collected_fnames = set()
+    for root, dirs, files in os.walk(spec.package.stage.source_path):
+        collected_fnames.update(files)
+    # The submodules generate files with the prefix "r0_file_"
+    assert set(['r0_file_0', 'r0_file_1', t.file]) < collected_fnames
+
+
 @pytest.mark.skipif(str(spack.platforms.host()) == 'windows',
                     reason=('Git fails to clone because the src/dst paths'
                             ' are too long: the name of the staging directory'

var/spack/repos/builtin.mock/packages/git-test/package.py

@@ -9,6 +9,8 @@
 class GitTest(Package):
     """Mock package that uses git for fetching."""
     homepage = "http://www.git-fetch-example.com"
+    # To be set by test
+    git = None
 
     submodules = True
 

var/spack/repos/builtin/packages/axom/package.py

@@ -38,21 +38,20 @@ class Axom(CachedCMakePackage, CudaPackage):
     git      = "https://github.com/LLNL/axom.git"
     tags     = ['radiuss']
 
-    version('main', branch='main', submodules=True)
-    version('develop', branch='develop', submodules=True)
-    version('0.6.1', tag='v0.6.1', submodules=True)
-    version('0.6.0', tag='v0.6.0', submodules=True)
-    version('0.5.0', tag='v0.5.0', submodules=True)
-    version('0.4.0', tag='v0.4.0', submodules=True)
-    version('0.3.3', tag='v0.3.3', submodules=True)
-    version('0.3.2', tag='v0.3.2', submodules=True)
-    version('0.3.1', tag='v0.3.1', submodules=True)
-    version('0.3.0', tag='v0.3.0', submodules=True)
-    version('0.2.9', tag='v0.2.9', submodules=True)
+    version('main', branch='main')
+    version('develop', branch='develop')
+    version('0.6.1', tag='v0.6.1')
+    version('0.6.0', tag='v0.6.0')
+    version('0.5.0', tag='v0.5.0')
+    version('0.4.0', tag='v0.4.0')
+    version('0.3.3', tag='v0.3.3')
+    version('0.3.2', tag='v0.3.2')
+    version('0.3.1', tag='v0.3.1')
+    version('0.3.0', tag='v0.3.0')
+    version('0.2.9', tag='v0.2.9')
 
     @property
     def submodules(self):
-        # All git checkouts should also initialize submodules
         return True
 
     patch('scr_examples_gtest.patch', when='@0.6.0:0.6.1')