Consolidate how Spack uses git (#34700)

Local `git` tests will fail with `fatal: transport 'file' not allowed` when using git 2.38.1 or higher, due to a fix for `CVE-2022-39253`.

This was fixed in CI in #33429, but that doesn't help the issue for anyone's local environment. Instead of fixing this with git config in CI, we should ensure that the tests run anywhere.

- [x] Introduce `spack.util.git`.
- [x] Use `spack.util.git.get_git()` to get a git executable, instead of `which("git")` everywhere.
- [x] Make all `git` tests use a `git` fixture that goes through `spack.util.git.get_git()`.
- [x] Add `-c protocol.file.allow=always` to all `git` invocations under `pytest`.
- [x] Revert changes from #33429, which are no longer needed.

.github/workflows/setup_git.ps1

@@ -4,10 +4,6 @@ git config --global user.email "spack@example.com"
 git config --global user.name "Test User"
 git config --global core.longpaths true
 
-# See https://github.com/git/git/security/advisories/GHSA-3wp6-j8xr-qw85 (CVE-2022-39253)
-# This is needed to let some fixture in our unit-test suite run
-git config --global protocol.file.allow always
-
 if ($(git branch --show-current) -ne "develop")
 {
     git branch develop origin/develop

.github/workflows/setup_git.sh

@@ -2,10 +2,6 @@
 git config --global user.email "spack@example.com"
 git config --global user.name "Test User"
 
-# See https://github.com/git/git/security/advisories/GHSA-3wp6-j8xr-qw85 (CVE-2022-39253)
-# This is needed to let some fixture in our unit-test suite run
-git config --global protocol.file.allow always
-
 # create a local pr base branch
 if [[ -n $GITHUB_BASE_REF ]]; then
     git fetch origin "${GITHUB_BASE_REF}:${GITHUB_BASE_REF}"

lib/spack/spack/ci.py

@@ -33,7 +33,7 @@
 import spack.mirror
 import spack.paths
 import spack.repo
-import spack.util.executable as exe
+import spack.util.git
 import spack.util.gpg as gpg_util
 import spack.util.spack_yaml as syaml
 import spack.util.url as url_util
@@ -486,7 +486,7 @@ def get_stack_changed(env_path, rev1="HEAD^", rev2="HEAD"):
     whether or not the stack was changed.  Returns True if the environment
     manifest changed between the provided revisions (or additionally if the
     `.gitlab-ci.yml` file itself changed).  Returns False otherwise."""
-    git = exe.which("git")
+    git = spack.util.git.git()
     if git:
         with fs.working_dir(spack.paths.prefix):
             git_log = git(
@@ -1655,7 +1655,7 @@ def get_spack_info():
     entry, otherwise, return a string containing the spack version."""
     git_path = os.path.join(spack.paths.prefix, ".git")
     if os.path.exists(git_path):
-        git = exe.which("git")
+        git = spack.util.git.git()
         if git:
             with fs.working_dir(spack.paths.prefix):
                 git_log = git("log", "-1", output=str, error=os.devnull, fail_on_error=False)
@@ -1695,7 +1695,7 @@ def setup_spack_repro_version(repro_dir, checkout_commit, merge_commit=None):
 
     spack_git_path = spack.paths.prefix
 
-    git = exe.which("git")
+    git = spack.util.git.git()
     if not git:
         tty.error("reproduction of pipeline job requires git")
         return False

lib/spack/spack/cmd/blame.py

@@ -14,9 +14,9 @@
 
 import spack.paths
 import spack.repo
+import spack.util.git
 import spack.util.spack_json as sjson
 from spack.cmd import spack_is_git_repo
-from spack.util.executable import which
 
 description = "show contributors to packages"
 section = "developer"
@@ -116,7 +116,7 @@ def blame(parser, args):
     # make sure this is a git repo
     if not spack_is_git_repo():
         tty.die("This spack is not a git clone. Can't use 'spack blame'")
-    git = which("git", required=True)
+    git = spack.util.git.git(required=True)
 
     # Get name of file to blame
     blame_file = None

lib/spack/spack/cmd/clone.py

@@ -9,7 +9,8 @@
 from llnl.util.filesystem import mkdirp, working_dir
 
 import spack.paths
-from spack.util.executable import ProcessError, which
+import spack.util.git
+from spack.util.executable import ProcessError
 
 _SPACK_UPSTREAM = "https://github.com/spack/spack"
 
@@ -32,7 +33,7 @@ def setup_parser(subparser):
 
 def get_origin_info(remote):
     git_dir = os.path.join(spack.paths.prefix, ".git")
-    git = which("git", required=True)
+    git = spack.util.git.git(required=True)
     try:
         branch = git("symbolic-ref", "--short", "HEAD", output=str)
     except ProcessError:
@@ -69,13 +70,13 @@ def clone(parser, args):
     if files_in_the_way:
         tty.die(
             "There are already files there! " "Delete these files before boostrapping spack.",
-            *files_in_the_way
+            *files_in_the_way,
         )
 
     tty.msg("Installing:", "%s/bin/spack" % prefix, "%s/lib/spack/..." % prefix)
 
     with working_dir(prefix):
-        git = which("git", required=True)
+        git = spack.util.git.git(required=True)
         git("init", "--shared", "-q")
         git("remote", "add", "origin", origin_url)
         git("fetch", "origin", "%s:refs/remotes/origin/%s" % (branch, branch), "-n", "-q")

lib/spack/spack/cmd/debug.py

@@ -17,6 +17,7 @@
 import spack.config
 import spack.paths
 import spack.platforms
+import spack.util.git
 from spack.main import get_version
 from spack.util.executable import which
 
@@ -35,7 +36,7 @@ def _debug_tarball_suffix():
     now = datetime.now()
     suffix = now.strftime("%Y-%m-%d-%H%M%S")
 
-    git = which("git")
+    git = spack.util.git.git()
     if not git:
         return "nobranch-nogit-%s" % suffix
 

lib/spack/spack/cmd/license.py

@@ -13,15 +13,11 @@
 import llnl.util.tty as tty
 
 import spack.paths
-from spack.util.executable import which
 
 description = "list and check license headers on files in spack"
 section = "developer"
 level = "long"
 
-#: need the git command to check new files
-git = which("git")
-
 #: SPDX license id must appear in the first <license_lines> lines of a file
 license_lines = 7
 
@@ -238,9 +234,6 @@ def setup_parser(subparser):
 
 
 def license(parser, args):
-    if not git:
-        tty.die("spack license requires git in your environment")
-
     licensed_files[:] = [re.compile(regex) for regex in licensed_files]
 
     commands = {

lib/spack/spack/cmd/style.py

@@ -13,6 +13,7 @@
 from llnl.util.filesystem import working_dir
 
 import spack.paths
+import spack.util.git
 from spack.util.executable import which
 
 description = "runs source code style checks on spack"
@@ -81,7 +82,7 @@ def changed_files(base="develop", untracked=True, all_files=False, root=None):
     if root is None:
         root = spack.paths.prefix
 
-    git = which("git", required=True)
+    git = spack.util.git.git(required=True)
 
     # ensure base is in the repo
     base_sha = git(

lib/spack/spack/cmd/tutorial.py

@@ -15,8 +15,8 @@
 import spack.cmd.common.arguments as arguments
 import spack.config
 import spack.paths
+import spack.util.git
 import spack.util.gpg
-from spack.util.executable import which
 from spack.util.spack_yaml import syaml_dict
 
 description = "set up spack for our tutorial (WARNING: modifies config!)"
@@ -84,7 +84,7 @@ def tutorial(parser, args):
     # If you don't put this last, you'll get import errors for the code
     # that follows (exacerbated by the various lazy singletons we use)
     tty.msg("Ensuring we're on the releases/v{0}.{1} branch".format(*spack.spack_version_info[:2]))
-    git = which("git", required=True)
+    git = spack.util.git.git(required=True)
     with working_dir(spack.paths.prefix):
         git("checkout", tutorial_branch)
     # NO CODE BEYOND HERE

lib/spack/spack/container/images.py

@@ -10,7 +10,7 @@
 import llnl.util.filesystem as fs
 import llnl.util.tty as tty
 
-import spack.util.executable as executable
+import spack.util.git
 
 #: Global variable used to cache in memory the content of images.json
 _data = None
@@ -97,7 +97,7 @@ def _verify_ref(url, ref, enforce_sha):
     # Do a checkout in a temporary directory
     msg = 'Cloning "{0}" to verify ref "{1}"'.format(url, ref)
     tty.info(msg, stream=sys.stderr)
-    git = executable.which("git", required=True)
+    git = spack.util.git.git(required=True)
     with fs.temporary_dir():
         git("clone", "-q", url, ".")
         sha = git(

lib/spack/spack/fetch_strategy.py

@@ -48,6 +48,7 @@
 import spack.error
 import spack.url
 import spack.util.crypto as crypto
+import spack.util.git
 import spack.util.pattern as pattern
 import spack.util.url as url_util
 import spack.util.web as web_util
@@ -765,7 +766,7 @@ def version_from_git(git_exe):
     @property
     def git(self):
         if not self._git:
-            self._git = which("git", required=True)
+            self._git = spack.util.git.git()
 
             # Disable advice for a quieter fetch
             # https://github.com/git/git/blob/master/Documentation/RelNotes/1.7.2.txt

lib/spack/spack/main.py

@@ -45,7 +45,7 @@
 import spack.store
 import spack.util.debug
 import spack.util.environment
-import spack.util.executable as exe
+import spack.util.git
 import spack.util.path
 from spack.error import SpackError
 
@@ -136,7 +136,7 @@ def get_version():
     version = spack.spack_version
     git_path = os.path.join(spack.paths.prefix, ".git")
     if os.path.exists(git_path):
-        git = exe.which("git")
+        git = spack.util.git.git()
         if not git:
             return version
         rev = git(

lib/spack/spack/repo.py

@@ -41,9 +41,9 @@
 import spack.spec
 import spack.tag
 import spack.util.file_cache
+import spack.util.git
 import spack.util.naming as nm
 import spack.util.path
-from spack.util.executable import which
 
 #: Package modules are imported as spack.pkg.<repo-namespace>.<pkg-name>
 ROOT_PYTHON_NAMESPACE = "spack.pkg"
@@ -198,27 +198,16 @@ class GitExe:
     #
     # Not using -C as that is not supported for git < 1.8.5.
     def __init__(self):
-        self._git_cmd = which("git", required=True)
+        self._git_cmd = spack.util.git.git(required=True)
 
     def __call__(self, *args, **kwargs):
         with working_dir(packages_path()):
             return self._git_cmd(*args, **kwargs)
 
 
-_git = None
-
-
-def get_git():
-    """Get a git executable that runs *within* the packages path."""
-    global _git
-    if _git is None:
-        _git = GitExe()
-    return _git
-
-
 def list_packages(rev):
     """List all packages associated with the given revision"""
-    git = get_git()
+    git = GitExe()
 
     # git ls-tree does not support ... merge-base syntax, so do it manually
     if rev.endswith("..."):
@@ -270,7 +259,7 @@ def get_all_package_diffs(type, rev1="HEAD^1", rev2="HEAD"):
 
     removed, added = diff_packages(rev1, rev2)
 
-    git = get_git()
+    git = GitExe()
     out = git("diff", "--relative", "--name-only", rev1, rev2, output=str).strip()
 
     lines = [] if not out else re.split(r"\s+", out)
@@ -293,7 +282,7 @@ def get_all_package_diffs(type, rev1="HEAD^1", rev2="HEAD"):
 
 def add_package_to_git_stage(packages):
     """add a package to the git stage with `git add`"""
-    git = get_git()
+    git = GitExe()
 
     for pkg_name in packages:
         filename = spack.repo.path.filename_for_package_name(pkg_name)

lib/spack/spack/reporters/cdash.py

@@ -22,11 +22,11 @@
 import spack.fetch_strategy
 import spack.package_base
 import spack.platforms
+import spack.util.git
 from spack.error import SpackError
 from spack.reporter import Reporter
 from spack.reporters.extract import extract_test_parts
 from spack.util.crypto import checksum
-from spack.util.executable import which
 from spack.util.log_parse import parse_log_events
 
 __all__ = ["CDash"]
@@ -108,7 +108,7 @@ def __init__(self, args):
         )
         self.buildIds = collections.OrderedDict()
         self.revision = ""
-        git = which("git")
+        git = spack.util.git.git()
         with working_dir(spack.paths.spack_root):
             self.revision = git("rev-parse", "HEAD", output=str).strip()
         self.generator = "spack-{0}".format(spack.main.get_version())

lib/spack/spack/test/ci.py

@@ -18,6 +18,7 @@
 import spack.environment as ev
 import spack.error
 import spack.paths as spack_paths
+import spack.util.git
 import spack.util.gpg
 import spack.util.spack_yaml as syaml
 
@@ -180,14 +181,13 @@ def test_setup_spack_repro_version(tmpdir, capfd, last_two_git_commits, monkeypa
     monkeypatch.setattr(spack.paths, "prefix", "/garbage")
 
     ret = ci.setup_spack_repro_version(repro_dir, c2, c1)
-    out, err = capfd.readouterr()
+    _, err = capfd.readouterr()
 
     assert not ret
     assert "Unable to find the path" in err
 
     monkeypatch.setattr(spack.paths, "prefix", prefix_save)
-
+    monkeypatch.setattr(spack.util.git, "git", lambda: None)
-    monkeypatch.setattr(spack.util.executable, "which", lambda cmd: None)
 
     ret = ci.setup_spack_repro_version(repro_dir, c2, c1)
     out, err = capfd.readouterr()
@@ -208,39 +208,39 @@ def __call__(self, *args, **kwargs):
 
     git_cmd = mock_git_cmd()
 
-    monkeypatch.setattr(spack.util.executable, "which", lambda cmd: git_cmd)
+    monkeypatch.setattr(spack.util.git, "git", lambda: git_cmd)
 
     git_cmd.check = lambda *a, **k: 1 if len(a) > 2 and a[2] == c2 else 0
     ret = ci.setup_spack_repro_version(repro_dir, c2, c1)
-    out, err = capfd.readouterr()
+    _, err = capfd.readouterr()
 
     assert not ret
     assert "Missing commit: {0}".format(c2) in err
 
     git_cmd.check = lambda *a, **k: 1 if len(a) > 2 and a[2] == c1 else 0
     ret = ci.setup_spack_repro_version(repro_dir, c2, c1)
-    out, err = capfd.readouterr()
+    _, err = capfd.readouterr()
 
     assert not ret
     assert "Missing commit: {0}".format(c1) in err
 
     git_cmd.check = lambda *a, **k: 1 if a[0] == "clone" else 0
     ret = ci.setup_spack_repro_version(repro_dir, c2, c1)
-    out, err = capfd.readouterr()
+    _, err = capfd.readouterr()
 
     assert not ret
     assert "Unable to clone" in err
 
     git_cmd.check = lambda *a, **k: 1 if a[0] == "checkout" else 0
     ret = ci.setup_spack_repro_version(repro_dir, c2, c1)
-    out, err = capfd.readouterr()
+    _, err = capfd.readouterr()
 
     assert not ret
     assert "Unable to checkout" in err
 
     git_cmd.check = lambda *a, **k: 1 if "merge" in a else 0
     ret = ci.setup_spack_repro_version(repro_dir, c2, c1)
-    out, err = capfd.readouterr()
+    _, err = capfd.readouterr()
 
     assert not ret
     assert "Unable to merge {0}".format(c1) in err

lib/spack/spack/test/cmd/blame.py

@@ -13,11 +13,8 @@
 import spack.paths
 import spack.util.spack_json as sjson
 from spack.main import SpackCommand
-from spack.util.executable import which
 
-pytestmark = pytest.mark.skipif(
+pytestmark = pytest.mark.usefixtures("git")
-    not which("git") or not spack.cmd.spack_is_git_repo(), reason="needs git"
-)
 
 blame = SpackCommand("blame")
 

lib/spack/spack/test/cmd/ci.py

@@ -31,7 +31,6 @@
 from spack.schema.database_index import schema as db_idx_schema
 from spack.schema.gitlab_ci import schema as gitlab_ci_schema
 from spack.spec import CompilerSpec, Spec
-from spack.util.executable import which
 from spack.util.pattern import Bunch
 
 ci_cmd = spack.main.SpackCommand("ci")
@@ -54,14 +53,13 @@ def ci_base_environment(working_env, tmpdir):
 
 
 @pytest.fixture(scope="function")
-def mock_git_repo(tmpdir):
+def mock_git_repo(git, tmpdir):
     """Create a mock git repo with two commits, the last one creating
     a .gitlab-ci.yml"""
 
     repo_path = tmpdir.join("mockspackrepo").strpath
     mkdirp(repo_path)
 
-    git = which("git", required=True)
     with working_dir(repo_path):
         git("init")
 

lib/spack/spack/test/cmd/is_git_repo.py

@@ -13,37 +13,21 @@
 from llnl.util.filesystem import mkdirp, working_dir
 
 import spack
-from spack.util.executable import which
 from spack.version import ver
 
-git = which("git")
-git_required_version = "2.17.0"
-
-
-def check_git_version():
-    """Check if git version is new enough for worktree functionality.
-    Return True if requirements are met.
-
-    The latest required functionality is `worktree remove` which was only added
-    in 2.17.0.
-
-    Refer:
-    https://github.com/git/git/commit/cc73385cf6c5c229458775bc92e7dbbe24d11611
-    """
-    git_version = spack.fetch_strategy.GitFetchStrategy.version_from_git(git)
-    return git_version >= ver(git_required_version)
-
-
-pytestmark = pytest.mark.skipif(
-    not git or not check_git_version(), reason="we need git to test if we are in a git repo"
-)
-
 
 @pytest.fixture(scope="function")
-def git_tmp_worktree(tmpdir, mock_git_version_info):
+def git_tmp_worktree(git, tmpdir, mock_git_version_info):
     """Create new worktree in a temporary folder and monkeypatch
     spack.paths.prefix to point to it.
     """
+
+    # We need `git worktree remove` for this fixture, which was added in 2.17.0.
+    # See https://github.com/git/git/commit/cc73385cf6c5c229458775bc92e7dbbe24d11611
+    git_version = spack.fetch_strategy.GitFetchStrategy.version_from_git(git)
+    if git_version < ver("2.17.0"):
+        pytest.skip("git_tmp_worktree requires git v2.17.0")
+
     with working_dir(mock_git_version_info[0]):
         # TODO: This is fragile and should be high priority for
         # follow up fixes. 27021

lib/spack/spack/test/cmd/pkg.py

@@ -16,9 +16,6 @@
 import spack.cmd.pkg
 import spack.main
 import spack.repo
-from spack.util.executable import which
-
-pytestmark = pytest.mark.skipif(not which("git"), reason="spack pkg tests require git")
 
 #: new fake package template
 pkg_template = """\
@@ -40,7 +37,7 @@ def install(self, spec, prefix):
 
 # Force all tests to use a git repository *in* the mock packages repo.
 @pytest.fixture(scope="module")
-def mock_pkg_git_repo(tmpdir_factory):
+def mock_pkg_git_repo(git, tmpdir_factory):
     """Copy the builtin.mock repo and make a mutable git repo inside it."""
     tmproot = tmpdir_factory.mktemp("mock_pkg_git_repo")
     repo_path = tmproot.join("builtin.mock")
@@ -49,7 +46,6 @@ def mock_pkg_git_repo(tmpdir_factory):
     mock_repo = spack.repo.RepoPath(str(repo_path))
     mock_repo_packages = mock_repo.repos[0].packages_path
 
-    git = which("git", required=True)
     with working_dir(mock_repo_packages):
         git("init")
 
@@ -110,7 +106,7 @@ def test_mock_packages_path(mock_packages):
     assert spack.repo.packages_path() == spack.repo.path.get_repo("builtin.mock").packages_path
 
 
-def test_pkg_add(mock_pkg_git_repo):
+def test_pkg_add(git, mock_pkg_git_repo):
     with working_dir(mock_pkg_git_repo):
         mkdirp("pkg-e")
         with open("pkg-e/package.py", "w") as f:
@@ -118,7 +114,6 @@ def test_pkg_add(mock_pkg_git_repo):
 
     pkg("add", "pkg-e")
 
-    git = which("git", required=True)
     with working_dir(mock_pkg_git_repo):
         try:
             assert "A  pkg-e/package.py" in git("status", "--short", output=str)

lib/spack/spack/test/cmd/style.py

@@ -24,18 +24,12 @@
 style = spack.main.SpackCommand("style")
 
 
-def has_develop_branch():
+@pytest.fixture(autouse=True)
-    git = which("git")
+def has_develop_branch(git):
-    if not git:
+    """spack style requires git and a develop branch to run -- skip if we're missing either."""
-        return False
     git("show-ref", "--verify", "--quiet", "refs/heads/develop", fail_on_error=False)
-    return git.returncode == 0
+    if git.returncode != 0:
-
+        pytest.skip("requires git and a develop branch")
-
-# spack style requires git to run -- skip the tests if it's not there
-pytestmark = pytest.mark.skipif(
-    not has_develop_branch(), reason="requires git with develop branch"
-)
 
 
 @pytest.fixture(scope="function")
@@ -77,9 +71,8 @@ def flake8_package_with_errors(scope="function"):
     yield tmp
 
 
-def test_changed_files_from_git_rev_base(tmpdir, capfd):
+def test_changed_files_from_git_rev_base(git, tmpdir, capfd):
     """Test arbitrary git ref as base."""
-    git = which("git", required=True)
     with tmpdir.as_cwd():
         git("init")
         git("checkout", "-b", "main")
@@ -97,10 +90,9 @@ def test_changed_files_from_git_rev_base(tmpdir, capfd):
         assert changed_files(base="HEAD~") == ["bin/spack"]
 
 
-def test_changed_no_base(tmpdir, capfd):
+def test_changed_no_base(git, tmpdir, capfd):
     """Ensure that we fail gracefully with no base branch."""
     tmpdir.join("bin").ensure("spack")
-    git = which("git", required=True)
     with tmpdir.as_cwd():
         git("init")
         git("config", "user.name", "test user")
@@ -165,10 +157,8 @@ def test_style_is_package(tmpdir):
 
 
 @pytest.fixture
-def external_style_root(flake8_package_with_errors, tmpdir):
+def external_style_root(git, flake8_package_with_errors, tmpdir):
     """Create a mock git repository for running spack style."""
-    git = which("git", required=True)
-
     # create a sort-of spack-looking directory
     script = tmpdir / "bin" / "spack"
     script.ensure()

lib/spack/spack/test/conftest.py

@@ -46,6 +46,7 @@
 import spack.subprocess_context
 import spack.test.cray_manifest
 import spack.util.executable
+import spack.util.git
 import spack.util.gpg
 import spack.util.spack_yaml as syaml
 import spack.util.url as url_util
@@ -66,12 +67,20 @@ def ensure_configuration_fixture_run_before(request):
         request.getfixturevalue("mutable_config")
 
 
+@pytest.fixture(scope="session")
+def git():
+    """Fixture for tests that use git."""
+    if not spack.util.git.git():
+        pytest.skip("requires git to be installed")
+
+    return spack.util.git.git(required=True)
+
+
 #
 # Return list of shas for latest two git commits in local spack repo
 #
 @pytest.fixture(scope="session")
-def last_two_git_commits():
+def last_two_git_commits(git):
-    git = spack.util.executable.which("git", required=True)
     spack_git_path = spack.paths.prefix
     with working_dir(spack_git_path):
         git_log_out = git("log", "-n", "2", output=str, error=os.devnull)
@@ -98,7 +107,7 @@ def override_git_repos_cache_path(tmpdir):
 
 
 @pytest.fixture
-def mock_git_version_info(tmpdir, override_git_repos_cache_path):
+def mock_git_version_info(git, tmpdir, override_git_repos_cache_path):
     """Create a mock git repo with known structure
 
     The structure of commits in this repo is as follows::
@@ -123,7 +132,6 @@ def mock_git_version_info(tmpdir, override_git_repos_cache_path):
     version tags on multiple branches, and version order is not equal to time
     order or topological order.
     """
-    git = spack.util.executable.which("git", required=True)
     repo_path = str(tmpdir.mkdir("git_repo"))
     filename = "file.txt"
 
@@ -1100,7 +1108,9 @@ def mock_archive(request, tmpdir_factory):
     """Creates a very simple archive directory with a configure script and a
     makefile that installs to a prefix. Tars it up into an archive.
     """
-    tar = spack.util.executable.which("tar", required=True)
+    tar = spack.util.executable.which("tar")
+    if not tar:
+        pytest.skip("requires tar to be installed")
 
     tmpdir = tmpdir_factory.mktemp("mock-archive-dir")
     tmpdir.ensure(spack.stage._source_path_subdir, dir=True)
@@ -1299,7 +1309,7 @@ def get_date():
 
 
 @pytest.fixture(scope="session")
-def mock_git_repository(tmpdir_factory):
+def mock_git_repository(git, tmpdir_factory):
     """Creates a git repository multiple commits, branches, submodules, and
     a tag. Visual representation of the commit history (starting with the
     earliest commit at c0)::
@@ -1323,8 +1333,6 @@ def mock_git_repository(tmpdir_factory):
     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
@@ -1452,7 +1460,9 @@ def mock_git_repository(tmpdir_factory):
 @pytest.fixture(scope="session")
 def mock_hg_repository(tmpdir_factory):
     """Creates a very simple hg repository with two commits."""
-    hg = spack.util.executable.which("hg", required=True)
+    hg = spack.util.executable.which("hg")
+    if not hg:
+        pytest.skip("requires mercurial to be installed")
 
     tmpdir = tmpdir_factory.mktemp("mock-hg-repo-dir")
     tmpdir.ensure(spack.stage._source_path_subdir, dir=True)
@@ -1490,7 +1500,10 @@ def mock_hg_repository(tmpdir_factory):
 @pytest.fixture(scope="session")
 def mock_svn_repository(tmpdir_factory):
     """Creates a very simple svn repository with two commits."""
-    svn = spack.util.executable.which("svn", required=True)
+    svn = spack.util.executable.which("svn")
+    if not svn:
+        pytest.skip("requires svn to be installed")
+
     svnadmin = spack.util.executable.which("svnadmin", required=True)
 
     tmpdir = tmpdir_factory.mktemp("mock-svn-stage")

lib/spack/spack/test/git_fetch.py

@@ -16,17 +16,13 @@
 from spack.fetch_strategy import GitFetchStrategy
 from spack.spec import Spec
 from spack.stage import Stage
-from spack.util.executable import which
 from spack.version import ver
 
-pytestmark = pytest.mark.skipif(not which("git"), reason="requires git to be installed")
-
-
 _mock_transport_error = "Mock HTTP transport error"
 
 
 @pytest.fixture(params=[None, "1.8.5.2", "1.8.5.1", "1.7.10", "1.7.1", "1.7.0"])
-def git_version(request, monkeypatch):
+def git_version(git, request, monkeypatch):
     """Tests GitFetchStrategy behavior for different git versions.
 
     GitFetchStrategy tries to optimize using features of newer git
@@ -34,7 +30,6 @@ def git_version(request, monkeypatch):
     paths for old versions still work, we fake it out here and make it
     use the backward-compatibility code paths with newer git versions.
     """
-    git = which("git", required=True)
     real_git_version = spack.fetch_strategy.GitFetchStrategy.version_from_git(git)
 
     if request.param is None:
@@ -83,6 +78,7 @@ def test_bad_git(tmpdir, mock_bad_git):
 @pytest.mark.parametrize("type_of_test", ["default", "branch", "tag", "commit"])
 @pytest.mark.parametrize("secure", [True, False])
 def test_fetch(
+    git,
     type_of_test,
     secure,
     mock_git_repository,
@@ -217,7 +213,7 @@ def test_debug_fetch(
             assert os.path.isdir(s.package.stage.source_path)
 
 
-def test_git_extra_fetch(tmpdir):
+def test_git_extra_fetch(git, tmpdir):
     """Ensure a fetch after 'expanding' is effectively a no-op."""
     testpath = str(tmpdir)
 
@@ -228,7 +224,7 @@ def test_git_extra_fetch(tmpdir):
         shutil.rmtree(stage.source_path)
 
 
-def test_needs_stage():
+def test_needs_stage(git):
     """Trigger a NoStageError when attempt a fetch without a stage."""
     with pytest.raises(
         spack.fetch_strategy.NoStageError, match=r"set_stage.*before calling fetch"

lib/spack/spack/test/main.py

@@ -3,7 +3,6 @@
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
-import os
 import sys
 
 import pytest
@@ -11,6 +10,8 @@
 import llnl.util.filesystem as fs
 
 import spack.paths
+import spack.util.executable as exe
+import spack.util.git
 from spack.main import get_version, main
 
 pytestmark = pytest.mark.skipif(
@@ -18,7 +19,7 @@
 )
 
 
-def test_version_git_nonsense_output(tmpdir, working_env):
+def test_version_git_nonsense_output(tmpdir, working_env, monkeypatch):
     git = str(tmpdir.join("git"))
     with open(git, "w") as f:
         f.write(
@@ -28,11 +29,11 @@ def test_version_git_nonsense_output(tmpdir, working_env):
         )
     fs.set_executable(git)
 
-    os.environ["PATH"] = str(tmpdir)
+    monkeypatch.setattr(spack.util.git, "git", lambda: exe.which(git))
     assert spack.spack_version == get_version()
 
 
-def test_version_git_fails(tmpdir, working_env):
+def test_version_git_fails(tmpdir, working_env, monkeypatch):
     git = str(tmpdir.join("git"))
     with open(git, "w") as f:
         f.write(
@@ -43,11 +44,11 @@ def test_version_git_fails(tmpdir, working_env):
         )
     fs.set_executable(git)
 
-    os.environ["PATH"] = str(tmpdir)
+    monkeypatch.setattr(spack.util.git, "git", lambda: exe.which(git))
     assert spack.spack_version == get_version()
 
 
-def test_git_sha_output(tmpdir, working_env):
+def test_git_sha_output(tmpdir, working_env, monkeypatch):
     git = str(tmpdir.join("git"))
     sha = "26552533be04e83e66be2c28e0eb5011cb54e8fa"
     with open(git, "w") as f:
@@ -60,7 +61,7 @@ def test_git_sha_output(tmpdir, working_env):
         )
     fs.set_executable(git)
 
-    os.environ["PATH"] = str(tmpdir)
+    monkeypatch.setattr(spack.util.git, "git", lambda: exe.which(git))
     expected = "{0} ({1})".format(spack.spack_version, sha)
     assert expected == get_version()
 
@@ -70,18 +71,22 @@ def test_get_version_no_repo(tmpdir, monkeypatch):
     assert spack.spack_version == get_version()
 
 
-def test_get_version_no_git(tmpdir, working_env):
+def test_get_version_no_git(tmpdir, working_env, monkeypatch):
-    os.environ["PATH"] = str(tmpdir)
+    monkeypatch.setattr(spack.util.git, "git", lambda: None)
     assert spack.spack_version == get_version()
 
 
-def test_main_calls_get_version(tmpdir, capsys, working_env):
+def test_main_calls_get_version(tmpdir, capsys, working_env, monkeypatch):
-    os.environ["PATH"] = str(tmpdir)
+    # act like git is not found in the PATH
+    monkeypatch.setattr(spack.util.git, "git", lambda: None)
+
+    # make sure we get a bare version (without commit) when this happens
     main(["-V"])
-    assert spack.spack_version == capsys.readouterr()[0].strip()
+    out, err = capsys.readouterr()
+    assert spack.spack_version == out.strip()
 
 
-def test_get_version_bad_git(tmpdir, working_env):
+def test_get_version_bad_git(tmpdir, working_env, monkeypatch):
     bad_git = str(tmpdir.join("git"))
     with open(bad_git, "w") as f:
         f.write(
@@ -91,5 +96,5 @@ def test_get_version_bad_git(tmpdir, working_env):
         )
     fs.set_executable(bad_git)
 
-    os.environ["PATH"] = str(tmpdir)
+    monkeypatch.setattr(spack.util.git, "git", lambda: exe.which(bad_git))
     assert spack.spack_version == get_version()

lib/spack/spack/test/mirror.py

@@ -104,33 +104,24 @@ def test_url_mirror(mock_archive):
     repos.clear()
 
 
-@pytest.mark.skipif(not which("git"), reason="requires git to be installed")
+def test_git_mirror(git, mock_git_repository):
-def test_git_mirror(mock_git_repository):
     set_up_package("git-test", mock_git_repository, "git")
     check_mirror()
     repos.clear()
 
 
-@pytest.mark.skipif(
-    not which("svn") or not which("svnadmin"), reason="requires subversion to be installed"
-)
 def test_svn_mirror(mock_svn_repository):
     set_up_package("svn-test", mock_svn_repository, "svn")
     check_mirror()
     repos.clear()
 
 
-@pytest.mark.skipif(not which("hg"), reason="requires mercurial to be installed")
 def test_hg_mirror(mock_hg_repository):
     set_up_package("hg-test", mock_hg_repository, "hg")
     check_mirror()
     repos.clear()
 
 
-@pytest.mark.skipif(
-    not all([which("svn"), which("hg"), which("git")]),
-    reason="requires subversion, git, and mercurial to be installed",
-)
 def test_all_mirror(mock_git_repository, mock_svn_repository, mock_hg_repository, mock_archive):
 
     set_up_package("git-test", mock_git_repository, "git")

lib/spack/spack/test/versions.py

@@ -16,7 +16,6 @@
 
 import spack.package_base
 import spack.spec
-from spack.util.executable import which
 from spack.version import (
     GitVersion,
     Version,
@@ -593,7 +592,7 @@ def test_invalid_versions(version_str):
 
 
 @pytest.mark.skipif(sys.platform == "win32", reason="Not supported on Windows (yet)")
-def test_versions_from_git(mock_git_version_info, monkeypatch, mock_packages):
+def test_versions_from_git(git, mock_git_version_info, monkeypatch, mock_packages):
     repo_path, filename, commits = mock_git_version_info
     monkeypatch.setattr(
         spack.package_base.PackageBase, "git", "file://%s" % repo_path, raising=False
@@ -607,7 +606,7 @@ def test_versions_from_git(mock_git_version_info, monkeypatch, mock_packages):
         ]
 
         with working_dir(repo_path):
-            which("git")("checkout", commit)
+            git("checkout", commit)
         with open(os.path.join(repo_path, filename), "r") as f:
             expected = f.read()
 

lib/spack/spack/util/git.py

@@ -0,0 +1,30 @@
+# Copyright 2013-2022 Lawrence Livermore National Security, LLC and other
+# Spack Project Developers. See the top-level COPYRIGHT file for details.
+#
+# SPDX-License-Identifier: (Apache-2.0 OR MIT)
+"""Single util module where Spack should get a git executable."""
+
+import sys
+from typing import Optional
+
+import llnl.util.lang
+
+import spack.util.executable as exe
+
+
+@llnl.util.lang.memoized
+def git(required: bool = False):
+    """Get a git executable.
+
+    Arguments:
+        required: if ``True``, fail if ``git`` is not found. By default return ``None``.
+    """
+    git: Optional[exe.Executable] = exe.which("git", required=required)
+
+    # If we're running under pytest, add this to ignore the fix for CVE-2022-39253 in
+    # git 2.38.1+. Do this in one place; we need git to do this in all parts of Spack.
+    if git and "pytest" in sys.modules:
+        git.add_default_arg("-c")
+        git.add_default_arg("protocol.file.allow=always")
+
+    return git