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

Possibly misleading warning about including ninja in build-system.requires #953

Open
gareth-cross opened this issue Nov 28, 2024 · 5 comments

Comments

@gareth-cross
Copy link

  • scikit-build-core version: 0.10.7
  • python version: 3.12.7
  • OS: Mac OS X (although this happens on linux as well)

My pyproject.toml (full source) contains the following lines in the build-system section:

[build-system]
requires = [
    "scikit-build-core==0.10.7",
    "cmake>=3.20",
    "ninja>=1.5",
    "mypy==1.9.0",
]
build-backend = "scikit_build_core.build"

I also have the following in the sckit-build section:

[tool.scikit-build]
cmake.version = ">=3.20"
# ...
ninja.version = ">=1.5"

When running pip wheel --verbose, I always see the following warnings:

  Running command Preparing metadata (pyproject.toml)
  2024-11-28 11:30:58,845 - scikit_build_core - WARNING - cmake should not be in build-system.requires - scikit-build-core will inject it as needed
  2024-11-28 11:30:58,845 - scikit_build_core - WARNING - ninja should not be in build-system.requires - scikit-build-core will inject it as needed
  *** scikit-build-core 0.10.7 using CMake 3.31.1 (metadata_wheel)
  Preparing metadata (pyproject.toml) ... done
...
  2024-11-28 11:30:59,407 - scikit_build_core - WARNING - cmake should not be in build-system.requires - scikit-build-core will inject it as needed
  2024-11-28 11:30:59,407 - scikit_build_core - WARNING - ninja should not be in build-system.requires - scikit-build-core will inject it as needed

The warning about ninja appears to be incorrect though, since removing it from build-system.requires breaks the build with:

  *** scikit-build-core 0.10.7 using CMake 3.29.2 (wheel)
  *** Configuring CMake...
  Traceback (most recent call last):
  ...
    File "/private/var/folders/j7/mfkd_rrx1lx_qnvj8mpylqrh0000gn/T/pip-build-env-y0a9qq7d/overlay/lib/python3.12/site-packages/scikit_build_core/build/__init__.py", line 33, in build_wheel
      return _build_wheel_impl(
             ^^^^^^^^^^^^^^^^^^
    File "/private/var/folders/j7/mfkd_rrx1lx_qnvj8mpylqrh0000gn/T/pip-build-env-y0a9qq7d/overlay/lib/python3.12/site-packages/scikit_build_core/build/wheel.py", line 175, in _build_wheel_impl
      return _build_wheel_impl_impl(
             ^^^^^^^^^^^^^^^^^^^^^^^
    File "/private/var/folders/j7/mfkd_rrx1lx_qnvj8mpylqrh0000gn/T/pip-build-env-y0a9qq7d/overlay/lib/python3.12/site-packages/scikit_build_core/build/wheel.py", line 402, in _build_wheel_impl_impl
      builder.configure(
    File "/private/var/folders/j7/mfkd_rrx1lx_qnvj8mpylqrh0000gn/T/pip-build-env-y0a9qq7d/overlay/lib/python3.12/site-packages/scikit_build_core/builder/builder.py", line 166, in configure
      local_def = set_environment_for_gen(
                  ^^^^^^^^^^^^^^^^^^^^^^^^
    File "/private/var/folders/j7/mfkd_rrx1lx_qnvj8mpylqrh0000gn/T/pip-build-env-y0a9qq7d/overlay/lib/python3.12/site-packages/scikit_build_core/builder/generator.py", line 130, in set_environment_for_gen
      raise NinjaNotFoundError(msg)
  scikit_build_core.errors.NinjaNotFoundError: Ninja is required to build
  error: subprocess-exited-with-error

It would be good to get clarification on this point - is ninja required in build-system.requires? Maybe the warning is incorrect.

@henryiii
Copy link
Collaborator

henryiii commented Nov 28, 2024

This looks like it might be a bug. Any special setup on your system? I can try it soonish.

Also, if you enable logging, it should tell you if it finds ninja.

@henryiii
Copy link
Collaborator

henryiii commented Nov 28, 2024

Ah, got it. You have the default ninja.make-fallback = true, but you are forcing Ninja via -GNinja. I think we might be able to change the default for this parameter is the generator is selected, we do have the ability to detect what you've selected. You can set ninja.make-fallback = false to fix. Or remove the explicit generator selection if you are okay with the fallback.

@gareth-cross
Copy link
Author

Ah, got it. You have the default ninja.make-fallback = true, but you are forcing Ninja via -GNinja. I think we might be able to change the default for this parameter is the generator is selected, we do have the ability to detect what you've selected. You can set ninja.make-fallback = false to fix. Or remove the explicit generator selection if you are okay with the fallback.

I think maybe that is backwards? I have ninja.make-fallback = false presently on line 52. If I understand that setting, it means ninja will always be required/used (which is what I want). Do I have that correct?

The -G Ninja arg was explicitly set because I found that scikit-build-core would fall back to MSBuild on Windows (at least as far as I recall, it is has been a while since I set that), and I would rather force it to use Ninja everywhere.

@LecrisUT LecrisUT assigned LecrisUT and unassigned LecrisUT Nov 29, 2024
@LecrisUT
Copy link
Collaborator

Yes, the option would allow to fallback to make (and maybe msvc). I think we should flip the default because it would make the builds more predictable by default. Maybe the tricky build to consider would be msvc

@gareth-cross
Copy link
Author

The behavior I would like to achieve is:

  • ninja is the only build system used on all platforms.
  • If ninja is not available or cannot be installed, fail (no fallback).
  • scikit-build-core adds ninja to the requirements automatically (I sort of thought this was the expected behavior because of the warning, but maybe not?)

My understanding is that my current configuration already achieves this because:

  • I have set make_fallback = false
  • I have placed ninja in my build-system.requirements.

Do I have all that correct?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants