Skip to content

Fix EventPipe source using COMPILE_OPTIONS on old CMake 3.6.2.#48809

Merged
lateralusX merged 1 commit into
dotnet:masterfrom
lateralusX:lateralusX/fix-ep-on-old-cmake
Feb 26, 2021
Merged

Fix EventPipe source using COMPILE_OPTIONS on old CMake 3.6.2.#48809
lateralusX merged 1 commit into
dotnet:masterfrom
lateralusX:lateralusX/fix-ep-on-old-cmake

Conversation

@lateralusX

@lateralusX lateralusX commented Feb 26, 2021

Copy link
Copy Markdown
Member

Older CMake < 3.11 doesn't include COMPILE_OPTIONS source file property. This property is needed by clang or it will issue warning (causing build error) when building files with .c extension under c++ compiler.

Fallback to add_compile_options if CMake is older than 3.11. Even if that option has wider scope, this is only applied to files in eventpipe directory or in debug-pal.

Keeping the more precise set_source_files_properties on newer CMake and when we can upgrade CMake to 3.11 or newer we could get rid of this.

Older CMake < 3.11 doesn't include COMPILE_OPTIONS source file property.
This property is needed by clang or it will issue warning when building
files with .c extension under c++ compiler.

Fallback to add_compile_options if CMake is older than 3.11. Even if
that option has wider scope, this is only applied to files in eventpipe
directory or in debug-pal.

Keeping the more precise set_source_files_properties on newer CMake and
when we can upgrade CMake to 3.11 or newer we could get rid of this.
@ghost

ghost commented Feb 26, 2021

Copy link
Copy Markdown

Tagging subscribers to this area: @tommcdon
See info in area-owners.md if you want to be subscribed.

Issue Details

Older CMake < 3.11 doesn't include COMPILE_OPTIONS source file property. This property is needed by clang or it will issue warning when building files with .c extension under c++ compiler.

Fallback to add_compile_options if CMake is older than 3.11. Even if that option has wider scope, this is only applied to files in eventpipe directory or in debug-pal.

Keeping the more precise set_source_files_properties on newer CMake and when we can upgrade CMake to 3.11 or newer we could get rid of this.

Author: lateralusX
Assignees: -
Labels:

area-Diagnostics-coreclr

Milestone: -

@hoyosjs hoyosjs left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

Keeping the more precise set_source_files_properties on newer CMake and when we can upgrade CMake to 3.11 or newer we could get rid of this.

Yes, necessary sadness of leaving the feature out to ensure we have wide platform reach for official distribution builds. Thanks for testing the fix, I tried on centos8 earlier with the PowerTools enabled, but looks like they are already on 3.11.4. Then tested against the source-build image in centos7 which has cmake-3.6.2 and the generated command has -xc++ as expected.

cc: @omaji @tmds

@lateralusX lateralusX merged commit c7fa295 into dotnet:master Feb 26, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Mar 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error building debug-pal's ds-ipc-posix.c with older cmake

2 participants