cmake: make simdutf optional. #10778
Merged
+12
−7
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
…itly to disable it). Signed-off-by: Phillip Whelan <[email protected]>
pwhelan
requested review from
fujimotos,
niedbalski,
patrick-stephens and
celalettin1286
as code owners
WalkthroughRespect FLB_USE_SIMDUTF external overrides, only enable the simdutf subdirectory when both FLB_UNICODE_ENCODER and FLB_USE_SIMDUTF are true, define FLB_HAVE_UNICODE_ENCODER when enabling the SIMDUTF path, and emit a fatal error if FLB_UNICODE_ENCODER is requested without FLB_USE_SIMDUTF. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User / CMake CLI
participant CMake as CMake configuration
participant Build as Build system
rect rgb(245, 250, 240)
note over User,CMake: User may pass -D flags
User->>CMake: -DFLB_UNICODE_ENCODER=ON/OFF\n(optional) -DFLB_USE_SIMDUTF=ON/OFF
CMake->>CMake: If `FLB_USE_SIMDUTF` NOT DEFINED -> set default
end
alt FLB_UNICODE_ENCODER == ON and FLB_USE_SIMDUTF == ON
CMake->>CMake: add definition `FLB_HAVE_UNICODE_ENCODER`
CMake->>Build: add_subdirectory(simdutf)
note over Build: SIMDUTF path and Unicode encoder enabled
else FLB_UNICODE_ENCODER == ON and FLB_USE_SIMDUTF == OFF
CMake->>User: fatal_error("FLB_UNICODE_ENCODER requires FLB_USE_SIMDUTF")
note over CMake,User: Configuration fails
else otherwise
CMake-->>Build: skip simdutf
note over Build: SIMDUTF not built
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 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. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
✨ 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 (
|
pwhelan
requested review from
cosmo0920
and removed request for
niedbalski,
patrick-stephens,
fujimotos and
celalettin1286
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
CMakeLists.txt (1)
602-607: Action: add strict C++ settings and fix CMake gating inconsistencies for simdutfShort summary: rg shows no unguarded uses of FLB_HAVE_UNICODE_ENCODER (all usages are behind #ifdef), but CMake gating is inconsistent — some subdirectories/install rules are still enabled by FLB_UNICODE_ENCODER alone. Please apply the fixes below.
Files / changes to make:
- CMakeLists.txt (top-level, lines ~602-607) — add REQUIRED + disable extensions:
if(FLB_UNICODE_ENCODER AND FLB_USE_SIMDUTF) enable_language(CXX) - set (CMAKE_CXX_STANDARD 11) + set (CMAKE_CXX_STANDARD 11) + set (CMAKE_CXX_STANDARD_REQUIRED ON) + set (CMAKE_CXX_EXTENSIONS OFF) add_subdirectory(${FLB_PATH_LIB_SIMDUTF} EXCLUDE_FROM_ALL) FLB_DEFINITION(FLB_HAVE_UNICODE_ENCODER) endif()
- src/CMakeLists.txt (around line ~349) — gate adding the local simdutf connector on FLB_USE_SIMDUTF too:
-if(FLB_UNICODE_ENCODER) +if(FLB_UNICODE_ENCODER AND FLB_USE_SIMDUTF) add_subdirectory(simdutf) set(src ${src}
- include/CMakeLists.txt (around line ~37) — gate header installation the same way:
-if(FLB_UNICODE_ENCODER) +if(FLB_UNICODE_ENCODER AND FLB_USE_SIMDUTF) file(GLOB headers "fluent-bit/simdutf/*.h") install(FILES ${headers} DESTINATION ${FLB_INSTALL_INCLUDEDIR}/fluent-bit/simdutf/ COMPONENT headersOptional: tighten the FLB_COVERAGE CXX flags gating (lines ~433-437) to require FLB_USE_SIMDUTF when setting CXX flags so flags are only set when C++ targets will be built.
Reasoning / result of verification:
- ripgrep found all FLB_HAVE_UNICODE_ENCODER usages guarded by #ifdef (no unconditional references).
- ripgrep also showed CMake still adds/installs simdutf artifacts when only FLB_UNICODE_ENCODER is set (src/CMakeLists.txt, include/CMakeLists.txt). That creates a mismatch with your top-level change which defines FLB_HAVE_UNICODE_ENCODER only when FLB_USE_SIMDUTF is true — causing incomplete/incorrect C++ target wiring or header installs when FLB_USE_SIMDUTF is off.
Please make the three small CMake edits above to keep gating consistent and to enforce a strict C++ standard.
🧹 Nitpick comments (1)
CMakeLists.txt (1)
135-143: Make FLB_USE_SIMDUTF a cached option with a computed default; normalize bools to ON/OFF.Intent looks good and the guard respects external -D overrides. To improve UX and consistency with CMake conventions:
- Expose FLB_USE_SIMDUTF as a CACHE BOOL so it shows up in ccmake/GUI and persists across re-configures.
- Use canonical ON/OFF values (CMake treats Yes/No as truthy, but ON/OFF is more idiomatic).
- Emit a single summary message of the resolved value.
Suggested diff confined to this hunk:
-if (NOT DEFINED FLB_USE_SIMDUTF) - if(CMAKE_CXX_COMPILER) - message(STATUS "CXX compiler found, enable simdutf.") - set(FLB_USE_SIMDUTF Yes) - else() - message(STATUS "CXX compiler not found, disable simdutf.") - set(FLB_USE_SIMDUTF No) - endif() -endif() +if (NOT DEFINED FLB_USE_SIMDUTF) + set(_FLB_USE_SIMDUTF_DEFAULT OFF) + if (CMAKE_CXX_COMPILER) + set(_FLB_USE_SIMDUTF_DEFAULT ON) + endif() + option(FLB_USE_SIMDUTF "Use simdutf for Unicode encoding (requires C++)" ${_FLB_USE_SIMDUTF_DEFAULT}) +else() + # Surface user-provided value in cache (no FORCE, to respect -D overrides) + set(FLB_USE_SIMDUTF "${FLB_USE_SIMDUTF}" CACHE BOOL "Use simdutf for Unicode encoding (requires C++)") +endif() +message(STATUS "FLB_USE_SIMDUTF=${FLB_USE_SIMDUTF}")This keeps override semantics intact and aligns with how other toggles are exposed.
📜 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(2 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_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
- 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, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=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_SIMD=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=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_UNDEFINED=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
- 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)
- 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
|
Some instances are also showing linking errors: I still have not been able to get to the root cause as is yet. |
…disabled. Signed-off-by: Phillip Whelan <[email protected]>
…in if. Signed-off-by: Phillip Whelan <[email protected]>
cosmo0920
approved these changes
Aug 25, 2025
|
This patch should be backported to 4.0 branch as well. Could you handle on that, @pwhelan ? |
7 tasks
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
allow setting FLB_USE_SIMDUTF=No explicitly to disable it. This is the only way to compile fluent-bit on ubuntu 22.04 and earlier or other linux distributions with gcc<=12. The simdutf library otherwise fails to build with the following error:
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).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
New Features
Bug Fixes