Add proposal to deal with multiple build-systems using directives #3
Conversation
alalazo
added
enhancement
There was a problem hiding this comment.
This is an interesting proposal. I spoke to @tgamblin about this a while back and he suggested simply wrapping each class with a @when decorator so that the Hdf5 class could inherit from different base classes based on version. However, it was never clear to me how this would work, and it doesn't allow for the same version to build with multiple build systems. I think the logic proposed in this PR is one of the simplest I've seen for handling this. As for how to implement it internally in Spack's core, I guess I'll have to see what we come up with. My main concern is that the package files remain simple, and this approach seems simple to me.
|
|
||
| ## The problem | ||
|
|
||
| There are multiple packages that are either changing their build-system during the evolution of the project, or using different build-systems for different platforms. Spack, at the moment, provides no support to model this use case. As a result we wrote some [documentation](https://github.com/spack/spack/pull/25174) to enumerate various workarounds that people adopted - each with its own drawbacks. What we would like to have in the long term is proper support for packages that can be built using multiple build-systems. |
There was a problem hiding this comment.
using different build-systems for different platforms
This is a very good point that I didn't think about while writing spack/spack#25174, glad to see more core developers thinking about Windows support!
There was a problem hiding this comment.
Very much so! Consider spack/spack@5818a54, are we going to turn all ./configure-based packages into CMake packages across all platforms just to support Visual Studio? I really hope not... zlib is a teeny tiny package that should be able to build with 0 other deps than a shell and a compiler.
| ``` | ||
| Each build-system can be subject to a constraint. In the example above, for instance, `cmake` can be used only if the version of `hdf5` is greater or equal than `X.Y` (while `autotools` is always available). | ||
|
|
||
| We can also assume that there is an __implicit preference order__ based on on the order of declaration in the class. In the example above this means that, if no other requests are made by the user, `cmake` is preferred to `autotools` when available. |
There was a problem hiding this comment.
I agree with this, but this "implicit preference order based on the order of declaration in the class" is different than all other directives in Spack, so it may be good to make this very clear in the docs.
| ``` | ||
| hdf5 build_system=cmake | ||
| ``` | ||
| __which requires the keyword `build_system` to be reserved by Spack__ (similarly to what is done for `dev_path`). It is interesting to note that modeling build-systems this way make them similar to signle-valued variants with the only difference that the set of allowed values is "dynamic" and subject to constraints. |
There was a problem hiding this comment.
| __which requires the keyword `build_system` to be reserved by Spack__ (similarly to what is done for `dev_path`). It is interesting to note that modeling build-systems this way make them similar to signle-valued variants with the only difference that the set of allowed values is "dynamic" and subject to constraints. | |
| __which requires the keyword `build_system` to be reserved by Spack__ (similarly to what is done for `dev_path`). It is interesting to note that modeling build-systems this way make them similar to single-valued variants with the only difference that the set of allowed values is "dynamic" and subject to constraints. |
A lot of users have been asking for this for swapping between Ninja and Makefile in CMakePackage. Would you consider Ninja vs. Makefile to be a build system? We could do build_system=cmake_ninja or build_system=cmake_makefile. Or it could be a different reserved keyword.
There was a problem hiding this comment.
I have to think a bit about this. A solution like build_system=cmake_ninja does not require any change in the proposed semantics, but we are effectively splitting the cmake build-system into many build-systems based on the generator we use.
Using hdf5 build_system=cmake generator=ninjaseems more appealing, but we need the ability to attach specific variants based on the build-system being used. That may require the "conditional" variant PR from @becker33:
variant('generator', values=('makefile', 'ninja'), default='makefile', when='build_system=cmake')| class CMakePackage(PackageBase): | ||
| # Can be overridden if conditional in derived classes | ||
| build_system('cmake') | ||
| depends_on('cmake', type='build') |
There was a problem hiding this comment.
If I write a package that supports both CMake and Autotools, will I have to manually add:
depends_on('cmake', when='build_system=cmake', type='build')or can that be included in CMakeBuildSystem? Is the build_system=cmake even available in the spec?
There was a problem hiding this comment.
Is the build_system=cmake even available in the spec?
Yes, that is available in the spec. There are legitimate use cases where you want to use it in recipes too, e.g. for bootstrapping tools. Think for instance foo can build with cmake and makefile and is a cmake dependency. In CMake's recipe you probably want to specify:
depends_on('foo build_system=makefile')to avoid circular dependencies.
If I write a package that supports both CMake and Autotools, will I have to manually add: [ ... ]
I think that should be implied and something similar needs to be included only if there are additional constraints to be met e.g. on the version of cmake.
Allow declaring possible values for variants with an associated condition. If the variant takes one of those values, the condition is imposed as a further constraint. The idea of this PR is to implement part of the mechanisms needed for modeling [packages with multiple build-systems]( spack/seps#3). After this PR the build-system directive can be implemented as: ```python variant( 'build-system', default='cmake', values=( 'autotools', conditional('cmake', when='@X.Y:') ), description='...', ) ``` Modifications: - [x] Allow conditional possible values in variants - [x] Add a unit-test for the feature - [x] Add documentation
Allow declaring possible values for variants with an associated condition. If the variant takes one of those values, the condition is imposed as a further constraint. The idea of this PR is to implement part of the mechanisms needed for modeling [packages with multiple build-systems]( spack/seps#3). After this PR the build-system directive can be implemented as: ```python variant( 'build-system', default='cmake', values=( 'autotools', conditional('cmake', when='@X.Y:') ), description='...', ) ``` Modifications: - [x] Allow conditional possible values in variants - [x] Add a unit-test for the feature - [x] Add documentation
Allow declaring possible values for variants with an associated condition. If the variant takes one of those values, the condition is imposed as a further constraint. The idea of this PR is to implement part of the mechanisms needed for modeling [packages with multiple build-systems]( spack/seps#3). After this PR the build-system directive can be implemented as: ```python variant( 'build-system', default='cmake', values=( 'autotools', conditional('cmake', when='@X.Y:') ), description='...', ) ``` Modifications: - [x] Allow conditional possible values in variants - [x] Add a unit-test for the feature - [x] Add documentation
|
@tgamblin This is the first SEP that has been implemented. Should we review the current early proposal, fix it, and merge it? Or should we leave it as it is since the SEP repository is rarely used? |
This SEP propose to deal with multiple build systems using a new directive:
The semantics of the new directive is that of a single valued variant with a dynamic set of allowed values subject to constraint. If we treat multiple build-systems this way:
hdf5 build_system=cmake)More details in the SEP.
Other attempts to solve the same issue in Spack:
PRs related to build-systems that we might want to consider: