"spack develop": always pull full history for git repos (#38343)

lib/spack/spack/cmd/develop.py

@@ -66,8 +66,7 @@ def develop(parser, args):
             # Both old syntax `spack develop pkg@x` and new syntax `spack develop pkg@=x`
             # are currently supported.
             spec = spack.spec.parse_with_version_concrete(entry["spec"])
-            pkg_cls = spack.repo.path.get_pkg_class(spec.name)
+            env.develop(spec=spec, path=path, clone=True)
-            pkg_cls(spec).stage.steal_source(abspath)
 
         if not env.dev_specs:
             tty.warn("No develop specs to download")

lib/spack/spack/environment/environment.py

@@ -1256,16 +1256,23 @@ def develop(self, spec: Spec, path: str, clone: bool = False) -> bool:
             tty.msg("Configuring spec %s for development at path %s" % (spec, path))
 
         if clone:
-            # "steal" the source code via staging API
+            # "steal" the source code via staging API. We ask for a stage
-            abspath = spack.util.path.canonicalize_path(path, default_wd=self.path)
+            # to be created, then copy it afterwards somewhere else. It would be
-
-            # Stage, at the moment, requires a concrete Spec, since it needs the
-            # dag_hash for the stage dir name. Below though we ask for a stage
-            # to be created, to copy it afterwards somewhere else. It would be
             # better if we can create the `source_path` directly into its final
             # destination.
+            abspath = spack.util.path.canonicalize_path(path, default_wd=self.path)
             pkg_cls = spack.repo.path.get_pkg_class(spec.name)
-            pkg_cls(spec).stage.steal_source(abspath)
+            # We construct a package class ourselves, rather than asking for
+            # Spec.package, since Spec only allows this when it is concrete
+            package = pkg_cls(spec)
+            if isinstance(package.fetcher[0], spack.fetch_strategy.GitFetchStrategy):
+                package.fetcher[0].get_full_repo = True
+                # If we retrieved this version before and cached it, we may have
+                # done so without cloning the full git repo; likewise, any
+                # mirror might store an instance with truncated history.
+                package.stage[0].disable_mirrors()
+
+            package.stage.steal_source(abspath)
 
         # If it wasn't already in the list, append it
         entry = {"path": path, "spec": str(spec)}

lib/spack/spack/stage.py

@@ -429,6 +429,12 @@ def source_path(self):
         """Returns the well-known source directory path."""
         return os.path.join(self.path, _source_path_subdir)
 
+    def disable_mirrors(self):
+        """The Stage will not attempt to look for the associated fetcher
+        target in any of Spack's mirrors (including the local download cache).
+        """
+        self.mirror_paths = []
+
     def fetch(self, mirror_only=False, err_msg=None):
         """Retrieves the code or archive
 

lib/spack/spack/test/cmd/develop.py

@@ -146,3 +146,48 @@ def check_path(stage, dest):
 
             # Check modifications actually worked
             assert spack.spec.Spec("mpich@1.0").concretized().satisfies("dev_path=%s" % abspath)
+
+
+def _git_commit_list(git_repo_dir):
+    git = spack.util.git.git()
+    with fs.working_dir(git_repo_dir):
+        output = git("log", "--pretty=format:%h", "-n", "20", output=str)
+    return output.strip().split()
+
+
+def test_develop_full_git_repo(
+    mutable_mock_env_path,
+    mock_git_version_info,
+    install_mockery,
+    mock_packages,
+    monkeypatch,
+    tmpdir,
+    mutable_config,
+    request,
+):
+    repo_path, filename, commits = mock_git_version_info
+    monkeypatch.setattr(
+        spack.package_base.PackageBase, "git", "file://%s" % repo_path, raising=False
+    )
+
+    spec = spack.spec.Spec("git-test-commit@1.2").concretized()
+    try:
+        spec.package.do_stage()
+        commits = _git_commit_list(spec.package.stage[0].source_path)
+        # Outside of "spack develop" Spack will only pull exactly the commit it
+        # needs, with no additional history
+        assert len(commits) == 1
+    finally:
+        spec.package.do_clean()
+
+    # Now use "spack develop": look at the resulting stage directory and make
+    # sure the git repo pulled includes the full branch history (or rather,
+    # more than just one commit).
+    env("create", "test")
+    with ev.read("test"):
+        develop("git-test-commit@1.2")
+
+        location = SpackCommand("location")
+        develop_stage_dir = location("git-test-commit").strip()
+        commits = _git_commit_list(develop_stage_dir)
+        assert len(commits) > 1