Skip to content

performance bugfix: only expose buildable oses to solver - #52604

Merged
tgamblin merged 6 commits into
developfrom
bugfix/performance-regression-macos
Jun 25, 2026
Merged

tgamblin merged 6 commits into
developfrom
bugfix/performance-regression-macos

Conversation

@becker33

Copy link
Copy Markdown
Member

Fixes #52602

Macos needs to know about older operating systems, to reconstruct specs from them, but it doesn't need to expose them to the solver unless they are coming in from a reused spec.

Generalized to a buildable_oses method on the Platform class.

@alecbcs

Signed-off-by: Gregory Becker <[email protected]>
@tgamblin
tgamblin requested a review from alalazo June 24, 2026 09:07

@tgamblin tgamblin left a comment

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.

can you also add a test to ensure that older OS's can be reused on macOS? #52390 didn't add one.

@alecbcs

alecbcs commented Jun 24, 2026

Copy link
Copy Markdown
Member

@spackbot fix style

@spackbot-app

spackbot-app Bot commented Jun 24, 2026

Copy link
Copy Markdown

Let me see if I can fix that for you!

@spackbot-app

spackbot-app Bot commented Jun 24, 2026

Copy link
Copy Markdown

I was able to run spack style --fix for you!

spack style --fix
==> Running style checks on spack
  selected: import, ruff-format, ruff-check, mypy
==> Running import checks
  import checks were clean
==> Running ruff-format checks
  ruff-format checks were clean
==> Running ruff-check checks
F841 Local variable `default_macos` is assigned to but never used
   --> lib/spack/spack/test/architecture.py:140:5
    |
138 | def test_instantiate_non_default_macos(mock_packages):
139 |     darwin = spack.platforms.Darwin()
140 |     default_macos = darwin.operating_system("default_os")
    |     ^^^^^^^^^^^^^
141 |
142 |     for name, macos in darwin.operating_sys.items():
    |
help: Remove assignment to unused variable `default_macos`
  ruff-check found errors
==> Running mypy checks
Success: no issues found in 639 source files
  mypy checks were clean
Keep in mind that I cannot fix your flake8 or mypy errors, so if you have any you'll need to fix them and update the pull request. If I was able to push to your branch, if you make further changes you will need to pull from your updated branch before pushing again.

I've updated the branch with style fixes.

becker33 and others added 3 commits June 24, 2026 14:45
@becker33
becker33 force-pushed the bugfix/performance-regression-macos branch from e778b3c to 0c4bde3 Compare June 24, 2026 12:45
Signed-off-by: Gregory Becker <[email protected]>

@alecbcs alecbcs left a comment

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.

Looks good to me

@alecbcs
alecbcs requested a review from tgamblin June 24, 2026 17:36
@tgamblin
tgamblin merged commit e91c987 into develop Jun 25, 2026
33 of 34 checks passed
@tgamblin
tgamblin deleted the bugfix/performance-regression-macos branch June 25, 2026 07:30
@haampie haampie added the v1.2.1 PRs to backport for v1.2.1 label Jun 29, 2026
@haampie haampie mentioned this pull request Jun 29, 2026
17 tasks done
haampie pushed a commit that referenced this pull request Jul 6, 2026
Fixes #52602

Macos needs to know about older operating systems, to reconstruct specs
from them, but it doesn't need to expose them to the solver unless they
are coming in from a reused spec.

Generalized to a `buildable_oses` method on the `Platform` class.

---------

Signed-off-by: Gregory Becker <[email protected]>
Co-authored-by: becker33 <[email protected]>
Signed-off-by: Harmen Stoppels <[email protected]>
becker33 added a commit that referenced this pull request Jul 6, 2026
Fixes #52602

Macos needs to know about older operating systems, to reconstruct specs
from them, but it doesn't need to expose them to the solver unless they
are coming in from a reused spec.

Generalized to a `buildable_oses` method on the `Platform` class.

---------

Signed-off-by: Gregory Becker <[email protected]>
Co-authored-by: becker33 <[email protected]>
Signed-off-by: Harmen Stoppels <[email protected]>
Aiden2244 pushed a commit to Aiden2244/spack that referenced this pull request Jul 20, 2026
Fixes spack#52602 

Macos needs to know about older operating systems, to reconstruct specs
from them, but it doesn't need to expose them to the solver unless they
are coming in from a reused spec.

Generalized to a `buildable_oses` method on the `Platform` class.

---------

Signed-off-by: Gregory Becker <[email protected]>
Co-authored-by: becker33 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

solver unit-tests v1.2.1 PRs to backport for v1.2.1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants