Skip to content

Commit ebbdbde

Browse files
committed
Better cmake_binary Resolution
1 parent 038d20e commit ebbdbde

File tree

2 files changed

+53
-25
lines changed

2 files changed

+53
-25
lines changed

cppython/plugins/cmake/resolution.py

Lines changed: 50 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,62 @@
11
"""Builder to help resolve cmake state"""
22

3+
import logging
34
import os
5+
import shutil
46
from pathlib import Path
57
from typing import Any
68

79
from cppython.core.schema import CorePluginData
810
from cppython.plugins.cmake.schema import CMakeConfiguration, CMakeData
911

1012

13+
def _resolve_cmake_binary(configured_path: Path | None) -> Path | None:
14+
"""Resolve the cmake binary path with validation.
15+
16+
Resolution order:
17+
1. CMAKE_BINARY environment variable (highest priority)
18+
2. Configured path from cmake_binary setting
19+
3. cmake from PATH (fallback)
20+
21+
If a path is specified (via env or config) but doesn't exist,
22+
a warning is logged and we fall back to PATH lookup.
23+
24+
Args:
25+
configured_path: The cmake_binary path from configuration, if any
26+
27+
Returns:
28+
Resolved cmake path, or None if not found anywhere
29+
"""
30+
logger = logging.getLogger('cppython.cmake')
31+
32+
# Environment variable takes precedence
33+
if env_binary := os.environ.get('CMAKE_BINARY'):
34+
env_path = Path(env_binary)
35+
if env_path.exists():
36+
return env_path
37+
logger.warning(
38+
'CMAKE_BINARY environment variable points to non-existent path: %s. '
39+
'Falling back to PATH lookup.',
40+
env_binary,
41+
)
42+
43+
# Try configured path
44+
if configured_path:
45+
if configured_path.exists():
46+
return configured_path
47+
logger.warning(
48+
'Configured cmake_binary path does not exist: %s. '
49+
'Falling back to PATH lookup.',
50+
configured_path,
51+
)
52+
53+
# Fall back to PATH lookup
54+
if cmake_in_path := shutil.which('cmake'):
55+
return Path(cmake_in_path)
56+
57+
return None
58+
59+
1160
def resolve_cmake_data(data: dict[str, Any], core_data: CorePluginData) -> CMakeData:
1261
"""Resolves the input data table from defaults to requirements
1362
@@ -27,11 +76,7 @@ def resolve_cmake_data(data: dict[str, Any], core_data: CorePluginData) -> CMake
2776
modified_preset_file = root_directory / modified_preset_file
2877

2978
# Resolve cmake binary: environment variable takes precedence over configuration
30-
cmake_binary: Path | None = None
31-
if env_binary := os.environ.get('CMAKE_BINARY'):
32-
cmake_binary = Path(env_binary)
33-
elif parsed_data.cmake_binary:
34-
cmake_binary = parsed_data.cmake_binary
79+
cmake_binary = _resolve_cmake_binary(parsed_data.cmake_binary)
3580

3681
return CMakeData(
3782
preset_file=modified_preset_file, configuration_name=parsed_data.configuration_name, cmake_binary=cmake_binary

cppython/plugins/conan/plugin.py

Lines changed: 3 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
"""
77

88
import os
9-
import shutil
109
from logging import Logger, getLogger
1110
from pathlib import Path
1211
from typing import Any
@@ -242,23 +241,6 @@ def sync_data(self, consumer: SyncConsumer) -> SyncData:
242241

243242
raise NotSupportedError(f'Unsupported sync types: {consumer.sync_types()}')
244243

245-
@staticmethod
246-
def _resolve_cmake_binary(cmake_path: Path | str | None) -> str | None:
247-
"""Resolve the cmake binary path.
248-
249-
If an explicit path is provided, use it. Otherwise, try to find cmake
250-
in the current Python environment (venv) to ensure we use the same
251-
cmake version for all operations including dependency builds.
252-
253-
Args:
254-
cmake_path: Explicit cmake path, or None to auto-detect
255-
256-
Returns:
257-
Resolved cmake path as string, or None if not found
258-
"""
259-
resolved = cmake_path or shutil.which('cmake')
260-
return str(Path(resolved).resolve()) if resolved else None
261-
262244
def _sync_with_cmake(self, consumer: SyncConsumer) -> CMakeSyncData:
263245
"""Synchronize with CMake generator and create sync data.
264246
@@ -269,8 +251,9 @@ def _sync_with_cmake(self, consumer: SyncConsumer) -> CMakeSyncData:
269251
CMakeSyncData configured for Conan integration
270252
"""
271253
# Extract cmake_binary from CMakeGenerator if available
272-
if isinstance(consumer, CMakeGenerator) and not os.environ.get('CMAKE_BINARY'):
273-
self._cmake_binary = self._resolve_cmake_binary(consumer.data.cmake_binary)
254+
# The cmake_binary is already validated and resolved during CMake data resolution
255+
if isinstance(consumer, CMakeGenerator) and consumer.data.cmake_binary:
256+
self._cmake_binary = str(consumer.data.cmake_binary.resolve())
274257

275258
return self._create_cmake_sync_data()
276259

0 commit comments

Comments
 (0)