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

Fix rlua - lua51 not building #88

Merged
merged 1 commit into from
Jul 21, 2024

Conversation

bhavyakukkar
Copy link
Contributor

rlua::Value::Integer is only available in lua 5.3+ as mentioned here.

This commit fixes tealr not building at all for flags rlua_builtin-lua51 and rlua_system-lua51.

Before this commit:

$ cargo run --example rlua_manual -F rlua,rlua_system-lua51
<65 errors>
$ cargo run --example rlua_manual -F rlua,rlua_builtin-lua51
<65 errors>

After this commit, all of these build successfully:

cargo run --example rlua_manual -F rlua,rlua_system-lua51
cargo run --example rlua_manual -F rlua,rlua_builtin-lua51

cargo run --example rlua_manual -F rlua,rlua_system-lua53
cargo run --example rlua_manual -F rlua,rlua_builtin-lua53

cargo run --example rlua_manual -F rlua,rlua_system-lua54
cargo run --example rlua_manual -F rlua,rlua_builtin-lua54

@bhavyakukkar bhavyakukkar marked this pull request as draft July 17, 2024 14:48
@bhavyakukkar bhavyakukkar force-pushed the bugfix/build_rlua_lua51 branch from 7a8413b to 23ea309 Compare July 17, 2024 14:58
@bhavyakukkar bhavyakukkar marked this pull request as ready for review July 17, 2024 14:59
@lenscas
Copy link
Owner

lenscas commented Jul 21, 2024

Can you rebase it onto master? I fixed the clippy errors.

And thanks for the PR, sorry it took a while to get to it.

- Excluded rlua::Value::Integer for tealr features rlua_builtin-lua51 &
  rlua_system-lua51 since it is available in lua 5.3+ only as mentioned
  here:
  https://github.com/mlua-rs/rlua/blob/6152008989fb43a63c0632dc39dea25f40390ac9/src/value.rs#L31
- Implemented as excluding for lua51 instead of including for lua53, 54
  so that future lua versions wouldn't require rechanging.
@bhavyakukkar bhavyakukkar marked this pull request as draft July 21, 2024 15:56
@bhavyakukkar bhavyakukkar force-pushed the bugfix/build_rlua_lua51 branch from 23ea309 to 6113540 Compare July 21, 2024 15:56
@bhavyakukkar bhavyakukkar marked this pull request as ready for review July 21, 2024 15:56
@bhavyakukkar
Copy link
Contributor Author

@lenscas done. all 6 examples still build

@lenscas lenscas merged commit 39d83ac into lenscas:master Jul 21, 2024
18 checks passed
@lenscas
Copy link
Owner

lenscas commented Jul 21, 2024

Awesome. Merged it.

Thank you for the PR

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

Successfully merging this pull request may close these issues.

2 participants