-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
build: Fix release builds with enabled FLB_DEBUG #8058
Conversation
Enabling FLB_RELEASE should enable compiler optimizations. However, if FLB_DEBUG was enabled at the same time, no optimization settings were applied. Now, FLB_RELEASE will always enable optimizations and includes debug information (CMAKE_BUILD_TYPE is RelWithDebInfo). Signed-off-by: Christian Norbert Menges <[email protected]>
Probably want to target 2.1 branch as well @Garfield96 as this will go into 2.2 otherwise. |
Hi @patrick-stephens, |
Hi @patrick-stephens, |
@niedbalski @patrick-stephens |
Just the size of the review queue is probably the only blocker @Garfield96 , I'll ping @niedbalski on it |
@patrick-stephens @niedbalski I understand that there is a huge number of open PRs and it takes a considerable amount of time to review them. However, the long review times are slowly becoming a problem, because users are forced to build fluent-bit locally in order to have their features included, and people refrain from contributing. This PR, which changes 2 LOCs, is open for 3 months and my other PR #6087 is open for almost 1.5 years. |
my apologies for the delay on this, thanks @patrick-stephens for the heads up. thanks for the fix, indeed the code is wrongly evaluating FLB_DEBUG first before FLB_RELEASE |
Enabling
FLB_RELEASE
should enable compiler optimizations. However, ifFLB_DEBUG
(enabled by default) was enabled at the same time, no optimization settings were applied. Now,FLB_RELEASE
will always enable optimizations and includes debug information (CMAKE_BUILD_TYPE
isRelWithDebInfo
).This issue affects all published Docker images and most likely severely degrades performance.
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:
If this is a change to packaging of containers or native binaries then please confirm it works for all targets.
ok-package-test
label to test for all targets (requires maintainer to do).Documentation
Backporting
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.