Skip to content

ASP-based solver: always consider version of installed packages#29933

Merged
becker33 merged 4 commits intospack:developfrom
alalazo:fixes/consider_unknown_version_of_installed_software
Apr 25, 2022
Merged

ASP-based solver: always consider version of installed packages#29933
becker33 merged 4 commits intospack:developfrom
alalazo:fixes/consider_unknown_version_of_installed_software

Conversation

@alalazo
Copy link
Copy Markdown
Member

@alalazo alalazo commented Apr 7, 2022

fixes #29201
fixes #30064

Explicitly add facts for versions of installed software when using the --reuse option, so that we could consider versions that are not declared in package.py

Modifications:

  • Fixed the bug
  • Add unit test

@spackbot-app spackbot-app bot added new-variant new-version tests General test capability(ies) labels Apr 7, 2022
@alalazo alalazo force-pushed the fixes/consider_unknown_version_of_installed_software branch 2 times, most recently from c7a4e74 to c63d55d Compare April 7, 2022 12:47
@alalazo alalazo marked this pull request as ready for review April 7, 2022 15:04
@alalazo alalazo requested review from becker33 and haampie April 7, 2022 15:15
@alalazo alalazo force-pushed the fixes/consider_unknown_version_of_installed_software branch from c63d55d to 5dcb505 Compare April 12, 2022 16:52
Copy link
Copy Markdown
Member

@becker33 becker33 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you tested the following scenario:

Package foo declares versions a, b
foo@c installed
foo@b preferred
foo@b conflicts with something in spec

I think in that case this will lead to concretizing foo@c even though it's only a valid version for reusing.


# add OS to possible OS's
for dep in spec.traverse():
self.possible_versions[dep.name].add(dep.version)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment above this block probably should change to reflect the new content, maybe

# ensure all attributes of installed specs are considered possible values.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 7727cd1

alalazo added 3 commits April 15, 2022 09:24
fixes spack#29201

Explicitly add facts for versions of installed software when
using the --reuse option, so that we could consider versions
that are not declared in package.py
@alalazo alalazo force-pushed the fixes/consider_unknown_version_of_installed_software branch from 5dcb505 to 7727cd1 Compare April 15, 2022 08:09
@alalazo
Copy link
Copy Markdown
Member Author

alalazo commented Apr 15, 2022

Have you tested the following scenario: [ ... ]

That scenario was working, but I came up with something along the same lines that wasn't. Fixed and added a new unit test for it. Thanks!

Comment on lines +1552 to +1554
"""Test that we can reuse installed specs with versions not
declared in package.py
"""
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This docstring looks like it was mistakenly copied from the previous test.

Suggested change
"""Test that we can reuse installed specs with versions not
declared in package.py
"""
"""Test that we cannot build with versions only declared for reuse.
"""

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 9876f66

@alalazo alalazo requested a review from becker33 April 23, 2022 12:49
@becker33 becker33 merged commit 268c671 into spack:develop Apr 25, 2022
@alalazo alalazo deleted the fixes/consider_unknown_version_of_installed_software branch April 25, 2022 17:29
tgamblin pushed a commit that referenced this pull request Apr 25, 2022
* ASP-based solver: always consider version of installed packages

fixes #29201

Explicitly add facts for versions of installed software when
using the --reuse option, so that we could consider versions
that are not declared in package.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Spack --reuse does not recognize 'non-preferred' installs --reuse does not reuse versions known in the db, but unknown in package.py

3 participants