Skip to content

Commit

Permalink
fix patched dependencies across repositories (spack#42463)
Browse files Browse the repository at this point in the history
Currently, if a package has a dependency from another repository and patches it,
generation of the patch cache will fail. Concretization succeeds if a fixed patch
cache is in place.

- [x] don't assume that patched dependencies are in the same repo when indexing
- [x] add some test fixtures to support multi-repo tests.

---------

Signed-off-by: Todd Gamblin <tgamblin@llnl.gov>
Co-authored-by: Todd Gamblin <tgamblin@llnl.gov>
  • Loading branch information
becker33 and tgamblin authored Nov 9, 2024
1 parent c6997e1 commit da1d533
Show file tree
Hide file tree
Showing 11 changed files with 80 additions and 12 deletions.
2 changes: 1 addition & 1 deletion lib/spack/spack/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,7 @@ def setup_main_options(args):
key = syaml.syaml_str("repos")
key.override = True
spack.config.CONFIG.scopes["command_line"].sections["repos"] = syaml.syaml_dict(
[(key, [spack.paths.mock_packages_path])]
[(key, [spack.paths.mock_packages_path, spack.paths.mock_packages_path2])]
)
spack.repo.PATH = spack.repo.create(spack.config.CONFIG)

Expand Down
9 changes: 3 additions & 6 deletions lib/spack/spack/patch.py
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,7 @@ def update_package(self, pkg_fullname: str) -> None:

# update the index with per-package patch indexes
pkg_cls = self.repository.get_pkg_class(pkg_fullname)
partial_index = self._index_patches(pkg_cls, self.repository)
partial_index = self._index_patches(pkg_cls)
for sha256, package_to_patch in partial_index.items():
p2p = self.index.setdefault(sha256, {})
p2p.update(package_to_patch)
Expand All @@ -546,14 +546,11 @@ def update(self, other: "PatchCache") -> None:
p2p.update(package_to_patch)

@staticmethod
def _index_patches(
pkg_class: Type["spack.package_base.PackageBase"], repository: "spack.repo.RepoPath"
) -> Dict[Any, Any]:
def _index_patches(pkg_class: Type["spack.package_base.PackageBase"]) -> Dict[Any, Any]:
"""Patch index for a specific patch.
Args:
pkg_class: package object to get patches for
repository: repository containing the package
Returns:
The patch index for that package.
Expand All @@ -571,7 +568,7 @@ def _index_patches(
for dependency in deps_by_name.values():
for patch_list in dependency.patches.values():
for patch in patch_list:
dspec_cls = repository.get_pkg_class(dependency.spec.name)
dspec_cls = spack.repo.PATH.get_pkg_class(dependency.spec.fullname)
patch_dict = patch.to_dict()
patch_dict.pop("sha256") # save some space
index[patch.sha256] = {dspec_cls.fullname: patch_dict}
Expand Down
1 change: 1 addition & 0 deletions lib/spack/spack/paths.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
repos_path = os.path.join(var_path, "repos")
packages_path = os.path.join(repos_path, "builtin")
mock_packages_path = os.path.join(repos_path, "builtin.mock")
mock_packages_path2 = os.path.join(repos_path, "builtin.mock2")

#
# Writable things in $spack/var/spack
Expand Down
16 changes: 11 additions & 5 deletions lib/spack/spack/test/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -575,6 +575,11 @@ def mock_repo_path():
yield spack.repo.from_path(spack.paths.mock_packages_path)


@pytest.fixture(scope="session")
def mock_repo_path2():
yield spack.repo.from_path(spack.paths.mock_packages_path2)


def _pkg_install_fn(pkg, spec, prefix):
# sanity_check_prefix requires something in the install directory
mkdirp(prefix.bin)
Expand All @@ -588,19 +593,20 @@ def mock_pkg_install(monkeypatch):


@pytest.fixture(scope="function")
def mock_packages(mock_repo_path, mock_pkg_install, request):
"""Use the 'builtin.mock' repository instead of 'builtin'"""
def mock_packages(mock_repo_path, mock_repo_path2, mock_pkg_install, request):
"""Use the 'builtin.mock' and 'builtin.mock2' repositories instead of 'builtin'"""
ensure_configuration_fixture_run_before(request)
with spack.repo.use_repositories(mock_repo_path) as mock_repo:
with spack.repo.use_repositories(mock_repo_path, mock_repo_path2) as mock_repo:
yield mock_repo


@pytest.fixture(scope="function")
def mutable_mock_repo(mock_repo_path, request):
def mutable_mock_repo(request):
"""Function-scoped mock packages, for tests that need to modify them."""
ensure_configuration_fixture_run_before(request)
mock_repo = spack.repo.from_path(spack.paths.mock_packages_path)
with spack.repo.use_repositories(mock_repo) as mock_repo_path:
mock_repo2 = spack.repo.from_path(spack.paths.mock_packages_path2)
with spack.repo.use_repositories(mock_repo, mock_repo2) as mock_repo_path:
yield mock_repo_path


Expand Down
6 changes: 6 additions & 0 deletions lib/spack/spack/test/patch.py
Original file line number Diff line number Diff line change
Expand Up @@ -499,3 +499,9 @@ def test_invalid_from_dict(mock_packages, config):
}
with pytest.raises(spack.fetch_strategy.ChecksumError, match="sha256 checksum failed for"):
spack.patch.from_dict(dictionary)


@pytest.mark.regression("43097")
def test_cross_repo_patch(mock_packages, config):
cross_repo_patch = Spec("patch-a-foreign-dependency")
cross_repo_patch.concretize()
7 changes: 7 additions & 0 deletions var/spack/repos/builtin.mock/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# `builtin.mock`

This repository and the secondary mock repo `builtin.mock2` contain mock packages used
by Spack tests.

Most tests are in `builtin.mock`, but `builtin.mock2` is used for scenarios where we
need multiple repos for testing.
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
--- patch-a-dependency/configure 2018-08-13 23:13:51.000000000 -0700
+++ patch-a-dependency/configure.patched 2018-08-13 23:14:15.000000000 -0700
@@ -2,7 +2,7 @@
prefix=$(echo $1 | sed 's/--prefix=//')
cat > Makefile <<EOF
all:
- echo Building...
+ echo Patched!

install:
mkdir -p $prefix
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# Copyright 2013-2024 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)

from spack.package import *


class PatchAForeignDependency(Package):
"""Package that requries a patched version of a dependency."""

homepage = "http://www.example.com"
url = "http://www.example.com/patch-a-dependency-1.0.tar.gz"

version("1.0", md5="0123456789abcdef0123456789abcdef")

depends_on("mock2-patched-dependency", patches=patch("mock2-package.patch"))
6 changes: 6 additions & 0 deletions var/spack/repos/builtin.mock2/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# `builtin.mock2`

This is the secondary mock repo for testing Spack, along with `builtin.mock`.

Most tests are in `builtin.mock`, but this repository is used for scenarios where we
need multiple repos for testing.
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# Copyright 2013-2024 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)

from spack.package import *


class Mock2PatchedDependency(Package):
"""Package patched by patch-a-foreign-dependency in builtin.mock."""

homepage = "http://www.example.com"
url = "http://www.example.com/mock2-patch-dependency-1.0.tar.gz"

version("1.0", md5="0123456789abcdef0123456789abcdef")
2 changes: 2 additions & 0 deletions var/spack/repos/builtin.mock2/repo.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
repo:
namespace: 'builtin.mock2'

0 comments on commit da1d533

Please sign in to comment.