Set PlatformTarget to AnyCPU for modern .NET #48599
Conversation
src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.RuntimeIdentifierInference.targets
Outdated
Show resolved
Hide resolved
|
Thanks for starting this! It looks like the tests got removed in one of the more recent commits, though? |
…imeIdentifierInference.targets Co-authored-by: Adeel Mujahid <3840695+am11@users.noreply.github.com>
src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.RuntimeIdentifierInference.targets
Show resolved
Hide resolved
| Modern .NET assemblies don't rely on the platform bit in the binary; they're CPU-agnostic, | ||
| and the JIT determines the actual architecture behavior at runtime. |
There was a problem hiding this comment.
| Modern .NET assemblies don't rely on the platform bit in the binary; they're CPU-agnostic, | |
| and the JIT determines the actual architecture behavior at runtime. | |
| Outside .NET Framework, PlatformTarget does not have a real purpose since the process architecture is | |
| determined by the host used to launch the application. Defaulting to AnyCPU naturally works on all platforms | |
| where modern .NET is supported and makes it unnecessary to recognize these platforms as PlatformTargets. |
JIT does not determine the actual architecture. The actual architecture is determined by the host.
There was a problem hiding this comment.
Did you verify the impact of this on VSTest?
For modern .NET, we determine which testhost will be copied to output directory based on that.
I think the usage in VSTest is bad, but we need to understand how we will approach handling this breaking change.
It's also passed to VSTestTask here:
https://github.com/microsoft/vstest/blob/dbcd471883202088496ab81d3ec2d3f3be0981b5/src/Microsoft.TestPlatform.Build/Microsoft.TestPlatform.targets#L53
But for the specific usage in this task, I don't know if it's .NET Framework only or if it also affects .NET Core. @nohwnd will be the best contact here.
|
Looks like the effect of this change is that we will copy always testhost.exe (the x64 testhost) to the output, and then when we see that the preference of the user is to run x86 tests, we will search for testhost.86.exe and won't find it, and will fall back to resolving dotnet for x86 architecture, and running x86/dotnet.exe testhost.dll. But who knows what other things we will see fail in the wild, we've seen users checking the process name to be testhost.exe and testhost.x86.exe, which was the reason we keep shipping those exes, but did not add one for arm64 or other architectures. If we go down this path it would be nice to detect that the fix is in place, and never copy the testhost.exe to output when new dotnet SDK is used, to unify how we run tests in the future, to always go through dotnet.exe. |
Fixes #42558.
cc @jkotas, @baronfel