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

lua: update lua crate with better header copying #12129

Closed
wants to merge 3 commits into from

Conversation

jasonish
Copy link
Member

Update lua crate to 0.1.0-alpha.5. This update will rewrite the headers if the env var SURICATA_LUA_SYS_HEADER_DST changes. This fixes the issue where the headers may not be written.

The cause is that Rust dependencies are cached, and if your editor is using rust-analyzer, it might cache the built without this var being set, so these headers are not available to Suricata. This crate update forces the re-run of the Lua build.rs if this env var changes, fixing this issue.

This new Lua crate is also smarter about writing out the headers, don't overwrite if the destination appears to be the same as the source, as this can force subsequent invocations of make to rebuild the C source.

Fixes this issue in PR #12104:

  • The MacOS build had different environment variables between the calls to make <build> and make install, which smarter build systems can detect, if there are vars that matter to the build. In this case it was the library search path.
  • But was able to fix this on the Lua crate side by not always copying the headers as part of the build, for example if the destination exists and is newer then the source header.

@jasonish jasonish mentioned this pull request Nov 18, 2024
Copy link

codecov bot commented Nov 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.11%. Comparing base (5d766df) to head (94dd70e).

Additional details and impacted files
@@             Coverage Diff             @@
##           master   #12129       +/-   ##
===========================================
+ Coverage   62.68%   83.11%   +20.43%     
===========================================
  Files         840      909       +69     
  Lines      153669   257883   +104214     
===========================================
+ Hits        96323   214349   +118026     
+ Misses      57346    43534    -13812     
Flag Coverage Δ
fuzzcorpus 60.93% <ø> (?)
livemode 19.43% <ø> (?)
pcap 44.43% <ø> (?)
suricata-verify 62.69% <ø> (+0.01%) ⬆️
unittests 59.27% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

---- 🚨 Try these New Features:

version = "0.1.0-alpha.3"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "aed3f46aa1b92feb15880f722bbce121e769bda1770f0090121cd6c920d89450"
version = "0.1.0-alpha.4"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be 5 instead?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dang.

Update lua crate to 0.1.0-alpha.5. This update will rewrite the
headers if the env var SURICATA_LUA_SYS_HEADER_DST changes. This fixes
the issue where the headers may not be written.

The cause is that Rust dependencies are cached, and if your editor is
using rust-analyzer, it might cache the built without this var being
set, so these headers are not available to Suricata. This crate update
forces the re-run of the Lua build.rs if this env var changes, fixing
this issue.

This new Lua crate is also smarter about writing out the headers,
don't overwrite if the destination appears to be the same as the
source, as this can force subsequent invocations of make to rebuild
the C source.
@jasonish jasonish closed this Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants