-
-
Notifications
You must be signed in to change notification settings - Fork 141
WIP Luau support for Lupa #285
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
base: master
Are you sure you want to change the base?
Conversation
Ok, so update on this Turns out theres a lot of really weird behavior on Lupa. For example, function calls use thread state and calling lua_pcall from thread state isnt really a great idea (im surprised lupa's test suite works on lua 5.1 but luau crashes because L->status != 0 when you try doing a lua_pcall on a function that comes from a yielded coroutine |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, thanks!
It's a bit verbose and can be stripped down, but here are some comments so far.
option_use_luau = has_option('--use-luau') | ||
if option_use_luau and option_no_bundle: | ||
print("Cannot use --use-luau together with --no-bundle") | ||
sys.exit(1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather build everything by default if we're using the bundled libraries and require users to exclude what they don't want in that case. Let's spell it the other way round, --no-luau
, as with --no-luajit
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One question here about this: I’m not sure how to support non bundled luau in lupa since I don’t think luau provides shared libs with the required compiler options (or even shared libs at all). What is the best way to handle this?
This whole issue of bundling is why I initially chose to make it use luau and not no luau as well for backwards compatibility but yeah, will change it to no luau
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That sounds like there are only three cases:
- not bundled, without Luau (
--no-bundle
) - bundled, including Luau (default or
--use-bundle
) - bundled, without Luau (default or
--use-bundle
, plus--no-luau
)
if option_use_luau: | ||
configs = [ | ||
use_bundled_luau(lua_bundle_path, c_defines) | ||
for lua_bundle_path in glob.glob(os.path.join(basedir, 'third-party', 'luau*' + os.sep)) | ||
] | ||
else: | ||
configs = [ | ||
use_bundled_lua(lua_bundle_path, c_defines) | ||
for lua_bundle_path in glob.glob(os.path.join(basedir, 'third-party', 'lua*' + os.sep)) | ||
if not ( | ||
False | ||
# LuaJIT 2.0 on macOS requires a CPython linked with "-pagezero_size 10000 -image_base 100000000" | ||
# http://t-p-j.blogspot.com/2010/11/lupa-on-os-x-with-macports-python-26.html | ||
# LuaJIT 2.1-alpha3 fails at runtime. | ||
or (platform == 'darwin' and 'luajit' in os.path.basename(lua_bundle_path.rstrip(os.sep))) | ||
# Let's restrict LuaJIT to x86_64 for now. | ||
or (get_machine() not in ("x86_64", "AMD64") and 'luajit' in os.path.basename(lua_bundle_path.rstrip(os.sep))) | ||
) | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These can probably be merged.
lauxlib_h_path = os.path.join(src_dir, "build", "lauxlib.h") | ||
if not os.path.exists(lauxlib_h_path): | ||
with open(lauxlib_h_path, 'w', encoding='us-ascii') as f: | ||
f.write(""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather not have this spelled out in setup.py
but in a real file. Makes it substantially more visible and maintainable. Then, either copy it somewhere from setup.py
or provide an additional include directory for the C compiler (-I…
) when compiling luau.
#include "lualib.h" | ||
#include "luacode.h" | ||
#include <stdbool.h> | ||
#define USE_LUAU 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
USE_LUAU
seems to be a macro local to Lupa. Then I'd prefix it accordingly, i.e. LUPA_USE_LUAU
.
# Compile the library | ||
object_files = [] | ||
for cpp_file in cpp_files: | ||
obj_file = os.path.splitext(os.path.basename(cpp_file))[0] + '.o' | ||
obj_file_path = os.path.join(src_dir, "build", lib[1], obj_file) | ||
compile_command = ["g++", "-c", cpp_file, "-o", obj_file_path] + lib_build_env['CXXFLAGS'] | ||
print("Compiling %s" % cpp_file) | ||
output = subprocess.check_output(compile_command, cwd=lib_src_dir) | ||
object_files.append(obj_file_path) | ||
|
||
# Create the static library | ||
static_lib_path = os.path.join(src_dir, "build", "lib" + lib[1] + ".a") | ||
ar_command = ["ar", "rcs", static_lib_path] + object_files | ||
print("Creating static library %s" % static_lib_path) | ||
output = subprocess.check_output(ar_command, cwd=lib_src_dir) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be possible to leave this to setuptools. It can build static libraries as part of an extension build. But it might also be enough to just list the .cpp
files as additional sources of the Lupa extension module, as done by the Lua module build. Either of the two is better than 'manually' starting a g++
process.
lua.lua_pushcclosure(L, py_args, 1) # lib function | ||
lua.lua_pushcclosured(L, py_args, 1) # lib function |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than renaming all of these in the source code, you can use explicit cnames in Cython to make it generate a different name in C than in the Cython code:
cdef extern from "whatever.h":
"""
#if LUAU
#define __lupa_pushcclosure(L, fn, n) lua_pushcclosured(L, fn, n)
#else
#define __lupa_pushcclosure(L, fn, n) lua_pushcclosure(L, fn, n)
#endif
"""
void lua_pushcclosure "__lupa_pushcclosure" (lua_State *L, lua_CFunction fn, int n)
This PR adds WIP Luau support to Lupa.
Note that this is still highly unstable, not tested outside of my ubuntu laptop, userdata dtors arent supported yet so it has memory leaks etc but basic vm init, gc collection, shutdown and listing table contents and evaling/compiling Lua chunks all works (at least, it looks like it (?)). Luau compiler is also not exposed (so you cant change the compiler opts yet). Env index is always 0 as well for now,
To achieve this, my PR creates a on-the-fly lauxlib.h file (as Luau doesnt ship with lauxlib.h) containing backports from both aux lib and from Lua (like lua panic function stuff).
I also had to make a psuedo build system for Luau as the default one in Luau either needs cmake or only complies debug builds from what I see (and also includes extra stuff like type solver which takes a long time to compile).
Closes #204 (once finished ofc)