Auto-import spack.build_systems._package_api_v1#50496
Merged
Conversation
spackbot-app
bot
added
build-systems
core
There was a problem hiding this comment.
Pull Request Overview
This PR introduces an automatic injection of a v1 package API import to maintain backward compatibility for v1 repositories.
- Adds a new module (spack/build_systems/_package_api_v1.py) that re-exports legacy API symbols.
- Replaces the default loader with a custom _PrependFileLoader in spack/repo.py to inject the import line.
- Updates tests by removing an obsolete test (test_is_package_module) and adding a new xfail test to cover deprecated audit behavior.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| lib/spack/spack/test/repo.py | Removed tests for is_package_module, aligning with the new API injection requirements. |
| lib/spack/spack/test/audit.py | Added a new xfail test for package audit behavior in presence of the injected line; note a typo in the test name. |
| lib/spack/spack/repo.py | Introduced _PrependFileLoader to prepend the import statement to module source code. |
| lib/spack/spack/build_systems/_package_api_v1.py | New module that re-exports symbols from v1 package API for backward compatibility. |
alalazo
reviewed
May 16, 2025
When loading packages from a v1.0 repository, inject a line from spack.build_systems._package_api import * This allows removal of certain names in `spack.package` as part of api v2 without breaking backward compatibility in Spack.
haampie
force-pushed
the
hs/fix/reintroduce-magic-import
branch
from
d2673ea to
5e76a65
Compare
alalazo
approved these changes
May 16, 2025
tgamblin
pushed a commit
that referenced
this pull request
May 19, 2025
Builders and package classes refer to packages from the builtin package repo and are often modified together with packages. That means that these classes should move into `spack_repo.builtin`. * move `spack.build_systems` -> `spack_repo.builtin.build_systems` * Remove the following re-exports from the `spack.package` module: - `AspellDictPackage` - `LuaPackage` - `AutotoolsPackage` - `MakefilePackage` - `BundlePackage` - `MavenPackage` - `CachedCMakePackage` - `MesonPackage` - `cmake_cache_filepath` - `MSBuildPackage` - `cmake_cache_option` - `NMakePackage` - `cmake_cache_path` - `OctavePackage` - `cmake_cache_string` - `PerlPackage` - `CargoPackage` - `PythonExtension` - `CMakePackage` - `PythonPackage` - `generator` - `QMakePackage` - `CompilerPackage` - `RacketPackage` - `CudaPackage` - `RPackage` - `Package` - `ROCmPackage` - `GNUMirrorPackage` - `RubyPackage` - `GoPackage` - `SConsPackage` - `IntelPackage` - `SIPPackage` - `IntelOneApiLibraryPackageWithSdk` - `SourceforgePackage` - `IntelOneApiLibraryPackage` - `SourcewarePackage` - `IntelOneApiStaticLibraryList` - `WafPackage` - `IntelOneApiPackage` - `XorgPackage` - `INTEL_MATH_LIBRARIES` * update mock packages to repo v2.0 and add copies of packages/build systems they use from builtin * add missing imports to build systems in `package.py` from builtin and test repos * update various tests This PR is breaking because of removal of various names from `spack.package`, but breakage should be minimal thanks to #50496, which ensures the above names are always imported in repo v1 packages. Specifically this PR breaks imports like the following in `package.py` files: ```python from spack.package import Package ``` but if your repo is v1.0 (see `spack repo list`) and has the following much more common pattern: ```python from spack.package import * ``` nothing should break.
jezwilkinson
pushed a commit
to jezwilkinson/spack
that referenced
this pull request
May 20, 2025
When loading packages from a v1.0 repository, inject a line from spack.build_systems._package_api import * This allows removal of certain names in `spack.package` as part of api v2 without breaking backward compatibility in Spack.
jezwilkinson
pushed a commit
to jezwilkinson/spack
that referenced
this pull request
May 20, 2025
Builders and package classes refer to packages from the builtin package repo and are often modified together with packages. That means that these classes should move into `spack_repo.builtin`. * move `spack.build_systems` -> `spack_repo.builtin.build_systems` * Remove the following re-exports from the `spack.package` module: - `AspellDictPackage` - `LuaPackage` - `AutotoolsPackage` - `MakefilePackage` - `BundlePackage` - `MavenPackage` - `CachedCMakePackage` - `MesonPackage` - `cmake_cache_filepath` - `MSBuildPackage` - `cmake_cache_option` - `NMakePackage` - `cmake_cache_path` - `OctavePackage` - `cmake_cache_string` - `PerlPackage` - `CargoPackage` - `PythonExtension` - `CMakePackage` - `PythonPackage` - `generator` - `QMakePackage` - `CompilerPackage` - `RacketPackage` - `CudaPackage` - `RPackage` - `Package` - `ROCmPackage` - `GNUMirrorPackage` - `RubyPackage` - `GoPackage` - `SConsPackage` - `IntelPackage` - `SIPPackage` - `IntelOneApiLibraryPackageWithSdk` - `SourceforgePackage` - `IntelOneApiLibraryPackage` - `SourcewarePackage` - `IntelOneApiStaticLibraryList` - `WafPackage` - `IntelOneApiPackage` - `XorgPackage` - `INTEL_MATH_LIBRARIES` * update mock packages to repo v2.0 and add copies of packages/build systems they use from builtin * add missing imports to build systems in `package.py` from builtin and test repos * update various tests This PR is breaking because of removal of various names from `spack.package`, but breakage should be minimal thanks to spack#50496, which ensures the above names are always imported in repo v1 packages. Specifically this PR breaks imports like the following in `package.py` files: ```python from spack.package import Package ``` but if your repo is v1.0 (see `spack repo list`) and has the following much more common pattern: ```python from spack.package import * ``` nothing should break.
abhishek1297
pushed a commit
to abhishek1297/spack-melissa
that referenced
this pull request
May 22, 2025
When loading packages from a v1.0 repository, inject a line from spack.build_systems._package_api import * This allows removal of certain names in `spack.package` as part of api v2 without breaking backward compatibility in Spack.
abhishek1297
pushed a commit
to abhishek1297/spack-melissa
that referenced
this pull request
May 22, 2025
Builders and package classes refer to packages from the builtin package repo and are often modified together with packages. That means that these classes should move into `spack_repo.builtin`. * move `spack.build_systems` -> `spack_repo.builtin.build_systems` * Remove the following re-exports from the `spack.package` module: - `AspellDictPackage` - `LuaPackage` - `AutotoolsPackage` - `MakefilePackage` - `BundlePackage` - `MavenPackage` - `CachedCMakePackage` - `MesonPackage` - `cmake_cache_filepath` - `MSBuildPackage` - `cmake_cache_option` - `NMakePackage` - `cmake_cache_path` - `OctavePackage` - `cmake_cache_string` - `PerlPackage` - `CargoPackage` - `PythonExtension` - `CMakePackage` - `PythonPackage` - `generator` - `QMakePackage` - `CompilerPackage` - `RacketPackage` - `CudaPackage` - `RPackage` - `Package` - `ROCmPackage` - `GNUMirrorPackage` - `RubyPackage` - `GoPackage` - `SConsPackage` - `IntelPackage` - `SIPPackage` - `IntelOneApiLibraryPackageWithSdk` - `SourceforgePackage` - `IntelOneApiLibraryPackage` - `SourcewarePackage` - `IntelOneApiStaticLibraryList` - `WafPackage` - `IntelOneApiPackage` - `XorgPackage` - `INTEL_MATH_LIBRARIES` * update mock packages to repo v2.0 and add copies of packages/build systems they use from builtin * add missing imports to build systems in `package.py` from builtin and test repos * update various tests This PR is breaking because of removal of various names from `spack.package`, but breakage should be minimal thanks to spack#50496, which ensures the above names are always imported in repo v1 packages. Specifically this PR breaks imports like the following in `package.py` files: ```python from spack.package import Package ``` but if your repo is v1.0 (see `spack repo list`) and has the following much more common pattern: ```python from spack.package import * ``` nothing should break.
kshea21
pushed a commit
to kshea21/spack
that referenced
this pull request
May 22, 2025
When loading packages from a v1.0 repository, inject a line from spack.build_systems._package_api import * This allows removal of certain names in `spack.package` as part of api v2 without breaking backward compatibility in Spack.
kshea21
pushed a commit
to kshea21/spack
that referenced
this pull request
May 22, 2025
Builders and package classes refer to packages from the builtin package repo and are often modified together with packages. That means that these classes should move into `spack_repo.builtin`. * move `spack.build_systems` -> `spack_repo.builtin.build_systems` * Remove the following re-exports from the `spack.package` module: - `AspellDictPackage` - `LuaPackage` - `AutotoolsPackage` - `MakefilePackage` - `BundlePackage` - `MavenPackage` - `CachedCMakePackage` - `MesonPackage` - `cmake_cache_filepath` - `MSBuildPackage` - `cmake_cache_option` - `NMakePackage` - `cmake_cache_path` - `OctavePackage` - `cmake_cache_string` - `PerlPackage` - `CargoPackage` - `PythonExtension` - `CMakePackage` - `PythonPackage` - `generator` - `QMakePackage` - `CompilerPackage` - `RacketPackage` - `CudaPackage` - `RPackage` - `Package` - `ROCmPackage` - `GNUMirrorPackage` - `RubyPackage` - `GoPackage` - `SConsPackage` - `IntelPackage` - `SIPPackage` - `IntelOneApiLibraryPackageWithSdk` - `SourceforgePackage` - `IntelOneApiLibraryPackage` - `SourcewarePackage` - `IntelOneApiStaticLibraryList` - `WafPackage` - `IntelOneApiPackage` - `XorgPackage` - `INTEL_MATH_LIBRARIES` * update mock packages to repo v2.0 and add copies of packages/build systems they use from builtin * add missing imports to build systems in `package.py` from builtin and test repos * update various tests This PR is breaking because of removal of various names from `spack.package`, but breakage should be minimal thanks to spack#50496, which ensures the above names are always imported in repo v1 packages. Specifically this PR breaks imports like the following in `package.py` files: ```python from spack.package import Package ``` but if your repo is v1.0 (see `spack repo list`) and has the following much more common pattern: ```python from spack.package import * ``` nothing should break.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When loading packages from a v1.0 repository, inject a line
This allows removal of certain names in
spack.packageas part of apiv2 without breaking backward compatibility with v1 package repos.