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

Remove vendored libraries and rely on the Zig package manager #34

Closed
Tracked by #32
natecraddock opened this issue Dec 29, 2023 · 3 comments · Fixed by #43
Closed
Tracked by #32

Remove vendored libraries and rely on the Zig package manager #34

natecraddock opened this issue Dec 29, 2023 · 3 comments · Fixed by #43
Labels
cleanup feature New feature or request
Milestone

Comments

@natecraddock
Copy link
Owner

My understanding is that the Zig package manager will eventually support pulling down non-zig code and using it in the build process. Maybe this is already possible?

But this would remove all of the files in ./lib. Doing this would make it much easier to update subprojects like Luau or LuaJIT, which have more frequent updates. It also makes it more clear that Ziglua actually does rely on Lua, and that we haven't tampered with the source code.

We could rely on the upstream project for everything except Lua 5.1. #2 reported a CVE that some distros have patched in Lua 5.1, but that Lua hasn't patched. So I would make a lua repo that contains the full history of Lua, and then that patch.

@natecraddock natecraddock added feature New feature or request cleanup labels Dec 29, 2023
@nurpax
Copy link
Contributor

nurpax commented Jan 5, 2024

I actually generally like vendored in libs because then the source code to the libraries is handily available in the project. However, in the case of Luau, I notice that some parts of Luau are missing from the currently vendored code, like the Analysis folder.

I recently realized that luau-analyze is missing some useful functionality like being able to specify type declaration files on the command line. Again, the docs for this are sorely missing from luau, but the idea is that you can declare types for any classes, types or functions that your "engine" (the thing embedding luau into) might declare. Like if you have a game engine and it registers a bunch of classes and functions when you load the Lua state, then you can static check your game code against those types. Except that right now you can't, since luau-analyze doesn't seem to have a way to specify what type defs to load. This is supported in https://github.com/JohnnyMorganz/luau-lsp and that's how I've become familiar with this feature, I'd just like to be able to have the analysis option available on the command line too.

Why is this relevant here? Well, if the Luau lib is pulled from the Zip package manager as the original source tarball, I could amend the Zig luau parts to expose everything that's needed for analysis. Then it'd be straightforward to implement a zig-luau-analyze tool that's more fully featured that Luau's own luau-analyze.

EDIT: actually scratch that. :) Given that it's a completely new C++ API, I don't think it makes sense to expose. Bummer that the luau-analyze isn't fully featured.

@natecraddock
Copy link
Owner Author

I saw the email notification a few days ago and came here just now prepared to reply. Guess I don't have to anymore 😂

I started work on this today. I am currently updating Ziglua to use the changes in ziglang/zig#18160 and it makes sense to also remove the vendored libraries at the same time for simplicity.

I guess I'm mostly commenting this to let you know I saw your many issues and PRs over the last week. I'm slowly working through them now :)

Bummer that the luau-analyze isn't fully featured.

I've given this about 2 seconds of thought, but it's possible that we could include a more fully featured version of luau-analyze with ziglua. Let's just see how the Luau bindings progress over time and what is needed

@nurpax
Copy link
Contributor

nurpax commented Jan 9, 2024

I guess I'm mostly commenting this to let you know I saw your many issues and PRs over the last week. I'm slowly working through them now :)

Thanks for the heads up! I've been playing a lot with Lua(u) bindings (even doing some light perf measurements with Lua vs. Luau) and have a gained some useful experience with this code.

I've given this about 2 seconds of thought, but it's possible that we could include a more fully featured version of luau-analyze with ziglua. Let's just see how the Luau bindings progress over time and what is needed

Yeah, I thought about this some too, and one idea I had would be to include luau-compile and luau-analyze as outputs from Ziglua. This makes sense for luau-compile because it's a good idea to try to compile bytecode using the same Luau version that loads the bytecode. But it's somewhat optional to bundle these tools -- it may be that a pretty workable workflow is to just use "analysis" in VSCode with luau-lsp and wait for the Luau upstream to improve on CLI tooling (some background).

@natecraddock natecraddock linked a pull request Jan 9, 2024 that will close this issue
4 tasks
@natecraddock natecraddock added this to the 0.3.0 milestone Jan 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup feature New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants