Skip to content

Commit

Permalink
Move to 8.0 (#71507)
Browse files Browse the repository at this point in the history
* Move to 8.0

This updates our TFMS to recognize that .NET 8 has shipped and .NET 6 is
no longer supported.

At the same time I am trying to update our code base to be more driven
off of `$(NetCurrent)` and `$(NetPrevious)`. This is important for
source build and VMR work as these properties are often overriden to
drive the larger build efforts. Hard coded .NET Core based TFMs
interfere with that and cause friction for those teams.

* Apply suggestions from code review

Co-authored-by: Jason Malinowski <jason@jason-m.com>

* Clean up NETSDK1206 warnings

* NETSDK1206 warnings

* Include a check for duplicate TFMs

* fix

* more

* more

* more

* more

* more

* Fix BuildBoss checks

* More

* more

* more

* more

* more

---------

Co-authored-by: Jason Malinowski <jason@jason-m.com>
  • Loading branch information
jaredpar and jasonmalinowski committed Jan 12, 2024
1 parent c05a561 commit 5fad265
Show file tree
Hide file tree
Showing 122 changed files with 287 additions and 208 deletions.
1 change: 1 addition & 0 deletions Compilers.slnf
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
"src\\Scripting\\VisualBasicTest\\Microsoft.CodeAnalysis.VisualBasic.Scripting.UnitTests.vbproj",
"src\\Scripting\\VisualBasic\\Microsoft.CodeAnalysis.VisualBasic.Scripting.vbproj",
"src\\Test\\PdbUtilities\\Roslyn.Test.PdbUtilities.csproj",
"src\\Tools\\BuildBoss\\BuildBoss.csproj",
"src\\Tools\\BuildValidator\\BuildValidator.csproj",
"src\\Tools\\PrepareTests\\PrepareTests.csproj",
"src\\Tools\\Source\\CompilerGeneratorTools\\Source\\BoundTreeGenerator\\CompilersBoundTreeGenerator.csproj",
Expand Down
40 changes: 31 additions & 9 deletions docs/contributing/Target Framework Strategy.md
Original file line number Diff line number Diff line change
@@ -1,17 +1,37 @@
Target Framework Strategy
===
# Target Framework Strategy

## Layers

# Layers
The roslyn repository produces components for a number of different products that push varying ship and TFM constraints on us. A summary of some of our dependencies are :

- Build Tools: requires us to ship compilers on `net472`
- .NET SDK: requires us to ship compilers on current servicing target framework (presently `net6.0`)
- Source build: requires us to ship `$(NetCurrent)` and `$(NetPrevious)` in workspaces and below (presently `net8.0` and `net7.0` respectively)
- Visual Studio: requires us to ship `net472` for base IDE components and `net6.0` for private runtime components.
- .NET SDK: requires us to ship compilers on current servicing target framework (presently `net8.0`)
- Source build: requires us to ship `$(NetCurrent)` and `$(NetPrevious)` in workspaces and below (presently `net9.0` and `net8.0` respectively)
- Visual Studio: requires us to ship `net472` for base IDE components and `$(NetVisualStudio)` (presently `net8.0`) for private runtime components.
- Visual Studio Code: expects us to ship against the same runtime as DevKit (presently `net7.0`) to avoid two runtime downloads.

It is not reasonable for us to take the union of all TFM and multi-target every single project to them. That would add several hundred compilations to any build operation which would in turn negatively impact our developer throughput. Instead we attempt to use the TFM where needed. That keeps our builds smaller but increases complexity a bit as we end up shipping a mix of TFM for binaries across our layers.

# Require consistent API across Target Frameworks
## Picking the right TargetFramework

Projects in our repository should include the following values in `<TargetFramework(s)>` based on the rules below:

1. `$(NetRoslynSourceBuild)`: code that needs to be part of source build. This property will change based on whether the code is building in a source build context or official builds. In official builds this will include the TFMs for `$(NetVSShared)`
2. `$(NetVS)`: code that needs to execute on the private runtime of Visual Studio.
3. `$(NetVSCode)`: code that needs to execute in DevKit host
4. `$(NetVSShared)`: code that needs to execute in both Visual Studio and VS Code but does not need to be source built.
5. `$(NetRoslynToolset)`: packages that ship the Roslyn toolset. The compiler often builds against multiple target frameworks. This property controls which of those frameworks 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. `$(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 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.

**DO NOT** hard code .NET Core TFMs in project files. Instead use the properties above as that lets us centrally manage them and structure the properties to avoid duplication. It is fine to hard code other TFMs like `netstandard2.0` or `net472` as those are not expected to change.

**DO NOT** use `$(NetCurrent)` or `$(NetPrevious)` in project files. These should only be used inside of `TargetFrameworks.props` to initialize the above values in certain configurations.

## Require consistent API across Target Frameworks

It is important that our shipping APIs maintain consistent API surface area across target frameworks. That is true whether the API is `public` or `internal`.

The reason for `public` is standard design pattern. The reason for `internal` is a combination of the following problems:
Expand All @@ -35,7 +55,8 @@ 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.

Example:
Expand Down Expand Up @@ -63,7 +84,8 @@ namespace System.Runtime.CompilerServices
#endif
```

## Pattern for extension methods
### Pattern for extension methods

When creating a polyfill for an extension use the `#if NET...` to declare the extension method and the `#else` to declare the same method without `this`. That will put a method with the expected signature in the binary but avoids it appearing as an extension method within that target framework.

```csharp
Expand Down
4 changes: 2 additions & 2 deletions eng/build.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ function TestUsingRunTests() {
$env:ROSLYN_TEST_USEDASSEMBLIES = "true"
}

$runTests = GetProjectOutputBinary "RunTests.dll" -tfm "net7.0"
$runTests = GetProjectOutputBinary "RunTests.dll" -tfm "net8.0"

if (!(Test-Path $runTests)) {
Write-Host "Test runner not found: '$runTests'. Run Build.cmd first." -ForegroundColor Red
Expand Down Expand Up @@ -538,7 +538,7 @@ function EnablePreviewSdks() {
# deploying at build time.
function Deploy-VsixViaTool() {

$vsixExe = Join-Path $ArtifactsDir "bin\RunTests\$configuration\net7.0\VSIXExpInstaller\VSIXExpInstaller.exe"
$vsixExe = Join-Path $ArtifactsDir "bin\RunTests\$configuration\net8.0\VSIXExpInstaller\VSIXExpInstaller.exe"
Write-Host "VSIX EXE path: " $vsixExe
if (-not (Test-Path $vsixExe)) {
Write-Host "VSIX EXE not found: '$vsixExe'." -ForegroundColor Red
Expand Down
2 changes: 1 addition & 1 deletion eng/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,6 @@ if [[ "$test_core_clr" == true ]]; then
if [[ "$ci" != true ]]; then
runtests_args="$runtests_args --html"
fi
dotnet exec "$scriptroot/../artifacts/bin/RunTests/${configuration}/net7.0/RunTests.dll" --runtime core --configuration ${configuration} --logs ${log_dir} --dotnet ${_InitializeDotNetCli}/dotnet $runtests_args
dotnet exec "$scriptroot/../artifacts/bin/RunTests/${configuration}/net8.0/RunTests.dll" --runtime core --configuration ${configuration} --logs ${log_dir} --dotnet ${_InitializeDotNetCli}/dotnet $runtests_args
fi
ExitWithExitCode 0
2 changes: 1 addition & 1 deletion eng/generate-compiler-code.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ function Run-IOperation($coreDir, $ioperationProject) {
$operationsDir = Join-Path $coreDir "Operations"
$operationsXml = Join-Path $operationsDir "OperationInterfaces.xml"
$generationDir = Join-Path $coreDir "Generated"
$targetFramework = "net7.0"
$targetFramework = "net8.0"

if (-not $test) {
Run-Tool $ioperationProject "`"$operationsXml`" `"$generationDir`"" $targetFramework
Expand Down
6 changes: 3 additions & 3 deletions eng/pipelines/test-integration-helix.yml
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ stages:
- task: BatchScript@1
displayName: Rehydrate RunTests
inputs:
filename: ./artifacts/bin/RunTests/${{ parameters.configuration }}/net7.0/rehydrate.cmd
filename: ./artifacts/bin/RunTests/${{ parameters.configuration }}/net8.0/rehydrate.cmd
env:
HELIX_CORRELATION_PAYLOAD: '$(Build.SourcesDirectory)\.duplicate'

Expand All @@ -69,9 +69,9 @@ stages:

# This is a temporary step until the actual test run moves to helix (then we would need to rehydrate the tests there instead)
- task: BatchScript@1
displayName: Rehydrate Microsoft.CodeAnalysis.Workspaces.MSBuild.UnitTests (net6.0-windows)
displayName: Rehydrate Microsoft.CodeAnalysis.Workspaces.MSBuild.UnitTests (net8.0-windows)
inputs:
filename: ./artifacts\bin\Microsoft.CodeAnalysis.Workspaces.MSBuild.UnitTests/${{ parameters.configuration }}/net6.0-windows/rehydrate.cmd
filename: ./artifacts\bin\Microsoft.CodeAnalysis.Workspaces.MSBuild.UnitTests/${{ parameters.configuration }}/net8.0-windows/rehydrate.cmd
env:
HELIX_CORRELATION_PAYLOAD: '$(Build.SourcesDirectory)\.duplicate'

Expand Down
2 changes: 1 addition & 1 deletion eng/pipelines/test-unix-job.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ jobs:
- task: ShellScript@2
displayName: Rehydrate RunTests
inputs:
scriptPath: ./artifacts/bin/RunTests/${{ parameters.configuration }}/net7.0/rehydrate.sh
scriptPath: ./artifacts/bin/RunTests/${{ parameters.configuration }}/net8.0/rehydrate.sh
env:
HELIX_CORRELATION_PAYLOAD: '$(Build.SourcesDirectory)/.duplicate'

Expand Down
2 changes: 1 addition & 1 deletion eng/pipelines/test-windows-job.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ jobs:
- task: BatchScript@1
displayName: Rehydrate RunTests
inputs:
filename: ./artifacts/bin/RunTests/${{ parameters.configuration }}/net7.0/rehydrate.cmd
filename: ./artifacts/bin/RunTests/${{ parameters.configuration }}/net8.0/rehydrate.cmd
env:
HELIX_CORRELATION_PAYLOAD: '$(Build.SourcesDirectory)\.duplicate'

Expand Down
2 changes: 1 addition & 1 deletion eng/prepare-tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ try {
$dotnet = Ensure-DotnetSdk
# permissions issues make this a pain to do in PrepareTests itself.
Remove-Item -Recurse -Force "$RepoRoot\artifacts\testPayload" -ErrorAction SilentlyContinue
Exec-Console $dotnet "$RepoRoot\artifacts\bin\PrepareTests\$configuration\net7.0\PrepareTests.dll --source $RepoRoot --destination $RepoRoot\artifacts\testPayload --dotnetPath `"$dotnet`""
Exec-Console $dotnet "$RepoRoot\artifacts\bin\PrepareTests\$configuration\net8.0\PrepareTests.dll --source $RepoRoot --destination $RepoRoot\artifacts\testPayload --dotnetPath `"$dotnet`""
exit 0
}
catch {
Expand Down
2 changes: 1 addition & 1 deletion eng/prepare-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,4 @@ InitializeDotNetCli true
# permissions issues make this a pain to do in PrepareTests itself.
rm -rf "$repo_root/artifacts/testPayload"

dotnet "$repo_root/artifacts/bin/PrepareTests/Debug/net7.0/PrepareTests.dll" --source "$repo_root" --destination "$repo_root/artifacts/testPayload" --unix --dotnetPath ${_InitializeDotNetCli}/dotnet
dotnet "$repo_root/artifacts/bin/PrepareTests/Debug/net8.0/PrepareTests.dll" --source "$repo_root" --destination "$repo_root/artifacts/testPayload" --unix --dotnetPath ${_InitializeDotNetCli}/dotnet
6 changes: 3 additions & 3 deletions eng/targets/Imports.BeforeArcade.targets
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,14 @@
<!-- use the source-built version of the reference packs if building in source-build -->
<ItemGroup Condition="'$(DotNetBuildFromSource)' == 'true'">
<KnownFrameworkReference Update="Microsoft.NETCore.App">
<TargetingPackVersion Condition="'%(TargetFramework)' == 'net6.0'">6.0.0</TargetingPackVersion>
<TargetingPackVersion Condition="'%(TargetFramework)' == 'net8.0'">8.0.0</TargetingPackVersion>
</KnownFrameworkReference>
<KnownFrameworkReference Update="Microsoft.AspNetCore.App">
<TargetingPackVersion Condition="'%(TargetFramework)' == 'net6.0'">6.0.0</TargetingPackVersion>
<TargetingPackVersion Condition="'%(TargetFramework)' == 'net8.0'">8.0.0</TargetingPackVersion>
</KnownFrameworkReference>
</ItemGroup>

<!-- do not restore or use the 6.0 app host in source-build -->
<!-- do not restore or use the 8.0 app host in source-build -->
<PropertyGroup Condition="'$(DotNetBuildFromSource)' == 'true'">
<UseAppHost>false</UseAppHost>
</PropertyGroup>
Expand Down
4 changes: 4 additions & 0 deletions eng/targets/Imports.targets
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@
-->
<RoslynCheckCodeStyle Condition="'$(RoslynCheckCodeStyle)' == '' AND '$(DisableNullableWarnings)' == 'true'">false</RoslynCheckCodeStyle>
<RoslynCheckCodeStyle Condition="'$(RoslynCheckCodeStyle)' == '' AND ('$(ContinuousIntegrationBuild)' != 'true' OR '$(RoslynEnforceCodeStyle)' == 'true')">true</RoslynCheckCodeStyle>

<!--https://github.com/dotnet/sdk/issues/37826 -->
<MSBuildWarningsAsMessages Condition="'$(OutputType)' == 'Library'">$(MSBuildWarningsAsMessages);NETSDK1206</MSBuildWarningsAsMessages>

</PropertyGroup>

<!--
Expand Down
50 changes: 3 additions & 47 deletions eng/targets/Settings.props
Original file line number Diff line number Diff line change
Expand Up @@ -51,54 +51,10 @@
<PublishWindowsPdb>false</PublishWindowsPdb>
<EnforceExtendedAnalyzerRules>false</EnforceExtendedAnalyzerRules>
<EnforceExtendedAnalyzerRules Condition="'$(IsAnalyzer)' == 'true'">true</EnforceExtendedAnalyzerRules>
</PropertyGroup>

<!--
There are effectively three modes that are needed for our source build TFMs and this is where
we calculate them
-->
<Choose>
<!--
1. CI source build leg: this needs to build the current and previous source build TFM. Both are
necessary as the output of this leg is used in other CI source build legs. Those could be
targeting NetCurrent or NetPrevious hence we must produce both.
However the toolset package we produce must target NetPrevious. This package gets used as the
bootstrap toolset in other repos doing (1). Those can be using a NetPrevious runtime hence
the toolset must support that.
-->
<When Condition="'$(DotNetBuildFromSource)' == 'true' AND '$(DotNetBuildFromSourceFlavor)' != 'Product'">
<PropertyGroup>
<SourceBuildToolsetTargetFramework>$(NetPrevious)</SourceBuildToolsetTargetFramework>
<SourceBuildToolsetTargetFrameworks>$(SourceBuildToolsetTargetFramework)</SourceBuildToolsetTargetFrameworks>
<SourceBuildTargetFrameworks>$(NetCurrent);$(NetPrevious)</SourceBuildTargetFrameworks>
</PropertyGroup>
</When>

<!--
2. Source build the product: this is the all up build of the product which needs only NetCurrent
-->
<When Condition="'$(DotNetBuildFromSource)' == 'true' AND '$(DotNetBuildFromSourceFlavor)' == 'Product'">
<PropertyGroup>
<SourceBuildToolsetTargetFramework>$(NetCurrent)</SourceBuildToolsetTargetFramework>
<SourceBuildToolsetTargetFrameworks>$(SourceBuildToolsetTargetFramework)</SourceBuildToolsetTargetFrameworks>
<SourceBuildTargetFrameworks>$(NetCurrent)</SourceBuildTargetFrameworks>
</PropertyGroup>
</When>
</PropertyGroup>

<!--
3. Everything else including normal CI, developer machines and official builds. This brings in enough
TFM that source build will go smoothly but doesn't bring in all source build TFMs to avoid adding
too many extra compiles to our builds
-->
<Otherwise>
<PropertyGroup>
<SourceBuildToolsetTargetFramework>net6.0</SourceBuildToolsetTargetFramework>
<SourceBuildToolsetTargetFrameworks>$(SourceBuildToolsetTargetFramework);net7.0</SourceBuildToolsetTargetFrameworks>
<SourceBuildTargetFrameworks>$(SourceBuildToolsetTargetFrameworks)</SourceBuildTargetFrameworks>
</PropertyGroup>
</Otherwise>
</Choose>
<Import Project="$(MSBuildThisFileDirectory)TargetFrameworks.props" />

<!--
Disable Source Link and Xliff in WPF temp projects to avoid generating non-deterministic file names to obj dir.
Expand All @@ -124,7 +80,7 @@
These may be overridden by projects that need to be skipped.
-->
<TestTargetFrameworks Condition="'$(TestRuntime)' == 'Mono'">net46;net472</TestTargetFrameworks>
<TestTargetFrameworks Condition="'$(TestRuntime)' == 'Core'">net6.0;net7.0</TestTargetFrameworks>
<TestTargetFrameworks Condition="'$(TestRuntime)' == 'Core'">$(NetRoslyn)</TestTargetFrameworks>

<XUnitDesktopSettingsFile>$(MSBuildThisFileDirectory)..\config\xunit.runner.json</XUnitDesktopSettingsFile>
<XUnitCoreSettingsFile>$(MSBuildThisFileDirectory)..\config\xunit.runner.json</XUnitCoreSettingsFile>
Expand Down
67 changes: 67 additions & 0 deletions eng/targets/TargetFrameworks.props
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
<?xml version="1.0" encoding="utf-8"?>
<Project>

<!--
This props file manages the target framework properties for Roslyn. The strategy for these properties
is convered in "docs/contributing/Target Framework Strategy.md". Please see that for documentation
on what these values mean.
Requirements:
- NetVSShared must include both NetVS and NetVSCode
- NetRoslynSourceBuild must include NetRoslynToolset
- NetRoslynAll must include all .NET Core TFMS in any property below
-->
<PropertyGroup>
<NetRoslyn>net8.0</NetRoslyn>
<NetRoslynAll>net7.0;net8.0</NetRoslynAll>
<NetVS>net8.0</NetVS>
<NetVSCode>net7.0</NetVSCode>
<NetVSShared>net7.0;net8.0</NetVSShared>
</PropertyGroup>

<!--
There are effectively three modes that are needed for our source build TFMs and this is where
we calculate them
-->
<Choose>
<!--
1. CI source build leg: this needs to build the current and previous source build TFM. Both are
necessary as the output of this leg is used in other CI source build legs. Those could be
targeting NetCurrent or NetPrevious hence we must produce both.
However the toolset package we produce must target NetPrevious. This package gets used as the
bootstrap toolset in other repos doing (1). Those can be using a NetPrevious runtime hence
the toolset must support that.
-->
<When Condition="'$(DotNetBuildFromSource)' == 'true' AND '$(DotNetBuildFromSourceFlavor)' != 'Product'">
<PropertyGroup>
<NetRoslynToolset>$(NetPrevious)</NetRoslynToolset>
<NetRoslynSourceBuild>$(NetCurrent);$(NetPrevious)</NetRoslynSourceBuild>
<NetRoslynAll>$(NetCurrent);$(NetPrevious)</NetRoslynAll>
</PropertyGroup>
</When>

<!--
2. Source build the product: this is the all up build of the product which needs only NetCurrent
-->
<When Condition="'$(DotNetBuildFromSource)' == 'true' AND '$(DotNetBuildFromSourceFlavor)' == 'Product'">
<PropertyGroup>
<NetRoslynToolset>$(NetCurrent)</NetRoslynToolset>
<NetRoslynSourceBuild>$(NetCurrent);$(NetPrevious)</NetRoslynSourceBuild>
<NetRoslynAll>$(NetCurrent);$(NetPrevious)</NetRoslynAll>
</PropertyGroup>
</When>

<!--
3. Everything else including normal CI, developer machines and official builds. This brings in enough
TFM that source build will go smoothly but doesn't bring in all source build TFMs to avoid adding
too many extra compiles to our builds
-->
<Otherwise>
<PropertyGroup>
<NetRoslynToolset>net8.0</NetRoslynToolset>
<NetRoslynSourceBuild>net7.0;net8.0</NetRoslynSourceBuild>
</PropertyGroup>
</Otherwise>
</Choose>
</Project>
2 changes: 1 addition & 1 deletion eng/test-rebuild.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ try {
# Rebuilds with missing references
# Rebuilds with other issues
" --exclude net472\Microsoft.CodeAnalysis.EditorFeatures2.UnitTests.dll" +
" --exclude net6.0\Microsoft.CodeAnalysis.Collections.Package.dll" +
" --exclude net8.0\Microsoft.CodeAnalysis.Collections.Package.dll" +
" --exclude netcoreapp3.1\Microsoft.CodeAnalysis.Collections.Package.dll" +
" --exclude netstandard2.0\Microsoft.CodeAnalysis.Collections.Package.dll" +
" --exclude netstandard2.0\Microsoft.CodeAnalysis.Debugging.Package.dll" +
Expand Down
2 changes: 1 addition & 1 deletion global.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"sdk": {
"version": "8.0.100",
"allowPrerelease": false,
"rollForward": "disable"
"rollForward": "patch"
},
"tools": {
"dotnet": "8.0.100",
Expand Down
Loading

0 comments on commit 5fad265

Please sign in to comment.