riconnon
commented
1 year ago I think https://github.com/scoder/lupa/pull/247 might be the fix for this.
Closed riconnon closed 11 months ago
riconnon
commented
1 year ago I think https://github.com/scoder/lupa/pull/247 might be the fix for this.
scoder
commented
1 year ago Bringing the general discussion back here from #247.
It was found that only one luaXY module can be loaded with global symbols enabled. Otherwise, symbols can get mixed up and link incorrectly when loading Lua extensions.
To allow users an easy selection of the one module to enable it for, we could use lazy loading for the extension modules, but that would probably just make it more obscure when they are actually being loaded. And you'd get random failures late on first usage if anything is wrong with them or the way they are loaded.
One proposal that would actually be possible is an import wrapper, e.g.
lua = lupa.load_lua("5.4", dlopen=True)
Then users would only ever import lupa and leave the rest to that package. The advantage is that users wouldn't have to care about the platform specific details.
I'm uncertain whether this is worth the complexity, though. I doubt that Lua extensions are a sufficiently common thing to use in Lupa's context to make all users go through a non-standard import mechanism.
riconnon
commented
1 year ago Great summary @scoder and I really appreciate your engagement on this issue!
I think, to perhaps aid discussion, I want summarise the state as of the end of the lupa 1.x series:
What changed in 2.x is the position of the loading of the module and now the system lua dynamic linking no longer works out of the box with system lua.
I, personally, have no qualms adopting explicit use of a context manager to set the flags as required, but I believe it's worth noting this is a regression, specifically for system lua users, compared to 1.x
I do wonder if, for system lua users, where we can guarantee there is only one lua runtime due to the way that setup.py works we could still automatically enable the same dlopenflags.
riconnon
commented
1 year ago I'm uncertain whether this is worth the complexity, though. I doubt that Lua extensions are a sufficiently common thing to use in Lupa's context to make all users go through a non-standard import mechanism.
A further thought on this topic... On the one hand I'm the only person to report this and 2.x has been out for a while, but on the other hand I've been tracking this for a while but was happy to stick on 1.x until the Python 3.12 release prompted me to need a lupa built with a newer Cython.
scoder
commented
1 year ago I'm still not sure regarding the default behaviour. I mean, there were cases where it used to work before, as you wrote in the PR, notably the "I built my own lupa" case with a single extension module. It doesn't seem wrong to enable module loading when you just say "import lupa; lua = lupa.LuaRuntime()`. It would then even work in a few more cases than before.
However, larger applications contexts are an issue where more than one party wants to use Lupa, e.g. two independent dependencies. Then one might say from lupa import LuaRuntime and the other with modload(): from lupa import lua53, and that kind of innocent code would lead to a collision now. Any such setup would run into problems, really, because only one Lua module can support Lua extensions and the first one necessarily wins.
So, definitely pros and cons for both, which speaks for leaving it to the users entirely.
riconnon
commented
10 months ago @scoder I think you said you were keen to release 2.1 with this fix (among others) Do you have any plans for the timescale of that?
I'm using the system Lua installation with lupa such that I can load C modules built against Lua.
Upgrading from 1.14.1 to 2.x I start to get undefined symbol errors when loading such a module.
See the following output for minimal steps to reproduce in the official Python docker image:
By comparison, using 1.14.1, this works fine: