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

CMakeLists.txt: fix build with ninja #8835

Merged
merged 1 commit into from
Jun 6, 2024

Conversation

ThomasDevoogdt
Copy link
Contributor

@ThomasDevoogdt ThomasDevoogdt commented May 19, 2024

$ cmake -GNinja -B build/ && cmake --build build/

Results in this error:

ninja: error: build.ninja:158: bad $-escape (literal $ must be written as $$)

Replacing the $(MAKE) command with make gives us this new error:

ninja: error: 'backtrace-prefix/lib/libbacktrace.a', needed by 'bin/fluent-bit', missing and no known rule to make it

So fix that by properly defining the BUILD_BYPRODUCTS. (Also see https://cmake.org/cmake/help/latest/module/ExternalProject.html#build-step-options)


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
  • [N/A] 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.
  • 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
Copy link
Contributor Author

@edsiper, can you consider this PR?

@edsiper edsiper added this to the Fluent Bit v3.0.7 milestone May 29, 2024
Copy link
Contributor

@cosmo0920 cosmo0920 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just specifying fixed "make" is not proper option for Windows.
Instead, how about using the following lines:

if (CMAKE_GENERATOR MATCHES "Ninja")
  if (FLB_SYSTEM_WINDOWS)
    MESSAGE(FATAL_ERROR "Building with Ninja is not supported on Windows")
  else()
    set(EXTERNAL_BUILD_TOOL "make")
  endif()
else()
  set(EXTERNAL_BUILD_TOOL "$(MAKE)")
endif()

and using ${EXTERNAL_BUILD_TOOL} in the following ExternalProject_Add?
This could be supporting Windows environment and preserve the previous behavior without specifying Ninja genator.

    $ cmake -GNinja -B build/ && cmake --build build/

Results in this error:

    ninja: error: build.ninja:158: bad $-escape (literal $ must be written as $$)

Replacing the $(MAKE) command with make gives us this new error:

    ninja: error: 'backtrace-prefix/lib/libbacktrace.a', needed by 'bin/fluent-bit', missing and no known rule to make it

So fix that by properly defining the BUILD_BYPRODUCTS.
(Also see https://cmake.org/cmake/help/latest/module/ExternalProject.html#build-step-options)

Signed-off-by: Thomas Devoogdt <[email protected]>
@ThomasDevoogdt ThomasDevoogdt force-pushed the bugfix/build-with-ninja branch from b9360a0 to f3fbcda Compare June 5, 2024 11:57
@ThomasDevoogdt ThomasDevoogdt requested a review from cosmo0920 June 5, 2024 12:00
@edsiper edsiper merged commit 5218f1f into fluent:master Jun 6, 2024
69 of 70 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs-required ok-package-test Run PR packaging tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants