Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: fix Kafka support needs a toolchain w/ C++ #7765

Conversation

ThomasDevoogdt
Copy link
Contributor

@ThomasDevoogdt ThomasDevoogdt commented Jul 28, 2023

While building fluent-bit on buildroot, I saw some build failures for toolchains that are lacking C++ support.
Kafka needs a C++ compiler to get compiled, but given the C codebase, it should be possible to compile it without.

Issue:

Logs:

Upstream:

Buildroot:


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:

  • [N/A] Example configuration file for the change
  • Debug log output from testing the change
  • [N/A] Attached Valgrind output that shows no leaks or memory corruption was found

If this is a change to packaging of containers or native binaries then please confirm it works for all targets.

  • [N/A] Run local packaging test showing all targets (including any new ones) build.
  • [N/A] Set ok-package-test label to test for all targets (requires maintainer to do).

Documentation

  • [N/A] Documentation required for this feature

Backporting

  • Backport to latest stable release.

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.

@ThomasDevoogdt ThomasDevoogdt changed the title Kafka support needs a toolchain w/ C++ build: fix Kafka support needs a toolchain w/ C++ Jul 28, 2023
@patrick-stephens
Copy link
Contributor

It looks like AppVeyor is failing so suggests a Windows build issue.
Triggering a full set of target builds to confirm but it looks like a unit test issue: https://ci.appveyor.com/project/fluent/fluent-bit-2e87g/builds/47658687/job/1w6aewjhg84vkw87

Test cache_basic_timeout...                     
------------------------
[ FAILED ]
  log.c:16: Check (*interval >= timeout - 1) && *interval <= timeout... failed
    interval error. got=3 expect=4-5
  log.c:92: Check ret == 0... failed
    update_and_check_interval for TEST_RECORD_01 failed. i=9
  log.c:16: Check (*interval >= timeout - 1) && *interval <= timeout... failed
    interval error. got=3 expect=4-5
  log.c:98: Check ret == 0... failed
    update_and_check_interval for TEST_RECORD_02 failed. i=9
Test cache_one_slot...                          
[ OK ]
FAILED: 1 of 2 unit tests has failed.
      Start 32: flb-it-processor

I cannot comment on the specific change itself: I think the typical approach is to fix it in the upstream dependency then update to the fixed version in Fluent Bit. We don't want to keep patching future dependency updates but it sounds like this is what you've done...

@patrick-stephens patrick-stephens added ok-to-test ok-package-test Run PR packaging tests and removed docs-required labels Aug 1, 2023
@patrick-stephens patrick-stephens temporarily deployed to integration August 1, 2023 19:33 — with GitHub Actions Inactive
@patrick-stephens patrick-stephens temporarily deployed to integration August 1, 2023 19:33 — with GitHub Actions Inactive
@patrick-stephens patrick-stephens temporarily deployed to integration August 1, 2023 19:39 — with GitHub Actions Inactive
@patrick-stephens patrick-stephens temporarily deployed to integration August 1, 2023 19:40 — with GitHub Actions Inactive
@ThomasDevoogdt
Copy link
Contributor Author

ThomasDevoogdt commented Aug 1, 2023 via email

@patrick-stephens
Copy link
Contributor

macOS unit tests failures can be ignored.
I've no idea about the appveyor one I'm afraid, that's probably better on @leonardo-albertovich

Copy link
Contributor

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.

…fluent#7741

e.g. fluent-bit is a c-only library, so allow compilation without cxx

Upstream: confluentinc/librdkafka#4366
Signed-off-by: Thomas Devoogdt <[email protected]>
Fluent-bit is c only, so no need to compile cxx.
This fixes also a compile error in buildroot.

Signed-off-by: Thomas Devoogdt <[email protected]>
@ThomasDevoogdt ThomasDevoogdt force-pushed the bugfix/7741-librdkafka-without-cxx-support branch from c9b4c0c to f1ec3b7 Compare August 25, 2024 16:13
@ThomasDevoogdt
Copy link
Contributor Author

@edsiper @patrick-stephens @celalettin1286 My upstream patch confluentinc/librdkafka#4366 is still not taken upstream, even not considered or an answer on my PR. I don't know what to do with it. I rebased this PR to the current master. But I can understand that you guys are not really keen to merge something that is not yet upstream.

@patrick-stephens
Copy link
Contributor

I'll leave this with @edsiper as not really a CI question for me :)

@ThomasDevoogdt
Copy link
Contributor Author

This PR is overruled by #9277, so I will close it. (Please have a look at that one!)

@ThomasDevoogdt ThomasDevoogdt deleted the bugfix/7741-librdkafka-without-cxx-support branch November 9, 2024 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants