build: add options for plugins at group granularity#12160
Conversation
Add `FLB_ALL_INPUTS`, `FLB_ALL_PROCESSORS`, `FLB_ALL_FILTERS`, and `FLB_ALL_OUTPUTS` options to control the build for each plugins group individually. Three options for each variable: - `Inherit`: Default, doesn't do anything. - `On`: Plugins for the entire group enabled. - `Off`: Plugins for the entire group disabled. Respects precedence from least to most specific (right overrides left): FLB_MINIMAL => FLB_ALL_* => Individual Option e.g to disable everything except outputs and HTTP input: -DFLB_MINIMAL=On -DFLB_ALL_OUTPUTS=On -DFLB_IN_HTTP=On Signed-off-by: Ra'Jiska <dodo.lasticot@gmail.com>
📝 WalkthroughWalkthroughChangesThe CMake plugin options module adds group-level Plugin group overrides
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmake/plugins_options.cmake`:
- Around line 6-9: Update the plugin option setup around FLB_GROUP_OVERRIDE and
option() so inherited options are refreshed from the current group setting when
FLB_ALL_* changes during reconfiguration, while explicitly selected plugin
overrides remain unchanged. Ensure the cache-backed option is force-updated only
for inherited values, preserving the existing option_name, description, and
override behavior.
- Around line 16-23: The FLB_ALL_INPUTS, FLB_ALL_PROCESSORS, FLB_ALL_FILTERS,
and FLB_ALL_OUTPUTS cache variables currently accept arbitrary command-line
values. Validate each value immediately after its cache definition against
exactly On, Off, or Inherit, and call message(FATAL_ERROR ...) for any invalid
value before DEFINE_OPTION(...) or option() processing occurs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 366f0c78-ba9c-437d-88e0-d40f1720fa1c
📒 Files selected for processing (1)
cmake/plugins_options.cmake
| if(NOT "${FLB_GROUP_OVERRIDE}" STREQUAL "Inherit") | ||
| set(temp_value ${FLB_GROUP_OVERRIDE}) | ||
| endif() | ||
| option(${option_name} "${description}" ${temp_value}) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
tmp_dir="$(mktemp -d)"
trap 'rm -rf "$tmp_dir"' EXIT
cat > "$tmp_dir/CMakeLists.txt" <<'EOF'
cmake_minimum_required(VERSION 3.13)
project(cache_probe NONE)
set(FLB_GROUP_OVERRIDE "On")
macro(DEFINE_OPTION option_name description default_value)
set(temp_value "${default_value}")
if(NOT "${FLB_GROUP_OVERRIDE}" STREQUAL "Inherit")
set(temp_value "${FLB_GROUP_OVERRIDE}")
endif()
option("${option_name}" "${description}" "${temp_value}")
endmacro()
DEFINE_OPTION(FLB_TEST_PLUGIN "test" OFF)
message(STATUS "FLB_TEST_PLUGIN=${FLB_TEST_PLUGIN}")
EOF
cmake -S "$tmp_dir" -B "$tmp_dir/build"
cmake -S "$tmp_dir" -B "$tmp_dir/build" -DFLB_GROUP_OVERRIDE=Off
grep -F 'FLB_TEST_PLUGIN=ON' "$tmp_dir/build/CMakeCache.txt"Repository: fluent/fluent-bit
Length of output: 199
🏁 Script executed:
#!/bin/bash
set -eu
echo "== file =="
wc -l cmake/plugins_options.cmake
sed -n '1,120p' cmake/plugins_options.cmake
echo "== usages =="
rg -n "FLB_GROUP_OVERRIDE|FLB_ALL_|DEFINE_OPTION|option\\(" cmake CMakeLists.txt 2>/dev/null | head -200Repository: fluent/fluent-bit
Length of output: 32147
Reapply inherited plugin options when group settings change.
option() leaves an existing cache variable unchanged, so switching FLB_ALL_* from On to Off on reconfigure leaves already created plugin options in their previous state. Make explicitly selected plugin overrides persist while updating options still inheriting from the flipped group setting.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmake/plugins_options.cmake` around lines 6 - 9, Update the plugin option
setup around FLB_GROUP_OVERRIDE and option() so inherited options are refreshed
from the current group setting when FLB_ALL_* changes during reconfiguration,
while explicitly selected plugin overrides remain unchanged. Ensure the
cache-backed option is force-updated only for inherited values, preserving the
existing option_name, description, and override behavior.
| set(FLB_ALL_INPUTS "Inherit" CACHE STRING "Enable/Disable all Input plugins (On, Off, Inherit)") | ||
| set(FLB_ALL_PROCESSORS "Inherit" CACHE STRING "Enable/Disable all Processor plugins (On, Off, Inherit)") | ||
| set(FLB_ALL_FILTERS "Inherit" CACHE STRING "Enable/Disable all Filter plugins (On, Off, Inherit)") | ||
| set(FLB_ALL_OUTPUTS "Inherit" CACHE STRING "Enable/Disable all Output plugins (On, Off, Inherit)") | ||
| set_property(CACHE FLB_ALL_INPUTS PROPERTY STRINGS "On;Off;Inherit") | ||
| set_property(CACHE FLB_ALL_PROCESSORS PROPERTY STRINGS "On;Off;Inherit") | ||
| set_property(CACHE FLB_ALL_FILTERS PROPERTY STRINGS "On;Off;Inherit") | ||
| set_property(CACHE FLB_ALL_OUTPUTS PROPERTY STRINGS "On;Off;Inherit") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
tmp_dir="$(mktemp -d)"
trap 'rm -rf "$tmp_dir"' EXIT
cat > "$tmp_dir/CMakeLists.txt" <<'EOF'
cmake_minimum_required(VERSION 3.13)
project(validation_probe NONE)
set(FLB_ALL_INPUTS "Inherit" CACHE STRING "")
set_property(CACHE FLB_ALL_INPUTS PROPERTY STRINGS "On;Off;Inherit")
if(NOT FLB_ALL_INPUTS IN_LIST _valid_group_values)
set(_valid_group_values On Off Inherit)
endif()
if(NOT FLB_ALL_INPUTS IN_LIST _valid_group_values)
message(FATAL_ERROR "FLB_ALL_INPUTS must be On, Off, or Inherit")
endif()
EOF
if cmake -S "$tmp_dir" -B "$tmp_dir/build" -DFLB_ALL_INPUTS=Typo; then
echo "Expected invalid value to fail" >&2
exit 1
fiRepository: fluent/fluent-bit
Length of output: 195
🏁 Script executed:
#!/bin/bash
set -eu
printf 'Files matching plugins_options.cmake:\n'
fd -a 'plugins_options\.cmake$' . || true
file="$(fd 'plugins_options\.cmake$' . | head -n 1 || true)"
if [ -n "$file" ]; then
printf '\nOutline:\n'
ast-grep outline "$file" || true
printf '\nFile contents:\n'
cat -n "$file"
fi
printf '\nUsages of FLB_ALL_* override variables:\n'
rg -n 'FLB_ALL_(INPUTS|PROCESSORS|FILTERS|OUTPUTS)|option\($' -S . | head -n 200Repository: fluent/fluent-bit
Length of output: 17621
🌐 Web query:
CMake option command accepts non boolean values option(OFF ON) documentation
💡 Result:
The CMake option command is specifically designed to provide a boolean option for the user to select [1][2]. According to official documentation, its syntax is option( "<help_text>" [value]), where the initial value is treated as a boolean [1][3][4]. While the documentation defines it as a "boolean option," CMake does not strictly enforce this at the command level; the command effectively acts as a shortcut for set( CACHE BOOLEAN "<help_text>") [3]. Because it creates a cache entry with the BOOLEAN type, tools like cmake-gui(1) and ccmake(1) will interpret the value as a boolean and provide a simple toggle interface (ON/OFF) [3][5]. In practice, if you pass a non-boolean value to option, CMake will store it in the cache variable [6]. However, this is generally discouraged because: 1. Logic behavior: If you use the variable in an if statement, CMake will evaluate it using its standard boolean condition rules (e.g., 1, ON, YES, TRUE, Y are true; 0, OFF, NO, FALSE, N, IGNORE, NOTFOUND are false) [6]. 2. User Experience: Tools meant for editing cache entries (like cmake-gui) expect boolean values for variables created with option, and may not handle non-boolean strings in a user-friendly way [3]. If you need a selection mechanism that supports more than two states (like an enum or a list of choices), it is recommended to use the set command with the CACHE STRING option instead of option [3]. For example: set(MY_VAR "ValueA" CACHE STRING "Description") set_property(CACHE MY_VAR PROPERTY STRINGS "ValueA" "ValueB" "ValueC")
Citations:
- 1: https://cmake.org/cmake/help/latest/command/option.html
- 2: https://cmake.org/cmake/help/v3.25/command/option.html
- 3: https://stackoverflow.com/questions/52913642/optionoption-variable-help-string-describing-option-initial-value-sets-i
- 4: https://cmake.org/cmake/help/latest/command/option.html?highlight=option
- 5: https://cmake.org/cmake/help/latest/module/CMakeDependentOption.html
- 6: https://stackoverflow.com/questions/68409032/why-cmake-option-command-should-be-on-or-off
🌐 Web query:
CMake STRINGS property cached string does not restrict value set_property CACHE STRING STRINGS documentation
💡 Result:
The STRINGS property for a CMake cache entry does not restrict the value of the variable [1][2]. According to the official CMake documentation, the STRINGS property is intended for convenience only [1][3]. Its primary purpose is to inform CMake-based GUI tools (such as cmake-gui or ccmake) that they should provide a selection widget (like a drop-down combo box) containing the enumerated values instead of a generic text entry field [1][4][5]. CMake does not perform any validation to enforce that a cache entry's value matches one of the values listed in its STRINGS property [1][6]. Users can still set the variable to any arbitrary value, for example, by using the command line flag -DVAR=value [4][6]. If you require strict enforcement of allowed values, you must manually implement validation logic within your CMakeLists.txt file [4][6]. A common approach is to check the variable against a list of allowed values and issue a fatal error if the value is invalid [6]: # Example of manual validation set(ALLOWED_VALUES "OptionA" "OptionB") set(MY_VAR "InvalidValue" CACHE STRING "Description") set_property(CACHE MY_VAR PROPERTY STRINGS ${ALLOWED_VALUES}) if(NOT MY_VAR IN_LIST ALLOWED_VALUES) message(FATAL_ERROR "MY_VAR must be one of ${ALLOWED_VALUES}") endif
Citations:
- 1: https://cmake.org/cmake/help/latest/prop_cache/STRINGS.html
- 2: https://cmake.org/cmake/help/git-stage/prop_cache/STRINGS.html
- 3: https://cmake.org/cmake/help/git-master/prop_cache/STRINGS.html
- 4: https://www.kitware.com/constraining-values-with-comboboxes-in-cmake-cmake-gui/
- 5: https://cmake.org/cmake/help/book/mastering-cmake/chapter/CMake%20Cache.html
- 6: https://stackoverflow.com/questions/39670879/cmake-variable-with-limited-valid-values
Reject invalid group override values before applying them.
STRINGS only sets GUI choices for CACHE STRING entries; -DFLB_ALL_* can still supply any value. Values that are not exactly Inherit fall through to DEFINE_OPTION(...) and become the initial value for option(), where typos can be interpreted as boolean plugin selection or be handled differently by CMake tooling. Add a check for each FLB_ALL_* against On;Off;Inherit and message(FATAL_ERROR ...) on invalid values.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmake/plugins_options.cmake` around lines 16 - 23, The FLB_ALL_INPUTS,
FLB_ALL_PROCESSORS, FLB_ALL_FILTERS, and FLB_ALL_OUTPUTS cache variables
currently accept arbitrary command-line values. Validate each value immediately
after its cache definition against exactly On, Off, or Inherit, and call
message(FATAL_ERROR ...) for any invalid value before DEFINE_OPTION(...) or
option() processing occurs.
Source: Coding guidelines
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 52a64a143c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if(NOT "${FLB_GROUP_OVERRIDE}" STREQUAL "Inherit") | ||
| set(temp_value ${FLB_GROUP_OVERRIDE}) | ||
| endif() |
There was a problem hiding this comment.
When an existing build directory is reconfigured with a different group value, such as changing FLB_ALL_INPUTS from Inherit to Off, every FLB_IN_* entry is already cached and remains unchanged. As cmake --help-command option documents, option() does nothing when its variable is already set, so the override here only affects the first configuration and the new group control silently stops controlling subsequent builds. Propagate group changes to cached plugin options while preserving explicitly supplied individual overrides.
Useful? React with 👍 / 👎.
| set(FLB_ALL_INPUTS "Inherit" CACHE STRING "Enable/Disable all Input plugins (On, Off, Inherit)") | ||
| set(FLB_ALL_PROCESSORS "Inherit" CACHE STRING "Enable/Disable all Processor plugins (On, Off, Inherit)") | ||
| set(FLB_ALL_FILTERS "Inherit" CACHE STRING "Enable/Disable all Filter plugins (On, Off, Inherit)") | ||
| set(FLB_ALL_OUTPUTS "Inherit" CACHE STRING "Enable/Disable all Output plugins (On, Off, Inherit)") |
There was a problem hiding this comment.
On Windows with the default FLB_WINDOWS_DEFAULTS=On, these new controls do not govern most inputs, outputs, or filters: CMakeLists.txt:441-444 includes cmake/windows-setup.cmake later, whose lines 51-143 unconditionally reset many individual plugin variables. Consequently, for example, a fresh -DFLB_ALL_INPUTS=Off configuration still enables the default Windows inputs, while On still leaves several inputs disabled. Apply the group override after the platform defaults or make those defaults respect the selected group value.
AGENTS.md reference: AGENTS.md:L11-L15
Useful? React with 👍 / 👎.
1 participant
Add
FLB_ALL_INPUTS,FLB_ALL_PROCESSORS,FLB_ALL_FILTERS, andFLB_ALL_OUTPUTSoptions to control the build for each plugins group individually.Three options for each variable:
Inherit: Default, doesn't do anything.On: Plugins for the entire group enabled.Off: Plugins for the entire group disabled.Respects precedence from least to most specific (right overrides left):
e.g to disable everything except outputs and HTTP input:
The above configuration results in:
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-testlabel to test for all targets (requires maintainer to do).Documentation
fluent/fluent-bit-docs#2632
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.
Summary by CodeRabbit
On,Off, orInheritsettings, allowing users to apply broad defaults while preserving individual plugin behavior when needed.