Some tests on Windows fail with ProcessNotRegisteredError
When building the package for conda, the following tests in tests/test_cmake_config.py fail on Windows:
test_cmake_pathstest_init_cachetest_cmake_args
at
config.configure()
with error ProcessNotRegisteredError:
Running e.g.:
pytest -vv -ra --showlocals -k "test_cmake_args"
...
if not cls.process_list[-1]._allow_unregistered:
> raise exceptions.ProcessNotRegisteredError(
"The process '%s' was not registered."
% (
command
if isinstance(command, str)
else " ".join(str(item) for item in command),
)
)
E pytest_subprocess.exceptions.ProcessNotRegisteredError: The process '%PREFIX%\Library\bin\cmake.EXE -S%SRC_DIR%\tests\packages\simple_pure -BC:\Users\buildu.
cls = <class 'pytest_subprocess.process_dispatcher.ProcessDispatcher'>
command = ['C:\\Users\\builidusername\\miniconda3\\envs\\c3i\\conda-bld\\scikit-build-core_1722583656353\\_test_env\\Library\\bin\\cmake.EXE',
'-SC:\\Users\\builidusername\\miniconda3\\envs\\c3i\\conda-bld\\scikit-build-core_1722583656353\\test_tmp\\tests\\packages\\simple_pure',
'-BC:\\Users\\builidusername\\AppData\\Local\\Temp\\pytest-of-builidusername\\pytest-86\\test_cmake_args0\\build',
'-DCMAKE_BUILD_TYPE:STRING=Release',
'-DSOMETHING=one']
command_instance = None
process_instance = None
processes = None
...
full log: scikit-build-core-0.9.9-pytest.log
Something is not working with the parameters in the registration of the commands here:
https://github.com/scikit-build/scikit-build-core/blob/715777d94d57ab0f9ae158f8690d609356a69753/tests/test_cmake_config.py#L140
https://github.com/scikit-build/scikit-build-core/blob/715777d94d57ab0f9ae158f8690d609356a69753/tests/test_cmake_config.py#L110
https://github.com/scikit-build/scikit-build-core/blob/715777d94d57ab0f9ae158f8690d609356a69753/tests/test_cmake_config.py#L56
as a test, if I replace *cmd with fp.any() to ignore the parameters, the registration works and the three tests pass.
On other platforms (linux, darwin) these three tests pass.
I could not determine if it was a \\ problem or something else.
scikit-build-core version: 0.9.9 or 0.9.8
Python: 3.8 to 3.11
Windows: 11
pytest-subprocess: 1.5.0
cmake: 3.26.4
Compiler: vs2019 win-64
The versions of the dependencies would be useful in this case. It seems that fp.program didn't expand the executable path correctly.
Maybe can also check on https://github.com/conda-forge/scikit-build-core-feedstock, there the tests don't seem to be covered properly
Thank you for your fast reply!
Apologies, here is the list of the dependencies used in the test environment:
attrs: 23.1.0-py312haa95532_0
blas: 1.0-mkl
bzip2: 1.0.8-h2bbff1b_6
ca-certificates: 2024.7.2-haa95532_0
cattrs: 23.1.2-py312haa95532_0
cmake: 3.26.4-h693b641_0
colorama: 0.4.6-py312haa95532_0
distlib: 0.3.8-py312haa95532_0
editables: 0.3-py312haa95532_0
expat: 2.6.2-hd77b12b_0
filelock: 3.13.1-py312haa95532_0
hatch-vcs: 0.3.0-py312haa95532_1
hatchling: 1.21.1-py312haa95532_0
iniconfig: 1.1.1-pyhd3eb1b0_0
intel-openmp: 2023.1.0-h59b6b97_46320
libffi: 3.4.4-hd77b12b_1
libuv: 1.48.0-h827c3e9_0
lz4-c: 1.9.4-h2bbff1b_1
markdown-it-py: 2.2.0-py312haa95532_1
mdurl: 0.1.0-py312haa95532_0
mkl: 2023.1.0-h6b88ed4_46358
mkl-service: 2.4.0-py312h2bbff1b_1
mkl_fft: 1.3.8-py312h2bbff1b_0
mkl_random: 1.2.4-py312h59b6b97_0
ninja: 1.10.2-haa95532_5
ninja-base: 1.10.2-h6d14046_5
numpy: 1.26.4-py312hfd52020_0
numpy-base: 1.26.4-py312h4dde369_0
openssl: 3.0.14-h827c3e9_0
packaging: 24.1-py312haa95532_0
pathspec: 0.10.3-py312haa95532_0
pip: 24.0-py312haa95532_0
platformdirs: 3.10.0-py312haa95532_0
pluggy: 1.0.0-py312haa95532_1
pybind11: 2.12.0-py312h59b6b97_0
pybind11-global: 2.12.0-py312h59b6b97_0
pygments: 2.15.1-py312haa95532_1
pyproject-metadata: 0.7.1-py312haa95532_0
pyproject_hooks: 1.0.0-py312haa95532_0
pytest: 7.4.4-py312haa95532_0
pytest-subprocess: 1.5.0-py312haa95532_0
python: 3.12.4-h14ffc60_1
python-build: 0.10.0-py312haa95532_0
rich: 13.7.1-py312haa95532_0
scikit-build-core: 0.9.9-py312h0158946_0 local
setuptools: 69.5.1-py312haa95532_0
setuptools-scm: 8.1.0-py312haa95532_0
sqlite: 3.45.3-h2bbff1b_0
tbb: 2021.8.0-h59b6b97_0
tk: 8.6.14-h0416ee5_0
trove-classifiers: 2023.10.18-py312haa95532_0
tzdata: 2024a-h04d1e81_0
vc: 14.2-h2eaa2aa_4
virtualenv: 20.26.1-py312haa95532_0
vs2015_runtime: 14.29.30133-h43f2093_4
vs2019_win-64: 19.29.30154-h96f319f_4
vswhere: 2.8.4-haa95532_0
wheel: 0.43.0-py312haa95532_0
xz: 5.4.6-h8cc25b3_1
zlib: 1.2.13-h8cc25b3_1
zstd: 1.5.5-hd43e919_2
There is a more recent version of pytest-subprocess: 1.5.0 but It does not seem to have changes regarding the expansion of the executable path: https://github.com/aklajnert/pytest-subprocess/compare/1.5.0...1.5.2
Indeed. From what I see in the CI jwlawson/actions-setup-cmake is used to setup cmake on the windows CI. Hopefully there's nothing funky there where it strips the .exe suffix and we are not covering that. We are also using vanilla windows runners though so it shouldn't be the case. 🤔
The thing that looks weird to me is the un-expanded environment variables. %PREFIX%\Library\bin\cmake.EXE instead of C:\\Users\\builidusername\\miniconda3\\envs\\c3i\\conda-bld\\scikit-build-core_1722583656353\\_test_env\\Library\\bin\\cmake.EXE. If pytest-subprocess is just comparing strings, those are not the same string.
Edit: oh, I can't read. That's what you were talking about...
The scikit-build-core feedstock only tests Linux, as it's hard to get it to test pure-Python feedstocks on macOS and Windows.
Could you try https://github.com/scikit-build/scikit-build-core/pull/844 ?
Could you try #844 ?
I did try it. It did not work.
I believe the expansion might happen but with this problem: https://github.com/conda/conda-build/issues/4264#issuecomment-1050817754
Or some other thing weird is happening.
If I have:
print("---config.source_dir-----")
print(str(config.source_dir))
print(repr(config.source_dir))
print("--------")
for c in configure_args(config):
print(str(c), "****", repr(c))
print("--------")
cmd = [os.fspath(c) for c in configure_args(config)]
print("---cmd-----")
print(cmd)
print("--------")
print("---*cmd-----")
print(*cmd)
print("--------")
I will see printed:
---config.source_dir-----
%SRC_DIR%\tests\packages\simple_pure
WindowsPath('C:/Users/lorepirri/miniconda3/envs/c3i/conda-bld/scikit-build-core_1722646771709/test_tmp/tests/packages/simple_pure')
--------
-S%SRC_DIR%\tests\packages\simple_pure **** '-SC:\\Users\\lorepirri\\miniconda3\\envs\\c3i\\conda-bld\\scikit-build-core_1722646771709\\test_tmp\\tests\\packages\\simple_pure'
-BC:\Users\lorepirri\AppData\Local\Temp\pytest-of-lorepirri\pytest-104\test_cmake_args0\build **** '-BC:\\Users\\lorepirri\\AppData\\Local\\Temp\\pytest-of-lorepirri\\pytest-104\\test_cmake_args0\\build'
--------
---cmd-----
['-SC:\\Users\\lorepirri\\miniconda3\\envs\\c3i\\conda-bld\\scikit-build-core_1722646771709\\test_tmp\\tests\\packages\\simple_pure', '-BC:\\Users\\lorepirri\\AppData\\Local\\Temp\\pytest-of-lorepirri\\pytest-104\\test_cmake_args0\\build']
--------
---*cmd-----
-S%SRC_DIR%\tests\packages\simple_pure -BC:\Users\lorepirri\AppData\Local\Temp\pytest-of-lorepirri\pytest-104\test_cmake_args0\build
--------
Oh yeah forgot about that azure/conda "feature". They just replace in the generated log the known values of environment variables. It does not make things confusing at all 😑.
o the two paths are different?
Ah, we are seeing the same string twice, and it's replacing one of them. We aren't told what has been registered here.
How hard is it to set up in CI?
We could run a modified version of conda-build next week to disable that annoying log replacement and see if we can get the real paths.
Thank you all for helping out!
I found the problem. I apologize I got hallucinated by the nice conda feature and thought that the problem was in the expansion of the path, and I stopped there.
I am also grateful for the truncated log that conda build offers us.
There is a parameter too much: -DCMAKE_BUILD_TYPE:STRING=Release
The function configure_args in the tests that is used to prepare the arguments does not consider that there might be a CMAKE_GENERATOR set in the environment (and I have it set to Ninja, for good or bad):
https://github.com/scikit-build/scikit-build-core/blob/901b1d31d9b7b47ee93aa2ef6f887c15e82e7f7a/tests/test_cmake_config.py#L21-L30
The configure() in src/scikit_build_core/cmake.py mutates single_config just right before calling the Run(...).live(...)
https://github.com/scikit-build/scikit-build-core/blob/901b1d31d9b7b47ee93aa2ef6f887c15e82e7f7a/src/scikit_build_core/cmake.py#L210-L212
I tested this code in tests/test_cmake_config.py and it works:
def configure_args(
config: CMaker,
*,
init: bool = False,
defines: Mapping[str, str | os.PathLike[str] | bool] | None = None,
cmake_args: Sequence[str] = (),) -> Generator[str, None, None]:
yield f"-S{config.source_dir}"
yield f"-B{config.build_dir}"
_cmake_args = config._compute_cmake_args(defines or {})
all_args = [*_cmake_args, *cmake_args]
gen = config.get_generator(*all_args)
if gen:
single_config = gen == "Ninja" or "Makefiles" in gen
if (single_config or config.single_config) and config.build_type:
yield f"-DCMAKE_BUILD_TYPE:STRING={config.build_type}"
if init:
cmake_init = config.build_dir / "CMakeInit.txt"
yield f"-C{cmake_init}"
I don't know if it's the best solution for you. One might think of extracting the part leading to mutating self.single_config into another method and using that from the tests.
If it is, please let me know and I will fix the PR with this change.
On the other hand, it might not be ideal to have the environmental variable CMAKE_GENERATOR set for testing.
I think we probably should monkeypatch the test to remove the envvar. If it’s important, we could also make a new or parameterized test with the envvar set. You generally don’t want a test to depend on the environment and run differently.
I think we probably should monkeypatch the test to remove the envvar. If it’s important, we could also make a new or parameterized test with the envvar set. You generally don’t want a test to depend on the environment and run differently.
Thank you @henryiii.
PR #847 should set things right for this particular problem. Let me know if the solution is acceptable.
Is this fixed in the latest release?
Yes, thanks for working on it and merging it!