Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can not build with lua-lib option #218

Closed
synodriver opened this issue Aug 13, 2022 · 7 comments
Closed

Can not build with lua-lib option #218

synodriver opened this issue Aug 13, 2022 · 7 comments
Milestone

Comments

@synodriver
Copy link
Contributor

synodriver commented Aug 13, 2022

With the latest commit(3016db8), lupa seems to generate lua.pyx when lua-lib and lua-includes is set, which conflict with lua.pxd because cython thinks lua.pyx is the implementation for lua.pxd. Choosing another name for it should be fine. For instance, _lua.pyx.

The environment:

pip list|grep Cython
Cython                       0.29.32

python -V
Python 3.10.4

uname -a
Linux codespaces-f9df62 5.4.0-1086-azure #91~18.04.1-Ubuntu SMP Thu Jun 23 20:33:05 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
@synodriver
Copy link
Contributor Author

What about changing ext_name = config.get('libversion', 'lua') to ext_name = config.get('libversion', '_lua') ? And so do __init__.py

@scoder
Copy link
Owner

scoder commented Aug 13, 2022

Hmm. Yes, calling it lua.pyx is a bad idea. We could call it lupa instead, although lupa.lupa would also seem confusing.

The intention is really to allow from lupa import lua54. The problem arises when we cannot know the library version before (configuring and) building the extension module, because the library was provided by the user in the form of a path to an arbitrary external file. If we know the header path, then we can try to parse those, but they might just be installed somewhere in the system.

I think a generic name is really the best we can (and should) do here.

@scoder scoder added this to the 2.0 milestone Aug 13, 2022
@scoder
Copy link
Owner

scoder commented Aug 13, 2022

Thanks for the timely report, BTW. I appreciate the testing and the feedback.

@synodriver
Copy link
Contributor Author

synodriver commented Aug 14, 2022

I think we can add a prefix to all modules, like _, so it becomes from lupa import _lua54 and from lupa import _lua. If the version is really necessary,maybe we can use ctypes to load those libraries to get the version. Or we can just change lua.pxd's name

@synodriver
Copy link
Contributor Author

synodriver commented Aug 14, 2022

Anyway, the easiest way should be change the pxd file's name. Maybe luaapi.pxd?

@scoder
Copy link
Owner

scoder commented Aug 14, 2022

I think we can add a prefix to all modules, like _, so it becomes from lupa import _lua54 and from lupa import _lua.

That's not what you'd want to write as a user, is it?

maybe we can use ctypes to load those libraries to get the version.

It's not about loading the Lua library. Each of the version modules is a complete (and usually statically linked) copy of the lupa module. ctypes won't help.

Or we can just change lua.pxd's name

It's possible that someone uses that .pxd file for talking to the Lua library directly. But I doubt that that's a big risk. Lupa doesn't expose its underlying Lua implementation anywhere. luaapi.pxd sounds good. I'll change the file name.

@scoder scoder closed this as completed in f63d7f8 Aug 14, 2022
@synodriver
Copy link
Contributor Author

BTW, there is another issue related to this commit. If you build with this option on windows, lupa will inject a lua51.dll (or maybe some name else) to the wheel, thus lupa will think lua51.dll is a valid python extension (but it's not) and fails to import unless you use from lupa.lua import LuaRuntime. So in my opinion, the _import_newest_lib should avoid grabing any .dll on windows.

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

No branches or pull requests

2 participants