Ad-hoc Git commit versions: support submodules (#30037)

* Allow packages to add a 'submodules' property that determines when ad-hoc Git-commit-based versions should initialize submodules

* add support for ad-hoc git-commit-based versions to instantiate submodules if the associated package has a 'submodules' property and it indicates this should happen for the associated spec

* allow Package-level submodule request to influence all explicitly-defined version() in the Package

* skip test on windows which fails because of long paths

5 files changed, 90 insertions, 11 deletions

diff --git a/lib/spack/spack/fetch_strategy.py b/lib/spack/spack/fetch_strategy.py
index 1a7976bf64..a8675550ff 100644
--- a/lib/spack/spack/fetch_strategy.py
+++ b/lib/spack/spack/fetch_strategy.py
@@ -1596,7 +1596,12 @@ def for_package_version(pkg, version):
     if version.is_commit and hasattr(pkg, "git"):
         # Populate the version with comparisons to other commits
         version.generate_commit_lookup(pkg.name)
-        fetcher = GitFetchStrategy(git=pkg.git, commit=str(version))
+        kwargs = {
+            'git': pkg.git,
+            'commit': str(version)
+        }
+        kwargs['submodules'] = getattr(pkg, 'submodules', False)
+        fetcher = GitFetchStrategy(**kwargs)
         return fetcher
 
     # If it's not a known version, try to extrapolate one by URL
@@ -1612,6 +1617,8 @@ def for_package_version(pkg, version):
     for fetcher in all_strategies:
         if fetcher.url_attr in args:
             _check_version_attributes(fetcher, pkg, version)
+            if fetcher.url_attr == 'git' and hasattr(pkg, 'submodules'):
+                args.setdefault('submodules', pkg.submodules)
             return fetcher(**args)
 
     # if a version's optional attributes imply a particular fetch
diff --git a/lib/spack/spack/test/conftest.py b/lib/spack/spack/test/conftest.py
index d4d2c64aaa..420910b764 100644
--- a/lib/spack/spack/test/conftest.py
+++ b/lib/spack/spack/test/conftest.py
@@ -1239,12 +1239,30 @@ def mock_cvs_repository(tmpdir_factory):
 
 @pytest.fixture(scope='session')
 def mock_git_repository(tmpdir_factory):
-    """Creates a simple git repository with two branches,
-    two commits and two submodules. Each submodule has one commit.
+    """Creates a git repository multiple commits, branches, submodules, and
+    a tag. Visual representation of the commit history (starting with the
+    earliest commit at c0)::
+
+       c3       c1 (test-branch, r1)  c2 (tag-branch)
+        |______/_____________________/
+       c0 (r0)
+
+    There are two branches aside from 'master': 'test-branch' and 'tag-branch';
+    each has one commit; the tag-branch has a tag referring to its commit
+    (c2 in the diagram).
+
+    Two submodules are added as part of the very first commit on 'master'; each
+    of these refers to a repository with a single commit.
+
+    c0, c1, and c2 include information to define explicit versions in the
+    associated builtin.mock package 'git-test'. c3 is a commit in the
+    repository but does not have an associated explicit package version.
     """
     git = spack.util.executable.which('git', required=True)
 
     suburls = []
+    # Create two git repositories which will be used as submodules in the
+    # main repository
     for submodule_count in range(2):
         tmpdir = tmpdir_factory.mktemp('mock-git-repo-submodule-dir-{0}'
                                        .format(submodule_count))
@@ -1252,7 +1270,6 @@ def mock_git_repository(tmpdir_factory):
         repodir = tmpdir.join(spack.stage._source_path_subdir)
         suburls.append((submodule_count, 'file://' + str(repodir)))
 
-        # Initialize the repository
         with repodir.as_cwd():
             git('init')
             git('config', 'user.name', 'Spack')
@@ -1269,7 +1286,7 @@ def mock_git_repository(tmpdir_factory):
     tmpdir.ensure(spack.stage._source_path_subdir, dir=True)
     repodir = tmpdir.join(spack.stage._source_path_subdir)
 
-    # Initialize the repository
+    # Create the main repository
     with repodir.as_cwd():
         git('init')
         git('config', 'user.name', 'Spack')
@@ -1279,7 +1296,7 @@ def mock_git_repository(tmpdir_factory):
             git('submodule', 'add', suburl,
                 'third_party/submodule{0}'.format(number))
 
-        # r0 is just the first commit
+        # r0 is the first commit: it consists of one file and two submodules
         r0_file = 'r0_file'
         repodir.ensure(r0_file)
         git('add', r0_file)
@@ -1293,13 +1310,13 @@ def mock_git_repository(tmpdir_factory):
         tag_file = 'tag_file'
         git('branch', tag_branch)
 
-        # Check out first branch
+        # Check out test branch and add one commit
         git('checkout', branch)
         repodir.ensure(branch_file)
         git('add', branch_file)
         git('-c', 'commit.gpgsign=false', 'commit', '-m' 'r1 test branch')
 
-        # Check out a second branch and tag it
+        # Check out the tag branch, add one commit, and then add a tag for it
         git('checkout', tag_branch)
         repodir.ensure(tag_file)
         git('add', tag_file)
@@ -1310,11 +1327,24 @@ def mock_git_repository(tmpdir_factory):
 
         git('checkout', 'master')
 
-        # R1 test is the same as test for branch
+        r2_file = 'r2_file'
+        repodir.ensure(r2_file)
+        git('add', r2_file)
+        git('-c', 'commit.gpgsign=false', 'commit', '-m', 'mock-git-repo r2')
+
         rev_hash = lambda x: git('rev-parse', x, output=str).strip()
+        r2 = rev_hash('master')
+
+        # Record the commit hash of the (only) commit from test-branch and
+        # the file added by that commit
         r1 = rev_hash(branch)
         r1_file = branch_file
 
+    # Map of version -> bunch. Each bunch includes; all the args
+    # that must be specified as part of a version() declaration (used to
+    # manufacture a version for the 'git-test' package); the associated
+    # revision for the version; a file associated with (and particular to)
+    # that revision/branch.
     checks = {
         'master': Bunch(
             revision='master', file=r0_file, args={'git': url}
@@ -1338,7 +1368,7 @@ def mock_git_repository(tmpdir_factory):
     }
 
     t = Bunch(checks=checks, url=url, hash=rev_hash,
-              path=str(repodir), git_exe=git)
+              path=str(repodir), git_exe=git, unversioned_commit=r2)
     yield t
 
 
diff --git a/lib/spack/spack/test/git_fetch.py b/lib/spack/spack/test/git_fetch.py
index 6e8993978c..acefb98175 100644
--- a/lib/spack/spack/test/git_fetch.py
+++ b/lib/spack/spack/test/git_fetch.py
@@ -137,6 +137,36 @@ def test_fetch(type_of_test,
             assert h('HEAD') == h(t.revision)
 
 
+@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'
+                            ' for ad-hoc Git commit versions is longer than'
+                            ' other staged sources'))
+@pytest.mark.disable_clean_stage_check
+def test_adhoc_version_submodules(
+        mock_git_repository,
+        config,
+        mutable_mock_repo,
+        monkeypatch,
+        mock_stage):
+
+    t = mock_git_repository.checks['tag']
+    # Construct the package under test
+    pkg_class = spack.repo.path.get_pkg_class('git-test')
+    monkeypatch.setitem(pkg_class.versions, ver('git'), t.args)
+    monkeypatch.setattr(pkg_class, 'git', 'file://%s' % mock_git_repository.path,
+                        raising=False)
+
+    spec = Spec('git-test@{0}'.format(mock_git_repository.unversioned_commit))
+    spec.concretize()
+    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']) < collected_fnames
+
+
 @pytest.mark.parametrize("type_of_test", ['branch', 'commit'])
 def test_debug_fetch(
         mock_packages, type_of_test, mock_git_repository, config, monkeypatch
@@ -227,7 +257,12 @@ def test_get_full_repo(get_full_repo, git_version, mock_git_repository,
 def test_gitsubmodule(submodules, mock_git_repository, config,
                       mutable_mock_repo, monkeypatch):
     """
-    Test GitFetchStrategy behavior with submodules
+    Test GitFetchStrategy behavior with submodules. This package
+    has a `submodules` property which is always True: when a specific
+    version also indicates to include submodules, this should not
+    interfere; if the specific version explicitly requests that
+    submodules *not* be initialized, this should override the
+    Package-level request.
     """
     type_of_test = 'tag-branch'
     t = mock_git_repository.checks[type_of_test]
diff --git a/var/spack/repos/builtin.mock/packages/git-test/package.py b/var/spack/repos/builtin.mock/packages/git-test/package.py
index 3e9b259106..ab72e5921e 100644
--- a/var/spack/repos/builtin.mock/packages/git-test/package.py
+++ b/var/spack/repos/builtin.mock/packages/git-test/package.py
@@ -10,4 +10,6 @@ class GitTest(Package):
     """Mock package that uses git for fetching."""
     homepage = "http://www.git-fetch-example.com"
 
+    submodules = True
+
     version('git', git='to-be-filled-in-by-test')
diff --git a/var/spack/repos/builtin/packages/axom/package.py b/var/spack/repos/builtin/packages/axom/package.py
index 3719459be3..7b02b25c56 100644
--- a/var/spack/repos/builtin/packages/axom/package.py
+++ b/var/spack/repos/builtin/packages/axom/package.py
@@ -50,6 +50,11 @@ class Axom(CachedCMakePackage, CudaPackage):
     version('0.3.0', tag='v0.3.0', submodules=True)
     version('0.2.9', tag='v0.2.9', submodules=True)
 
+    @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')
 
     root_cmakelists_dir = 'src'