Update to net8.0 SDK #45879

wtgodbe · 2023-01-04T20:27:31Z

No description provided.

wtgodbe · 2023-01-04T20:49:38Z

Blocked on getting an SDK from dotnet/installer#15151

mitchdenny

Change looks good (once you've got the installer available).

dougbu · 2023-01-05T01:38:00Z

eng/tools/RepoTasks/RepoTasks.csproj

@@ -1,6 +1,6 @@
 <Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
-    <TargetFrameworks>net7.0</TargetFrameworks>
+    <TargetFrameworks>$(DefaultNetCoreTargetFramework)</TargetFrameworks>


Changes here cannot work until we have an 8.0.0 SDK containing net8.0 bits. The basic idea is RepoTasks and GenerateFiles aren't part of the product. They're instead used in msbuild (or dotnet msbuild) and to generate the extra Directory.Build.props, Directory.Build.targets, and dotnet-tools.json files used in almost every other project in the repo.

Why did this file and RepoTasks.tasks need to change in the first place❔ Note GenerateFiles still targets net5.0 and (other than the deprecated TFM warning we're suppressing somewhere) that's fine.

See for example

System.IO.FileNotFoundException: Could not load file or assembly 'System.Runtime, Version=8.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a'. The system cannot find the file specified.

Yep, that's why this is blocked on dotnet/installer#15151 as I mentioned above. It needs to change because SourceBuild breaks if it doesn't target the latest TFM (though I don't know why the same isn't true for GenerateFiles) - dotnet/installer#15151 (comment)

though I don't know why the same isn't true for GenerateFiles

@dougbu @wtgodbe, GenerateFiles.csproj doesn't need to use the latest TFM because it doesn't build during source-build:

aspnetcore/eng/tools/GenerateFiles/GenerateFiles.csproj

Line 5 in cefc6cc

<ExcludeFromSourceBuild>false</ExcludeFromSourceBuild>

wtgodbe · 2023-01-05T03:55:38Z

Here's a new one, will look tomorrow:

/Users/runner/work/1/s/.dotnet/sdk/8.0.100-alpha.1.23054.7/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.Sdk.FrameworkReferenceResolution.targets(90,5): error NETSDK1195: Unable to optimize assemblies for size: a valid runtime package was not found. Either set the PublishTrimmed property to false, or use a supported target framework when publishing. [/Users/runner/work/1/s/src/Servers/Connections.Abstractions/src/Microsoft.AspNetCore.Connections.Abstractions.csproj::TargetFramework=netstandard2.1]

wtgodbe · 2023-01-05T04:51:52Z

@JamesNK can we just disable trimming for netstandard2.1 targets?

JamesNK · 2023-01-05T04:57:04Z

What do you mean by disable trimming? Our projects are annotated to help trimming succeed, but that's it.

Trimming happens when publishing an app. Who is publishing?

mitchdenny · 2023-01-05T06:04:45Z

Who is publishing?

I think our Helix tests publish. Not sure if they would be impacted by this.

wtgodbe · 2023-01-05T16:13:44Z

Trimming happens when publishing an app. Who is publishing?

This error happens very early on, during framework reference resolution. Presumably the SDK is doing some scanning that sees we have PublishTrimmed=true for projects with a netstandard2.1 configuration, and errors out. I'm proposing setting that to false whenever TargetFramework=netstandard2.1

wtgodbe · 2023-01-05T17:28:20Z

@sbomer looks like this was caused by dotnet/sdk#29441 - is it intentional that we get these errors for netstandard2.1 projects that enable trimming?

#45879 (comment)

sbomer · 2023-01-05T18:03:11Z

PublishTrimmed is incorrect for a netstandard2.1 library both because we don't support publishing self-contained trimmed libraries, and because trimming wasn't available until netcoreapp3.0.

@wtgodbe Is it definitely the case that PublishTrimmed is set, or could these projects have IsTrimmable or EnableTrimAnalyzer set instead? netstandard2.1 didn't have annotations for trimming, but the 7.0 SDK didn't block enabling the trim analyzer, and it was possible to get warnings by for example compiling a copy of RequiresUnreferencedCodeAttribute into user code.

@vitek-karas is this considered a supported scenario? If so, we might need to add a KnownILLinkPack for netstandard2.1, or factor out the build-only targets from the ILLink pack, to make this work with the approach in dotnet/sdk#29441.

wtgodbe · 2023-01-05T18:13:23Z

@wtgodbe Is it definitely the case that PublishTrimmed is set, or could these projects have IsTrimmable or EnableTrimAnalyzer set instead? netstandard2.1 didn't have annotations for trimming, but the 7.0 SDK didn't block enabling the trim analyzer, and it was possible to get warnings by for example compiling a copy of RequiresUnreferencedCodeAttribute into user code.

They set IsTrimmable. Maybe these projects could conditionally set PublishTrimmed=false when TargetFramework=netstandard2.1

aspnetcore/src/Servers/Connections.Abstractions/src/Microsoft.AspNetCore.Connections.Abstractions.csproj

Line 10 in 5b43ed1

sbomer · 2023-01-05T19:00:51Z

Setting IsTrimmable to false would prevent the library from being trimmed when consumed in apps, so it may not be what you want. I think our team needs to discuss the intended UX around this. For now, I can suggest a workaround:

  <Target Name="_FixKnownILLinkPack"
          BeforeTargets="ProcessFrameworkReferences">
    <ItemGroup>
      <KnownILLinkPack Include="@(KnownILLinkPack)"
                       Condition="'%(TargetFramework)' == 'net7.0'"
                       TargetFramework="netstandard2.1"
                       ILLinkPackVersion="%(KnownILLinkPack.ILLinkPackVersion)" />
    </ItemGroup>
  </Target>

This should keep it working as before (by adding a netstandard2.1 KnownILLinkPack matching the 7.0 version).

wtgodbe · 2023-01-05T19:57:37Z

Setting IsTrimmable to false would prevent the library from being trimmed when consumed in apps, so it may not be what you want. I think our team needs to discuss the intended UX around this. For now, I can suggest a workaround:

I'm suggesting keeping IsTrimmable=true everywhere, but also adding PublishTrimmed=false when TFM=netstandard2.1. Would that not work? If not I'll try your suggested workaround

sbomer · 2023-01-05T21:46:27Z

No, I think it will hit the same error because some of the logic for IsTrimmable is imported from the KnownILLinkPack.

…105.3 Microsoft.DotNet.Arcade.Sdk , Microsoft.DotNet.Build.Tasks.Installers , Microsoft.DotNet.Build.Tasks.Templating , Microsoft.DotNet.Helix.Sdk From Version 8.0.0-beta.22630.1 -> To Version 8.0.0-beta.23055.3

brunolins16 · 2023-01-06T19:36:34Z

global.json

  },
  "tools": {
-    "dotnet": "8.0.100-alpha.1.22531.1",


Because of the new SDK, I believe we will need to update the CI pipeline to use the binlogtosln that supports v15 (dotnet/arcade#11995), if it is not updated yet.

Just merged that PR into this one

wtgodbe · 2023-01-06T22:13:25Z

Remaining errors are all:

##[error]src/Tools/LinkabilityChecker/LinkabilityChecker.csproj(47,5): error MSB4036: (NETCORE_ENGINEERING_TELEMETRY=Build) The "ILLink" task was not found. Check the following: 1.) The name of the task in the project file is the same as the name of the task class. 2.) The task class is "public" and implements the Microsoft.Build.Framework.ITask interface. 3.) The task is correctly declared with in the project file, or in the *.tasks files located in the "/Users/runner/work/1/s/.dotnet/sdk/8.0.100-alpha.1.23055.1" directory.

@sbomer are you aware of any other recent linker changes that might have caused this to start happening?

sbomer · 2023-01-07T01:34:28Z

The same change dotnet/sdk#29441 removes ILLink as a bundled SDK - it now needs to be downloaded based on the TFM - but it looks like aspnetcore was relying on it being present in the SDK layout. I think someone from aspnetcore needs to look into this. Probably https://github.com/dotnet/aspnetcore/blob/main/src/Tools/LinkabilityChecker/LinkabilityChecker.csproj needs to set some properties to tell the SDK to download ILLink - I think it would suffice to add PublishTrimmed there (even if the project itself doesn't need to be published).

dougbu · 2023-01-07T01:51:55Z

Removing ILLink sounds like a regression, especially if the SDK doesn't treat references to the SDK in the same way it does missing targeting packs. What's the mid-term story here❔

JamesNK · 2023-01-08T03:25:22Z

@dougbu It sounds like ILLink isn't in the SDK by default, but it is automatically downloaded when <PublishTrimmed>true</PublishTrimmed> is set on a project. What we're doing here, calling the ILLink task directly, is something people would rarely do and is an acceptable tradeoff for a smaller SDK size (I'm guessing that's why).

All we need to do is add <PublishTrimmed>true</PublishTrimmed> to WasmLinkerTest.csproj and LinkabilityChecker.csproj.

@sbomer Correct me if I'm wrong here.

ViktorHofer

LGTM. If you want to avoid this for .NET 9 and onwards, consider setting DisableImplicitFrameworkReferences to true in RepoTasks.csproj as presumably that project doesn't need the framework references and the underlying targeting pack.

wtgodbe · 2023-01-09T18:15:24Z

f you want to avoid this for .NET 9 and onwards, consider setting DisableImplicitFrameworkReferences to true in RepoTasks.csproj as presumably that project doesn't need the framework references and the underlying targeting pack.

Thanks, testing that out pre-SDK update here: #45971

wtgodbe · 2023-01-09T18:23:09Z

Don't think that'll work since our RepoTasks do use stuff from runtime. We can probably just apply the FrameworkReference workaround we give to non-Tools projects to RepoTasks to fix this going forward.

ViktorHofer · 2023-01-09T18:34:17Z

I don't think we need to fix this right now and would instead love to see this go in. Should we file an issue to track the follow-up work?

wtgodbe · 2023-01-09T18:43:23Z

#45973

sbomer · 2023-01-09T18:52:10Z

ILLink isn't in the SDK by default, but it is automatically downloaded when true is set on a project. [...] is an acceptable tradeoff for a smaller SDK size (I'm guessing that's why).

Just some additional context - the main reason we made the change is for versioning. We need to download a linker that matches the TFM, and it would bloat the SDK if we bundled a version of the linker for each supported TFM.

ghost · 2023-01-09T18:52:14Z

Hi @sbomer. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.

dougbu · 2023-01-09T23:10:19Z

@wtgodbe this change broke official builds. See https://dev.azure.com/dnceng/internal/_build/results?buildId=2083576&view=results for example. Please immediately either revert this change or update the binlogtosln tool version we use.

wtgodbe · 2023-01-09T23:17:39Z

I see, this did bring in the new SourceIndexPackageVersion from dotnet/arcade#11995, but we overwrite that in ci.yml with an old version from 2021. We should probably just start using the default from Arcade. I'll open a PR now

wtgodbe · 2023-01-09T23:20:46Z

Never mind, can't remove that because it's used directly before we get to eng/common. Opened #45982 to just update it

- 8.0.100-alpha.1.23055.1 -> 8.0.100-alpha.1.23059.8 - not a big jump because dotnet#45879 went in just this morning - that PR was relatively up to date

* Move to latest .NET SDK - 8.0.100-alpha.1.23055.1 -> 8.0.100-alpha.1.23059.8 - not a big jump because #45879 went in just this morning - that PR was relatively up to date * Hopefully fix analyzer warning Co-authored-by: Steve Sanderson <[email protected]>

Update to net8.0 SDK

004e098

wtgodbe requested review from a team and dougbu as code owners January 4, 2023 20:27

ghost added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Jan 4, 2023

build-analysis bot mentioned this pull request Jan 4, 2023

Request failed "500 Internal Server Error" dotnet/arcade#11737

Open

5 tasks

mitchdenny approved these changes Jan 4, 2023

View reviewed changes

dougbu reviewed Jan 5, 2023

View reviewed changes

Update global.json

72c71bf

sbomer mentioned this pull request Jan 5, 2023

Define UX for IsTrimmable in netstandard2.1 libraries dotnet/linker#3175

Closed

wtgodbe added 2 commits January 5, 2023 14:27

Update Directory.Build.targets.in

5e80747

Update RedisHubLifetimeManager.cs

169ba9c

wtgodbe requested review from BrennanConroy and halter73 as code owners January 6, 2023 18:31

brunolins16 reviewed Jan 6, 2023

View reviewed changes

wtgodbe added 2 commits January 6, 2023 12:28

Update Arcade

93d3c46

Fix conflict

89e27af

wtgodbe mentioned this pull request Jan 6, 2023

[main] Update dependencies from dotnet/arcade #45930

Closed

wtgodbe requested review from JamesNK and mgravell as code owners January 6, 2023 20:59

Update WasmLinkerTest.csproj

67b1647

wtgodbe requested a review from a team as a code owner January 8, 2023 15:22

Update LinkabilityChecker.csproj

b2ce0c2

ViktorHofer approved these changes Jan 9, 2023

View reviewed changes

ViktorHofer mentioned this pull request Jan 9, 2023

[main] Update dependencies from dotnet/sdk dotnet/installer#15151

Merged

wtgodbe merged commit c4cb55c into main Jan 9, 2023

wtgodbe deleted the wtgodbe/net8SDK branch January 9, 2023 18:43

ghost added this to the 8.0-preview1 milestone Jan 9, 2023

wtgodbe mentioned this pull request Jan 9, 2023

Update sourceIndexPackageVersion #45982

Merged

dougbu added a commit to dougbu/aspnetcore that referenced this pull request Jan 10, 2023

Move to latest .NET SDK

c2306c6

- 8.0.100-alpha.1.23055.1 -> 8.0.100-alpha.1.23059.8 - not a big jump because dotnet#45879 went in just this morning - that PR was relatively up to date

dougbu mentioned this pull request Jan 10, 2023

Move to latest .NET SDK #45983

Merged

JamesNK mentioned this pull request Jan 19, 2023

Trimming issue workaround in .NET 8 grpc/grpc-dotnet#2022

Merged

This was referenced Jun 5, 2023

Document NETSDK1195 dotnet/docs#35655

Merged

Update SDK to 8.0.100-preview.6.23305.3 #48619

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update to net8.0 SDK #45879

Update to net8.0 SDK #45879

wtgodbe commented Jan 4, 2023

wtgodbe commented Jan 4, 2023

mitchdenny left a comment

dougbu Jan 5, 2023

wtgodbe Jan 5, 2023 •

edited

Loading

ViktorHofer Jan 9, 2023

wtgodbe commented Jan 5, 2023

wtgodbe commented Jan 5, 2023

JamesNK commented Jan 5, 2023

mitchdenny commented Jan 5, 2023

wtgodbe commented Jan 5, 2023

wtgodbe commented Jan 5, 2023

sbomer commented Jan 5, 2023 •

edited

Loading

wtgodbe commented Jan 5, 2023

sbomer commented Jan 5, 2023

wtgodbe commented Jan 5, 2023

sbomer commented Jan 5, 2023

brunolins16 Jan 6, 2023

wtgodbe Jan 6, 2023

wtgodbe commented Jan 6, 2023

sbomer commented Jan 7, 2023

dougbu commented Jan 7, 2023

JamesNK commented Jan 8, 2023 •

edited

Loading

ViktorHofer left a comment

wtgodbe commented Jan 9, 2023

wtgodbe commented Jan 9, 2023

ViktorHofer commented Jan 9, 2023

wtgodbe commented Jan 9, 2023

sbomer commented Jan 9, 2023

ghost commented Jan 9, 2023

dougbu commented Jan 9, 2023

wtgodbe commented Jan 9, 2023 •

edited

Loading

wtgodbe commented Jan 9, 2023

Update to net8.0 SDK #45879

Update to net8.0 SDK #45879

Conversation

wtgodbe commented Jan 4, 2023

wtgodbe commented Jan 4, 2023

mitchdenny left a comment

Choose a reason for hiding this comment

dougbu Jan 5, 2023

Choose a reason for hiding this comment

wtgodbe Jan 5, 2023 • edited Loading

Choose a reason for hiding this comment

ViktorHofer Jan 9, 2023

Choose a reason for hiding this comment

wtgodbe commented Jan 5, 2023

wtgodbe commented Jan 5, 2023

JamesNK commented Jan 5, 2023

mitchdenny commented Jan 5, 2023

wtgodbe commented Jan 5, 2023

wtgodbe commented Jan 5, 2023

sbomer commented Jan 5, 2023 • edited Loading

wtgodbe commented Jan 5, 2023

sbomer commented Jan 5, 2023

wtgodbe commented Jan 5, 2023

sbomer commented Jan 5, 2023

brunolins16 Jan 6, 2023

Choose a reason for hiding this comment

wtgodbe Jan 6, 2023

Choose a reason for hiding this comment

wtgodbe commented Jan 6, 2023

sbomer commented Jan 7, 2023

dougbu commented Jan 7, 2023

JamesNK commented Jan 8, 2023 • edited Loading

ViktorHofer left a comment

Choose a reason for hiding this comment

wtgodbe commented Jan 9, 2023

wtgodbe commented Jan 9, 2023

ViktorHofer commented Jan 9, 2023

wtgodbe commented Jan 9, 2023

sbomer commented Jan 9, 2023

ghost commented Jan 9, 2023

dougbu commented Jan 9, 2023

wtgodbe commented Jan 9, 2023 • edited Loading

wtgodbe commented Jan 9, 2023

wtgodbe Jan 5, 2023 •

edited

Loading

sbomer commented Jan 5, 2023 •

edited

Loading

JamesNK commented Jan 8, 2023 •

edited

Loading

wtgodbe commented Jan 9, 2023 •

edited

Loading