-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Remove DCLR_DOTNET_HOST_PATH cmake parameter, use dotnet from PATH #122211
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: main
Are you sure you want to change the base?
Conversation
The CLR_DOTNET_HOST_PATH cmake option doesn't support spaces in paths. Instead of trying to fix the quoting, simplify by using 'dotnet' from PATH. This aligns with the comment in clrdatadescriptors.cmake that says the code "just uses `dotnet` from the PATH", and with the design intent where dotnet.sh places dotnet on the PATH. Co-authored-by: agocke <[email protected]>
|
Why not simply change |
|
The problem isn’t in this file, it’s all the way up the chain of our Shell scripts. They aren’t preserving the quotes properly. In the container scenario, dotnet.sh should be placing dotnet on the path, so I don’t see why it wouldn’t work. |
That's what cmake will take care of, we just need to quote first two path args. On this same line, we have |
I know you haven't validated this because I did, and that doesn't work. |
|
@agocke, do you mean having spaces in |
They may -- in my repro the paths do not have spaces. But my Mac does have spaces in the dotnet path, and it's broken. |
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 removes the DCLR_DOTNET_HOST_PATH cmake parameter that fails when paths contain spaces on Unix systems. The build system already ensures dotnet is available in PATH through dotnet.sh and InitializeDotNetCli, making the explicit path parameter redundant.
- Removes the cmake argument that passes
DOTNET_HOST_PATHto the build system - Updates the cmake command to use
dotnetdirectly from PATH instead of the variable
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/coreclr/runtime.proj | Removes the -DCLR_DOTNET_HOST_PATH cmake argument from the build configuration |
| src/coreclr/clrdatadescriptors.cmake | Changes from using ${CLR_DOTNET_HOST_PATH} variable to directly invoking dotnet from PATH |
The
DCLR_DOTNET_HOST_PATHcmake option fails when the path contains spaces on Unix systems.Changes
-DCLR_DOTNET_HOST_PATHcmake argument${CLR_DOTNET_HOST_PATH}todotnetdirectlyThe build system already places
dotneton PATH viadotnet.shandInitializeDotNetCli, making the explicit path parameter unnecessary. This eliminates the quoting issue and aligns with the existing comment inclrdatadescriptors.cmakestating "this just usesdotnetfrom the PATH."Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.