Fix: resolve compiler paths from env vars with flags#435
Open
zhusy54 wants to merge 1 commit intohw-native-sys:mainfrom
Open
Fix: resolve compiler paths from env vars with flags#435zhusy54 wants to merge 1 commit intohw-native-sys:mainfrom
zhusy54 wants to merge 1 commit intohw-native-sys:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a _resolve_compiler utility to correctly parse compiler paths from environment variables, specifically handling cases where the variables include additional flags. The Gxx15Toolchain and GxxToolchain classes were updated to use this utility. Review feedback recommends moving the resolution logic into the class constructors to ensure that all compiler invocations, including those outside of CMake, consistently respect the environment variables.
Added _resolve_compiler() helper to properly extract and resolve compiler executables from environment variables that may contain compiler flags (e.g., 'gcc -pthread -B /path' from sysconfig). Updated Gxx15Toolchain and GxxToolchain to resolve CC/CXX in __init__ so all usages (CMake and direct subprocess invocations via KernelCompiler) benefit from the resolution. - Handle compiler env vars with embedded flags by splitting and extracting exe name - Use shutil.which() to resolve exe name to full path via PATH - Fix IndexError when CC/CXX contains only whitespace - Resolve CC/CXX in __init__ so KernelCompiler direct calls also use resolved paths - Gxx15Toolchain defaults to gcc-15/g++-15 for c++23 consistency - GxxToolchain uses self.cc_path/self.cxx_path in get_cmake_args for simplicity
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
_resolve_compiler()helper inpython/toolchain.pyto safely handleCC/CXXenv vars that contain compiler flags (e.g. conda-derivedgcc -pthread -B /path/compiler_compat)IndexErrorwhen env var is set to whitespace-only stringGxxToolchainandGxx15Toolchainto useself.cc_path/self.cxx_pathas defaults, ensuring consistency between the C and C++ compiler defaultsGxx15Toolchainnow defaults togcc-15/g++-15, consistent with its c++23 requirementRoot Cause
When
pip installuses scikit-build-core's isolated build environment, Python'ssysconfig.CC(gcc -pthread -B .../compiler_compat) is injected as theCCenvironment variable. This value contains flags and is not a validCMAKE_C_COMPILERpath, causing CMake configuration to fail.Testing
python3 -m pip install -e . --user --force-reinstallsucceeds without settingCC/CXXsysconfig.CCcontains extra flags