lib: zstd: only require a C compiler

[ThomasDevoogdt]

Upstream: facebook/zstd@769723a

Summary by CodeRabbit

• Chores

  • Streamlined CI build setup by removing an unnecessary system package while retaining required native dependencies.
  • Kept external Kafka support configuration intact.

• Refactor

  • Improved build configuration so C++ support is enabled only when needed.
  • Made compiler and linker settings apply more selectively across build types.

• Tests

  • Updated build pathways to better isolate test-related components.

• No user-facing functionality changed.

[edsiper]

(cc: @cosmo0920) the changes should not touch the bundled dependencies because when we upgrade those changes will be lost. I recommend trying to merge the changes in libzstd repo first

[ThomasDevoogdt]

@edsiper The changes are already upstream. See the git commit message.

[ThomasDevoogdt]

@edsiper @cosmo0920 this has been approved (and upstreamed) for quite some time, can this be merged?

[ThomasDevoogdt]

@cosmo0920 @edsiper It's again a bit silent around this PR. Everything is approved and ready to be merged, on what do we exactly wait? Can you just merge this one?

[ThomasDevoogdt]

@cosmo0920 Can you merge this one? Or add milestone that might land anytime soon. I have the feeling that next means forgotten.

[cosmo0920]

We don't want to merge this PR without merging for upstream libzstd library itself.

[cosmo0920]

This commit facebook/zstd@769723a is not released as a stable version officially, so we need to wait for merging this.

[coderabbitai]

📝 Walkthrough

Walkthrough

The change makes zstd C++ support conditional on ZSTD_BUILD_TESTS, routes compiler flags through language-specific parameters, and updates CI to build without the system zstd library.

Changes

Zstd CMake configuration

Layer / File(s)	Summary
Conditional language and compilation flag setup lib/zstd-1.5.7/build/cmake/CMakeLists.txt, lib/zstd-1.5.7/build/cmake/CMakeModules/AddZstdCompilationFlags.cmake	The project no longer declares CXX by default. It enables CXX when ZSTD_BUILD_TESTS is enabled. ADD_ZSTD_COMPILATION_FLAGS now accepts C, CXX, and linker controls.
CI compile-check configuration .github/workflows/pr-compile-check.yaml	The pr-compile-without-cxx job no longer installs libzstd-dev or sets FLB_PREFER_SYSTEM_LIB_ZSTD=ON. The Kafka system-library preference remains enabled.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
    participant CI as CI compile check
    participant CMake as zstd CMake
    participant Flags as compilation flags module

    CI->>CI: Omit system zstd package and preference
    CI->>CMake: Configure vendored zstd
    CMake->>CMake: Set ZSTD_ENABLE_CXX from ZSTD_BUILD_TESTS
    alt Tests enabled
        CMake->>CMake: Enable CXX
    else Tests disabled
        CMake->>CMake: Keep CXX disabled
    end
    CMake->>Flags: Configure C, CXX, and linker flags
    Flags-->>CMake: Apply enabled flags

Loading

Possibly related PRs

• fluent/fluent-bit#9277: Introduced the pr-compile-without-cxx job that this change updates.

Suggested reviewers: patrick-stephens, celalettin1286, niedbalski

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: making the zstd build require only a C compiler by default.

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with 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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

lib/zstd-1.5.7/build/cmake/CMakeLists.txt (1)

119-130: LGTM with maintenance concern: bundled dependency modification before upstream stable release.

The conditional C++ enablement logic is technically correct:

• C++ is enabled only when tests are built (ZSTD_ENABLE_CXX is tied to ZSTD_BUILD_TESTS)
• The macro call ADD_ZSTD_COMPILATION_FLAGS(ON ZSTD_ENABLE_CXX ON) correctly passes C (always on), C++ (conditional), and linker (always on) flag enablement parameters

However, per the PR discussion, modifying bundled dependencies (lib/zstd-1.5.7/) before the upstream changes are released in a stable zstd version creates maintenance risk. When fluent-bit updates to the next stable zstd release, these local modifications will be overwritten, requiring re-application or coordination with upstream timing.

Recommendation: Consider one of the following approaches to reduce maintenance burden:

1. Wait for the upstream zstd stable release that includes commit 769723aee2540aaff8951ac432a1babed358aa71 before merging this PR
2. If immediate merge is required, document this local modification in the fluent-bit repository (e.g., in a ZSTD_PATCHES.md file) to ensure the changes are re-applied during the next zstd bundle update
3. Track the upstream zstd release schedule and coordinate the bundle update timing with this PR merge

Based on learnings from the PR discussion.

📜 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 163b99c.

📒 Files selected for processing (3)

• .github/workflows/pr-compile-check.yaml (2 hunks)
• lib/zstd-1.5.7/build/cmake/CMakeLists.txt (1 hunks)
• lib/zstd-1.5.7/build/cmake/CMakeModules/AddZstdCompilationFlags.cmake (3 hunks)

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-08-31T12:46:11.940Z

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 CMakeLists.txt, the system library preference flags are defined as FLB_PREFER_SYSTEM_LIB_ZSTD and FLB_PREFER_SYSTEM_LIB_KAFKA with the FLB_ prefix.

Applied to files:

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

⏰ 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_ARROW=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_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_SIMD=Off, 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_SMALL=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_COVERAGE=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_SIMD=On, 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 (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
• GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=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_SANITIZE_MEMORY=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 (-DFLB_SMALL=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=On, 3.31.6, clang, clang++)

🔇 Additional comments (5)

.github/workflows/pr-compile-check.yaml (2)

138-138: LGTM! Consistent with the goal of testing bundled zstd.

Removing libzstd-dev installation from the pr-compile-without-cxx job is correct since this job now tests the bundled zstd library (which no longer requires C++) rather than the system-provided one.

---

152-152: LGTM! Correctly uses bundled zstd.

Removing the -DFLB_PREFER_SYSTEM_LIB_ZSTD=ON flag ensures the job uses the bundled zstd library, which aligns with testing the C-only compilation capability.

lib/zstd-1.5.7/build/cmake/CMakeModules/AddZstdCompilationFlags.cmake (2)

52-52: LGTM! Macro signature correctly extended for conditional flag control.

The updated signature ADD_ZSTD_COMPILATION_FLAGS(_C _CXX _LD) enables fine-grained control over which compiler flags to enable, supporting the conditional C++ enablement pattern introduced in the related CMakeLists.txt changes.

---

66-111: LGTM! All EnableCompilerFlag calls consistently updated.

All calls to EnableCompilerFlag have been correctly updated to pass the new _C, _CXX, and _LD parameters, enabling conditional flag application based on language availability.

lib/zstd-1.5.7/build/cmake/CMakeLists.txt (1)

36-40: LGTM! Correctly removes C++ from default languages.

Removing CXX from the initial LANGUAGES list is appropriate for making zstd buildable with C-only toolchains. C++ is now enabled conditionally based on ZSTD_BUILD_TESTS (see lines 119-124).

[ThomasDevoogdt]

@cosmo0920 @edsiper I understand that this is not yet part of an upstream release, but the commit at least has been merged upstream, AND there are tests which are checking the functionality. So is there any reason to keep waiting for it and to not take this along?