cmake: explicitly disable FLB_UNICODE_ENCODER when FLB_USE_SIMDUTF is disabled. #10786
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… disabled. Signed-off-by: Phillip Adair Stewart Whelan <[email protected]>
pwhelan
requested review from
fujimotos,
niedbalski,
patrick-stephens and
celalettin1286
as code owners
WalkthroughAdds a CMake pre-check that automatically disables FLB_UNICODE_ENCODER when FLB_USE_SIMDUTF is not enabled, emitting a status message and short-circuiting encoder configuration. No other logic paths are modified. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer
participant CMake as CMake Configure
Dev->>CMake: Configure project
rect rgba(230,246,255,0.6)
note right of CMake: New pre-check
CMake->>CMake: Is FLB_UNICODE_ENCODER ON?
alt Encoder ON
CMake->>CMake: Is FLB_USE_SIMDUTF ON?
alt SIMDUTF OFF (new)
CMake->>CMake: set(FLB_UNICODE_ENCODER OFF)
CMake-->>Dev: STATUS: Disabling FLB_UNICODE_ENCODER
else SIMDUTF ON
CMake->>CMake: Proceed with encoder configuration (unchanged)
end
else Encoder OFF
CMake->>CMake: No action
end
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested labels
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
CMakeLists.txt (3)
433-440: Good guard; prevents inconsistent config when SIMDUTF is offThis early guard cleanly avoids the build failure scenario by forcing
FLB_UNICODE_ENCODERoff whenFLB_USE_SIMDUTFis disabled. Placement (before coverage flags and the later simdutf block) is correct.Optional polish:
- Consider updating the cache entry so CMake-GUI/ccmake reflect the final OFF value, even if the user passed
-DFLB_UNICODE_ENCODER=ON. Currentset()only overrides in the normal scope and can look confusing in GUIs.- Optionally bump the message to WARNING to make CI logs more visible.
Examples:
- message(STATUS "FLB_USE_SIMDUTF is disabled. Disabling FLB_UNICODE_ENCODER support.") - set(FLB_UNICODE_ENCODER OFF) + message(WARNING "FLB_USE_SIMDUTF is disabled. Disabling FLB_UNICODE_ENCODER support.") + set(FLB_UNICODE_ENCODER OFF CACHE BOOL "Build with Unicode (UTF-16LE, UTF-16BE) encoding support" FORCE)
609-613: Remove unreachable fatal check (dead code)This inner check can never trigger because the outer condition already requires
FLB_USE_SIMDUTF. Simplify to reduce noise:-if(FLB_UNICODE_ENCODER AND FLB_USE_SIMDUTF) - if (NOT FLB_USE_SIMDUTF) - message(FATAL_ERROR "FLB_UNICODE_ENCODER requires FLB_USE_SIMDUTF") - endif() +if(FLB_UNICODE_ENCODER AND FLB_USE_SIMDUTF)
163-165: Clarify option help to document dependencyMake the dependency explicit in the option description so users see it immediately in GUIs and cache listings:
-option(FLB_UNICODE_ENCODER "Build with Unicode (UTF-16LE, UTF-16BE) encoding support" ${FLB_USE_SIMDUTF}) +option(FLB_UNICODE_ENCODER "Build with Unicode (UTF-16LE/BE) encoding support (requires FLB_USE_SIMDUTF=ON)" ${FLB_USE_SIMDUTF})If desired, you could also convert this to a dependent option via CMake’s
CMakeDependentOptionmodule, but the current guard plus clearer help text is sufficient.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
CMakeLists.txt(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (24)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-centos-7
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-24.04, clang-14)
3 tasks
cosmo0920
approved these changes
Aug 27, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Due to how cmake works and how the
CMakeLists.txtis designed, one can explicitly enableFLB_UNICODE_ENCODERwhenFLB_USE_SIMDUTFis disabled. This should not happen often but when it does it will lead to a failed build.This code should guard against it.
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
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