draft -- lua: example of using crate to vendor/bundle lua 5.4.6 - v0 #10843
Conversation
| AC_DEFINE([HAVE_LUA], [1], [lua support available]) | ||
| AM_CONDITIONAL([HAVE_LUA], [true]) | ||
| AC_SUBST([LUA_INT8], ["lua_int8"]) |
There was a problem hiding this comment.
It would be easy to keep a --disable-lua. What is harder is letting a distribution opt-out of our Lua in favor of theirs. I'm tempted to not support that.
There was a problem hiding this comment.
Also, the "lua_int8" feature can go away now that we pin to a specific version of Lua.
| mkdir -p $(abs_top_builddir)/rust/gen | ||
| cp -a $(RUST_SURICATA_LIBDIR)/build/suricata-lua-sys-*/out/lua/*.h \ | ||
| $(abs_top_builddir)/rust/gen/ |
There was a problem hiding this comment.
A little bit janky but I don't have a better way at this time.
| cbindgen --config $(abs_top_srcdir)/rust/cbindgen.toml \ | ||
| cd $(abs_top_srcdir)/rust && \ | ||
| cbindgen --config $(abs_top_srcdir)/rust/cbindgen.toml \ | ||
| --quiet --verify --output $(abs_top_builddir)/rust/gen/rust-bindings.h || true |
There was a problem hiding this comment.
This is actually not related, but needs to be done in master as part of the other PR that change the dist pattern for Rust.
| #[allow(unused_imports)] | ||
| pub use suricata_lua_sys; |
There was a problem hiding this comment.
This is how we keep Lua library into the resulting binary without actually using it. Needs comment stating this.
| @@ -67,6 +67,8 @@ time = "=0.3.20" | |||
|
|
|||
| suricata-derive = { path = "./derive", version = "@PACKAGE_VERSION@" } | |||
|
|
|||
| suricata-lua-sys = { git = "https://github.com/jasonish/suricata-lua-sys", version = "0.1.0-alpha.1" } | |||
There was a problem hiding this comment.
This repo contains a copy of the Lua source. We could also use a git sub-module, but that does open us up to breakage by upstream still. So I opt to create a copy, so we're in full control.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #10843 +/- ##
==========================================
- Coverage 82.83% 81.52% -1.31%
==========================================
Files 913 928 +15
Lines 246847 254189 +7342
==========================================
+ Hits 204474 207226 +2752
- Misses 42373 46963 +4590
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
Information: QA ran without warnings. Pipeline 20062 |
|
Interesting. Are you planning to still support a way to optionally use an external Lua library? |
|
My CI results are showing multiple failures: FreeBSD 14:CentOS 7Debian Buster Arm 32:Debian 12 (arm64)scan-build 16 (ubuntu 23.10)reports 4 new bugs/warnings, but doesn't show them. Could they be hidden by cargo? |
|
Information: ERROR: QA failed on SURI_TLPW2_autofp_suri_time. ERROR: QA failed on SURI_TLPR1_alerts_cmp.
Pipeline 20064 |
Yes. As expected. This is more about review of the bundling technique. I'll probably need access to each failing setup to fix that those. But first let's decide if this a method of bundling. The other option is just copying the Lua source right into ours. |
Probably in the Lua source. |
Remove lua-dev(el) from all CI tests.
The vendored Lua code triggers some scan-build failures, so exclude the rust/ directory for now. Might want to look at these separately though.
Not really. It would be hard around what includes to use, etc. The other option for vendoring is to pull the Lua source right in, in which case it would be a little easier. But my question, is to what value? Is the effort worth it? |
[...]
Not sure. In Debian it is generally frowned upon to build using vendored code. Mainly because that means having to identify, patch and rebuild binary packages when security updates to vendored code are released. But given the fact that we already build a lot of vendored Rust code I don't think adding Lua would matter much (or is worth patching Suricata within Debian). |
Remove maintainer-clean-local, this is not needed. In distclean-local, remove "rust/dist" and "rust/vendor" as they are created during "make dist". In "clean-local", remove "rust/target" and "rust/gen" as they are created during a normal "make".
Modify the CentOS 9 Stream build to not have cbdingen available, as its already building from the dist. But add a "make clean" followed by a "make" to test that it still builds after a clean.
|
Continued at #10854. |
Use a Rust crate to bundle Lua.
I first started using mlua (https://crates.io/crates/mlua) but it requires too new of a Rust toolchain and bundles multiple versions, so has a larger footprint than we need. Instead I created a minimal crate: https://github.com/jasonish/suricata-lua-sys
Might need fixups for other platforms not covered by GitHub CI.