-
Notifications
You must be signed in to change notification settings - Fork 6.3k
modify NetdataYAML.cmake to fix compilation error #21295
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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(<target> 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.
|
@Ferroin @vkalintiris |
There was a problem hiding this 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_DEFINITIONSandINTERFACE_COMPILE_OPTIONSfor bundled libyaml to prevent definitions from being treated as bare compiler flags - Introduced
NETDATA_YAML_DEFINITIONSvariable to store preprocessor definitions separately - Added
target_compile_definitions()call to properly apply definitions with-Dprefix
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@cubic-dev-ai review this PR |
@kkzhsh I've started the AI code review. It'll take a few minutes to complete. |
There was a problem hiding this 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
Summary
There is an error while building netdata without module yaml-1.0, thils PR is to fix this issue.
Test Plan
cmake -DENABLE_PLUGIN_XENSTAT=Falsecmake information is as following
and some incorrect building command is generated in compile_commands.json
NOTE:only
-DYAML_DECLARE_STATICis expected, butYAML_DECLARE_STATICis 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
Additional Information
A new variable
NETDATA_YAML_DEFINITIONSis 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.
Written for commit 520a211. Summary will update automatically on new commits.