Add a static binary workflow #966
Conversation
This commit moves the nix expression for vast into an overlay. It also adds a package for zeek's broker library to make it possible to build zeek-to-vast. At the smae time, overrides for CAF and Apache Arrow are added to fix their build with a compiler that only targets static binaries, and link-time optimization in case of CAF. Lastly, the shell.nix file has been updated to work with the new overlay and make use of the CMake changes that were added in the previous commits.
| option(VAST_STATIC_EXECUTABLE "Link VAST statically" $ENV{VAST_STATIC_EXECUTABLE}) | ||
| option(VAST_USE_JEMALLOC "Use jemalloc instead of libc malloc" "${VAST_STATIC_EXECUTABLE}") |
There was a problem hiding this comment.
Changed by mistake? This doesn't look right to me.
There was a problem hiding this comment.
The intention is to use the environment variable if the option has not been given explicitly. The order of precedence is now:
explicit option > environment > default (implicitly off because the empty string is treated as False).
The reason I changed these is that I can use nix to set up an environment for a static build like so:
nix-shell -E 'let pkgs = import ./nix { }; in import ./shell.nix {pkgs = pkgs.pkgsStatic;}'The toolchain in this environment is not capable to produce shared libraries, so one has to pass several options to CMake to get a working build tree with a configuration similar to the static build in CI:
cmake -Bbuild -S . -DVAST_STATIC_EXECUTABLE=ON -DZSTD_ROOT="$(nix-build ./nix --no-out-link -A pkgsStatic.zstd)" -DVAST_USE_JEMALLOC=ONWith the changes to CMake adding these options is not required, while the regular build is not affected.
There was a problem hiding this comment.
I really don't like how inconsistent this is. Can you create a follow-up story for a) prefixing all CMake variables that the user can set with VAST_, and b) for exposing them on the command line like this?
| # We look for an environment variable of the same name too, because | ||
| # that can be provided by a tool like nix-shell or docker. Wo do this | ||
| # so the user doesn't have to remember to add "-DZSTD_ROOT=..." to their | ||
| # cmake invocations. |
There was a problem hiding this comment.
Having this for some variables but not all is really confusing and makes maintenance of the CMake way harder. I'm already always afraid to change CMake code because it might break the Nix build.
There was a problem hiding this comment.
This is essentially a workaround for a bug in the generated .pc files in ZSTD. We can probably get rid of FindZSTD.cmake when upgrading to the next Arrow release thanks to apache/arrow#7388.
The script already exists in event-horizon, maybe it's even fine to leave it there since only tenzir developers will have permissions to trigger manual dispatches anyways. However, it will not be possible to test the manual trigger until the workflow that responds to it lands on master, so not in this PR. |
Co-authored-by: Dominik Lohmann <mail@dominiklohmann.de>
tobim
force-pushed
the
story/ch17797/static-binary
branch
from
237535f
to
4fa59f2
Compare
|
The name for the workflow seems still pretty cryptic. Can we strip the technical details? |
The name is now "VAST Static / Static Binary (trigger)". |
tobim
force-pushed
the
story/ch17797/static-binary
branch
from
1627cab
to
11986e9
Compare
With this change the links will always point to a tarball with the latest release build.
tobim
force-pushed
the
story/ch17797/static-binary
branch
from
58b4fa5
to
2847264
Compare
Co-authored-by: Dominik Lohmann <mail@dominiklohmann.de>
Co-authored-by: Dominik Lohmann <mail@dominiklohmann.de>
Co-authored-by: Dominik Lohmann <mail@dominiklohmann.de>
Adds a new workflow that uses nix to build a static binary. It runs after every push to master and can be manually triggered by the
repository_dispatchevent.TODO: