lib: librdkafka: only require a C compiler by ThomasDevoogdt · Pull Request #10341 · fluent/fluent-bit

ThomasDevoogdt · 2025-05-16T12:53:26Z

Upstream: confluentinc/librdkafka#4366

Summary by CodeRabbit

Chores
- Updated pull request compilation checks to use the default Kafka-library selection.
- Removed the Kafka development package from the check’s system package setup.

coderabbitai · 2025-10-04T07:12:15Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults
Review profile: CHILL
Plan: Advanced
Run ID: b04c28b6-b478-4a12-839a-5bd0f9198e4f

📥 Commits

Reviewing files that changed from the base of the PR and between 5e875e5 and 354d4bb.

📒 Files selected for processing: 1

.github/workflows/pr-compile-check.yaml

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

📝 Walkthrough

The CXX-disabled compilation workflow no longer installs librdkafka-dev or sets FLB_PREFER_SYSTEM_LIB_KAFKA=ON. The system Zstandard preference remains enabled.

Changes: CI Kafka dependency removal

Layer / File(s)	Summary
Update CXX-disabled compilation workflow `.github/workflows/pr-compile-check.yaml`	The job no longer installs `librdkafka-dev` or passes `-DFLB_PREFER_SYSTEM_LIB_KAFKA=ON`. It continues to prefer the system Zstandard library.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~2 minutes

Change: Feature

Merge Risk: ⚪ Minimal · up to 354d4

The CI job can build with bundled librdkafka without requiring a C++ compiler; no actionable merge-blocking risk remains.

Pre-merge checks: 5

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely states the main change: librdkafka requires only a C compiler.
Docstring Coverage		No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.)

✨ Finishing Touches: 💡 1

🛠️ Fix failing CI checks: 💡

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Create a new PR

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

lib/librdkafka-2.10.1/CMakeLists.txt (1)
304-307: Also gate tests on actual C++ availability

Even without the option() call, a cache entry like -DRDKAFKA_BUILD_TESTS=ON keeps this block true. On hosts lacking a usable C++ toolchain (the exact case this PR targets), we would still dive into add_subdirectory(tests) and the build fails. Please guard this block with the same CMAKE_CXX_COMPILER check used for src-cpp, e.g.:
-if(RDKAFKA_BUILD_TESTS)
+if(CMAKE_CXX_COMPILER AND RDKAFKA_BUILD_TESTS)
   enable_testing()
   add_subdirectory(tests)
 endif()

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4d9593e and 0a124d4.

📒 Files selected for processing (3)

.github/workflows/pr-compile-check.yaml (2 hunks)
lib/librdkafka-2.10.1/CMakeLists.txt (3 hunks)
lib/librdkafka-2.10.1/examples/CMakeLists.txt (2 hunks)

🧰 Additional context used

🧠 Learnings (1)

📓 Common learnings

Learnt from: ThomasDevoogdt
PR: fluent/fluent-bit#9277
File: .github/workflows/pr-compile-check.yaml:147-151
Timestamp: 2025-08-31T12:46:11.940Z
Learning: In fluent-bit, the correct CMake flag for using system librdkafka is `FLB_PREFER_SYSTEM_LIB_KAFKA=ON`.

⏰ 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). (20)

GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 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_ARROW=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_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=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_UNDEFINED=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_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_SIMD=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 (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
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, gcc, g++)
GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 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 (-DFLB_SMALL=On, 3.31.6, clang, clang++)
GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)

coderabbitai · 2025-10-04T07:16:45Z

+include(CheckLanguage)
+check_language(CXX)
+if(CMAKE_CXX_COMPILER)
+  enable_language(CXX)
+else()
+  message(STATUS "C++ compiler not found, skipping C++ support")
+endif()


⚠️ Potential issue | 🔴 Critical

Handle bogus CXX values before calling enable_language

When CXX is preset to something non-working (e.g. our no-C++ workflow exports /bin/false), check_language(CXX) still leaves CMAKE_CXX_COMPILER pointing at that path instead of *-NOTFOUND. The current if (CMAKE_CXX_COMPILER) conditional therefore fires and enable_language(CXX) aborts configuration, defeating the “C-only” fallback we’re trying to enable. Please tighten the guard so we only enable C++ when CMAKE_CXX_COMPILER is both defined and not *-NOTFOUND. One way is:

-check_language(CXX) -if(CMAKE_CXX_COMPILER) +check_language(CXX) +if(CMAKE_CXX_COMPILER AND NOT CMAKE_CXX_COMPILER MATCHES "-NOTFOUND$") enable_language(CXX) else() message(STATUS "C++ compiler not found, skipping C++ support") endif()

🤖 Prompt for AI Agents

In lib/librdkafka-2.10.1/CMakeLists.txt around lines 9-15, the current guard uses if(CMAKE_CXX_COMPILER) which can be true for bogus values like /bin/false left by check_language(CXX), causing enable_language(CXX) to abort; change the conditional to only enable C++ when the compiler is both defined and not a NOTFOUND marker, e.g. use if(DEFINED CMAKE_CXX_COMPILER AND NOT CMAKE_CXX_COMPILER MATCHES "-NOTFOUND$") then enable_language(CXX) else emit the existing status message.

github-actions · 2026-01-03T02:16:51Z

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.

github-actions · 2026-06-11T02:26:35Z

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.

Upstream: confluentinc/librdkafka#4366 Signed-off-by: Thomas Devoogdt <thomas.devoogdt@barco.com>

This reverts commit 32fc9dd. Not needed anymore since librdkafka has been fixed. Signed-off-by: Thomas Devoogdt <thomas@devoogdt.com>

ThomasDevoogdt requested review from edsiper, fujimotos, koleini and leonardo-albertovich as code owners May 16, 2025 12:53

github-actions Bot added the docs-required label May 16, 2025

ThomasDevoogdt mentioned this pull request May 16, 2025

workflows: add sanity check for toolchains w/ CXX support #9277

Merged

ThomasDevoogdt force-pushed the bugfix/lib-kafka-only-require-a-c-compiler branch 2 times, most recently from e69a7aa to d9a9108 Compare July 19, 2025 13:34

ThomasDevoogdt force-pushed the bugfix/lib-kafka-only-require-a-c-compiler branch from d9a9108 to 0a124d4 Compare October 4, 2025 07:11

ThomasDevoogdt requested review from celalettin1286, niedbalski and patrick-stephens as code owners October 4, 2025 07:11

coderabbitai Bot reviewed Oct 4, 2025

View reviewed changes

github-actions Bot added the Stale label Jan 3, 2026

ThomasDevoogdt force-pushed the bugfix/lib-kafka-only-require-a-c-compiler branch from 0a124d4 to 16d53c5 Compare March 7, 2026 17:50

ThomasDevoogdt requested a review from cosmo0920 as a code owner March 7, 2026 17:50

github-actions Bot removed the Stale label Mar 13, 2026

github-actions Bot added the Stale label Jun 11, 2026

ThomasDevoogdt force-pushed the bugfix/lib-kafka-only-require-a-c-compiler branch from 16d53c5 to 5e875e5 Compare August 4, 2026 13:55

github-actions Bot removed the Stale label Aug 5, 2026

ThomasDevoogdt mentioned this pull request Oct 9, 2026

lib: zstd: only require a C compiler #12538

Merged

ThomasDevoogdt and others added 2 commits October 18, 2026 18:30

lib: librdkafka: only require a C compiler

562273e

Upstream: confluentinc/librdkafka#4366 Signed-off-by: Thomas Devoogdt <thomas.devoogdt@barco.com>

Revert "workflows: exclude librdkafka-dev from pr-compile-without-cxx"

354d4bb

This reverts commit 32fc9dd. Not needed anymore since librdkafka has been fixed. Signed-off-by: Thomas Devoogdt <thomas@devoogdt.com>

ThomasDevoogdt force-pushed the bugfix/lib-kafka-only-require-a-c-compiler branch from 5e875e5 to 354d4bb Compare October 18, 2026 16:31

Navigation Menu

Repository navigation

lib: librdkafka: only require a C compiler - #10341

lib: librdkafka: only require a C compiler#10341
ThomasDevoogdt wants to merge 2 commits into
fluent:masterfrom
ThomasDevoogdt:bugfix/lib-kafka-only-require-a-c-compiler

ThomasDevoogdt commented May 16, 2025 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Oct 4, 2025 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot Oct 4, 2025

Uh oh!

github-actions Bot commented Jan 3, 2026

Uh oh!

github-actions Bot commented Jun 11, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

ThomasDevoogdt commented May 16, 2025 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Oct 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Oct 4, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jan 3, 2026

Uh oh!

github-actions Bot commented Jun 11, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

ThomasDevoogdt commented May 16, 2025 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Oct 4, 2025 •

edited

Loading