summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorHarmen Stoppels <harmenstoppels@gmail.com>2023-02-23 01:44:47 +0100
committerGitHub <noreply@github.com>2023-02-23 01:44:47 +0100
commit3d41b71664c626e3f30c10e1670c5d07fee252a7 (patch)
treefdd5e783526aa1a788398cf32564d60ade71e0ec
parente27d3c4f754e2d3d67f6e4c2754e8acb4e655754 (diff)
downloadspack-3d41b71664c626e3f30c10e1670c5d07fee252a7.tar.gz
spack-3d41b71664c626e3f30c10e1670c5d07fee252a7.tar.bz2
spack-3d41b71664c626e3f30c10e1670c5d07fee252a7.tar.xz
spack-3d41b71664c626e3f30c10e1670c5d07fee252a7.zip
buildcache push: ensure bool arguments for include_* (#35632)
Fixes a bug introduced in 44ed0de8c077630148c213d3c7f40a8965eb6f94 where the push method of binary_distribution now takes named args include_root and include_depedencies, to avoid the **kwarg hole. But the call site wasn't update and we passed a dict of keys/values instead of arguments, which resulted in a call like this: ``` push(include_root={"include_root": True, "include_dependencies": False}) ``` This commit fixes that, and adds a test to see if we push the correct packages.
-rw-r--r--lib/spack/spack/binary_distribution.py6
-rw-r--r--lib/spack/spack/ci.py3
-rw-r--r--lib/spack/spack/test/bindist.py67
3 files changed, 73 insertions, 3 deletions
diff --git a/lib/spack/spack/binary_distribution.py b/lib/spack/spack/binary_distribution.py
index e9e48b4010..9758e8b9ea 100644
--- a/lib/spack/spack/binary_distribution.py
+++ b/lib/spack/spack/binary_distribution.py
@@ -1332,7 +1332,7 @@ def nodes_to_be_packaged(specs, root=True, dependencies=True):
return list(filter(packageable, nodes))
-def push(specs, push_url, include_root=True, include_dependencies=True, **kwargs):
+def push(specs, push_url, include_root: bool = True, include_dependencies: bool = True, **kwargs):
"""Create a binary package for each of the specs passed as input and push them
to a given push URL.
@@ -1345,6 +1345,10 @@ def push(specs, push_url, include_root=True, include_dependencies=True, **kwargs
**kwargs: TODO
"""
+ # Be explicit about the arugment type
+ if type(include_root) != bool or type(include_dependencies) != bool:
+ raise ValueError("Expected include_root/include_dependencies to be True/False")
+
nodes = nodes_to_be_packaged(specs, root=include_root, dependencies=include_dependencies)
# TODO: This seems to be an easy target for task
diff --git a/lib/spack/spack/ci.py b/lib/spack/spack/ci.py
index 8be254be23..7afda4babf 100644
--- a/lib/spack/spack/ci.py
+++ b/lib/spack/spack/ci.py
@@ -1437,9 +1437,8 @@ def _push_mirror_contents(env, specfile_path, sign_binaries, mirror_url):
hashes = env.all_hashes() if env else None
matches = spack.store.specfile_matches(specfile_path, hashes=hashes)
push_url = spack.mirror.Mirror.from_url(mirror_url).push_url
- spec_kwargs = {"include_root": True, "include_dependencies": False}
kwargs = {"force": True, "allow_root": True, "unsigned": unsigned}
- bindist.push(matches, push_url, spec_kwargs, **kwargs)
+ bindist.push(matches, push_url, include_root=True, include_dependencies=False, **kwargs)
def push_mirror_contents(env, specfile_path, mirror_url, sign_binaries):
diff --git a/lib/spack/spack/test/bindist.py b/lib/spack/spack/test/bindist.py
index d959258c85..81f5f7377c 100644
--- a/lib/spack/spack/test/bindist.py
+++ b/lib/spack/spack/test/bindist.py
@@ -885,3 +885,70 @@ def test_default_index_json_404():
with pytest.raises(bindist.FetchIndexError, match="Could not fetch index"):
fetcher.conditional_fetch()
+
+
+@pytest.mark.parametrize(
+ "root,deps,expected",
+ [
+ (
+ True,
+ True,
+ [
+ "dttop",
+ "dtbuild1",
+ "dtbuild2",
+ "dtlink2",
+ "dtrun2",
+ "dtlink1",
+ "dtlink3",
+ "dtlink4",
+ "dtrun1",
+ "dtlink5",
+ "dtrun3",
+ "dtbuild3",
+ ],
+ ),
+ (
+ False,
+ True,
+ [
+ "dtbuild1",
+ "dtbuild2",
+ "dtlink2",
+ "dtrun2",
+ "dtlink1",
+ "dtlink3",
+ "dtlink4",
+ "dtrun1",
+ "dtlink5",
+ "dtrun3",
+ "dtbuild3",
+ ],
+ ),
+ (True, False, ["dttop"]),
+ (False, False, []),
+ ],
+)
+def test_correct_specs_are_pushed(
+ root, deps, expected, default_mock_concretization, tmpdir, temporary_store, monkeypatch
+):
+ # Concretize dttop and add it to the temporary database (without prefixes)
+ spec = default_mock_concretization("dttop")
+ temporary_store.db.add(spec, directory_layout=None)
+
+ # Create a mirror push url
+ push_url = spack.mirror.Mirror.from_local_path(str(tmpdir)).push_url
+
+ packages_to_push = []
+
+ def fake_build_tarball(node, push_url, **kwargs):
+ assert push_url == push_url
+ assert not kwargs
+ assert isinstance(node, Spec)
+ packages_to_push.append(node.name)
+
+ monkeypatch.setattr(bindist, "_build_tarball", fake_build_tarball)
+
+ bindist.push([spec], push_url, include_root=root, include_dependencies=deps)
+
+ assert packages_to_push == expected