-
Notifications
You must be signed in to change notification settings - Fork 347
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[main] Upgrade netcoreapp3.1 TFMs to net7.0 #9127
Changes from all commits
510f582
9d4bd2b
bc91aae
b8236d1
49e11af
1db4d43
0d9dcdc
b65afc0
f6af788
b88968b
f87cc56
d46523a
ef51e04
d262b23
08c7848
2e1ec30
3350777
3448629
9627a60
5200224
a881adf
a39a29f
9073eb7
2a9d198
d0090ab
b5794ff
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,7 @@ | ||
<Project> | ||
|
||
<!-- | ||
By default, build for an old TFM for wide applicability. When running source-build, target the | ||
current .NET SDK framework version to avoid dependency challenges. | ||
--> | ||
<PropertyGroup> | ||
<TargetFrameworkForNETSDK>netcoreapp3.1</TargetFrameworkForNETSDK> | ||
<TargetFrameworkForNETSDK Condition="'$(DotNetBuildFromSource)' == 'true'">net6.0</TargetFrameworkForNETSDK> | ||
<TargetFrameworkForNETSDK>net7.0</TargetFrameworkForNETSDK> | ||
</PropertyGroup> | ||
|
||
</Project> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,14 +2,14 @@ | |
<Project Sdk="Microsoft.NET.Sdk"> | ||
|
||
<PropertyGroup> | ||
<TargetFrameworks>netcoreapp3.1;net472</TargetFrameworks> | ||
<TargetFrameworks>$(TargetFrameworkForNETSDK);net472</TargetFrameworks> | ||
<SignAssembly>true</SignAssembly> | ||
|
||
<Description>This package provides support for publishing assets to a NuGet protocol based feed.</Description> | ||
<DevelopmentDependency>true</DevelopmentDependency> | ||
<PackageType>MSBuildSdk</PackageType> | ||
<ExcludeFromSourceBuild>true</ExcludeFromSourceBuild> | ||
<_ExcludeNuGetAssembliesTargetFramework>netcoreapp3.1</_ExcludeNuGetAssembliesTargetFramework> | ||
<_ExcludeNuGetAssembliesTargetFramework>$(TargetFrameworkForNETSDK)</_ExcludeNuGetAssembliesTargetFramework> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that this property and the target at the bottom of the project file aren't required anymore based on the underlying msbuild issue already being fixed. That said, this might be a good follow-up investigation, unrelated to this PR. |
||
</PropertyGroup> | ||
|
||
<ItemGroup> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
<Project ToolsVersion="14.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003"> | ||
<PropertyGroup> | ||
<MicrosoftDotNetBuildTasksInstallersTaskAssembly Condition="'$(MSBuildRuntimeType)' == 'Core'">$(MSBuildThisFileDirectory)..\tools\netcoreapp3.1\Microsoft.DotNet.Build.Tasks.Installers.dll</MicrosoftDotNetBuildTasksInstallersTaskAssembly> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried this change even with the preview 5 .NET 7 SDK and it still cant load 7.0.0 of System.Runtime for this task. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @AraHaan can you please post or message to me privately a link to the failure you're describing? |
||
<MicrosoftDotNetBuildTasksInstallersTaskAssembly Condition="'$(MSBuildRuntimeType)' == 'Core'">$(MSBuildThisFileDirectory)..\tools\net7.0\Microsoft.DotNet.Build.Tasks.Installers.dll</MicrosoftDotNetBuildTasksInstallersTaskAssembly> | ||
<MicrosoftDotNetBuildTasksInstallersTaskAssembly Condition="'$(MSBuildRuntimeType)' != 'Core'">$(MSBuildThisFileDirectory)..\tools\net472\Microsoft.DotNet.Build.Tasks.Installers.dll</MicrosoftDotNetBuildTasksInstallersTaskAssembly> | ||
<MicrosoftDotNetBuildTasksInstallersMSBuildDir Condition="'$(MicrosoftDotNetBuildTasksInstallersMSBuildDir)' == ''">$(MSBuildThisFileDirectory)</MicrosoftDotNetBuildTasksInstallersMSBuildDir> | ||
</PropertyGroup> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,7 @@ | ||
<!-- Licensed to the .NET Foundation under one or more agreements. The .NET Foundation licenses this file to you under the MIT license. --> | ||
<Project> | ||
<PropertyGroup> | ||
<DotNetBuildTasksTargetFrameworkAssembly Condition="'$(MSBuildRuntimeType)' == 'core'">..\tools\netcoreapp3.1\Microsoft.DotNet.Build.Tasks.TargetFramework.dll</DotNetBuildTasksTargetFrameworkAssembly> | ||
<DotNetBuildTasksTargetFrameworkAssembly Condition="'$(MSBuildRuntimeType)' == 'core'">..\tools\net7.0\Microsoft.DotNet.Build.Tasks.TargetFramework.dll</DotNetBuildTasksTargetFrameworkAssembly> | ||
<DotNetBuildTasksTargetFrameworkAssembly Condition="'$(MSBuildRuntimeType)' != 'core'">..\tools\net472\Microsoft.DotNet.Build.Tasks.TargetFramework.dll</DotNetBuildTasksTargetFrameworkAssembly> | ||
</PropertyGroup> | ||
</Project> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unsure who owns nugetrepack. @mmitche this will probably not work until microsoft.dotnet.nugetrepack.tasks is updated after this PR is merged and the newer package is available.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also it's weird that something outside the package hardcodes the path to the assemblies inside the package. Ideally the microsoft.dotnet.nugetrepack.tasks package would contain an msbuild file that already sets this property (as we do in all our other task projects in arcade).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ViktorHofer Not sure who owns nuget repack either. My guess with the properties is just that this is an older-style and an oversight,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested this change in fsharp, which I know uses NuGetRepack, and it does work. See here for my build of fsharp (internal MS link): https://dev.azure.com/dnceng/internal/_build/results?buildId=1829578&view=results. The windows failure is an unrelated, known issue: https://github.com/microsoft/dropvalidator/issues/455
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes this should work. My comment was referring to that ideally this code would reside in the package itself and not in the consumer's path. We should look at this in a follow-up.