Skip to content

Conversation

@kkzhsh
Copy link
Contributor

@kkzhsh kkzhsh commented Nov 13, 2025

Summary

There is an error while building netdata without module yaml-1.0, thils PR is to fix this issue.

Test Plan
  1. cmake -DENABLE_PLUGIN_XENSTAT=False
    cmake information is as following
....
-- Checking for module 'yaml-0.1'
--   Package 'yaml-0.1', required by 'virtual:world', not found
CMake Deprecation Warning at build/_deps/yaml-src/CMakeLists.txt:2 (cmake_minimum_required):
  Compatibility with CMake < 3.5 will be removed from a future version of
  CMake.

  Update the VERSION argument <min> value or use a ...<max> suffix to tell
  CMake that the project does not need compatibility with older versions.
....

and some incorrect building command is generated in compile_commands.json
NOTE:only -DYAML_DECLARE_STATIC is expected, but YAML_DECLARE_STATIC is also generated as a compilation flag.
"command": "/usr/bin/cc -DYAML_DECLARE_STATIC -D_GNU_SOURCE -I/usr/include/uuid -I/home/work-code/github/netdata/build/include -I/home/work-code/github/netdata/build/libbacktrace-install/include -I/home/work-code/github/netdata/build -I/home/work-code/github/netdata/src -I/home/work-code/github/netdata/build/_deps/yaml-src/include -I/home/work-code/github/netdata/build/_deps/yaml-build/include -I/home/work-code/github/netdata/src/libnetdata/libjudy/vendored -I/home/work-code/github/netdata/src/libnetdata/libjudy/vendored/JudyCommon -I/home/work-code/github/netdata/build/_deps/json-c-src -I/home/work-code/github/netdata/build/_deps/json-c-build -O2 -g -DNDEBUG -std=gnu11 -flto=auto -fno-fat-lto-objects -flto=auto -fexceptions -fno-omit-frame-pointer -funwind-tables -fasynchronous-unwind-tables -std=c++17 YAML_DECLARE_STATIC -o CMakeFiles/libnetdata.dir/src/libnetdata/adaptive_resortable_list/adaptive_resortable_list.c.o -c /home/work-code/github/netdata/src/libnetdata/adaptive_resortable_list/adaptive_resortable_list.c", "command": "/usr/bin/cc -DYAML_DECLARE_STATIC -D_GNU_SOURCE -I/usr/include/uuid -I/home/work-code/github/netdata/build/include -I/home/work-code/github/netdata/build/libbacktrace-install/include -I/home/work-code/github/netdata/build -I/home/work-code/github/netdata/src -I/home/work-code/github/netdata/build/_deps/yaml-src/include -I/home/work-code/github/netdata/build/_deps/yaml-build/include -I/home/work-code/github/netdata/src/libnetdata/libjudy/vendored -I/home/work-code/github/netdata/src/libnetdata/libjudy/vendored/JudyCommon -I/home/work-code/github/netdata/build/_deps/json-c-src -I/home/work-code/github/netdata/build/_deps/json-c-build -O2 -g -DNDEBUG -std=gnu11 -flto=auto -fno-fat-lto-objects -flto=auto -fexceptions -fno-omit-frame-pointer -funwind-tables -fasynchronous-unwind-tables -std=c++17 YAML_DECLARE_STATIC -o CMakeFiles/libnetdata.dir/src/libnetdata/aral/aral.c.o -c /home/work-code/github/netdata/src/libnetdata/aral/aral.c",
2)make
Building error is as following

cc: warning: YAML_DECLARE_STATIC: linker input file unused because linking not done
cc: error: YAML_DECLARE_STATIC: linker input file not found: No such file or directory
Additional Information

A new variable NETDATA_YAML_DEFINITIONS is to used to record yaml module related preprocessor macro‌, and target_compile_definitions cmake-command is used to append yaml module preprocessor macro‌ to building command.


Summary by cubic

Fixes build failures when system libyaml is missing by correctly applying libyaml compile definitions in CMake. Prevents “YAML_DECLARE_STATIC” from appearing as a raw token and causing cc errors.

  • Bug Fixes
    • Use target_compile_definitions for bundled libyaml or when YAML not found; use target_compile_options when using system libyaml.
    • Remove unconditional compile_options and set include dirs BEFORE to avoid invalid compile_commands entries and linker errors.

Written for commit 520a211. Summary will update automatically on new commits.

@github-actions github-actions bot added the area/packaging Packaging and operating systems support label Nov 13, 2025
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 1 file

Prompt for AI agents (all 1 issues)

Understand the root cause of the following 1 issues and fix them.


<file name="packaging/cmake/Modules/NetdataYAML.cmake">

<violation number="1" location="packaging/cmake/Modules/NetdataYAML.cmake:78">
When libyaml is provided by the system (ENABLE_BUNDLED_LIBYAML is false), NETDATA_YAML_DEFINITIONS is never set, so this command expands to `target_compile_definitions(&lt;target&gt; PUBLIC)` and CMake raises an argument error, halting the build. Please guard the call or initialize the variable so the target is only given definitions when some exist.</violation>
</file>

Since this is your first cubic review, here's how it works:

  • cubic automatically reviews your code and comments on bugs and improvements
  • Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
  • Ask questions if you need clarification on any suggestion

React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.

@kkzhsh
Copy link
Contributor Author

kkzhsh commented Nov 17, 2025

@Ferroin @vkalintiris
Could U help to review this PR? Thanks

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a compilation error when building netdata without the system yaml-0.1 library by properly separating compile definitions from compile options in the CMake configuration.

Key Changes:

  • Separated INTERFACE_COMPILE_DEFINITIONS and INTERFACE_COMPILE_OPTIONS for bundled libyaml to prevent definitions from being treated as bare compiler flags
  • Introduced NETDATA_YAML_DEFINITIONS variable to store preprocessor definitions separately
  • Added target_compile_definitions() call to properly apply definitions with -D prefix

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@kkzhsh
Copy link
Contributor Author

kkzhsh commented Nov 19, 2025

@cubic-dev-ai review this PR

@cubic-dev-ai
Copy link
Contributor

cubic-dev-ai bot commented Nov 19, 2025

@cubic-dev-ai review this PR

@kkzhsh I've started the AI code review. It'll take a few minutes to complete.

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 1 file

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/packaging Packaging and operating systems support

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant