Skip to content

[python] Report the dlopen failure reason from load_library - #63

Open
conrade-ctc wants to merge 2 commits into
compiler-research:mainfrom
chicagotrading:pr-f-dlerror-text
Open

[python] Report the dlopen failure reason from load_library#63
conrade-ctc wants to merge 2 commits into
compiler-research:mainfrom
chicagotrading:pr-f-dlerror-text

Conversation

@conrade-ctc

Copy link
Copy Markdown
Contributor

Cpp::LoadLibrary drops the loader's failure reason, so load_library raises a bare error. This change asks the loader again with dlopen and reports its dlerror text, when the captured stderr is empty. A companion CppInterOp PR (compiler-research/CppInterOp#1101) emits the same reason on stderr directly; this change stands on its own if that PR lags. It adds a regression test for a missing library and a truncated ELF header.

Cpp::LoadLibrary drops the loader message, so load_library asks the
loader again with dlopen and reports its dlerror text.

Co-developed-with-the-help-of: Claude Code (Opus 5, human in the loop)

@aaronj0 aaronj0 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a nice improvement! Perhaps we could have Cpp::LoadLibrary give us the diagnostics on why it failed so we can avoid re-attempting dlopen with ctypes on the Python side.

edit: Just saw compiler-research/CppInterOp#1101, I think that would be the best solution here

@conrade-ctc

Copy link
Copy Markdown
Contributor Author

Agreed, #1101 is the right place for this, and I think a LoadLibrary that hands back the reason is the proper end state. #63 already prefers the captured stderr text and only falls back to the ctypes probe when that text is empty, so with #1101 in the pin the probe is idle. I kept it because the pin lags CppInterOp and the exception is what a notebook user sees, not stderr. Two options, your call: merge #63 as the interim and I remove the probe when the pin catches up, or I close it and follow #1101 with a CppInterOp API that returns the reason (an out-parameter overload of Cpp::LoadLibrary, say) and wire cppjit to that. I am happy either way.

@aaronj0

aaronj0 commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Agreed, #1101 is the right place for this, and I think a LoadLibrary that hands back the reason is the proper end state. #63 already prefers the captured stderr text and only falls back to the ctypes probe when that text is empty, so with #1101 in the pin the probe is idle. I kept it because the pin lags CppInterOp and the exception is what a notebook user sees, not stderr. Two options, your call: merge #63 as the interim and I remove the probe when the pin catches up, or I close it and follow #1101 with a CppInterOp API that returns the reason (an out-parameter overload of Cpp::LoadLibrary, say) and wire cppjit to that. I am happy either way.

We can add an optional out param to LoadLibrary and once that lands, use that in this PR (you can bump the pinned commit here so it builds with latest CppInterOp containing #1101)

Cpp::LoadLibrary now hands back the loader's reason through an optional
out-parameter (compiler-research/CppInterOp#1107), so the ctypes
re-dlopen probe goes away. The pin bump to a CppInterOp commit that
carries #1107 is folded in when it lands.

Co-developed-with-the-help-of: Claude Code (Fable 5.1, human in the loop)
@conrade-ctc

Copy link
Copy Markdown
Contributor Author

Opened compiler-research/CppInterOp#1107 with the optional std::string* error out-parameter, stacked on #1101. The cppjit side is pushed here already: load_library passes a std.string and reports its text, and the ctypes probe is gone. CI on this PR stays red until the pin can point at a CppInterOp commit that carries #1107; I will bump CPPINTEROP_GIT_TAG and squash as soon as it lands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants