Don't allow replacement of root develop specs with --reuse (#28605)

* Fix to concretize.lp

do not allow dev specs to be reused

Co-authored-by: Gregory Becker <becker33@llnl.gov>
This commit is contained in:
psakievich 2022-04-12 11:37:24 -06:00 committed by GitHub
parent 2aec5b65f3
commit 7d534f38d6
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 48 additions and 4 deletions

View file

@ -1676,13 +1676,15 @@ def define_installed_packages(self, specs, possible):
# Specs from local store # Specs from local store
with spack.store.db.read_transaction(): with spack.store.db.read_transaction():
for spec in spack.store.db.query(installed=True): for spec in spack.store.db.query(installed=True):
self._facts_from_concrete_spec(spec, possible) if not spec.satisfies('dev_path=*'):
self._facts_from_concrete_spec(spec, possible)
# Specs from configured buildcaches # Specs from configured buildcaches
try: try:
index = spack.binary_distribution.update_cache_and_get_specs() index = spack.binary_distribution.update_cache_and_get_specs()
for spec in index: for spec in index:
self._facts_from_concrete_spec(spec, possible) if not spec.satisfies('dev_path=*'):
self._facts_from_concrete_spec(spec, possible)
except (spack.binary_distribution.FetchCacheError, IndexError): except (spack.binary_distribution.FetchCacheError, IndexError):
# this is raised when no mirrors had indices. # this is raised when no mirrors had indices.
# TODO: update mirror configuration so it can indicate that the source cache # TODO: update mirror configuration so it can indicate that the source cache

View file

@ -853,6 +853,9 @@ no_flags(Package, FlagType)
{ hash(Package, Hash) : installed_hash(Package, Hash) } 1 { hash(Package, Hash) : installed_hash(Package, Hash) } 1
:- node(Package), error("Internal error: package must resolve to at most one hash"). :- node(Package), error("Internal error: package must resolve to at most one hash").
% you can't choose an installed hash for a dev spec
:- hash(Package, Hash), variant_value(Package, "dev_path", _).
% if a hash is selected, we impose all the constraints that implies % if a hash is selected, we impose all the constraints that implies
impose(Hash) :- hash(Package, Hash). impose(Hash) :- hash(Package, Hash).

View file

@ -339,7 +339,6 @@ def concretize_difficult_packages(self, a, b):
assert s[a].version == ver(b) assert s[a].version == ver(b)
def test_concretize_two_virtuals(self): def test_concretize_two_virtuals(self):
"""Test a package with multiple virtual dependencies.""" """Test a package with multiple virtual dependencies."""
Spec('hypre').concretize() Spec('hypre').concretize()
@ -1165,6 +1164,46 @@ def test_external_package_versions(self, spec_str, is_external, expected):
assert s.external == is_external assert s.external == is_external
assert s.satisfies(expected) assert s.satisfies(expected)
@pytest.mark.parametrize('dev_first', [True, False])
@pytest.mark.parametrize('spec', [
'dev-build-test-install', 'dev-build-test-dependent ^dev-build-test-install'])
@pytest.mark.parametrize('mock_db', [True, False])
def test_reuse_does_not_overwrite_dev_specs(
self, dev_first, spec, mock_db, tmpdir, monkeypatch):
"""Test that reuse does not mix dev specs with non-dev specs.
Tests for either order (dev specs are not reused for non-dev, and
non-dev specs are not reused for dev specs)
Tests for a spec in which the root is developed and a spec in
which a dep is developed.
Tests for both reuse from database and reuse from buildcache"""
# dev and non-dev specs that are otherwise identical
spec = Spec(spec)
dev_spec = spec.copy()
dev_constraint = 'dev_path=%s' % tmpdir.strpath
dev_spec['dev-build-test-install'].constrain(dev_constraint)
# run the test in both orders
first_spec = dev_spec if dev_first else spec
second_spec = spec if dev_first else dev_spec
# concretize and setup spack to reuse in the appropriate manner
first_spec.concretize()
def mock_fn(*args, **kwargs):
return [first_spec]
if mock_db:
monkeypatch.setattr(spack.store.db, 'query', mock_fn)
else:
monkeypatch.setattr(
spack.binary_distribution, 'update_cache_and_get_specs', mock_fn)
# concretize and ensure we did not reuse
with spack.config.override("concretizer:reuse", True):
second_spec.concretize()
assert first_spec.dag_hash() != second_spec.dag_hash()
@pytest.mark.regression('20292') @pytest.mark.regression('20292')
@pytest.mark.parametrize('context', [ @pytest.mark.parametrize('context', [
{'add_variant': True, 'delete_variant': False}, {'add_variant': True, 'delete_variant': False},
@ -1172,7 +1211,7 @@ def test_external_package_versions(self, spec_str, is_external, expected):
{'add_variant': True, 'delete_variant': True} {'add_variant': True, 'delete_variant': True}
]) ])
@pytest.mark.xfail() @pytest.mark.xfail()
def test_reuse_installed_packages( def test_reuse_installed_packages_when_package_def_changes(
self, context, mutable_database, repo_with_changing_recipe self, context, mutable_database, repo_with_changing_recipe
): ):
# Install a spec # Install a spec