Delete NS2x runtime specific impls #64610
Conversation
Remove the remaining ten .NETStandard runtime specific implementations. For reasoning please see dotnet#64536.
|
Tagging subscribers to this area: @dotnet/area-infrastructure-libraries Issue DetailsFixes #64536 Remove the remaining ten .NETStandard runtime specific implementations. @ericstj I'm unsure if I got it right. Some of the NS2.x implementations will now throw a PNSE. Is that expected and OK? All the project changes are contained in the first commit. The second commit removes the infrastructure burden.
|
dotnet/runtime was the only consumer that dependent on runtime specific .NETStandard tfms. Now that we are removing them with dotnet/runtime#64610 we can also delete the hacks to make them work in the TargetFramework SDK.
ViktorHofer
force-pushed
the
DeleteNS2xRuntimeSpecificImpls
branch
3 times, most recently
from
a8901c7
to
691ce83
Compare
Also cleaning-up some logic and stop disabling AppendTargetFramework.
ViktorHofer
force-pushed
the
DeleteNS2xRuntimeSpecificImpls
branch
from
691ce83
to
56fedca
Compare
dotnet/runtime was the only consumer that dependent on runtime specific .NETStandard tfms. Now that we are removing them with dotnet/runtime#64610 we can also delete the hacks to make them work in the TargetFramework package. Also renaming the project from Microsoft.DotNet.Build.Tasks.TargetFramework.Sdk to Microsoft.DotNet.Build.Tasks.TargetFramework as doesn't need to be an msbuild sdk anymore and consuming it as a package will reduce evaluation time.
So long as that's what they did before this change that is OK. We don't want to transition any |
|
Thanks for double checking. Then the change is ready to be reviewed. I believe the test leg failures are all unrelated but let me retrigger CI to be sure. |
|
/azp run runtime-staging |
|
/azp run runtime |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run dotnet-linker-tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
1 similar comment
|
Azure Pipelines successfully started running 1 pipeline(s). |
| @@ -20,7 +20,7 @@ | |||
|
|
|||
| <_generateRuntimeGraphTargetFramework Condition="'$(MSBuildRuntimeType)' == 'core'">$(NetCoreAppToolCurrent)</_generateRuntimeGraphTargetFramework> | |||
| <_generateRuntimeGraphTargetFramework Condition="'$(MSBuildRuntimeType)' != 'core'">net472</_generateRuntimeGraphTargetFramework> | |||
| <_generateRuntimeGraphTask>$([MSBuild]::NormalizePath('$(BaseOutputPath)', '$(_generateRuntimeGraphTargetFramework)-$(Configuration)', '$(AssemblyName).dll'))</_generateRuntimeGraphTask> | |||
| <_generateRuntimeGraphTask>$([MSBuild]::NormalizePath('$(BaseOutputPath)', $(Configuration), '$(_generateRuntimeGraphTargetFramework)', '$(AssemblyName).dll'))</_generateRuntimeGraphTask> | |||
There was a problem hiding this comment.
How is this related to the rest of the change?
There was a problem hiding this comment.
I change the IntermediateOutputPath and the OutputPath by removing the AppendTargetFrameworkToOutputPath=false setting in the libraries Directory.Build.props file, hence a few hardcoded paths need to be updated.
| <AppleTestRunnerDir>$([MSBuild]::NormalizeDirectory('$(ArtifactsBinDir)', 'AppleTestRunner', '$(MobileRunnersDirSuffix)'))</AppleTestRunnerDir> | ||
| <AndroidTestRunnerDir>$([MSBuild]::NormalizeDirectory('$(ArtifactsBinDir)', 'AndroidTestRunner', '$(MobileRunnersDirSuffix)'))</AndroidTestRunnerDir> | ||
| <WasmTestRunnerDir>$([MSBuild]::NormalizeDirectory('$(ArtifactsBinDir)', 'WasmTestRunner', '$(MobileRunnersDirSuffix)'))</WasmTestRunnerDir> | ||
| <AppleTestRunnerDir>$([MSBuild]::NormalizeDirectory('$(ArtifactsBinDir)', 'AppleTestRunner', '$(Configuration)', '$(NetCoreAppCurrent)'))</AppleTestRunnerDir> |
There was a problem hiding this comment.
How is this related to the rest of the change?
There was a problem hiding this comment.
Same reason as above. The IntermediateOutputPath and the OutputPath of the test runners changed with the removal of the AppendTargetFrameworkToOutputPath=false setting and I'm updating hardcoded paths here.
| @@ -92,6 +92,3 @@ In order to mitigate design-time/build-time performance issues with source gener | |||
| <DisableSourceGeneratorPropertyName>CustomPropertyName</DisableSourceGeneratorPropertyName> | |||
| </PropertyGroup> | |||
| ``` | |||
There was a problem hiding this comment.
Should we add something to the docs around where the -runtime suffix is supported? Does the infra fail in a sensible way when it's misused (if not, should we explicitly error)?
There was a problem hiding this comment.
After dotnet/arcade#8418 merged and consumed, NuGet will throw during restore if a non .NETCoreApp tfm has a suffix in its alias. It makes sense to add section to the project guidelines clarifying which tfms can be runtime specific and how to apply conditions to specific runtimes correctly.
I plan to update that document with #64500.
|
The wasm failures are affecting all PRs. Merging. |
dotnet/runtime was the only consumer that dependent on runtime specific .NETStandard tfms. Now that we are removing them with dotnet/runtime#64610 we can also delete the hacks to make them work in the TargetFramework package. Also renaming the project from Microsoft.DotNet.Build.Tasks.TargetFramework.Sdk to Microsoft.DotNet.Build.Tasks.TargetFramework as doesn't need to be an msbuild sdk anymore and consuming it as a package will reduce evaluation time.
ghost
locked as resolved and limited conversation to collaborators
Fixes #64536
Remove the remaining ten .NETStandard runtime specific implementations.
For reasoning please see #64536.
@ericstj I'm unsure if I got it right. Some of the NS2.x implementations will now throw a PNSE. Is that expected and OK? All the project changes are contained in the first commit. The second commit removes the infrastructure burden.