-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Adds Linux CMake presets for coreclr #110029
Conversation
@dotnet-policy-service agree |
Tagging subscribers to this area: @hoyosjs |
Tagging @huoyaoyuan as per the original contributions to this file |
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.
I have local preset for WSL, which needs more remoting settings. This PR is mostly LGTM and WSL preset can base on it.
"inherits": "base", | ||
"cacheVariables": { | ||
"CLR_CMAKE_TARGET_OS": "linux" | ||
}, |
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.
"CMAKE_C_COMPILER": "clang",
"CMAKE_CXX_COMPILER": "clang++"
Is it using gcc for default? If so, let's default to clang since it's used for most cases.
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.
It is unspecified by default, so it would use whatever is discovered by CMake via environment variables, kit etc.. I initially set it that way because it is possible to build with both gcc
and clang
but if we want to be explicit to use clang
then that's fine too
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.
Hmm, for future usage in the repo, the choice of compiler may be passed with cmake command line. A user can always create a user preset with additional specification. I'm then OK to leave it unspecified.
@@ -233,6 +248,33 @@ | |||
"Checked", | |||
"ARM64" | |||
] | |||
}, | |||
{ | |||
"name": "linux.x64.Debug", |
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.
Let's add linux-arm64 too since it's in official support matrix.
@MattBolitho I am just curious - what are the steps to be able to debug code in CLion for coreclr repo with sources? |
The step should be similar to Visual Studio, see #109097 |
I am not asking about debugging with source stepping. I am interested in proper intellisense. From my understanding the workflow is a bit more complex |
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.
LGTM, requires an official member to check this in.
Thank you for your feedback! I will tag @jkoritzinsky as they merged the previous PR I referenced.
@EgorBo - I didn't do any deep debugging for the sake of raising this PR yet. My plan is to use |
I noticed when opening this project in CLion on Ubuntu that presets are supported for Windows and macOS but not Linux. As macOS support was added recently in #109061, I thought I'd raise this small PR to include Linux x64 as well.