Skip to content

Update SDK to 8.0.100-preview.6.23305.3 #48619

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

Merged
merged 2 commits into from
Jun 7, 2023
Merged

Update SDK to 8.0.100-preview.6.23305.3 #48619

merged 2 commits into from
Jun 7, 2023

Conversation

eerhardt
Copy link
Member

@eerhardt eerhardt commented Jun 5, 2023

No description provided.

@eerhardt eerhardt requested review from a team and wtgodbe as code owners June 5, 2023 16:36
@ghost ghost added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Jun 5, 2023
@ghost
Copy link

ghost commented Jun 5, 2023

Hey @dotnet/aspnet-build, looks like this PR is something you want to take a look at.

@eerhardt
Copy link
Member Author

eerhardt commented Jun 5, 2023

@sbomer - any idea what's going on here?

src/Servers/Connections.Abstractions/src/Microsoft.AspNetCore.Connections.Abstractions.csproj(0,0): error NU1603: (NETCORE_ENGINEERING_TELEMETRY=Restore) Warning As Error: Microsoft.AspNetCore.Connections.Abstractions depends on Microsoft.NET.ILLink.Analyzers (>= 8.0.0-preview.6.23304.2) but Microsoft.NET.ILLink.Analyzers 8.0.0-preview.6.23304.2 was not found. An approximate best match of Microsoft.NET.ILLink.Analyzers 8.0.100-1.22608.1 was resolved.

@sbomer
Copy link
Member

sbomer commented Jun 5, 2023

Writing down my answer from our conversation in case anyone else comes across this:

  • With Use linker matching TFM sdk#29441, the trim analyzer (imported from the ILLink pack) isn't supported for netstandard2.1. (The behavior was never correct for netstandard2.1, but now we intentionally block the scenario).
  • Aspnet has a workaround that defines a KnownILLinkPack for netstandard2.1, using the package version taken from 8.0:
    <KnownILLinkPack Include="@(KnownILLinkPack)"
    Condition="'%(TargetFramework)' == '${DefaultNetCoreTargetFramework}'"
    TargetFramework="netstandard2.1"
    ILLinkPackVersion="%(KnownILLinkPack.ILLinkPackVersion)" />
  • When targeting net7.0 and below, we add a separate packagereference to the analyzer (since it used to ship as a separate package). With the aspnet workaround this now looks for a non-existent 8.0 package, so restore fails.

I would suggest removing the workaround - then the scenario will fail (depending on the TFM) with an error that indicates the runtime pack is not available. Trimming and trim analysis should be disabled for those TFMs since they are not supported scenarios (see dotnet/linker#3175 for more context).

@sbomer
Copy link
Member

sbomer commented Jun 6, 2023

Adding more notes from investigation with @eerhardt: the specific issue was caused by the aspnet workaround using the 8.0 version for KnownILLinkPack, causing the SDK to look for non-existent Microsoft.NET.ILLink.Analyzers package (since we started shipping analyzers with the ILLink.Tasks package in 8.0). The workaround should be using net7.0 instaed of DefaultNetCoreTargetFramework in the condition, as suggested originally in #45879 (comment).

- Add static keyword for method that doesn't need to be an instance method.
- Change workaround for dotnet/linker#3175 to use net7.0 instead of net8.0. The ILLink.Analysis package no longer ships in net8.0. So use net7.0, like the rest of the "unsupported" TFMs use.
@eerhardt eerhardt requested a review from a team as a code owner June 6, 2023 21:24
@eerhardt eerhardt merged commit ed984e3 into main Jun 7, 2023
@eerhardt eerhardt deleted the UpdateSDK branch June 7, 2023 14:03
@ghost ghost added this to the 8.0-preview6 milestone Jun 7, 2023
@amcasey amcasey mentioned this pull request Jun 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants