scikit-build-core icon indicating copy to clipboard operation
scikit-build-core copied to clipboard

Ninja generator does not work on Windows due to (supposedly) scikit internal behavior

Open dograt opened this issue 5 months ago • 11 comments
trafficstars

Hi @henryiii . Is there a way to view and override the fact that scikit-build-core seems to be passing the architecture to CMake under the hood on Windows, even if I am using Ninja?

  CMake Error at CMakeLists.txt:16 (project):
    Generator

      Ninja

    does not support platform specification, but platform

      x64

    was specified.


  -- Configuring incomplete, errors occurred!

If it matters, I am using overrides to specify a CMake preset, which works fine on other OSs:

  if.platform-system = "^win32"
  build-dir = "build/desktop-windows"
  cmake.args = ["--preset=desktop-windows"]

I don't set the architecture anywhere myself, I so I'm assuming that scikit-build-core does this under the hood somewhere.

dograt avatar May 28 '25 01:05 dograt

Relates to https://github.com/scikit-build/scikit-build-core/issues/934, possibly?

dograt avatar May 28 '25 01:05 dograt

observed this without presets also by just passing

cmake.args = ["-G Ninja"]

I am using visual studio developer command shell, Ideally I want to use presets and compile using clang and directly invoking cmake works but I get the same error as reported when running scikit-build-core

BLaZeKiLL avatar Jun 01 '25 03:06 BLaZeKiLL

observed this without presets also by just passing

cmake.args = ["-G Ninja"]

I am using visual studio developer command shell, Ideally I want to use presets and compile using clang and directly invoking cmake works but I get the same error as reported when running scikit-build-core

First of all just -G Ninja failing was a cache issue but I looked further into presets

I looked into the code in cmake.py, builder.py and generator.py and found 2 things

def get_generator(self, *args: str) -> str | None:
    """
    Try to get the generator that will be used to build the project. If it's
    not set, return None (default generator will be used).
    """
    generators = [g for g in args if g.startswith("-G")]
    if generators:
        return generators[-1][2:].strip()
    return self.env.get("CMAKE_GENERATOR", None)

In the even if your generator is specified in presets the detected generator is None as it only looks for -G so the following in generator.py, will default the generator to msvc on windows and set the platform then when cmake starts it will read the preset and update the generator but the platform is still passed as x64 to the cache list

def set_environment_for_gen(
    generator: str | None,
    cmake: CMake,
    env: MutableMapping[str, str],
    ninja_settings: NinjaSettings,
) -> Mapping[str, str]:
    """
    This function modifies the environment as needed to safely set a generator.
    You should have used CMAKE_GENERATOR already to get the input generator
    string.

    A reasonable default generator is set if the environment does not already
    have one set; if ninja is present, ninja will be used over make on Unix.

    If gen is not None, then that will be the target generator.
    """
    allow_make_fallback = ninja_settings.make_fallback

    if generator:
        logger.debug("Set generator: {}", generator)
        allow_make_fallback = False
    else:
        generator = get_default(cmake) or ""
        if generator:
            logger.debug("Default generator: {}", generator)

    if sysconfig.get_platform().startswith("win") and "Visual Studio" in generator:
        # This must also be set when *_PLATFORM is set.
        env.setdefault("CMAKE_GENERATOR", generator)
        env.setdefault("CMAKE_GENERATOR_PLATFORM", get_cmake_platform(env))
        return {}

as a workaround just pass the generator along with preset

cmake.args = ["--preset=default", "-GNinja"]

BLaZeKiLL avatar Jun 01 '25 03:06 BLaZeKiLL

There's work to better integrate with presets in #994. I've been a bit unsure about how much we should integrate with presets; I've been using them a lot more lately so I think I've got a better idea now, but I think it's not critical, since for scikit-build-core you can always set values in pyproject.toml, which is really about the same thing (but better since you can use overrides, while presents don't have conditionals).

henryiii avatar Jun 01 '25 04:06 henryiii

Sorry for my late response. I did encounter this when trying to work on the presets, but I didn't look deeply. Thanks @BLaZeKiLL for checking the code. Just to double check my assumption, if you run with debug output, do you see CMAKE_GENERATOR_PLATFORM being set? I suspect that's thr conflicting part?

LecrisUT avatar Jun 01 '25 05:06 LecrisUT

Sorry for my late response. I did encounter this when trying to work on the presets, but I didn't look deeply. Thanks @BLaZeKiLL for checking the code. Just to double check my assumption, if you run with debug output, do you see CMAKE_GENERATOR_PLATFORM being set? I suspect that's thr conflicting part?

I don't see any debug logs for CMAKE_GENERATOR_PLATFORM but locally I did add print statements in the if condition and saw the following.

if sysconfig.get_platform().startswith("win") and "Visual Studio" in generator:
    # This must also be set when *_PLATFORM is set.
    print(f"GEN: {generator}, PLATFORM: {get_cmake_platform(env)}")
    env.setdefault("CMAKE_GENERATOR", generator)
    env.setdefault("CMAKE_GENERATOR_PLATFORM", get_cmake_platform(env))
    return {}
GEN: Visual Studio 17 2022, PLATFORM: x64

My pyproject.toml

cmake.args = [
    "--preset=default", 
    # "-GNinja"
]

My base config preset

{
	"name": "base",
	"hidden": true,
	"description": "Base congfig",
	"binaryDir": "${sourceDir}/build",
	"generator": "Ninja",
	"cacheVariables": {
		"CMAKE_INSTALL_PREFIX": "${sourceDir}/install"
	}
},

BLaZeKiLL avatar Jun 01 '25 06:06 BLaZeKiLL

Does removing CMAKE_GENERATOR_PLATFORM part help? Maybe we can use CMAKE_VS_PLATFORM_NAME_DEFAULT instead?

LecrisUT avatar Jun 01 '25 07:06 LecrisUT

Does removing CMAKE_GENERATOR_PLATFORM part help? Maybe we can use CMAKE_VS_PLATFORM_NAME_DEFAULT instead?

Yup removing CMAKE_GENERATOR_PLATFORM also helps

BLaZeKiLL avatar Jun 02 '25 07:06 BLaZeKiLL

Does removing CMAKE_GENERATOR_PLATFORM part help? Maybe we can use CMAKE_VS_PLATFORM_NAME_DEFAULT instead?

Yup removing CMAKE_GENERATOR_PLATFORM also helps

Removing it does have other side-effects also, my logs look a bit different will look into them further

BLaZeKiLL avatar Jun 02 '25 07:06 BLaZeKiLL

Great, appreciate the debugging effort. @henryiii thoughts on the CMAKE_GENERATOR_PLATFORM vs CMAKE_VS_PLATFORM_NAME_DEFAULT usage? It does seem to be ultimately a CMake bug that CMAKE_GENERATOR_PLATFORM is not ignored for the generators that don't support it.

LecrisUT avatar Jun 02 '25 08:06 LecrisUT

For the record, I was also hit by this. I could work around it by setting the generator via an environment variable (SKBUILD_CMAKE_ARGS="-G Ninja") opposed to setting it in pyproject.toml.

It looks like a bug in builder/generator.py to me: If the generator is set in pyproject.toml, it is still set to None in set_environment_for_gen, which results in it being set to the platform default (MSVC), which then triggers the if clause that adds CMAKE_GENERATOR_PLATFORM. If we start with the custom generator in the env, we never enter that branch and all builds fine.

dokempf avatar Sep 19 '25 12:09 dokempf