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

Support optional builds #112

Closed
4 tasks done
henryiii opened this issue Nov 18, 2022 · 15 comments
Closed
4 tasks done

Support optional builds #112

henryiii opened this issue Nov 18, 2022 · 15 comments
Milestone

Comments

@henryiii
Copy link
Collaborator

henryiii commented Nov 18, 2022

I think this can be supported with a few more override settings. We could provide:

  • if.failed: If the build fails, the config settings gets rerun, and if any match this, the new configuration is used. This only happens once.
  • if.system-cmake and if.cmake-wheel.
  • message.after-failure: This will display if the build fails.
  • fail = true: This will fail the build. Might be only allowed in overrides.

Should we allow failed to take the condition of failure? I think the conditions would be "configure", "build", and "install". Though maybe it's better to just wait until someone asks for the ability to differentiate?

The cmake one would trigger if you were not on a platform with a known cmake wheel and there's no sufficient CMake present, or always if there's no system CMake. If this matches,
then it affects if cmake is requested, as well.

This is what pure-python on non-supported CMake system would look like:

[[tool.scikit-build.overrides]]
if.no-system-cmake = "known-wheel"
wheel.cmake = false

Or if the build fails:

[[tool.scikit-build.overrides]]
if.failed = true
wheel.cmake = false

Custom message on failure:

[tool.scikit-build]
error-message = "The build failed!"

I think this would add a lot of flexibility; for an unrelated example, you could fail on unsupported Pythons:

[[tool.scikit-build.overrides]]
if.python-version = ">=3.13"
error-message = "Python 3.13+ is not supported yet!"
fail = true

I'm looking at a way to allow the cmake wheel to trigger the bootstrapping feature, and I think this would be a step in that direction too.

This is a very rough draft of what I'm thinking.

@maxbachmann
Copy link
Contributor

So this would mean, that on installation failure a hook is called, which would allow me to handle the error? This would be awesome. I currently handle this using try/except inside my setup.py: https://github.com/maxbachmann/RapidFuzz/blob/main/setup.py which I would love to get rid of.

@henryiii
Copy link
Collaborator Author

Yes, this is exactly what this refers to.

@maxbachmann
Copy link
Contributor

One thing that could lead to problems is the cmake/ninja dependency, because as far as I know there is no notion of try to install for dependencies in Python. So when cmake/ninja are not installed and no wheel is available on the corresponding platform the build could fail when building them. In rapidfuzz I currently handle this by always installing the Python fallback in this case. This is suboptimal when the dependency build would actually succeed, but at least it is guaranteed to succeed this way.

@henryiii
Copy link
Collaborator Author

Good point. I have the information about platforms (in https://github.com/scikit-build/scikit-build-core/blob/main/src/scikit_build_core/resources/known_wheels.toml ) so could do that, at least as an option.

@maxbachmann
Copy link
Contributor

An alternative I thought of when I was working on this for rapidfuzz was an optional_cmake package. This package would do the same as the cmake package, but on failure it would simply install an empty package.

@henryiii
Copy link
Collaborator Author

henryiii commented Nov 1, 2023

I think we do the following to make this mostly possible with the new overrides.

  1. Support wheel.cmake = False. This will disable requiring CMake/Ninja during the wheel step too, just like the opposite of what sdist.cmake = True does.
  2. Support wheel.compiled = False. The default will be to match wheel.cmake. This will target purelib instead of platlib.
  3. At this point, we could make scikit-build-core bootstrap itself if we wanted to. :)
  4. Add a way to opt-into an override. Maybe provide a user-extendable namespace like custom or opt? For example:

pip install -e -Copt.compile="true"

Would match

[[tool.scikit-build.overrides]]
opt.compile = ".+"

(API might need a little work there, maybe would be an inline table with options or something)

We could also add if.env.<VAR>=<regex>.

@maxbachmann
Copy link
Contributor

Hm so if I understand you correctly: wheel.cmake = False would

  1. try to install cmake/ninja. Probably based on whether it is already installed / a wheel is available.
  2. try to build the c extension
  3. if 1) + 2) succeeds install everything
  4. if 1) or 2) failed only install the pure python parts

A user would be able to override the behavior while installing using something like -Copt.compile="true".

The only other thing I would require is the ability to enforce the compile based on environment variables. In my case I do enforce the build when I am in a known packaging environment:

packaging = "1" in {
    os.environ.get("CIBUILDWHEEL", "0"),
    os.environ.get("CONDA_BUILD", "0"),
    os.environ.get("PIWHEELS_BUILD", "0"),
    os.environ.get("RAPIDFUZZ_BUILD_EXTENSION", "0"),
}
if packaging:
    run_setup(True)
else:
    try:
        run_setup(True)
    except BaseException:
        show_message(
            "WARNING: The C extension could not be compiled, speedups are not enabled.",
            "Failure information, if any, is above.",
            "Retrying the build without the C extension now.",
        )
        run_setup(False)
        show_message(
            "WARNING: The C extension could not be compiled, speedups are not enabled.",
            "Plain-Python build succeeded.",
        )

@henryiii
Copy link
Collaborator Author

henryiii commented Nov 2, 2023

No, I think it's the exact opposite part of the solution. This would give you the ability to enforce the compile based on environment variables. It doesn't handle the try/fail case. wheel.cmake = False would not add cmake to the build requirements, and would only do the pure Python part of the build. By putting it in an overrides section, that would make it conditional on whatever is in the if part of the overrides section (+ proposed opt section I guess).

Basically this would solve the if/else part above, but not the try/except. But I think the try/except part solution might be easy to get to from this. For example, maybe you could set one to be a fallback on failure or similar...

So if you had this:

packaging = "1" in {
    os.environ.get("CIBUILDWHEEL", "0"),
    os.environ.get("CONDA_BUILD", "0"),
    os.environ.get("PIWHEELS_BUILD", "0"),
    os.environ.get("RAPIDFUZZ_BUILD_EXTENSION", "0"),
}
if packaging:
    run_setup(True)
else:
    run_setup(False)

You could write this:

[tool.scikit-build]
wheel.cmake = false

[[tool.scikit-build.overrides]]
if.env.CIBUILDWHEEL = "1"
wheel.cmake = true

[[tool.scikit-build.overrides]]
if.env.CONDA_BUILD = "1"
wheel.cmake = true

[[tool.scikit-build.overrides]]
if.env.PIWHEELS_BUILD = "1"
wheel.cmake = true

[[tool.scikit-build.overrides]]
if.env.RAPIDFUZZ_BUILD_EXTENSION = "1"
wheel.cmake = true

(Probably should come up with a way to do OR...)

@maxbachmann
Copy link
Contributor

Thanks for the clarification. Yes I completely misunderstood which part this was going to solve.

henryiii added a commit that referenced this issue Nov 9, 2023
This supports skipping the CMake build. This can be used with overrides
to make packages that have a pure Python implementation (#112).

Signed-off-by: Henry Schreiner <[email protected]>
@henryiii
Copy link
Collaborator Author

henryiii commented Jul 5, 2024

See discussion at rapidfuzz/Levenshtein#65 for use case. I think this is what it would look like:

[tool.scikit-build]
wheel.cmake = true

[[tool.scikit-build.overrides]]
if.any.no-system-cmake = "known-wheel"
if.any.failed = true
wheel.cmake = false

[[tool.scikit-build.overrides]]
if.any.env.CIBUILDWHEEL = "1"
if.any.env.CONDA_BUILD = "1"
if.any.env.PIWHEELS_BUILD = "1"
if.any.env.RAPIDFUZZ_BUILD_EXTENSION = "1"
wheel.cmake = true
error-message = "failed to build C++ Extension in a packaged build"

@henryiii henryiii changed the title Support optional builds (probably via hook) Support optional builds Jul 5, 2024
@henryiii henryiii added this to the v0.10.0 milestone Jul 6, 2024
@cringeyburger
Copy link

Hi there!
We at PyBaMM are transitioning our build system from setuptools and wheel to scikit-build-core.
One challenge we've encountered is the extended build time on Windows OS. To address this, we aim to provide a pure Python wheel fallback for Windows, controlled via environment variables. This is precisely the behaviour we are looking for.
We would greatly appreciate your insights and advice on implementing this feature.
CC: @agriyakhetarpal

@henryiii
Copy link
Collaborator Author

We already support a pure fallback via environment variables. And if you upload a pure Python wheel, that will always be used if a native wheel is not available, no backend involvement needed. This proposal was to have a way to allow a failed build to turn into a pure Python install, as well as a way to select behavior based on if a compiled cmake/ninja wheel is available for the platform.

@agriyakhetarpal
Copy link

Thank you, @henryiii! To clarify further: yes, trying to compile a wheel but then receiving a pure one upon failure on the unavailability of compilers or package managers, etc., say, if MSVC or vcpkg are not installed on Windows, is sort of what we are trying to look out for local development purposes, but if it's possible not to attempt the compilation altogether, that's great to hear as well – we can very well try that for CI, as @cringeyburger mentions.

a pure fallback via environment variables

Is there somewhere where this environment variable you mentioned is documented, though? I couldn't find it anywhere (side note: maybe a dedicated section/page on "building packages with optional extension modules" as such would be nice), and then I looked at the CHANGELOG entries and found wheel.cmake = false, which seems to offer the fallback behaviour you suggest, but without a note about the environment variable. Based on other examples I noticed, I tried searching for SKBUILD_WHEEL_CMAKE or similar, but that doesn't exist. Either way, it doesn't matter much because we'll rely on the pyproject.toml-based overrides anyway by checking for the CIBUILDWHEEL environment variable as that should be neater, but I wished to ask for completeness.

P.S. We considered uploading a pure Python wheel in the past in addition to compiled ones, but we didn't want to offer users a higher tier of support for unsupported platforms because of some dependencies we know are not present/broken on, say, manylinux arm64 yet, or are tricky to compile from their sdist (or in some cases impossible to compile if they ship a broken sdist)

@henryiii
Copy link
Collaborator Author

henryiii commented Jul 18, 2024

We probably need a whole page on overrides rather than just a section; you'd do it with overrides. For example:

[tool.scikit-build]
wheel.cmake = false

[[tool.scikit-build.overrides]]
if.any.env.CIBUILDWHEEL = true
if.any.env.CONDA_BUILD = true
if.any.env.PIWHEELS_BUILD = true
if.any.env.MY_CUSTOM_BUILD_EXTENSION = true
wheel.cmake = true

For opt-in binary builds.

SKBUILD_WHEEL_CMAKE or similar, but that doesn't exist

All configuration is available via envvar too, so that actually would work too.

The tasks at the top are what need to happen to allow control over failed builds. Each one will likely look a bit like #812.

@henryiii
Copy link
Collaborator Author

Overrides page: https://scikit-build-core.readthedocs.io/en/latest/overrides.html

henryiii added a commit that referenced this issue Jul 22, 2024
One of the steps for #112.

---------

Signed-off-by: Henry Schreiner <[email protected]>
henryiii added a commit that referenced this issue Jul 24, 2024
Adding a `fail = true` setting. One possible use:

```toml
[[tool.scikit-build.overrides]]
if.python-version = ">=3.13"
fail = true
```

This will be nicer once we allow customizing the error message.

See #112.

Signed-off-by: Henry Schreiner <[email protected]>
henryiii added a commit that referenced this issue Jul 25, 2024
henryiii added a commit that referenced this issue Jul 25, 2024
See #112.

This includes `.format()` so that items can be added here later in a
backward-compatible way.

---------

Signed-off-by: Henry Schreiner <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

No branches or pull requests

4 participants