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

Correctly handle required compilation flags for dependencies. #17664

Merged
merged 1 commit into from
May 16, 2024

Conversation

Ferroin
Copy link
Member

@Ferroin Ferroin commented May 15, 2024

Summary

target_compile_definitions can only be used for actual command line macro definitions. But the *_CFLAGS_OTHER may contain flags that are not macro definitions, and when that happens, any use of target_compile_definitions with the relevant *_CFLAGS_OTHER variable will break the build.

Instead, we should be using target_compile_options for these variables.

Test Plan

Observe that the CI jobs for Rocky Linux 8 and CentOS 8 Stream pass on this PR, despite failing on the master branch.

`target_compile_definitions` can only be used for actual command
line _macro definitions_. But the `*_CFLAGS_OTHER` may contain flags
that are _not_ macro definitions, and when that happens, any use of
`target_compile_definitions` with the relevant `*_CFLAGS_OTHER` variable
will break the build.

Instead, we should be using `target_compile_options` for these
variables.
@github-actions github-actions bot added area/packaging Packaging and operating systems support area/build Build system (autotools and cmake). labels May 15, 2024
@Ferroin Ferroin marked this pull request as ready for review May 15, 2024 14:08
@Ferroin Ferroin requested review from vkalintiris and a team as code owners May 15, 2024 14:08
@Ferroin Ferroin merged commit bfdd228 into netdata:master May 16, 2024
147 of 149 checks passed
@Ferroin Ferroin deleted the build-fix branch May 16, 2024 10:47
@Ferroin Ferroin mentioned this pull request May 17, 2024
Ferroin added a commit to stelfrag/netdata that referenced this pull request May 20, 2024
…a#17664)

`target_compile_definitions` can only be used for actual command
line _macro definitions_. But the `*_CFLAGS_OTHER` may contain flags
that are _not_ macro definitions, and when that happens, any use of
`target_compile_definitions` with the relevant `*_CFLAGS_OTHER` variable
will break the build.

Instead, we should be using `target_compile_options` for these
variables.
Ferroin added a commit that referenced this pull request May 21, 2024
`target_compile_definitions` can only be used for actual command
line _macro definitions_. But the `*_CFLAGS_OTHER` may contain flags
that are _not_ macro definitions, and when that happens, any use of
`target_compile_definitions` with the relevant `*_CFLAGS_OTHER` variable
will break the build.

Instead, we should be using `target_compile_options` for these
variables.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/build Build system (autotools and cmake). area/packaging Packaging and operating systems support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants