Locate dotnet host for illink on desktop#3192
Conversation
| <_DotNetHostDirectory>$([System.IO.Path]::GetFullPath($(MSBuildThisFileDirectory)../../../../../))</_DotNetHostDirectory> | ||
| <_DotNetHostFileName>dotnet</_DotNetHostFileName> | ||
| <_DotNetHostFileName Condition=" '$(OS)' == 'Windows_NT' ">dotnet.exe</_DotNetHostFileName> | ||
| </PropertyGroup> |
There was a problem hiding this comment.
This ../../.. won't work in our test environment for full framework, where we set up things to point at the build output.
Thinking about the best approach here.
There was a problem hiding this comment.
If the SDK always sets a property DotNetHost similar to AppHost executable, I think that would be useful.
|
@fadimounir @swaroop-sridhar this is the env var issue I mentioned today. |
|
Ah, forgot about this, sorry. Haven't figured out what to do about the issue above yet, but back to thinking about this. |
|
@nguerrera any more thoughts on this? |
|
How about this: Set up NETCoreRoot next to NETCoreTargetingPackRoot here: This makes one fewer place in sdk where we have to .. (we already do that to find these bundled versions), and has prior art. We can optimize a bit and make NETCoreTargetingPackRoot a function of NETCoreRoot. Edit: it will also take care of the testing problem because we already override the bundled props location appropriately when testing. @dsplaisted What do you think? |
|
Ah, so that bundled props file is the place to put this. Thanks @nguerrera! If @dsplaisted agrees, I can go ahead and make a PR with the change in core-sdk. |
|
Sounds good. I do wonder if there are interactions between environment variables such as DOTNET_HOST or DOTNET_ROOT and this new MSBuild property that we should consider. |
|
I thought about those a bit, it's why I hesitated here. The problem I see is that there are existing uses of DOTNET_HOST_PATH meant to detect if running CLI or not, and DOTNET_ROOT has a bunch of baggage too. I think this is separate and just the location of the dotnet hive that has the bundled versions. Maybe there's a better name? |
|
As far as I can tell,
If I haven't missed anything, these interactions seem sensible to me. If we wanted to, I think we could even replace uses of |
When running on full framework MSBuild, the environment variable DOTNET_HOST_PATH is not set. This was not caught by the SDK tests because they are started by a test process that uses the dotnet cli, so this includes a change to prevent DOTNET_HOST_PATH from propagating to the MSBuild process.
sbomer
force-pushed
the
locateHostForLinker
branch
from
8176b5d to
cd96a1d
Compare
|
@nguerrera @dsplaisted PTAL |
…015.2 (#3192) - Microsoft.DotNet.Arcade.Sdk - 5.0.0-beta.19515.2
The harness was wiping DOTNET_HOST_PATH from the msbuild.exe environment to mirror the customer scenario where bare msbuild.exe is launched outside a Developer Command Prompt. That clear was added in 2019 by #3192 ("Locate dotnet host for illink on desktop") to expose ILLink's dependence on the env var, and ILLink got a project-level fallback at the same time. That customer scenario no longer reflects reality. Since #50066 (commit ea55de9, 2025-07-19, "set env vars from resolver APIs"), Microsoft.DotNet.MSBuildSdkResolver returns DOTNET_HOST_PATH via the EnvironmentVariablesToAdd API whenever it resolves Microsoft.NET.Sdk. The MSBuild engine then injects it into the resolver's process environment, so by the time tasks (Csc, Microsoft.DotNet.ApiCompat.Task, etc.) execute, DOTNET_HOST_PATH is set even if msbuild.exe was launched without it. Real customers running bare msbuild.exe today are auto-fixed by the resolver; Arcade's eng/common scripts also rely on this and never set DOTNET_HOST_PATH explicitly. Removing the clear restores natural propagation through the test harness: dotnet exec sets DOTNET_HOST_PATH on the test process (per Muxer.cs), the test process inherits it, and msbuild.exe inherits it from the test process. This unblocks .NETCoreApp tasks declared with `Runtime="NET"` (notably the new ApiCompat / GenAPI tasks introduced when removing .NET Framework support from src/Compatibility) when running on the FullFramework Helix leg, without needing per-test env-var workarounds. Also revert two ineffective workarounds previously added while diagnosing this: - The DOTNET_HOST_PATH set in build/SetupHelixEnvironment.cmd (it never reached msbuild.exe because the harness immediately cleared it). - The temporary _DiagnoseTaskHostProps target in test/TestAssets/TestProjects/PackageValidationTestProject. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
4 participants
When running on full framework MSBuild, the environment variable
DOTNET_HOST_PATHis not set, causing the linker to fail. This was not caught by the SDK tests because they are started by a test process that uses the dotnet cli, so this includes a change to preventDOTNET_HOST_PATHfrom propagating to the MSBuild process.For the desktop MSBuild case, I believe the correct thing to do is to use the dotnet host that comes with the SDK that is being used to build the app, so I'm trying to explicitly pass the location of that host (rather than searching the
PATH, for example). @dsplaisted and @nguerrera does that sound right to you, or should I be doing something else?The current fix has a problem: the relative path from the targets file to the dotnet host is correct for an actual product SDK, but because the tests run against a different layout, the computed location is incorrect when tests are run. @dsplaisted, @nguerrera do you have any suggestions? Perhaps the SDK already has some information about the dotnet host location that I can reuse.