Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Basic check for winsdk before concretizing #43752

Merged
merged 12 commits into from May 2, 2024

Conversation

johnwparent
Copy link
Contributor

@johnwparent johnwparent commented Apr 19, 2024

Adds a pre-concretization check in asp.py for the Windows SDK and WGL (Windows GL) packages as non-buildable externals.
Previously this was incorporated into ensure_core_depenendencies in the bootstrap module, but that scoped the config modifications to the bootstrap scope, which made the changes fundamentally useless for general purpose use, so we transitioned to a pre concretization call to ensure these packages are always available pre concretization.

The goal is to achieve parity with most unix-like systems where we have a consistent understanding of the system (potentially hardware accelerated) GL location and can reasonably rely on that heuristic to bake the external into packages.yaml.
Adds documentation to the startup guide denoting that the SDK is a hard requirement for Spack now.
Adds error handling if WGL and WSDK cannot be detected on system.

Adds pytest fixture mocking the check for WGL and WSDK as if they were present

Note: This fixture requires the session scope in order to avoid a race where certain fixtures are invoked first without the mocked WSDK check. Typical pytest monkeypatch fixtures are function scope, which is incompatible with session scoped fixtures. This required the introduction of a session scoped monkeypatch fixture named monkeypatch_session.

@spackbot-app spackbot-app bot added the core PR affects Spack core functionality label Apr 19, 2024
@scheibelp scheibelp self-assigned this Apr 19, 2024
@johnwparent johnwparent force-pushed the win/call-sdk-bootstrap-on-conc branch from ad78c71 to cdf6a6f Compare April 23, 2024 20:02
@spackbot-app spackbot-app bot added the tests General test capability(ies) label Apr 23, 2024
@johnwparent johnwparent force-pushed the win/call-sdk-bootstrap-on-conc branch from b4df63f to 93ab873 Compare April 26, 2024 18:14
@johnwparent johnwparent force-pushed the win/call-sdk-bootstrap-on-conc branch from 3636bd5 to 83c7dbf Compare April 30, 2024 21:06
Copy link
Member

@scheibelp scheibelp left a comment

Choose a reason for hiding this comment

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

I have a couple minor requests about documentation. This generally looks good.

lib/spack/spack/test/conftest.py Outdated Show resolved Hide resolved
lib/spack/spack/test/conftest.py Show resolved Hide resolved
lib/spack/spack/cmd/unit_test.py Show resolved Hide resolved
@johnwparent
Copy link
Contributor Author

@spackbot fix style

Copy link

spackbot-app bot commented May 1, 2024

Let me see if I can fix that for you!

Copy link

spackbot-app bot commented May 1, 2024

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

spack style --fix
==> Running style checks on spack
  selected: isort, black, flake8, mypy
==> Modified files
  lib/spack/spack/bootstrap/core.py
  lib/spack/spack/cmd/unit_test.py
  lib/spack/spack/solver/asp.py
  lib/spack/spack/test/conftest.py
==> Running isort checks
  isort checks were clean
==> Running black checks
reformatted lib/spack/spack/solver/asp.py
All done! ✨ 🍰 ✨
1 file reformatted, 3 files left unchanged.
  black checks were clean
==> Running flake8 checks
  flake8 checks were clean
==> Running mypy checks
lib/spack/spack/version/version_types.py:145: error: Argument 2 to "StandardVersion" has incompatible type "*Tuple[Tuple[Any, ...], Tuple[Any, ...]]"; expected "Tuple[Tuple[Any, ...], Tuple[Any, ...]]"  [arg-type]
lib/spack/spack/version/version_types.py:452: error: Argument 2 to "StandardVersion" has incompatible type "*Tuple[Tuple[Any, ...], Tuple[Any, ...]]"; expected "Tuple[Tuple[Any, ...], Tuple[Any, ...]]"  [arg-type]
lib/spack/spack/version/version_types.py:481: error: Argument 2 to "StandardVersion" has incompatible type "*Tuple[Tuple[Any, ...], Tuple[Any, ...]]"; expected "Tuple[Tuple[Any, ...], Tuple[Any, ...]]"  [arg-type]
Found 3 errors in 1 file (checked 623 source files)
  mypy found errors
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.

@scheibelp scheibelp enabled auto-merge (squash) May 1, 2024 18:15
@scheibelp scheibelp merged commit 7e8415a into spack:develop May 2, 2024
34 of 36 checks passed
wdconinc pushed a commit that referenced this pull request May 4, 2024
Adds a pre-concretization check for the Windows SDK and WGL (Windows
GL) packages as non-buildable externals.

This is a redo of #43459, but makes
sure to modify the configuration scope outside of the bootstrap scope:
whichever is highest-precedence in the user's environment at the time
the concretization runs, which should either be an env scope or the
~ scope.

Adds pytest fixture mocking the check for WGL and WSDK as if they were
present.
tldahlgren pushed a commit to tldahlgren/spack that referenced this pull request May 8, 2024
Adds a pre-concretization check for the Windows SDK and WGL (Windows
GL) packages as non-buildable externals.

This is a redo of spack#43459, but makes
sure to modify the configuration scope outside of the bootstrap scope:
whichever is highest-precedence in the user's environment at the time
the concretization runs, which should either be an env scope or the
~ scope.

Adds pytest fixture mocking the check for WGL and WSDK as if they were
present.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
commands core PR affects Spack core functionality documentation tests General test capability(ies)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants