-
Notifications
You must be signed in to change notification settings - Fork 93
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
Implement Cross Compilation #162
Conversation
This commit addresses various lint issues, mainly: * clippy flagging once_cell::Lazy variables consts * other simple fixes in the build command
A couple questions / notes on this, not reviewing yet since this is a draft:
|
Isn't native compilation handled at runtime? If that is the case, I don't think we'd need to worry about that since the target would be tailored to the platform which would be consuming the natively compiled bytecode anyway.
I've been troubled by this issue as well; atm, we're just assuming the user knows what they're doing, and are providing a base binary at their own risk. This isn't ideal, but there isn't a very straightforward solution to this problem either. The only other solution I could think of was to parse version increments for luau, but that was quickly shut down as I realized luau does not follow semver. The only other solution we could practically implement would be to statically keep track of breaking changes to bytecode for versions for different lune and luau versions, and alert the user when they are providing a base binary which may cause such a conflict. |
In order to address bytecode conflicts among varying lune and luau versions, it is currently hardcoded to only support cross compilation among targets for the same release. |
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.
Left some initial comments and changes to make. I have a couple more things that I didn't leave in specific places:
- We should probably use something like
zip_next
and aspawn_blocking
here instead, async zip is making things more difficult unnecessarily since zip body is not received and decompressed concurrently either way. It's being received in full and then decompressed Lua::compile
has JIT enabled, we should probably disable it for standalone binaries on other targets somehow, probably fine to keep enabled for the same os/arch as current though- Why has the output formatting changed, and extra prefix spacing been added to many output lines? The output is intentionally plain but maybe we can add a splash of color to things like file paths, following how the rest of Lune does things
* Moves caching logic into its own function (`cache_target`) * `get_base_exe_path` now returns a tuple including the output path
* The build command now checks whether the target specified is the same as the host. If so, it proceeds to use the current executable. * Instead of returning a `to_cross_compile` boolean, `get_base_exe_path` now directly returns the path to the current exe if required.
defaults In the previous commit, I had moved `LuaCompiler` construction into the CLI command, which was not necessary. This reverts that.
I've addressed all the specific comments in the review, would you mind addressing the general structural ones yourself after you merge this PR? |
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.
LGTM 👍
I've addressed all the specific comments in the review, would you mind addressing the general structural ones
Sure yeah, I'll take a look
See the linked issue for more details.
Closes #161.