fluent-bit
fluent-bit copied to clipboard
build: use the system provided packages if found
Some packages might already be present in the system, so use them if that is the case. E.g. in buildroot, it doesn't make sense that e.g. luajit is compiled twice.
Enter [N/A] in the box, if an item is not applicable to your change.
Testing Before we can approve your change; please submit the following in a comment:
- [N/A] Example configuration file for the change
- [N/A] Debug log output from testing the change
- [N/A] Attached Valgrind output that shows no leaks or memory corruption was found
If this is a change to packaging of containers or native binaries then please confirm it works for all targets.
- [ ] Run local packaging test showing all targets (including any new ones) build.
- [ ] Set
ok-package-testlabel to test for all targets (requires maintainer to do).
Documentation
- [N/A] Documentation required for this feature
Backporting
- [N/A] Backport to latest stable release.
Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.
This overrules these PRs:
- https://github.com/fluent/fluent-bit/pull/8929
- https://github.com/fluent/fluent-bit/pull/7286
I think it'll be reasonable. However, we shouldn't break the priority of building and including the bundled libraries. So, how about adding
FLB_USE_SYSTEM_LIBRARIRSor similar flag to provide an option to prioritize the system installed libraries? This is because using system installed libraries by default diversifies the version of dependent libraries.
Hi, I once did that in https://github.com/fluent/fluent-bit/pull/7286, but that PR is somewhat ignored in the meantime. I kept it open for reference. Please consider that one first, if it gets merged, then I can rework this PR to do the same for some other packages.
The only difference in the other PR is that I've used FLB_PREFER_SYSTEM_LIBS iso FLB_USE_SYSTEM_LIBRARIRS.
I found collisions for the storing values of pkg_config results. Maybe, we'd better to use FLB_ prefix to store the results of referring the system libraries?
Also, the searching mechanism should be follow the naming convension of FindXXX.cmake. But it's optional. I guess.
So, we'd better to use the following names:
- FindCares.cmake
- Find(Rd)Kadfa.cmake
- FindLuaJIT.cmake
- FindNGHttp2.cmake
Do you still expect a change here? Not sure if we have to add the FLB_ prefix, there was only one package for which there was a conflict.
About the FindXXX.cmake, what do you mean? Is pkg_config not fine?
I found collisions for the storing values of pkg_config results. Maybe, we'd better to use FLB_ prefix to store the results of referring the system libraries? Also, the searching mechanism should be follow the naming convension of FindXXX.cmake. But it's optional. I guess. So, we'd better to use the following names:
- FindCares.cmake
- Find(Rd)Kadfa.cmake
- FindLuaJIT.cmake
- FindNGHttp2.cmake
Do you still expect a change here? Not sure if we have to add the FLB_ prefix, there was only one package for which there was a conflict.
About the FindXXX.cmake, what do you mean? Is pkg_config not fine?
No worries, just my two cents. It's good time to approve! :+1:
can you please add a simple workflow that builds Fluent Bit in the CI with all those libs in shared mode ?
can you please add a simple workflow that builds Fluent Bit in the CI with all those libs in shared mode ?
I have no experience with GitHub workflows. So that would take me some time to get started with it. Also, not all packages are up-to-date in e.g. the ubuntu docker containers. I tested all of them by linking to /usr/local/lib installs. I know that the apple homebrew eco system is quite up to date, so perhaps, that can be a good start? But I would propose to not hamper this PR with this extra request.
@edsiper, sorry that I don't find the time to have a look at these GitHub actions. But is there any reason to still block this PR for this? I would like to have these changes so that I can improve the support in Buildroot.
What exactly has to be invoked to build this? I can potentially update CI to do that
What exactly has to be invoked to build this? I can potentially update CI to do that
@patrick-stephens A test job that compiles fluent-bit against a set of native system libraries, where we do enable the FLB_PREFER_SYSTEM_<LIB> flags.
e.g. for ubuntu 2022.04:
sudo apt-get install libc-ares-dev libluajit-5.1-dev libnghttp2-dev libjemalloc-dev librdkafka-dev
and then use
cmake -GNinja -DFLB_PREFER_SYSTEM_LIB_CARES=Yes -DFLB_PREFER_SYSTEM_LIB_LUAJIT=Yes -DFLB_PREFER_SYSTEM_LIB_NGHTTP2=Yes -DFLB_PREFER_SYSTEM_LIB_JEMALLOC=Yes -DFLB_PREFER_SYSTEM_LIB_KAFKA=Yes ../
The only problem is that Ubuntu doesn't always ship the latest libraries. A very similar test can be made, using the homebrew environment. Homebrew typically is more up-to-date, and if I'm right, homebrew does allow the installation of a fixed version.
What exactly has to be invoked to build this? I can potentially update CI to do that
@patrick-stephens A test job that compiles fluent-bit against a set of native system libraries, where we do enable the FLB_PREFER_SYSTEM_ flags.
e.g. for ubuntu 2022.04:
sudo apt-get install libc-ares-dev libluajit-5.1-dev libnghttp2-dev libjemalloc-dev librdkafka-devand then use
cmake -GNinja -DFLB_PREFER_SYSTEM_LIB_CARES=Yes -DFLB_PREFER_SYSTEM_LIB_LUAJIT=Yes -DFLB_PREFER_SYSTEM_LIB_NGHTTP2=Yes -DFLB_PREFER_SYSTEM_LIB_JEMALLOC=Yes -DFLB_PREFER_SYSTEM_LIB_KAFKA=Yes ../The only problem is that Ubuntu doesn't always ship the latest libraries. A very similar test can be made, using the homebrew environment. Homebrew typically is more up-to-date, and if I'm right, homebrew does allow the installation of a fixed version.
Probably not homebrew - macOS runners cost a lot and we already have Linux based package tests in place for all targets via simple container builds.
My preference would be just updating ./packaging/build.sh to support the new option to make it easy to then invoke all in one go via the existing test workflow which then means we do it for all targets and stays up-to-date plus gives users a simple approach too using our actual release tooling.
I can probably do this after this PR is merged though, it likely just needs some extra packages adding to the container base images for builds then passing the options in.
Another option is like using ArchLinux via docker containers. ArchLinux's packaging is blazingly fast and typically no patches are applied. If we need to test against the latest packages, we'll be able to consider this option. However, that distribution is sometimes too cutting edge. So, I feel that testing with system packages on Ubuntu 22.04 is enough for most cases. ;)
can you please add a simple workflow that builds Fluent Bit in the CI with all those libs in shared mode ?
In https://github.com/fluent/fluent-bit/pull/7286, we added simple workflow which uses system libluajit-5.1-dev and its associated libraries. So, once that PR is merged we can continue to add dependent libraries as apt packages.
@edsiper @cosmo0920 kind remember to take this up again
@edsiper thank you for merging #7286, this is a follow up pull request on top of the previous one. Can you also have a look at this one? I rebased the branch on top of the master branch.
@edsiper @cosmo0920 @patrick-stephens @leonardo-albertovich Hi, I rebased this PR to the current master. Is it possible to consider this PR?
Hi, thanks for continuously rebasing on top of master. Currently, I added a milestone for v3.2.0 in this PR.