[release/7.0] Target only net7.0 during source-build #75168
Conversation
|
Tagging subscribers to this area: @dotnet/area-infrastructure-libraries Issue DetailsSee dotnet/source-build#2966 for more context. This is needed for net7.0 RC2 in order to ship source-build without prebuilts. We have a working branch with these changes as a patch here, showing that the changes are at least functional and pass our tests: dotnet/installer#14167
|
src/libraries/System.Text.RegularExpressions/tools/GenerateRegexCasingTable.csproj
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
The compatibility suppression file changes result in package validation failing during a non source build as the suppressions are missing for net6.0. Instead consider disabling package validation during source build by adding the following switch to SourceBuild.props: /p:EnablePackageValidation=false.
ghost
added
the
needs-author-action
ghost
removed
the
needs-author-action
src/libraries/System.Diagnostics.DiagnosticSource/src/CompatibilitySuppressions.xml
Outdated
Show resolved
Hide resolved
| @@ -7,7 +7,7 @@ | |||
| <EnableTrimAnalyzer Condition="$([MSBuild]::GetTargetFrameworkIdentifier('$(TargetFramework)')) == '.NETFramework'">false</EnableTrimAnalyzer> | |||
| <IsPackable>true</IsPackable> | |||
| <!-- Lifetime rules introduced in C# 11 impact scenarios in net6 framework --> | |||
| <LangVersion Condition="'$(TargetFramework)' == 'net6.0'">10</LangVersion> | |||
| <LangVersion Condition="'$(TargetFramework)' == 'net6.0' or '$(TargetFramework)' == 'net7.0'">10</LangVersion> | |||
There was a problem hiding this comment.
This change doesn't look right. We were already targeting net7.0 before. Why wasn't that needed? cc @tarekgh
There was a problem hiding this comment.
CC @AaronRobinsonMSFT as he is the one introduced this.
There was a problem hiding this comment.
This was because of the ref field support and the project using the .NET 7 Preview ref assemblies instead of the live build assemblies. Once the SDK for the repo was updated, this can likely be removed and the code fixed up a bit.
/cc @jakobbotsch
There was a problem hiding this comment.
Based on Aaron's reply, this setting shouldn't be needed anymore:
| <LangVersion Condition="'$(TargetFramework)' == 'net6.0' or '$(TargetFramework)' == 'net7.0'">10</LangVersion> |
There was a problem hiding this comment.
This change should also be forward ported into main.
| @@ -23,7 +23,7 @@ | |||
|
|
|||
| <PropertyGroup> | |||
| <_NetCoreAppCurrent>net7.0</_NetCoreAppCurrent> | |||
| <_NetCoreAppToolCurrent>net6.0</_NetCoreAppToolCurrent> | |||
| <_NetCoreAppToolCurrent>net7.0</_NetCoreAppToolCurrent> | |||
There was a problem hiding this comment.
@akoeplinger can you please help me understand why this file duplicates these common properties?
There was a problem hiding this comment.
@ViktorHofer the intention of this file is to be used outside of the dotnet/runtime repo where we don't have access to the common properties (see the description above), i.e. you'd include this .props file in a test .csproj and set RuntimeSrcDir/RuntimeConfig and use your locally built runtime.
We could probably generate this file so we could inject the values for _NetCoreAppCurrent/_NetCoreAppToolCurrent
There was a problem hiding this comment.
Generating this file would be helpful to avoid multiple sources of truth for the tfm and the version properties.
There was a problem hiding this comment.
Can you please file an issue?
carlossanlop
changed the title
|
@ViktorHofer any chance you can take a look again? There are new commits. |
| @@ -54,6 +54,7 @@ | |||
| <NetCoreAppCurrentBrandName>.NET $(NetCoreAppCurrentVersion)</NetCoreAppCurrentBrandName> | |||
| <NetCoreAppCurrent>net$(NetCoreAppCurrentVersion)</NetCoreAppCurrent> | |||
| <NetCoreAppMinimum>net6.0</NetCoreAppMinimum> | |||
| <NetCoreAppMinimum Condition="'$(DotNetBuildFromSource)' == 'true'">$(NetCoreAppCurrent)</NetCoreAppMinimum> | |||
There was a problem hiding this comment.
This is the only change here that worries me a bit but if it works then LGTM.
|
Thanks for signing off, @ViktorHofer. |
ghost
locked as resolved and limited conversation to collaborators
See dotnet/source-build#2966 for more context.
This is needed for net7.0 RC2 in order to ship source-build without prebuilts. We have a working branch with these changes as a patch here, showing that the changes are at least functional and pass our tests: dotnet/installer#14167