Read CppInterOp location from CPPINTEROP_BIN_DIR env var#211
Open
conrade-ctc wants to merge 1 commit into
Open
Read CppInterOp location from CPPINTEROP_BIN_DIR env var#211conrade-ctc wants to merge 1 commit into
conrade-ctc wants to merge 1 commit into
Conversation
aaronj0
reviewed
Jun 26, 2026
Comment on lines
+135
to
+140
| #elif defined(_WIN32) | ||
| return dir + "/lib/libclangCppInterOp.dll"; | ||
| #elif defined(__APPLE__) | ||
| return dir + "/lib/libclangCppInterOp.dylib"; | ||
| #else | ||
| return dir + "/lib/libclangCppInterOp.so"; |
Collaborator
There was a problem hiding this comment.
Do we need these branches? Seems unlikely that this variable isn't set, do you have an example where this was the case?
Author
There was a problem hiding this comment.
We're not using cmake in our env, so setting it feels artificial... however, since cmake will set the suffix, the WIN32 and APPLE special cases seem unnecessary, and maybe we just go with the main CMAKE_SHARED_LIBRARY_SUFFIX branch with ".so" as the default if not defined. I'll update, let me know if that doesn't seem reasonable.
The CppInterOp install prefix was baked into a CPPINTEROP_DIR macro at build time, so a relocated backend could not find libclangCppInterOp. Add cppinterop_dir()/cppinterop_lib_path() helpers that prefer the CPPINTEROP_BIN_DIR environment variable, fall back to the compile-time default, and report a clear error (instead of relying on the macro always being defined) when the location is unknown. Rename the macro to CPPINTEROP_BIN_DIR to match. Co-developed-with-the-help-of: Claude Code (Claude Opus 4.8)
conrade-ctc
force-pushed
the
unify_path_defines_and_env
branch
from
198aceb to
98c296c
Compare
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
2 participants
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
CPPINTEROP_DIRmacro at build time, so a relocated backend could not findlibclangCppInterOp.cppinterop_dir()/cppinterop_lib_path()helpers that prefer theCPPINTEROP_BIN_DIRenvironment variable, fall back to the compile-time default, and report a clear error (instead of relying on the macro always being defined) when the location is unknown.CPPINTEROP_BIN_DIRto match.Test plan
test_cppyy: 501 passed, 69 skipped, 19 xfailed, 2 xpassed.import cppyywith a bogusCPPINTEROP_BIN_DIRprints a clear diagnostic instead of crashing.🤖 Done with the help of Claude Code (Claude Opus 4.8)