llvmlite
llvmlite copied to clipboard
Support loading llvmlite from the main executable.
Change library loading code to also check for the presence of llvmlite statically linked into the main executable if the dynamic library fails to load. This allows llvmlite to be used in hermetic and embedded build cases.
Check that the version function can be dynamically resolved before declaring success.
Exit the resource context manager correctly so that any extracted files are unlinked correctly.
On failure, raise OSError with the first error chained.
Change library loading code to also check for the presence of a statically linked version of llvmlite if the dynamic library fails to load. This allows llvmlite to be used in hermetic and embedded build cases.
Thank you for adding this. There are a few ways to build llvmlite. By default the Numba team will statically link LLVM in both wheels and conda packages. See also:
https://llvmlite.readthedocs.io/en/latest/admin-guide/install.html#pre-built-binaries
Hence I am a little confused as to what benefit this PR brings. May I kindly request, that you describe in slightly more detail: what problems did you encounter and how does this help with the build? Thank you.
I modified the PR description slightly to make the intention hopefully more clear.
This CL addresses the case where you link llvmlite (and llvm) directly into the main executable. You might do this, for example, if you're using an embedded python runtime and don't want to distribute lots of additional shared libraries. It's admittedly a niche use-case (although an important one for us).
I'm happy to maintain this patch downstream if you feel like it's not sufficiently general,. but I do think there are some other small quality of life improvements in this code and it would be pretty easy to just drop the contextlib.nullcontext(None)
that supports looking up the symbols in the main executable.
I modified the PR description slightly to make the intention hopefully more clear.
This CL addresses the case where you link llvmlite (and llvm) directly into the main executable. You might do this, for example, if you're using an embedded python runtime and don't want to distribute lots of additional shared libraries. It's admittedly a niche use-case (although an important one for us).
I'm happy to maintain this patch downstream if you feel like it's not sufficiently general,. but I do think there are some other small quality of life improvements in this code and it would be pretty easy to just drop the
contextlib.nullcontext(None)
that supports looking up the symbols in the main executable.
OK, if this patch has value for you I don't mind considering it. It's just important that this does not impact what is currently working. Also, would you be open to adding more comments to the source code? There are a few lines where it isn't immediately obvious to me what is going on and they could IMHO do with some additional comments.
Done. LMK if there's anything that needs more clarification.
Done. LMK if there's anything that needs more clarification.
This looks better, thank you.
/azp run
Azure Pipelines successfully started running 1 pipeline(s).
@folded thank you again for the PR. CI seems green now and I have given the patch a more thorough review. There are a few questions left to address. Oh, and also: do you have any thoughts on how I could test this patch to verify it does indeed work in the kinds of environments you describe.
@folded thank you again for submitting this. I will now continue to review this.
@folded do you, by any chance, have some simple instructions for creating they type of binary that has llvmlite statically linked so that I can test that this works?
I haven't had time to look at this for a while, but I think it might be sensible to split it into logically separate components. Dealing with all the separate issues that came up had quite a few follow-on effects.
Would that be helpful?
I haven't had time to look at this for a while, but I think it might be sensible to split it into logically separate components. Dealing with all the separate issues that came up had quite a few follow-on effects.
Would that be helpful?
Yes, that sounds useful.
@folded it's been a while since I last heard from you, how do you want to proceed with this one?
My work for this week is to do the internal upgrade to 0.39.0, and as a result I'll be going through these patches again and cleaning them up. I'll get back to you soon.
My work for this week is to do the internal upgrade to 0.39.0, and as a result I'll be going through these patches again and cleaning them up. I'll get back to you soon.
Thank you for the update!
I've split this into 3 PRs, starting with folded:refactor-native-library-loading