Merge branch 'develop' into psakiev/f/git-version-refactor

Ensure that if a commit variant is on the spec fetch operations
use the commit

lib/spack/docs/basic_usage.rst

@@ -1222,6 +1222,23 @@ A version specifier can also be a list of ranges and specific versions,
 separated by commas.  For example, ``@1.0:1.5,=1.7.1`` matches any version
 in the range ``1.0:1.5`` and the specific version ``1.7.1``.
 
+^^^^^^^^^^^^^^^^^
+Binary Provenance
+^^^^^^^^^^^^^^^^^
+
+Spack versions are paired to attributes that determine the source code Spack
+will use to build. Checksummed assets are preferred but there are a few 
+notable exceptions such as git branches and tags i.e ``pkg@develop``.
+These versions do not naturally have source provenance because they refer to a range
+of commits (branches) or can be changed outside the spack packaging infrastructure
+(tags). Without source provenace we can not have binary provenance.
+
+Spack has a reserved variant to allow users to complete source and binary provenance
+for these cases: ``pkg@develop commit=<SHA>``.  The ``commit`` variant must be supplied
+the full 40 character commit SHA. Using a partial commit SHA or assigning
+the ``commit`` variant to a version that is not using a branch or tag reference will
+lead to an error during concretization.
+
 ^^^^^^^^^^^^
 Git versions
 ^^^^^^^^^^^^

lib/spack/docs/packaging_guide.rst

@@ -1267,7 +1267,11 @@ Git fetching supports the following parameters to ``version``:
   If paths provided are directories then all the subdirectories and associated files
   will also be cloned.
 
-Only one of ``tag``, ``branch``, or ``commit`` can be used at a time.
+``tag`` and ``branch`` should not be combined in the version parameters. We strongly
+recommend that all ``tag`` entries be paired with ``commit``. Providing the full 
+``commit`` SHA hash allows for Spack to preserve binary provenance for all binaries. 
+This is due to the fact that git tags and branches are mutable references to commits, 
+but git commits are guaranteed to be unique points in the git history.
 
 The destination directory for the clone is the standard stage source path.
 

lib/spack/spack/build_systems/msbuild.py

@@ -24,10 +24,6 @@ class MSBuildPackage(spack.package_base.PackageBase):
     build_system("msbuild")
     conflicts("platform=linux", when="build_system=msbuild")
     conflicts("platform=darwin", when="build_system=msbuild")
-    conflicts("platform=freebsd", when="build_system=msbuild")
-
-    def define(self, msbuild_arg, value):
-        return define(msbuild_arg, value)
 
 
 @spack.builder.builder("msbuild")
@@ -91,7 +87,7 @@ def define_targets(self, *targets):
         return "/target:" + ";".join(targets) if targets else ""
 
     def define(self, msbuild_arg, value):
-        return define(msbuild_arg, value)
+        return "/p:{}={}".format(msbuild_arg, value)
 
     def msbuild_args(self):
         """Define build arguments to MSbuild. This is an empty list by default.
@@ -125,7 +121,3 @@ def install(
             pkg.module.msbuild(
                 *self.msbuild_install_args(), self.define_targets(*self.install_targets)
             )
-
-
-def define(msbuild_arg, value):
-    return "/p:{}={}".format(msbuild_arg, value)

lib/spack/spack/fetch_strategy.py

@@ -1710,13 +1710,15 @@ def for_package_version(pkg, version=None):
         version = pkg.version
 
     # if it's a commit, we must use a GitFetchStrategy
-    if isinstance(version, spack.version.GitVersion):
+    commit_sha = pkg.spec.variants.get("commit", None)
+    if isinstance(version, spack.version.GitVersion) or commit_sha:
         if not hasattr(pkg, "git"):
             raise spack.error.FetchError(
                 f"Cannot fetch git version for {pkg.name}. Package has no 'git' attribute"
             )
         # Populate the version with comparisons to other commits
-        version.attach_lookup(spack.version.git_ref_lookup.GitRefLookup(pkg.name))
+        if isinstance(version, spack.version.GitVersion):
+            version.attach_lookup(spack.version.git_ref_lookup.GitRefLookup(pkg.name))
 
         # For GitVersion, we have no way to determine whether a ref is a branch or tag
         # Fortunately, we handle branches and tags identically, except tags are
@@ -1724,16 +1726,27 @@ def for_package_version(pkg, version=None):
         # We call all non-commit refs tags in this context, at the cost of a slight
         # performance hit for branches on older versions of git.
         # Branches cannot be cached, so we tell the fetcher not to cache tags/branches
-        ref_type = "commit" if version.is_commit else "tag"
-        kwargs = {"git": pkg.git, ref_type: version.ref, "no_cache": True}
 
-        kwargs["submodules"] = getattr(pkg, "submodules", False)
+        # TODO(psakiev) eventually we should  only need to clone based on the commit
+        ref_type = None
+        ref_value = None
+        if commit_sha:
+            ref_type = "commit"
+            ref_value = commit_sha.value
+        else:
+            ref_type = "commit" if version.is_commit else "tag"
+            ref_value = version.ref
+
+        kwargs = {ref_type: ref_value, "no_cache": ref_type != "commit"}
+        kwargs["git"] = pkg.version_or_package_attr("git", version)
+        kwargs["submodules"] = pkg.version_or_package_attr("submodules", version, False)
 
         # if the ref_version is a known version from the package, use that version's
-        # submodule specifications
-        ref_version_attributes = pkg.versions.get(pkg.version.ref_version)
-        if ref_version_attributes:
-            kwargs["submodules"] = ref_version_attributes.get("submodules", kwargs["submodules"])
+        # attributes
+        ref_version = getattr(pkg.version, "ref_version", None)
+        if ref_version:
+            kwargs["git"] = pkg.version_or_package_attr("git", ref_version)
+            kwargs["submodules"] = pkg.version_or_package_attr("submodules", ref_version, False)
 
         fetcher = GitFetchStrategy(**kwargs)
         return fetcher

lib/spack/spack/package_base.py

@@ -1001,6 +1001,42 @@ def detect_dev_src_change(self) -> bool:
         assert dev_path_var and record, "dev_path variant and record must be present"
         return fsys.recursive_mtime_greater_than(dev_path_var.value, record.installation_time)
 
+    @classmethod
+    def version_or_package_attr(cls, attr, version, default=None):
+        """
+        Get an attribute that could be on the version or package with preference to the version
+        """
+        version_attrs = cls.versions.get(version)
+        if version_attrs and attr in version_attrs:
+            return version_attrs.get(attr)
+        value = getattr(cls, attr, default)
+        if value is None:
+            raise PackageError(f"{attr} attribute not defined on {cls.name}")
+        return value
+
+    @classmethod
+    def needs_commit(cls, version) -> bool:
+        """
+        Method for checking if the package instance needs a commit sha to be found
+        """
+        if isinstance(version, GitVersion):
+            return True
+
+        ver_attrs = cls.versions.get(version)
+        if ver_attrs:
+            return bool(ver_attrs.get("commit") or ver_attrs.get("tag") or ver_attrs.get("branch"))
+
+        return False
+
+    def resolve_binary_provenance(self) -> None:
+        """
+        Method to ensure concrete spec has binary provenance.
+        Base implementation will look up git commits when appropriate.
+        Packages may override this implementation for custom implementations
+        """
+        # TODO in follow on PR adding here so SNL team can begin work ahead of spack core
+        pass
+
     def all_urls_for_version(self, version: StandardVersion) -> List[str]:
         """Return all URLs derived from version_urls(), url, urls, and
         list_url (if it contains a version) in a package in that order.

lib/spack/spack/solver/asp.py

@@ -1519,6 +1519,9 @@ def __init__(self, tests: bool = False):
         self.assumptions: List[Tuple["clingo.Symbol", bool]] = []  # type: ignore[name-defined]
         self.declared_versions: Dict[str, List[DeclaredVersion]] = collections.defaultdict(list)
         self.possible_versions: Dict[str, Set[GitOrStandardVersion]] = collections.defaultdict(set)
