Unconditional preset python cmake variables

[wenjuno]

I run into problem using Python_EXECUTABLE variable in Windows build due to the backslash in the string. After some debugging I find out that scikit-build-core set Python_EXECUTABLE (and some other python related variables) via "-C" option of cmake that pre-populate the cmake cache. I don't find any configuration to override/disable this behavior. I would like to know what is the reason to impose these variables unconditionally to my build configuration. Thanks.

[henryiii]

Scikit-build-build has to communicate the Python that's running it to the CMake run. Otherwise CMake will possibly grab the wrong Python instead of the one that's running the process. It also communicates other things, like where to put the files that should go in the wheel. However, this file uses the CMake path syntax which is operating system independent, so it should not break (and we test extensively on Windows, many users use Windows, etc). Do you have more info on what's breaking? Logs, a repo, etc?

[wenjuno]

Hi @henryiii, thanks for the reply.

1. It's reasonable to make sure CMake picks the desired Python to be consistent. But I think the override could be improved. Instead of overriding a slew of Python_* variables, how about we just set Python_ROOT (as advised by find_package function) or Python_ROOT_DIR (as advised by FindPython module) and let the FindPython module set other variables properly?
2. To your question about what exactly is the problem, the "Python_EXECUTABLE" is set as STRING type, not PATH type. It preserves the backslash literally and when I use it in install(CODE ..) function, it chokes on invalid character escape. For instance, the path starts with "C:\Users" and it omplains about "\U". I can give you an exact repro, but I think this is moot if you take my suggestion in (1) to use just Python_ROOT.
   Thanks.

[LecrisUT]

Re point 1 the cross-compilation support with crossenv will be important. Will expand more in #1050.

But changing the values to be FILEPATH and PATH is reasonable. Here are some relevant parts if you want to make a quick PR

scikit-build-core/src/scikit_build_core/cmake.py

Lines 170 to 173 in d52df05

	elif isinstance(value, os.PathLike):
	# Convert to CMake's internal path format
	str_value = str(value).replace("\\", "/")
	f.write(f'set({key} [===[{str_value}]===] CACHE PATH "" FORCE)\n')

scikit-build-core/src/scikit_build_core/builder/builder.py

Lines 242 to 244 in d52df05

	cache_config[f"{prefix}_EXECUTABLE"] = sys.executable
	cache_config[f"{prefix}_ROOT_DIR"] = sys.prefix
	cache_config[f"{prefix}_INCLUDE_DIR"] = python_include_dir

[wenjuno]

Do you mean you can't use Python_ROOT but rather having to set whole bunch of Python_ variable individually? Do you mean FindPython is incapable of setting those variable correctly if only Python_ROOT is set?

[LecrisUT]

It is complicated. As you can see Python_ROOT_DIR is being used which is effectively the same, but for something like crossenv the Python_EXECUTABLE is from a different root (host instead of target). There is also Python_ARTIFACTS_PREFIX which we should see if we want to expose.

But you are right that these should be converted to Path() and just to be extra safe, the cache variables should try to make a FILEPATH if it exists and it is a file.

[henryiii]

Why does this work today on most packages, though? We have CI testing on Windows, I've seen this working for lots of packages, why is it only failing for you? And why would setting Python_ROOT work with backslashes but Python_EXECUTABLE not? Ahh, I see you are using it. I've also seen it used, but maybe it was always Python::Interpreter, which is much safer, by the way, and will error if Python is not found, instead of just be empty. (PS: I'd recommend trying this as a quick workaround, by the way! It works most but not all places)

I originally did just set one value, but had to add others due to bugs finding Python. I think I had to add this one almost immediately. One example is if you have multiple versions of Python side-by-side, which happens, for example, if you have python3.13 and python3.13t. I think there were other bugs too, but that's why we've had a slowly growing number of values set here.

We don't need or want FindPython to find Python. We literally are running from the Python we want it to find¹, and finding anything else is an error. However, we should set it with a CMake style path, not a Windows style path (true for all paths!), and it looks like that might not be happening. If it's a CMake style path, then it will behave just like if CMake itself set it. Looks like we just need to wrap it like Path(sys.executable). I'm quite surprised this was missed, the reason we have the custom handling around paths was from trial/error getting paths to work. And one (actually several) of the core paths is missed?

BTW, PATH vs. STRING doesn't matter, but the handling code @LecrisUT highlighted does matter². The CMake type is just for GUIs when displaying the cache options, like ccmake.

Edit: install(CODE might be a place where targets are harder to use. I'd have to see if it's possible via generator expression. It also might that this case is special, which is why the wrong path style causes a problem. The locations expecting a path might be able to convert if they get the wrong type of path.

Footnotes

1. Except when cross-compiling, but that's even more important to set all the values correctly. ↩

2. For the set command. The command line does actually convert this to an absolute CMake path. ↩

[wenjuno]

In my testing the variable type makes a world of difference. When I test manually by setting the variable in command line, if I use something like -DPython_EXECUTABLE=C:\User, it crashes when I run install due to the invalid escape sequence "\U". But if I use -DPython_EXECUTABLE:PATH=C:\User, it works. I'm using CMake 4.0.1, I'm not sure if older CMake has different behavior.

[henryiii]

Do you have a sample of an install(CODE that I could stick in a test? I just want to do something basic, like print hello, that should trigger the error.

[henryiii]

I said above the command line makes a difference, but not the set command. That has to be a CMake style path already. The command line converts if it's PATH or FILEPATH. The thing that we are doing wrong is this is avoiding our Windows -> CMake path conversion because the type is a string.

[wenjuno]

CMakeLists.txt:

cmake_minimum_required(VERSION 3.31)
project(Test1)
find_package(Python COMPONENTS Interpreter REQUIRED)
install(CODE "execute_process(COMMAND ${Python_EXECUTABLE} -V COMMAND_ECHO STDOUT)")

Command line:

cmake -G Ninja -DPython_EXECUTABLE=C:\uenv\python.exe
ninja install

Error message:

  when parsing string

    C:\uenv\python.exe

  Invalid character escape '\u'.

[wenjuno]

I said above the command line makes a difference, but not the set command. That has to be a CMake style path already. The command line converts if it's PATH or FILEPATH. The thing that we are doing wrong is this is avoiding our Windows -> CMake path conversion because the type is a string.

Set it via command line or via set() function is the same for me, STRING type keeps the backslash and breaks, PATH type handles the backslash and works. I don't think the manual conversion is the critical part.

[henryiii]

Interesting, I'm going from the documentation, haven't booted up a Windows machine. I was able to reproduce the failure in tests, working on a fix!