Skip to content

Commit

Permalink
Cleaning up the .NET 9 SDK issues
Browse files Browse the repository at this point in the history
This change adjusts our repo to working with the .NET 9 SDK.
  • Loading branch information
jaredpar committed May 16, 2024
1 parent da3e165 commit 95b3302
Show file tree
Hide file tree
Showing 14 changed files with 197 additions and 72 deletions.
10 changes: 9 additions & 1 deletion docs/contributing/Target Framework Strategy.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ Projects in our repository should include the following values in `<TargetFramew
5. `$(NetRoslyn)`: code that needs to execute on .NET but does not have any specific product deployment requirements. For example utilities that are used by our infra, compiler unit tests, etc ... This property also controls which of the frameworks the compiler builds against are shipped in the toolset packages. This value will potentially change in source builds.
6. `$(NetRoslynAll)`: code, generally test utilities, that need to build for all .NET runtimes that we support.
7. `$(NetRoslynBuildHostNetCoreVersion)`: the target used for the .NET Core BuildHost process used by MSBuildWorkspace.
8. `$(NetRoslynNext)`: code that needs to run on the ext .NET Core version. This is used during the transition to a new .NET Core version where we need to move forward but don't want to hard code a .NET Core TFM into the build files.

This properties `$(NetCurrent)`, `$(NetPrevious)` and `$(NetMinimum)` are not used in our project files because they change in ways that make it hard for us to maintain corect product deployments. Our product ships on VS and VS Code which are not captured by arcade `$(Net...)` macros. Further as the arcade properties change it's very easy for us to end up with duplicate entries in a `<TargetFarmeworks>` setting. Instead our repo uses the above values and when inside source build or VMR our properties are initialized with arcade properties.

Expand Down Expand Up @@ -58,7 +59,7 @@ This problem primarily comes from our use of polyfill APIs. To avoid this we emp
This comes up in two forms:

### Pattern for types
### Pattern for types

When creating a polyfill for a type use the `#if !NET...` to declare the type and in the `#else` use a `TypeForwardedTo` for the actual type.

Expand Down Expand Up @@ -99,6 +100,13 @@ When creating a polyfill for an extension use the `#if NET...` to declare the ex
#endif
```

## Transitioning to new .NET SDK

As the .NET team approaches releasing a new .NET SDK the Roslyn team will begin using preview versions of that SDK in our build. This will often lead to test failures in our CI system due to underlying behavior changes in the runtime. These failures will often not show up when running in Visual Studio due to the way the runtime for the test runner is chosen.

To ensure we have a simple developer environment such project should be moved to to the `$(NetRoslynNext)` target framework. That ensures the new runtime is loaded when running tests locally.

When the .NET SDK RTMs and Roslyn adopts it all occurrences of `$(NetRoslynNext)` will be moved to simply `$(NetRoslyn)`.

**DO NOT** include both `$(NetRoslyn)` and `$(NetRoslynNext)` in the same project unless there is a very specific reason that both tests are adding value. The most common case is that the runtime has changed behavior and we simply need to update our baselines to match it. Adding extra TFMs for this just increases test time for very little gain.

6 changes: 4 additions & 2 deletions eng/SourceBuildPrebuiltBaseline.xml
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,15 @@
<UsagePattern IdentityGlob="Microsoft.CodeAnalysis.VisualBasic.CodeStyle/*" />
<UsagePattern IdentityGlob="Microsoft.Net.Compilers.Toolset/*" />

<!-- Roslyn's source-build CI builds both NetPrevious and NetCurrent. This 7.0 ref pack shows up as
<!-- Roslyn's source-build CI builds both NetPrevious and NetCurrent. This 7.0 / 8.0 ref pack shows up as
prebuilt only for the repo CI build but not full source-build. -->
<UsagePattern IdentityGlob="Microsoft.AspNetCore.App.Ref/7.0*" />
<UsagePattern IdentityGlob="Microsoft.AspNetCore.App.Ref/8.0*" />

<!-- Roslyn's source-build CI builds both NetPrevious and NetCurrent. This 7.0 ref pack shows up as
<!-- Roslyn's source-build CI builds both NetPrevious and NetCurrent. This 7.0 / 8.0 ref pack shows up as
prebuilt only for the repo CI build but not full source-build. -->
<UsagePattern IdentityGlob="Microsoft.NETCore.App.Ref/7.0*" />
<UsagePattern IdentityGlob="Microsoft.NETCore.App.Ref/8.0*" />

<!-- This is upgraded to latest version in full source-build and can be baselined for repo build -->
<UsagePattern IdentityGlob="Microsoft.Extensions.Configuration*/8.0*" />
Expand Down
3 changes: 3 additions & 0 deletions eng/targets/TargetFrameworks.props
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
<NetVSCode>net8.0</NetVSCode>
<NetVSShared>net7.0;net8.0</NetVSShared>
<NetRoslynBuildHostNetCoreVersion>net6.0</NetRoslynBuildHostNetCoreVersion>
<NetRoslynNext>net9.0</NetRoslynNext>
</PropertyGroup>

<!--
Expand All @@ -42,6 +43,7 @@
<NetRoslynSourceBuild>$(NetCurrent);$(NetPrevious)</NetRoslynSourceBuild>
<NetRoslynAll>$(NetCurrent);$(NetPrevious)</NetRoslynAll>
<NetRoslynBuildHostNetCoreVersion>$(NetPrevious)</NetRoslynBuildHostNetCoreVersion>
<NetRoslynNext>$(NetPrevious)</NetRoslynNext>
</PropertyGroup>
</When>

Expand All @@ -54,6 +56,7 @@
<NetRoslynSourceBuild>$(NetCurrent);$(NetPrevious)</NetRoslynSourceBuild>
<NetRoslynAll>$(NetCurrent);$(NetPrevious)</NetRoslynAll>
<NetRoslynBuildHostNetCoreVersion>$(NetCurrent)</NetRoslynBuildHostNetCoreVersion>
<NetRoslynNext>$(NetCurrent)</NetRoslynNext>
</PropertyGroup>
</When>

Expand Down
Loading

0 comments on commit 95b3302

Please sign in to comment.