+        self.git_commit_versions: Dict[str, Dict[GitOrStandardVersion, str]] = (
+            collections.defaultdict(dict)
+        )
         self.deprecated_versions: Dict[str, Set[GitOrStandardVersion]] = collections.defaultdict(
             set
         )
@@ -1592,6 +1595,11 @@ def key_fn(version):
                 )
             )
 
+        for v in self.possible_versions[pkg.name]:
+            if pkg.needs_commit(v):
+                commit = pkg.version_or_package_attr("commit", v, "")
+                self.git_commit_versions[pkg.name][v] = commit
+
         # Declare deprecated versions for this package, if any
         deprecated = self.deprecated_versions[pkg.name]
         for v in sorted(deprecated):
@@ -2689,6 +2697,8 @@ def define_package_versions_and_validate_preferences(
             if pkg_name not in packages_yaml or "version" not in packages_yaml[pkg_name]:
                 continue
 
+            # TODO(psakiev) Need facts about versions
+            # - requires_commit (associated with tag or branch)
             version_defs: List[GitOrStandardVersion] = []
 
             for vstr in packages_yaml[pkg_name]["version"]:
@@ -2880,12 +2890,22 @@ def virtual_providers(self):
 
     def define_version_constraints(self):
         """Define what version_satisfies(...) means in ASP logic."""
+
+        for pkg_name, versions in sorted(self.possible_versions.items()):
+            for v in versions:
+                if v in self.git_commit_versions[pkg_name]:
+                    sha = self.git_commit_versions[pkg_name].get(v)
+                    if sha:
+                        self.gen.fact(fn.pkg_fact(pkg_name, fn.version_has_commit(v, sha)))
+                    else:
+                        self.gen.fact(fn.pkg_fact(pkg_name, fn.version_needs_commit(v)))
+        self.gen.newline()
+
         for pkg_name, versions in sorted(self.version_constraints):
             # generate facts for each package constraint and the version
             # that satisfies it
             for v in sorted(v for v in self.possible_versions[pkg_name] if v.satisfies(versions)):
                 self.gen.fact(fn.pkg_fact(pkg_name, fn.version_satisfies(versions, v)))
-
             self.gen.newline()
 
     def collect_virtual_constraints(self):
@@ -3180,6 +3200,10 @@ def setup(
             allow_deprecated=allow_deprecated, require_checksum=checksummed
         )
 
+        self.gen.h1("Infinity Versions")
+        for i, v in enumerate(spack.version.infinity_versions):
+            self.gen.fact(fn.infinity_version(v, i))
+
         self.gen.h1("Package Constraints")
         for pkg in sorted(self.pkgs):
             self.gen.h2("Package rules: %s" % pkg)
@@ -3188,6 +3212,7 @@ def setup(
 
         self.gen.h1("Special variants")
         self.define_auto_variant("dev_path", multi=False)
+        self.define_auto_variant("commit", multi=False)
         self.define_auto_variant("patches", multi=True)
 
         self.gen.h1("Develop specs")
@@ -4146,6 +4171,10 @@ def build_specs(self, function_tuples):
                         spack.version.git_ref_lookup.GitRefLookup(spec.fullname)
                     )
 
+        # check for commits must happen after all version adaptations are complete
+        for s in self._specs.values():
+            _specs_with_commits(s)
+
         specs = self.execute_explicit_splices()
         return specs
 
@@ -4186,6 +4215,29 @@ def execute_explicit_splices(self):
         return specs
 
 
+def _specs_with_commits(spec):
+    if not spec.package.needs_commit(spec.version):
+        return
+
+    # check integrity of specified commit shas
+    if "commit" in spec.variants:
+        invalid_commit_msg = (
+            f"Internal Error: {spec.name}'s assigned commit {spec.variants['commit'].value}"
+            " does not meet commit syntax requirements."
+        )
+        assert vn.is_git_commit_sha(spec.variants["commit"].value), invalid_commit_msg
+
+    spec.package.resolve_binary_provenance()
+    # TODO(psakiev) assert commit is associated with ref
+
+    if isinstance(spec.version, spack.version.GitVersion):
+        if not spec.version.commit_sha:
+            # TODO(psakiev) this will be a failure when commit look up is automated
+            return
+        if "commit" not in spec.variants:
+            spec.variants["commit"] = vt.SingleValuedVariant("commit", spec.version.commit_sha)
+
+
 def _inject_patches_variant(root: spack.spec.Spec) -> None:
     # This dictionary will store object IDs rather than Specs as keys
     # since the Spec __hash__ will change as patches are added to them

lib/spack/spack/solver/concretize.lp

@@ -343,9 +343,33 @@ attr("node_version_satisfies", node(ID, Package), Constraint)
   :- attr("version", node(ID, Package), Version),
      pkg_fact(Package, version_satisfies(Constraint, Version)).
 
+% if a version needs a commit or has one it can use the commit variant
+can_accept_commit(Package, Version) :- pkg_fact(Package, version_needs_commit(Version)).
+can_accept_commit(Package, Version) :- pkg_fact(Package, version_has_commit(Version, _)).
+
+% Specs with a commit variant can't use versions that don't need commits
+error(10, "Cannot use commit variant with '{0}@={1}'", Package, Version)
+  :- attr("version", node(ID, Package), Version),
+     not can_accept_commit(Package, Version),
+     attr("variant_value", node(ID, Package), "commit", _).
+
+error(10, "Commit '{0}' must match package.py value '{1}' for '{2}@={3}'", Vsha, Psha, Package, Version)
+  :- attr("version", node(ID, Package), Version),
+     attr("variant_value", node(ID, Package), "commit", Vsha),
+     pkg_fact(Package, version_has_commit(Version, Psha)),
+     Vsha != Psha.
+
+% need a rule for above, can't select a version that needs a commit if variant matches a version that has a commit
+  :- attr("version", node(ID, Package), VersionA),
+     attr("variant_value", node(ID, Package), "commit", Vsha),
+     pkg_fact(Package, version_has_commit(VersionB, Psha)),
+     Vsha == Psha,
+     VersionA != VersionB.
+
 #defined version_satisfies/3.
 #defined deprecated_versions_not_allowed/0.
 #defined deprecated_version/2.
+#defined can_accept_commit/2.
 
 %-----------------------------------------------------------------------------
 % Spec conditions and imposed constraints

lib/spack/spack/spec.py

@@ -2884,7 +2884,7 @@ def _validate_version(self):
             v.ref_version
         except vn.VersionLookupError:
             before = self.cformat("{name}{@version}{/hash:7}")
-            v._ref_version = vn.StandardVersion.from_string("develop")
+            v.std_version = vn.StandardVersion.from_string("develop")
             tty.debug(
                 f"the git sha of {before} could not be resolved to spack version; "
                 f"it has been replaced by {self.cformat('{name}{@version}{/hash:7}')}."
@@ -4483,7 +4483,7 @@ def attach_git_version_lookup(self):
         if not self.name:
             return
         for v in self.versions:
-            if isinstance(v, vn.GitVersion) and v._ref_version is None:
+            if isinstance(v, vn.GitVersion) and v.std_version is None:
                 v.attach_lookup(spack.version.git_ref_lookup.GitRefLookup(self.fullname))
 
     def original_spec_format(self) -> int:
@@ -4679,7 +4679,7 @@ def substitute_abstract_variants(spec: Spec):
         if v.concrete and v.type == vt.VariantType.MULTI:
             continue
 
-        if name == "dev_path":
+        if name in ("dev_path", "commit"):
             v.type = vt.VariantType.SINGLE
             v.concrete = True
             continue

lib/spack/spack/test/cmd/find.py

@@ -5,6 +5,7 @@
 import argparse
 import json
 import os
+import pathlib
 import sys
 from textwrap import dedent
 
@@ -14,6 +15,7 @@
 import spack.cmd.find
 import spack.concretize
 import spack.environment as ev
+import spack.package_base
 import spack.repo
 import spack.store
 import spack.user_environment as uenv
@@ -607,3 +609,18 @@ def _nresults(_qresult):
         assert _nresults(_query(e, "--tag=tag0")) == (1, 0)
         assert _nresults(_query(e, "--tag=tag1")) == (1, 1)
         assert _nresults(_query(e, "--tag=tag2")) == (0, 1)
+
+
+@pytest.mark.usefixtures("install_mockery", "mock_fetch", "mutable_mock_env_path")
+def test_find_based_on_commit_sha(mock_git_version_info, monkeypatch):
+    repo_path, filename, commits = mock_git_version_info
+    file_url = pathlib.Path(repo_path).as_uri()
+
+    monkeypatch.setattr(spack.package_base.PackageBase, "git", file_url, raising=False)
+
+    env("create", "test")
+    with ev.read("test"):
+        install("--fake", "--add", f"git-test-commit commit={commits[0]}")
+
+        output = find(f"commit={commits[0]}")
+        assert "git-test-commit" in output

lib/spack/spack/test/concretization/core.py

@@ -2,6 +2,7 @@
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 import os
+import pathlib
 import platform
 import sys
 
@@ -21,6 +22,7 @@
 import spack.detection
 import spack.error
 import spack.hash_types as ht
+import spack.package_base
 import spack.paths
 import spack.platforms
 import spack.platforms.test
@@ -2583,6 +2585,12 @@ def test_correct_external_is_selected_from_packages_yaml(self, mutable_config):
         assert s.satisfies("~opt")
         assert s.prefix == "/tmp/prefix2"
 
+    def test_git_based_version_must_exist_to_use_ref(self):
+        # gmake should fail, only has sha256
+        with pytest.raises(spack.error.UnsatisfiableSpecError) as e:
+            spack.concretize.concretize_one(f"gmake commit={'a' * 40}")
+            assert "Cannot use commit variant with" in e.value.message
+
 
 @pytest.fixture()
 def duplicates_test_repository():
@@ -3115,6 +3123,107 @@ def test_spec_unification(unify, mutable_config, mock_packages):
         _ = spack.cmd.parse_specs([a_restricted, b], concretize=True)
 
 
+@pytest.mark.usefixtures("mutable_config", "mock_packages", "do_not_check_runtimes_on_reuse")
+@pytest.mark.parametrize(
+    "spec_str, error_type",
+    [
+        (f"git-ref-package@main commit={'a' * 40}", None),
+        (f"git-ref-package@main commit={'a' * 39}", AssertionError),
+        (f"git-ref-package@2.1.6 commit={'a' * 40}", spack.error.UnsatisfiableSpecError),
+        (f"git-ref-package@git.2.1.6=2.1.6 commit={'a' * 40}", None),
+        (f"git-ref-package@git.{'a' * 40}=2.1.6 commit={'a' * 40}", None),
+    ],
+)
+def test_spec_containing_commit_variant(spec_str, error_type):
+    spec = spack.spec.Spec(spec_str)
+    if error_type is None:
+        spack.concretize.concretize_one(spec)
+    else:
+        with pytest.raises(error_type):
+            spack.concretize.concretize_one(spec)
+
+
+@pytest.mark.usefixtures("mutable_config", "mock_packages", "do_not_check_runtimes_on_reuse")
+@pytest.mark.parametrize(
+    "spec_str, error_type",
+    [
+        (f"git-test-commit@git.main commit={'a' * 40}", None),
+        (f"git-test-commit@git.v1.0 commit={'a' * 40}", None),
+        ("git-test-commit@{sha} commit={sha}", None),
+        ("git-test-commit@{sha} commit=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", None),
+    ],
+)
+def test_spec_with_commit_interacts_with_lookup(
+    mock_git_version_info, monkeypatch, spec_str, error_type
+):
+    # This test will be short lived. Technically we could do further checks with a Lookup
+    # but skipping impl since we are going to deprecate
+    repo_path, filename, commits = mock_git_version_info
+    file_url = pathlib.Path(repo_path).as_uri()
+    monkeypatch.setattr(spack.package_base.PackageBase, "git", file_url, raising=False)
+    spec = spack.spec.Spec(spec_str.format(sha=commits[-1]))
+    if error_type is None:
+        spack.concretize.concretize_one(spec)
+    else:
+        with pytest.raises(error_type):
+            spack.concretize.concretize_one(spec)
+
+
+@pytest.mark.usefixtures("mutable_config", "mock_packages", "do_not_check_runtimes_on_reuse")
+@pytest.mark.parametrize("version_str", [f"git.{'a' * 40}=main", "git.2.1.5=main"])
+def test_relationship_git_versions_and_commit_variant(version_str):
+    """
+    Confirm that GitVersions auto assign and populates the commit variant correctly
+    """
+    # This should be a short lived test and can be deleted when we remove GitVersions
+    spec = spack.spec.Spec(f"git-ref-package@{version_str}")
+    spec = spack.concretize.concretize_one(spec)
+    if spec.version.commit_sha:
+        assert spec.version.commit_sha == spec.variants["commit"].value
+    else:
+        assert "commit" not in spec.variants
+
+
+@pytest.mark.usefixtures("install_mockery", "do_not_check_runtimes_on_reuse")
+def test_abstract_commit_spec_reuse():
+    commit = "abcd" * 10
+    spec_str_1 = f"git-ref-package@develop commit={commit}"
+    spec_str_2 = f"git-ref-package commit={commit}"
+    spec1 = spack.concretize.concretize_one(spack.spec.Spec(spec_str_1))
+    PackageInstaller([spec1.package], fake=True, explicit=True).install()
+
+    with spack.config.override("concretizer:reuse", True):
+        spec2 = spack.spec.Spec(spec_str_2)
+        spec2 = spack.concretize.concretize_one(spec2)
+        assert spec1.dag_hash() == spec2.dag_hash()
+
+
+@pytest.mark.usefixtures("install_mockery", "do_not_check_runtimes_on_reuse")
+@pytest.mark.parametrize(
+    "installed_commit, incoming_commit, reusable",
+    [("a" * 40, "b" * 40, False), (None, "b" * 40, False), ("a" * 40, None, True)],
+)
+def test_commit_variant_can_be_reused(installed_commit, incoming_commit, reusable):
+    # install a non-default variant to test if reuse picks it
+    if installed_commit:
+        spec_str_1 = f"git-ref-package@develop commit={installed_commit} ~opt"
+    else:
+        spec_str_1 = "git-ref-package@develop ~opt"
+
+    if incoming_commit:
+        spec_str_2 = f"git-ref-package@develop commit={incoming_commit}"
+    else:
+        spec_str_2 = "git-ref-package@develop"
+
+    spec1 = spack.concretize.concretize_one(spack.spec.Spec(spec_str_1))
+    PackageInstaller([spec1.package], fake=True, explicit=True).install()
+
+    with spack.config.override("concretizer:reuse", True):
+        spec2 = spack.spec.Spec(spec_str_2)
+        spec2 = spack.concretize.concretize_one(spec2)
+        assert (spec1.dag_hash() == spec2.dag_hash()) == reusable
+
+
 def test_concretization_cache_roundtrip(
     mock_packages, use_concretization_cache, monkeypatch, mutable_config
 ):

lib/spack/spack/test/conftest.py

@@ -154,7 +154,7 @@ def mock_git_version_info(git, tmpdir, override_git_repos_cache_path):
        o second commit (v1.0)
        o first commit
 
-    The repo consists of a single file, in which the GitVersion._ref_version representation
+    The repo consists of a single file, in which the GitVersion.std_version representation
     of each commit is expressed as a string.
 
     Important attributes of the repo for test coverage are: multiple branches,
@@ -197,6 +197,11 @@ def latest_commit():
 
         # Get name of default branch (differs by git version)
         main = git("rev-parse", "--abbrev-ref", "HEAD", output=str, error=str).strip()
+        if main != "main":
+            # assure the default branch name is consistent for tests
+            git("branch", "-m", "main")
+            main = git("rev-parse", "--abbrev-ref", "HEAD", output=str, error=str).strip()
+        assert "main" == main
 
         # Tag second commit as v1.0
         write_file(filename, "[1, 0]")

lib/spack/spack/test/git_fetch.py

@@ -4,6 +4,7 @@
 
 import copy
 import os
+import pathlib
 import shutil
 
 import pytest
@@ -19,6 +20,7 @@
 from spack.fetch_strategy import GitFetchStrategy
 from spack.spec import Spec
 from spack.stage import Stage
+from spack.variant import SingleValuedVariant
 from spack.version import Version
 
 _mock_transport_error = "Mock HTTP transport error"
@@ -429,3 +431,19 @@ def test_git_sparse_paths_partial_clone(
 
         # fixture file is in the sparse-path expansion tree
         assert os.path.isfile(t.file)
+
+
+@pytest.mark.disable_clean_stage_check
+def test_commit_variant_clone(
+    git, default_mock_concretization, mutable_mock_repo, mock_git_version_info, monkeypatch
+):
+
+    repo_path, filename, commits = mock_git_version_info
+    test_commit = commits[-1]
+    s = default_mock_concretization("git-test")
+    args = {"git": pathlib.Path(repo_path).as_uri()}
+    monkeypatch.setitem(s.package.versions, Version("git"), args)
+    s.variants["commit"] = SingleValuedVariant("commit", test_commit)
+    s.package.do_stage()
+    with working_dir(s.package.stage.source_path):
+        assert git("rev-parse", "HEAD", output=str, error=str).strip() == test_commit

lib/spack/spack/test/packages.py

@@ -339,6 +339,30 @@ def test_package_can_have_sparse_checkout_properties(mock_packages, mock_fetch,
     assert fetcher.git_sparse_paths == pkg_cls.git_sparse_paths
 
 
+def test_package_can_depend_on_commit_of_dependency(mock_packages, config):
+    spec = spack.concretize.concretize_one(Spec("git-ref-commit-dep@1.0.0"))
+    assert spec.satisfies(f"^git-ref-package commit={'a' * 40}")
+    assert "surgical" not in spec["git-ref-package"].variants
+
+
+def test_package_condtional_variants_may_depend_on_commit(mock_packages, config):
+    spec = spack.concretize.concretize_one(Spec("git-ref-commit-dep@develop"))
+    assert spec.satisfies(f"^git-ref-package commit={'b' * 40}")
+    conditional_variant = spec["git-ref-package"].variants.get("surgical", None)
+    assert conditional_variant
+    assert conditional_variant.value
+
+
+def test_commit_variant_finds_matches_for_commit_versions(mock_packages, config):
+    """
+    test conditional dependence on `when='commit=<sha>'`
+    git-ref-commit-dep variant commit-selector depends on a specific commit of git-ref-package
+    that commit is associated with the stable version of git-ref-package
+    """
+    spec = spack.concretize.concretize_one(Spec("git-ref-commit-dep+commit-selector"))
+    assert spec.satisfies(f"^git-ref-package commit={'c' * 40}")
+
+
 def test_pkg_name_can_only_be_derived_when_package_module():
     """When the module prefix is not spack_repo (or legacy spack.pkg) we cannot derive
     a package name."""

lib/spack/spack/test/versions.py

@@ -811,6 +811,33 @@ def test_version_list_with_range_and_concrete_version_is_not_concrete():
     assert not v.concrete
 
 
+@pytest.mark.parametrize(
+    "git_ref, std_version",
+    (("foo", "develop"), ("a" * 40, "develop"), ("a" * 40, None), ("v1.2.0", "1.2.0")),
+)
+def test_git_versions_store_ref_requests(git_ref, std_version):
+    """
+    User requested ref's should be known on creation
+    Commit and standard version may not be known until concretization
+    To be concrete a GitVersion must have a commit and standard version
+    """
+    if std_version:
+        vstring = f"git.{git_ref}={std_version}"
+    else:
+        vstring = git_ref
+
+    v = Version(vstring)
+
+    assert isinstance(v, GitVersion)
+    assert v.ref == git_ref
+
+    if std_version:
+        assert v.std_version == Version(std_version)
+
+    if v.is_commit:
+        assert v.ref == v.commit_sha
+
+
 @pytest.mark.parametrize(
     "vstring, eq_vstring, is_commit",
     (

lib/spack/spack/variant.py

@@ -23,12 +23,16 @@
 RESERVED_NAMES = {
     "arch",
     "architecture",
+    "branch",
+    "commit",
     "dev_path",
     "namespace",
     "operating_system",
     "os",
     "patches",
     "platform",
+    "ref",
+    "tag",
     "target",
 }
 

lib/spack/spack/version/__init__.py

@@ -20,6 +20,7 @@
     VersionError,
     VersionLookupError,
     infinity_versions,
+    is_git_commit_sha,
     is_git_version,
 )
 from .version_types import (
@@ -58,6 +59,7 @@
     "any_version",
     "from_string",
     "infinity_versions",
+    "is_git_commit_sha",
     "is_git_version",
     "ver",
 ]

lib/spack/spack/version/common.py

@@ -23,13 +23,12 @@
 STRING_TO_PRERELEASE = {"alpha": ALPHA, "beta": BETA, "rc": RC, "final": FINAL}
 
 
+def is_git_commit_sha(string: str) -> bool:
+    return len(string) == 40 and bool(COMMIT_VERSION.match(string))
+
+
 def is_git_version(string: str) -> bool:
-    return (
-        string.startswith("git.")
-        or len(string) == 40
-        and bool(COMMIT_VERSION.match(string))
-        or "=" in string[1:]
-    )
+    return string.startswith("git.") or is_git_commit_sha(string) or "=" in string[1:]
 
 
 class VersionError(spack.error.SpackError):

lib/spack/spack/version/version_types.py

@@ -548,15 +548,19 @@ class GitVersion(ConcreteVersion):
     sufficient.
     """
 
-    __slots__ = ["ref", "has_git_prefix", "is_commit", "_ref_lookup", "_ref_version"]
+    __slots__ = ["has_git_prefix", "commit_sha", "ref", "std_version", "_ref_lookup"]
 
     def __init__(self, string: str):
+        # TODO will be required for concrete specs when commit lookup added
+        self.commit_sha: Optional[str] = None
+        self.std_version: Optional[StandardVersion] = None
+
+        # optional user supplied git ref
+        self.ref: Optional[str] = None
+
         # An object that can lookup git refs to compare them to versions
         self._ref_lookup: Optional[AbstractRefLookup] = None
 
-        # This is the effective version.
-        self._ref_version: Optional[StandardVersion]
-
         self.has_git_prefix = string.startswith("git.")
 
         # Drop `git.` prefix
@@ -565,23 +569,27 @@ def __init__(self, string: str):
         if "=" in normalized_string:
             # Store the git reference, and parse the user provided version.
             self.ref, spack_version = normalized_string.split("=")
-            self._ref_version = StandardVersion(
+            self.std_version = StandardVersion(
                 spack_version, *parse_string_components(spack_version)
             )
         else:
             # The ref_version is lazily attached after parsing, since we don't know what
             # package it applies to here.
-            self._ref_version = None
+            self.std_version = None
             self.ref = normalized_string
 
         # Used by fetcher
         self.is_commit: bool = len(self.ref) == 40 and bool(COMMIT_VERSION.match(self.ref))
 
+        # translations
+        if self.is_commit:
+            self.commit_sha = self.ref
+
     @property
     def ref_version(self) -> StandardVersion:
         # Return cached version if we have it
-        if self._ref_version is not None:
-            return self._ref_version
+        if self.std_version is not None:
+            return self.std_version
 
         if self.ref_lookup is None:
             raise VersionLookupError(
@@ -594,10 +602,10 @@ def ref_version(self) -> StandardVersion:
         # Add a -git.<distance> suffix when we're not exactly on a tag
         if distance > 0:
             version_string += f"-git.{distance}"
-        self._ref_version = StandardVersion(
+        self.std_version = StandardVersion(
             version_string, *parse_string_components(version_string)
         )
-        return self._ref_version
+        return self.std_version
 
     def intersects(self, other: VersionType) -> bool:
         # For concrete things intersects = satisfies = equality
@@ -629,7 +637,9 @@ def satisfies(self, other: VersionType) -> bool:
         raise TypeError(f"'satisfies()' not supported for instances of {type(other)}")
 
     def __str__(self) -> str:
-        s = f"git.{self.ref}" if self.has_git_prefix else self.ref
+        s = ""
+        if self.ref:
+            s += f"git.{self.ref}" if self.has_git_prefix else self.ref
         # Note: the solver actually depends on str(...) to produce the effective version.
         # So when a lookup is attached, we require the resolved version to be printed.
         # But for standalone git versions that don't have a repo attached, it would still
@@ -651,6 +661,7 @@ def __eq__(self, other: object) -> bool:
         return (
             isinstance(other, GitVersion)
             and self.ref == other.ref
+            # TODO(psakiev) this needs to chamge to commits when we turn on lookups
             and self.ref_version == other.ref_version
         )
 

share/spack/gitlab/cloud_pipelines/stacks/ml-linux-x86_64-rocm/spack.yaml

@@ -19,8 +19,6 @@ spack:
       - +rocm
       - amdgpu_target=gfx90a
       - ~flash_attention
-    miopen-hip:
-      require: ~ck
 
   specs:
     # Horovod

share/spack/setup-env.sh

@@ -232,10 +232,6 @@ _spack_determine_shell() {
         # If procfs is present this seems a more reliable
         # way to detect the current shell
         _sp_exe=$(readlink /proc/$$/exe)
-        # Qemu emulation has _sp_exe point to the emulator
-        if [ "${_sp_exe##*qemu*}" != "${_sp_exe}" ]; then
-            _sp_exe=$(cat /proc/$$/comm)
-        fi
         # Shell may contain number, like zsh5 instead of zsh
         basename ${_sp_exe} | tr -d '0123456789'
     elif [ -n "${BASH:-}" ]; then

var/spack/repos/spack_repo/builtin/packages/conduit/package.py

@@ -119,9 +119,9 @@ class Conduit(CMakePackage):
     # package dependencies
     ###########################################################################
 
-    depends_on("c", type="build")
-    depends_on("cxx", type="build")
-    depends_on("fortran", type="build", when="+fortran")
+    depends_on("c", type="build")  # generated
+    depends_on("cxx", type="build")  # generated
+    depends_on("fortran", type="build")  # generated
 
     #######################
     # BLT
@@ -184,10 +184,8 @@ class Conduit(CMakePackage):
     #######################
     # Parmetis
     #######################
-    depends_on("parmetis+shared", when="+parmetis+shared")
-    depends_on("parmetis~shared", when="+parmetis~shared")
-    depends_on("metis+shared", when="+parmetis+shared")
-    depends_on("metis~shared", when="+parmetis~shared")
+    depends_on("parmetis", when="+parmetis")
+    depends_on("metis", when="+parmetis")
 
     #######################
     # MPI

var/spack/repos/spack_repo/builtin/packages/kokkos/package.py

@@ -28,7 +28,6 @@ class Kokkos(CMakePackage, CudaPackage, ROCmPackage):
     version("master", branch="master")
     version("develop", branch="develop")
 
-    version("4.6.01", sha256="b9d70e4653b87a06dbb48d63291bf248058c7c7db4bd91979676ad5609bb1a3a")
     version("4.6.00", sha256="be72cf7fc6ef6b99c614f29b945960013a2aaa23859bfe1a560d8d9aa526ec9c")
     version("4.5.01", sha256="52d003ffbbe05f30c89966e4009c017efb1662b02b2b73190670d3418719564c")
     version("4.5.00", sha256="cbfb742feeb9e649db9eca0394e6ca9a22aa017a1e6aab8576990772a0e3135b")
@@ -247,8 +246,6 @@ class Kokkos(CMakePackage, CudaPackage, ROCmPackage):
         "86": "ampere86",
         "89": "ada89",
         "90": "hopper90",
-        "100": "blackwell100",
-        "120": "blackwell120",
     }
     cuda_arches = spack_cuda_arch_map.values()
     conflicts("+cuda", when="cuda_arch=none")

var/spack/repos/spack_repo/builtin/packages/kokkos_kernels/package.py

@@ -22,7 +22,6 @@ class KokkosKernels(CMakePackage, CudaPackage):
 
     version("develop", branch="develop")
     version("master", branch="master")
-    version("4.6.01", sha256="95b9357f37ab3b9c3913c00741acb2501831c28ea8664de67818ae79c69c5908")
     version("4.6.00", sha256="f2b18f3df78c3c8dd970fe8ead54e05c8c09c8ac6d1893655c2a1769d33aa364")
     version("4.5.01", sha256="c111a6561f23a85af9850d1df1e9015f37a586f1da0be4b6fb1e98001d75e074")
     version("4.5.00", sha256="94726a64e349adf6cd276e9fdc1b2bf7ff81efec833e479a5d3024b83f165a59")

var/spack/repos/spack_repo/builtin/packages/kokkos_nvcc_wrapper/package.py

@@ -23,7 +23,6 @@ class KokkosNvccWrapper(Package):
     version("master", branch="master")
     version("develop", branch="develop")
 
-    version("4.6.01", sha256="b9d70e4653b87a06dbb48d63291bf248058c7c7db4bd91979676ad5609bb1a3a")
     version("4.6.00", sha256="be72cf7fc6ef6b99c614f29b945960013a2aaa23859bfe1a560d8d9aa526ec9c")
     version("4.5.01", sha256="52d003ffbbe05f30c89966e4009c017efb1662b02b2b73190670d3418719564c")
     version("4.5.00", sha256="cbfb742feeb9e649db9eca0394e6ca9a22aa017a1e6aab8576990772a0e3135b")

var/spack/repos/spack_repo/builtin/packages/msmpi/burn_cbt.patch

@@ -1,84 +0,0 @@
-diff --git a/Directory.Build.props b/Directory.Build.props
-index 3177de8..6b5ec9c 100644
---- a/Directory.Build.props
-+++ b/Directory.Build.props
-@@ -56,7 +56,6 @@
-     properties before or after it depending on what you're injecting.  The "global" build.props will also import
-     files located in the $(CBTLocalPath) folder at certain points.
-   -->
--  <Import Project="$(EnlistmentRoot)\.build\CBT\build.props" />
-   
-   <!--
-     The NuProjPath property allows .nuproj projects to build without having NuProj installed.
-@@ -114,9 +113,6 @@
- 
-   <PropertyGroup>
-     <OutputPath>$(StagingOutputRootPath)$(MSBuildProjectName)\</OutputPath>
--    <VCToolsVersion>14.15.26726</VCToolsVersion>
--    <WindowsTargetPlatformVersion>10.0.16299.0</WindowsTargetPlatformVersion>
--    <PlatformToolset>v141</PlatformToolset>
-     <OutDir>$(OutputPath)</OutDir>
-     <O>$(Platform)\$(Configuration)</O>
-     <O Condition="'$(Platform)' == 'x86'">$(Configuration)</O>
-diff --git a/dirs.proj b/dirs.proj
-index ba6a006..2631d04 100644
---- a/dirs.proj
-+++ b/dirs.proj
-@@ -4,6 +4,5 @@
-   <ItemGroup>
-     <ProjectFile Include="$(MSBuildThisFileDirectory)src\dirs.proj" />
-   </ItemGroup>
--  <Import Project="$(TraversalTargets)" Condition=" '$(CBTModulesRestored)' == 'true' " />
- 
- </Project>
-\ No newline at end of file
-diff --git a/src/dirs.proj b/src/dirs.proj
-index b53afb1..e0eeca2 100644
---- a/src/dirs.proj
-+++ b/src/dirs.proj
-@@ -8,5 +8,4 @@
-     <ProjectFile Include="$(MSBuildThisFileDirectory)launchSvc\msmpiLaunchSvcMc.vcxproj"/>
-     <ProjectFile Include="$(MSBuildThisFileDirectory)mpi\dirs.proj"/>
-   </ItemGroup>
--  <Import Project="$(TraversalTargets)" Condition=" '$(CBTModulesRestored)' == 'true' " />
- </Project>
-diff --git a/src/mpi/dirs.proj b/src/mpi/dirs.proj
-index d8e9358..a8c329a 100644
---- a/src/mpi/dirs.proj
-+++ b/src/mpi/dirs.proj
-@@ -12,5 +12,4 @@
-     <ProjectFile Include="$(MSBuildThisFileDirectory)stub\mpistub.vcxproj" />
-   </ItemGroup>
- 
--  <Import Project="$(TraversalTargets)" Condition=" '$(CBTModulesRestored)' == 'true' " />
- </Project>
-diff --git a/src/mpi/msmpi/dirs.proj b/src/mpi/msmpi/dirs.proj
-index 9959160..548bc5b 100644
---- a/src/mpi/msmpi/dirs.proj
-+++ b/src/mpi/msmpi/dirs.proj
-@@ -14,5 +14,4 @@
-     <ProjectFile Include="$(MSBuildThisFileDirectory)util\mpiutil.vcxproj" />
-   </ItemGroup>
- 
--  <Import Project="$(TraversalTargets)" Condition=" '$(CBTModulesRestored)' == 'true' " />
- </Project>
-diff --git a/src/mpi/msmpi/fortran/dirs.proj b/src/mpi/msmpi/fortran/dirs.proj
-index d92d7fd..35370b6 100644
---- a/src/mpi/msmpi/fortran/dirs.proj
-+++ b/src/mpi/msmpi/fortran/dirs.proj
-@@ -11,5 +11,4 @@
-     <ProjectFile Include="$(MSBuildThisFileDirectory)msmpifms\msmpifms.vcxproj" />
-   </ItemGroup>
- 
--  <Import Project="$(TraversalTargets)" Condition=" '$(CBTModulesRestored)' == 'true' " />
- </Project>
-diff --git a/src/mpi/pmilib/dirs.proj b/src/mpi/pmilib/dirs.proj
-index a9c9a8a..3fe9821 100644
---- a/src/mpi/pmilib/dirs.proj
-+++ b/src/mpi/pmilib/dirs.proj
-@@ -8,5 +8,4 @@
-     <ProjectFile Include="$(MSBuildThisFileDirectory)lib\pmilib.vcxproj" />
-   </ItemGroup>
- 
--  <Import Project="$(TraversalTargets)" Condition=" '$(CBTModulesRestored)' == 'true' " />
- </Project>

var/spack/repos/spack_repo/builtin/packages/msmpi/ifort_compat.patch

@@ -1,28 +1,19 @@
 diff --git a/src/mpi/msmpi/dll/msmpi.vcxproj b/src/mpi/msmpi/dll/msmpi.vcxproj
-index 255b9f5..9eb7dff 100644
+index 255b9f5..cc4f096 100644
 --- a/src/mpi/msmpi/dll/msmpi.vcxproj
 +++ b/src/mpi/msmpi/dll/msmpi.vcxproj
-@@ -45,6 +45,7 @@
-         $(PUBLIC_SDK_LIB)\Iphlpapi.lib;
-         $(PUBLIC_SDK_LIB)\authz.lib;
-         $(PUBLIC_SDK_LIB)\ntdsapi.lib;
-+        $(ND_DIR)\ndutil.lib;
-         $(OutDir)\..\mpichannels\mpichannels.lib;
-         $(OutDir)\..\mpiio\mpiio.lib;
-         $(OutDir)\..\mpicommon\mpicommon.lib;
-@@ -57,6 +58,10 @@
+@@ -57,6 +57,9 @@
          $(OutDir)\..\mpi_debugger\mpi_debugger.lib;
          $(CRT_Libs);
        </AdditionalDependencies>
 +      <AdditionalLibraryDirectories>
-+        $(SPACK_IFORT)\lib;
-+        $(ND_DIR);
++        $(SPACK_IFORT)compiler\lib\intel64
 +      </AdditionalLibraryDirectories>
        <ModuleDefinitionFile>.\msmpi.def</ModuleDefinitionFile>
      </Link>
    </ItemDefinitionGroup>
 diff --git a/src/mpi/msmpi/fortran/lib/mpifort.vcxproj b/src/mpi/msmpi/fortran/lib/mpifort.vcxproj
-index 24bd29d..319153d 100644
+index 24bd29d..57d0292 100644
 --- a/src/mpi/msmpi/fortran/lib/mpifort.vcxproj
 +++ b/src/mpi/msmpi/fortran/lib/mpifort.vcxproj
 @@ -8,12 +8,12 @@
@@ -37,7 +28,7 @@ index 24bd29d..319153d 100644
 +      <Fort_Flags>/D _WIN64=1 /D _AMD64_=1 /D AMD64=1</Fort_Flags>
      </PropertyGroup>
 -    <Exec Command="$(GFORTRAN_BIN)\gfortran.exe -I$(MPI_INC_ROOT) -c %(ForCompile.Identity) $(Fort_Flags)  -o $(O)\%(ForCompile.FileName).obj" />
-+    <Exec Command="$(IFORT_BIN)\ifx.exe /I$(MPI_INC_ROOT) /c %(ForCompile.Identity) $(Fort_Flags) /names:lowercase /assume:nounderscore /o $(O)\%(ForCompile.FileName).obj" />
++    <Exec Command="$(IFORT_BIN)\ifort.exe /I$(MPI_INC_ROOT) /c %(ForCompile.Identity) $(Fort_Flags) /names:lowercase /assume:nounderscore /o $(O)\%(ForCompile.FileName).obj" />
      <ItemGroup>
        <Lib Condition="'$(ConfigurationType)'=='StaticLibrary'" Include="@(ForCompile->'$(O)\%(Filename).obj')" />
      </ItemGroup>

var/spack/repos/spack_repo/builtin/packages/msmpi/no_mc.patch

@@ -1,49 +0,0 @@
-# Patch partially based on: https://github.com/microsoft/vcpkg/pull/13218
-diff --git a/src/launchSvc/msmpiLaunchSvcMc.vcxproj b/src/launchSvc/msmpiLaunchSvcMc.vcxproj
-index 2a6952b..b9a9eba 100644
---- a/src/launchSvc/msmpiLaunchSvcMc.vcxproj
-+++ b/src/launchSvc/msmpiLaunchSvcMc.vcxproj
-@@ -15,12 +15,10 @@
-   </PropertyGroup>
-   <Import Project="$(VCTargetsPath)\Microsoft.Cpp.props" />
-   <ItemGroup>
--    <MessageCompile Include="launchsvcmsg.mc">
--      <GeneratedHeaderPath>true</GeneratedHeaderPath>
--      <HeaderFilePath>$(SrcRoot)\launchsvc\$(O)</HeaderFilePath>
--      <GeneratedRCAndMessagesPath>true</GeneratedRCAndMessagesPath>
--      <RCFilePath>$(SrcRoot)\launchsvc\$(O)</RCFilePath>
--    </MessageCompile>
-+    <CustomBuild Include="$(SrcRoot)\launchSvc\launchSvcMsg.mc">
-+      <Command>mc.exe -um -h $(SrcRoot)\launchSvc\$(O) -r $(SrcRoot)\launchSvc\$(O) $(SrcRoot)\launchSvc\launchSvcMsg.mc</Command>
-+      <Outputs>$(SrcRoot)\launchSvc\$(O)\launchSvcMsg.h;$(SrcRoot)\launchSvc\$(O)\launchSvcMsg.rc</Outputs>
-+   </CustomBuild>
-   </ItemGroup>
-   <Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" />
- </Project>
-\ No newline at end of file
-diff --git a/src/mpi/common/traceManifest.vcxproj b/src/mpi/common/traceManifest.vcxproj
-index f80e80b..eb9281d 100644
---- a/src/mpi/common/traceManifest.vcxproj
-+++ b/src/mpi/common/traceManifest.vcxproj
-@@ -15,16 +15,10 @@
-   <Import Project="$(VCTargetsPath)\Microsoft.Cpp.props" />
- 
-   <ItemGroup>
--    <MessageCompile Include="$(MPI_SRC_ROOT)\common\mpitrace.man">
--      <GeneratedHeaderPath>true</GeneratedHeaderPath>
--      <HeaderFilePath>$(MPI_SRC_ROOT)\common\$(O)</HeaderFilePath>
--      <GeneratedRCAndMessagesPath>true</GeneratedRCAndMessagesPath>
--      <RCFilePath>$(MPI_SRC_ROOT)\common\$(O)</RCFilePath>
--      <GeneratedFilesBaseName>MpiTraceEvents</GeneratedFilesBaseName>
--      <GenerateUserModeLoggingMacros>true</GenerateUserModeLoggingMacros>
--      <PrefixMacroName>Trace</PrefixMacroName>
--      <RemoveCharsFromSymbolName>EVENT_</RemoveCharsFromSymbolName>
--    </MessageCompile>
-+    <CustomBuild Include="$(MPI_SRC_ROOT)\common\mpitrace.man">
-+      <Command>mc.exe -um -p Trace -P EVENT_ -h $(MPI_SRC_ROOT)\common\$(O) -r $(MPI_SRC_ROOT)\common\$(O) -z MpiTraceEvents $(MPI_SRC_ROOT)\common\mpitrace.man</Command>
-+      <Outputs>$(MPI_SRC_ROOT)\common\$(O)\MpiTraceEvents.h;$(MPI_SRC_ROOT)\common\$(O)\MpiTraceEvents.rc</Outputs>
-+   </CustomBuild>
-   </ItemGroup>
-   <Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" />
- </Project>
-\ No newline at end of file

var/spack/repos/spack_repo/builtin/packages/msmpi/package.py

@@ -2,16 +2,15 @@
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
-import glob
 import os
 import re
 import sys
 
-import spack.build_systems.msbuild as msbuild
+from spack.build_systems.generic import GenericBuilder
 from spack.package import *
 
 
-class Msmpi(MSBuildPackage):
+class Msmpi(Package):
     """MSMPI is a Windows port of MPICH provided by the Windows team"""
 
     homepage = "https://docs.microsoft.com/en-us/message-passing-interface/microsoft-mpi"
@@ -32,24 +31,8 @@ class Msmpi(MSBuildPackage):
     depends_on("c", type="build")
 
     depends_on("win-wdk")
-    depends_on("networkdirect")
-    depends_on("perl")
 
-    # MSMPI's build system is hard coded to assume
-    # paths to gfortran and network direct
-    # This patch makes the build compatible with
-    # ifort/ifx and allows for a dynamically located
-    # network direct install
-    patch("ifort_nd_compat.patch")
-    # the cbt msbuild extension ms-mspi's build sytem
-    # uses has been removed. This patch removes the
-    # associated functionality from the build system
-    patch("burn_cbt.patch")
-    # For whatever reason, specifying the platform toolset
-    # prevents message compiler (mc) detection
-    # We know its present because we have a WDK
-    # patches the build system to just directly call the MC
-    patch("no_mc.patch")
+    patch("ifort_compat.patch")
 
     requires("platform=windows")
     requires("%msvc")
@@ -70,48 +53,27 @@ def setup_dependent_package(self, module, dependent_spec):
         # Note: This is not typical of MPI installations
         self.spec.mpicc = dependent_spec["c"].package.cc
         self.spec.mpicxx = dependent_spec["cxx"].package.cxx
-        if "fortran" in dependent_spec:
-            self.spec.mpifc = dependent_spec["fortran"].package.fc
-            self.spec.mpif77 = dependent_spec["fortran"].package.f77
+        self.spec.mpifc = dependent_spec["fortran"].package.fortran
+        self.spec.mpif77 = dependent_spec["fortran"].package.fortran
 
 
-class MSBuildBuilder(msbuild.MSBuildBuilder):
+class GenericBuilder(GenericBuilder):
     def setup_build_environment(self, env: EnvironmentModifications) -> None:
-        # os.path.join interprets C: as an indicator of a relative path
-        # so we need to use the traditional string join here
-        ifort_root = "\\".join(self.pkg.compiler.fc.split(os.path.sep)[:-2])
+        ifort_root = os.path.join(*self.pkg.compiler.fc.split(os.path.sep)[:-2])
         env.set("SPACK_IFORT", ifort_root)
-        env.prepend_path("IncludePath", self.spec["networkdirect"].prefix.include)
-        env.set("ND_DIR", self.spec["networkdirect"].prefix.ndutil)
 
-    @property
-    def std_msbuild_args(self):
-        return []
+    def is_64bit(self):
+        return "64" in str(self.pkg.spec.target.family)
 
-    # We will need to burn out the CBT build system and associated files
-    # and instead directly invoke msbuild per sln we're interested in
-    # and generate our own installer
-    def msbuild_args(self):
-        pkg = self.pkg
+    def build_command_line(self):
         args = ["-noLogo"]
-        ifort_bin = '"' + os.path.dirname(pkg.compiler.fc) + '"'
-        args.append(self.define("IFORT_BIN", ifort_bin))
-        args.append(self.define("PlatformToolset", "v" + pkg["msvc"].platform_toolset_ver))
-        args.append(self.define("VCToolsVersion", pkg["msvc"].msvc_version))
-        args.append(
-            self.define("WindowsTargetPlatformVersion", str(pkg["win-sdk"].version) + ".0")
-        )
-        args.append(self.define("Configuration", "Release"))
-        args.append("src\\msmpi.sln")
+        ifort_bin = self.pkg.compiler.fc
+        args.append("/p:IFORT_BIN=%s" % os.path.dirname(ifort_bin))
+        args.append("/p:VCToolsVersion=%s" % self.pkg.compiler.msvc_version)
+        args.append("/p:WindowsTargetPlatformVersion=%s" % str(self.pkg.spec["wdk"].version))
+        args.append("/p:PlatformToolset=%s" % self.pkg.compiler.cc_version)
         return args
 
-    def install(self, pkg, spec, prefix):
-        base_build = pkg.stage.source_path
-        out = os.path.join(base_build, "out")
-        build_configuration = glob.glob(os.path.join(out, "*"))[0]
-        for x in glob.glob(os.path.join(build_configuration, "**", "*.*")):
-            install_path = x.replace(build_configuration, prefix)
-            mkdirp(os.path.dirname(install_path))
-            install(x, install_path)
-        include_dir = os.path.join(os.path.join(base_build, "src"), "include")
-        install_tree(include_dir, prefix.include)
+    def install(self, spec, prefix):
+        with working_dir(self.pkg.stage.build_directory, create=True):
+            msbuild(*self.build_command_line())

var/spack/repos/spack_repo/builtin/packages/networkdirect/no_cbt.patch

@@ -1,52 +0,0 @@
-diff --git a/Directory.Build.props b/Directory.Build.props
-index a5e937a..d24da7b 100644
---- a/Directory.Build.props
-+++ b/Directory.Build.props
-@@ -56,7 +56,7 @@
-     properties before or after it depending on what you're injecting.  The "global" build.props will also import
-     files located in the $(CBTLocalPath) folder at certain points.
-   -->
--  <Import Project="$(EnlistmentRoot)\.build\CBT\build.props" />
-+  <!-- <Import Project="$(EnlistmentRoot)\.build\CBT\build.props" /> -->
-   
-   <!--
-     The NuProjPath property allows .nuproj projects to build without having NuProj installed.
-@@ -76,9 +76,9 @@
-   <PropertyGroup>
-     <OutputPath>$(StagingOutputRootPath)$(MSBuildProjectName)\</OutputPath>
-     <OutIncludePath>$(StagingOutputRootPath)include</OutIncludePath>
--    <VCToolsVersion>14.15.26726</VCToolsVersion>
--    <WindowsTargetPlatformVersion>10.0.16299.0</WindowsTargetPlatformVersion>
--    <PlatformToolset>v141</PlatformToolset>
-+    <!-- <VCToolsVersion>14.15.26726</VCToolsVersion> -->
-+    <!-- <WindowsTargetPlatformVersion>10.0.16299.0</WindowsTargetPlatformVersion> -->
-+    <!-- <PlatformToolset>v141</PlatformToolset> -->
-     <CharacterSet>Unicode</CharacterSet>
-     <OutDir>$(OutputPath)</OutDir>
-     <ResolveNugetpackages>false</ResolveNugetpackages>
-@@ -97,11 +97,12 @@
- 
-   <ItemDefinitionGroup>
-     <ClCompile>
--      <WarningLevel>Level3</WarningLevel>
-+      <!-- <WarningLevel>Level3</WarningLevel> -->
-       <SDLCheck>true</SDLCheck>
-       <ConformanceMode>true</ConformanceMode>
-       <DebugInformationFormat>ProgramDatabase</DebugInformationFormat>
-       <DisableSpecificWarnings>26439</DisableSpecificWarnings>
-+      <DisableSpecificWarnings>5205</DisableSpecificWarnings>
-     </ClCompile>
-     <Link>
-       <AdditionalDependencies>;%(AdditionalDependencies)</AdditionalDependencies>
-diff --git a/dirs.proj b/dirs.proj
-index 5a19bbf..e6c815c 100644
---- a/dirs.proj
-+++ b/dirs.proj
-@@ -4,5 +4,5 @@
-   <ItemGroup>
-     <ProjectFile Include="$(MSBuildThisFileDirectory)src\dirs.proj" />
-   </ItemGroup>
--  <Import Project="$(TraversalTargets)" Condition=" '$(CBTModulesRestored)' == 'true' " />
-+  <!-- <Import Project="$(TraversalTargets)" Condition=" '$(CBTModulesRestored)' == 'true' " /> -->
-   </Project>
-\ No newline at end of file

var/spack/repos/spack_repo/builtin/packages/networkdirect/package.py

@@ -1,72 +0,0 @@
-# Copyright Spack Project Developers. See COPYRIGHT file for details.
-#
-# SPDX-License-Identifier: (Apache-2.0 OR MIT)
-
-import glob
-import os
-
-import spack.build_systems.msbuild as msbuild
-from spack.package import *
-
-
-class Networkdirect(MSBuildPackage):
-    """NetworkDirect is a user-mode programming interface specification
-    for Remote Direct Memory Access (RDMA)"""
-
-    homepage = "https://learn.microsoft.com/en-us/previous-versions/windows/it-pro/windows-server-2012-r2-and-2012/hh997033(v=ws.11)"
-    url = "https://github.com/microsoft/NetworkDirect/archive/refs/tags/v2.0.zip"
-
-    maintainers("johnwparent")
-
-    license("MIT", checked_by="johnwparent")
-
-    version("2.0", sha256="ba05a9be32ff39e08766c5a726ee63d47ee3eb9cab53b2b4b656de2d9158460c")
-
-    requires("platform=windows")
-
-    depends_on("cxx")
-    requires("%msvc")
-
-    depends_on("win-sdk")
-    depends_on("win-wdk")
-
-    # Networkdirect uses a build system called CBT that is built on top of MSBuild
-    # CBT is entirely deprecated, and fully incompatible with modern dotnet versions
-    # so we disable the CBT system and drive the underlying MSBuild system directly
-    patch("no_cbt.patch")
-
-
-class MSBuildBuilder(msbuild.MSBuildBuilder):
-
-    build_targets = ["ndutil"]
-
-    # Networkdirect is a unique package where providing
-    # typically required information actually
-    # breaks expected behavior, override the defaults
-    @property
-    def std_msbuild_args(self):
-        return []
-
-    def msbuild_args(self):
-        args = ["-noLogo"]
-        args.append(
-            self.define("WindowsTargetPlatformVersion", str(self.pkg["win-sdk"].version) + ".0")
-        )
-        # one of the headers we need isn't generated during release builds
-        args.append(self.define("Configuration", "Debug"))
-        args.append("src\\netdirect.sln")
-        return args
-
-    def install(self, pkg, spec, prefix):
-        base_build = pkg.stage.source_path
-        out = os.path.join(base_build, "out")
-        build_configuration = glob.glob(os.path.join(out, "*"))[0]
-        for x in glob.glob(os.path.join(build_configuration, "**", "*.lib")):
-            install_path = x.replace(build_configuration, prefix)
-            mkdirp(os.path.dirname(install_path))
-            install(x, install_path)
-        include_dir = os.path.join(build_configuration, "include")
-        install_tree(include_dir, prefix.include)
-        # for whatever reason this header is not moved to the "out" prefix
-        # with the rest of the headers, ensure its there
-        install(os.path.join(base_build, "src", "ndutil", "ndsupport.h"), prefix.include)

var/spack/test_repos/builtin.mock/packages/git-ref-commit-dep/package.py

@@ -0,0 +1,25 @@
+# Copyright Spack Project Developers. See COPYRIGHT file for details.
+#
+# SPDX-License-Identifier: (Apache-2.0 OR MIT)
+
+from spack.package import *
+
+
+class GitRefCommitDep(AutotoolsPackage):
+    """
+    tests dependency using commit
+    """
+
+    homepage = "https://github.com/dummy/dummy"
+    git = "https://github.com/dummy/dummy.git"
+    url = git
+
+    version("develop", branch="develop")
+    version("main", branch="main")
+    version("1.0.0", sha256="a5d504c0d52e2e2721e7e7d86988dec2e290d723ced2307145dedd06aeb6fef2")
+
+    variant("commit-selector", default=False, description="test grabbing a specific commit")
+
+    depends_on(f"git-ref-package commit={'a' * 40}", when="@1.0.0")
+    depends_on(f"git-ref-package commit={'b' * 40}", when="@develop")
+    depends_on(f"git-ref-package commit={'c' * 40}", when="+commit-selector")

var/spack/test_repos/builtin.mock/packages/git-ref-package/package.py

@@ -14,6 +14,10 @@ class GitRefPackage(AutotoolsPackage):
     url = "https://github.com/dummy/dummy/archive/2.0.0.tar.gz"
     git = "https://github.com/dummy/dummy.git"
 
+    version("develop", branch="develop")
+    version("main", branch="main")
+    version("stable", tag="stable", commit="c" * 40)
+    version("3.0.1", tag="v3.0.1")
     version("2.1.6", sha256="a5d504c0d52e2e2721e7e7d86988dec2e290d723ced2307145dedd06aeb6fef2")
     version("2.1.5", sha256="3f6576971397b379d4205ae5451ff5a68edf6c103b2f03c4188ed7075fbb5f04")
     version("2.1.4", sha256="a0293475e6a44a3f6c045229fe50f69dc0eebc62a42405a51f19d46a5541e77a")
@@ -34,6 +38,12 @@ class GitRefPackage(AutotoolsPackage):
     variant("opt", default=True, description="Enable optimizations")
     variant("shared", default=True, description="Build shared library")
     variant("pic", default=True, description="Enable position-independent code (PIC)")
+    variant(
+        "surgical",
+        default=True,
+        when=f"commit={'b' * 40}",
+        description="Testing conditional on commit",
+    )
 
     conflicts("+shared~pic")
 

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

@@ -11,6 +11,7 @@ class GitTestCommit(Package):
     homepage = "http://www.git-fetch-example.com"
     # git='to-be-filled-in-by-test'
 
+    version("main", branch="main")
     version("1.0", tag="v1.0")
     version("1.1", tag="v1.1")
     version("1.2", tag="1.2")  # not a typo