mlua-rs / rlua

High level Lua bindings to Rust
Other
1.72k stars 115 forks source link

ios build error #255

Closed Yuri6037 closed 2 years ago

Yuri6037 commented 2 years ago

When rlua-54-sys is built on iOS the build fails with the following error: lua-5.4.3/src/loslib.c:146:10: error: 'system' is unavailable: not available on iOS.

In my project I require that the library builds embedded lua by itself on both windows, mac, linux and iOS. Building lua externally is unacceptable.

Can you provide a cargo feature to get rid of loslib.c? The os lib is not intended to be used in my project.

EDIT: There's actually a much easier fix not requiring any cargo feature: just add a define to #define system(s) ((s)==NULL ? 0 : -1) in case target_os == "ios".

jugglerchris commented 2 years ago

Hi, The rlua-*-sys crates contain unmodified Lua source - I'd ideally like to avoid modifying that to add defines (if nothing else it makes updates to upstream Lua releases harder).

So I think a cargo feature makes sense (and building without the os lib also seems reasonable in any case) - it looks like it just means leaving out loslib.c and linit.c and not calling luaopen_os().

I've pushed a test branch: https://github.com/amethyst/rlua/tree/optional-oslib which adds a lua-no-oslib feature - can you confirm whether that works for you? (I don't have any access to iOS devices or dev systems, but the tests passed) If so I can tidy it up (e.g. extend to the other Lua versions) and make a new release in the next few days with that or something like it.

Thanks!

Yuri6037 commented 2 years ago

Thanks for your quick answer I've just tried the new lua-no-oslib feature on a new branch and it does indeed appear to solve the compile error on ios when building with cargo build --target aarch64-apple-ios --features lua-no-oslib.

This is only a preliminary test which does appear to pass, I can't yet build the full project because the full project requires crates.io version to match so in the meantime I've just built locally but so far it's looking good.

I wonder if some other similar features could be setup for various libs which are dangerous for sandboxing, mainly the debug lib which is known to be a potential source of memory unsafely in Rust and the io lib, which in it's current state allows reading and writing all files of the user with no restriction. Even if I'm explicitely not loading these libs they could still be a source of rejection for AppStore submission because the code might still be compiled... This could also be considered as a way to reduce compilation time because in many projects where lua is used for sandboxed scripting these libs are usually disabled anyway.

Can features be also set-up for disabling compilation of both the io and debug libs?

jugglerchris commented 2 years ago

Hi @Yuri6037 - I've merged the branch with this added feature. I didn't mean to close this issue immediately - I'll reopen until to give you a chance to check it works for you!

jugglerchris commented 2 years ago

Sorry I missed your second question about leaving out the io/debug libs. I don't think (though I admit I haven't tested it) it would make much difference to compile time (a couple of C files compared to some rust crates!), but I can see that it would be useful to minimise executable size and leave out banned symbols. (Though are there any functions that the io/debug libraries bring in that aren't used elsewhere anyway?)

jugglerchris commented 2 years ago

I got it wrong - another release fixing it is imminent.

jugglerchris commented 2 years ago

I've just released 0.19.4 (and yanked 0.19.3).

Yuri6037 commented 2 years ago

I've just tried with the new update and now it works.

Thanks a lot for fixing this issue quickly.