Move builders into builtin repo by haampie · Pull Request #50452 · spack/spack

haampie · 2025-05-13T13:52:12Z

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, AutotoolsPackage, BundlePackage, CachedCMakePackage, cmake_cache_filepath, cmake_cache_option, cmake_cache_path, cmake_cache_string, CargoPackage, CMakePackage, generator, CompilerPackage, CudaPackage, Package, GNUMirrorPackage, GoPackage, IntelPackage, IntelOneApiLibraryPackageWithSdk, IntelOneApiLibraryPackage, IntelOneApiStaticLibraryList, IntelOneApiPackage, INTEL_MATH_LIBRARIES, LuaPackage, MakefilePackage, MavenPackage, MesonPackage, MSBuildPackage, NMakePackage, OctavePackage, PerlPackage, PythonExtension, PythonPackage, QMakePackage, RacketPackage, RPackage, ROCmPackage, RubyPackage, SConsPackage, SIPPackage, SourceforgePackage, SourcewarePackage, WafPackage, XorgPackage.
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:

from spack.package import Package

but if your repo is v1.0 (see spack repo list) and has the following much more common pattern

from spack.package import *

nothing should break.

tgamblin

I reviewed this commit by commit on the CLI. The changes are mostly mechanical (via spack repo migrate with additions to fix coupling between core and packages, mostly to handle name changes in tests.

Follow-ons:

Should remove the now-unused spack.paths.build_systems_path
(for me) Should add some proper fix for the windows grep issue (see #50520)
Not clear to me why we use relative imports in builtin and absolute in builtin_mock
This one might be more significant:

It's not clear to me that v1 packages that import other packages are handled by spack repo migrate -- are they? This type of stuff looks like it was a manual change needed after migration to v2:
```
-from ..cmake_client import CmakeClient
+from ..cmake_client.package import CmakeClient
```

In a follow-on we should modify spack repo migrate to do this.

Otherwise looks good to me.

haampie · 2025-05-17T09:56:38Z

* Not clear to me why we use relative imports in `builtin` and absolute in `builtin_mock`

Because some test repos have symlinks to packages from from builtin_mock, which makes ...build_systems.foo incorrect.

Edit: they are all absolute now.

* This one might be more significant:
  It's not clear to me that v1 packages that import other packages are handled by `spack repo migrate` -- are they? This type of stuff looks like it was a manual change needed after migration to v2:
  ```python
  -from ..cmake_client import CmakeClient
  +from ..cmake_client.package import CmakeClient
  ```

Correct, that isn't handled.

use absolute imports for build_systems to make copy/paste of recipes easier.

haampie · 2025-05-18T16:36:25Z

@spackbot run pipeline

spackbot-app · 2025-05-18T16:36:35Z

I've started that pipeline for you!

tgamblin · 2025-05-19T03:32:09Z

Merged because pipeline failures onml-aarch64-cpu are unrelated to this PR.

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.

Since #50452, build systems are no longer in core and need their own imports. Specifically, in addition to: ```python from spack.package import * ``` you now also need, e.g.: ```python from spack_repo.builtin.build_systems.python import PythonPackage ``` Or similar for other build systems, or things will break. - [x] Fix `py-deprecat` package - [x] Fix `cryoef` package Signed-off-by: Todd Gamblin <[email protected]>

Since spack#50452, build systems are no longer in core and need their own imports. Specifically, in addition to: ```python from spack.package import * ``` you now also need, e.g.: ```python from spack_repo.builtin.build_systems.python import PythonPackage ``` Or similar for other build systems, or things will break. - [x] Fix `py-deprecat` package - [x] Fix `cryoef` package Signed-off-by: Todd Gamblin <[email protected]>

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.

Since spack#50452, build systems are no longer in core and need their own imports. Specifically, in addition to: ```python from spack.package import * ``` you now also need, e.g.: ```python from spack_repo.builtin.build_systems.python import PythonPackage ``` Or similar for other build systems, or things will break. - [x] Fix `py-deprecat` package - [x] Fix `cryoef` package Signed-off-by: Todd Gamblin <[email protected]>

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.

Since spack#50452, build systems are no longer in core and need their own imports. Specifically, in addition to: ```python from spack.package import * ``` you now also need, e.g.: ```python from spack_repo.builtin.build_systems.python import PythonPackage ``` Or similar for other build systems, or things will break. - [x] Fix `py-deprecat` package - [x] Fix `cryoef` package Signed-off-by: Todd Gamblin <[email protected]>

spackbot-app bot added build-environment build-systems conflicts core PR affects Spack core functionality dependencies extends headers libraries new-variant stand-alone-tests Stand-alone (or smoke) tests for installed packages tests General test capability(ies) labels May 13, 2025

haampie force-pushed the hs/fix/builders-in-repo branch 14 times, most recently from ccf7922 to 72ba5ad Compare May 16, 2025 17:53

spackbot-app bot added commands documentation Improvements or additions to documentation environments intel update-package labels May 16, 2025

haampie force-pushed the hs/fix/builders-in-repo branch from 4ffd39f to 8f7f837 Compare May 17, 2025 08:04

haampie added the pipelines:urgent Skip "deferred pipelines" check. Only use if rebasing on a tested develop commit is unfeasible label May 17, 2025

tgamblin previously approved these changes May 17, 2025

View reviewed changes

tgamblin enabled auto-merge (squash) May 17, 2025 09:43

fix kokkos/pybind

7f382d8

haampie dismissed tgamblin’s stale review via 7f382d8 May 17, 2025 10:22

tgamblin previously approved these changes May 17, 2025

View reviewed changes

haampie disabled auto-merge May 17, 2025 10:44

use absolute imports again

59dd5b9

tgamblin enabled auto-merge (squash) May 19, 2025 03:06

tgamblin approved these changes May 19, 2025

View reviewed changes

tgamblin disabled auto-merge May 19, 2025 03:30

tgamblin merged commit 2929ea0 into develop May 19, 2025
45 of 48 checks passed

tgamblin deleted the hs/fix/builders-in-repo branch May 19, 2025 03:31

haampie mentioned this pull request May 19, 2025

environment.py: fix Environment.repo construction #50469

Closed

tgamblin mentioned this pull request May 21, 2025

bugfix: add build system imports to new packages #50587

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move builders into builtin repo#50452

Move builders into builtin repo#50452
tgamblin merged 47 commits intodevelopfrom
hs/fix/builders-in-repo

haampie commented May 13, 2025 •

edited

Loading

Uh oh!

tgamblin left a comment

Uh oh!

haampie commented May 17, 2025 •

edited

Loading

Uh oh!

haampie commented May 18, 2025

Uh oh!

spackbot-app bot commented May 18, 2025

Uh oh!

Uh oh!

tgamblin commented May 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

haampie commented May 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tgamblin left a comment

Choose a reason for hiding this comment

Uh oh!

haampie commented May 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

haampie commented May 18, 2025

Uh oh!

spackbot-app bot commented May 18, 2025

Uh oh!

Uh oh!

tgamblin commented May 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

haampie commented May 13, 2025 •

edited

Loading

haampie commented May 17, 2025 •

edited

Loading