Update SDK #48908

RussKie · 2023-06-20T02:22:42Z

No description provided.

RussKie · 2023-06-20T03:17:52Z

/home/vsts/work/1/s/src/Middleware/Spa/SpaProxy/src/SpaHostingStartup.cs(37,13): error IL2091: 'TOptions' generic argument does not satisfy 'DynamicallyAccessedMemberTypes.All' in 'Microsoft.Extensions.DependencyInjection.OptionsConfigurationServiceCollectionExtensions.Configure<TOptions>(IServiceCollection, IConfiguration)'. The generic parameter 'T' of 'ConfigureOptions<T>(IServiceCollection, IConfigurationSection)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. [/home/vsts/work/1/s/src/Middleware/Spa/SpaProxy/src/Microsoft.AspNetCore.SpaProxy.csproj]
##[error]src/Middleware/Spa/SpaProxy/src/SpaHostingStartup.cs(37,13): error IL2091: (NETCORE_ENGINEERING_TELEMETRY=Build) 'TOptions' generic argument does not satisfy 'DynamicallyAccessedMemberTypes.All' in 'Microsoft.Extensions.DependencyInjection.OptionsConfigurationServiceCollectionExtensions.Configure<TOptions>(IServiceCollection, IConfiguration)'. The generic parameter 'T' of 'ConfigureOptions<T>(IServiceCollection, IConfigurationSection)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.

@sbomer can you help with this?

halter73 · 2023-06-22T01:32:07Z

.dotnet/sdk/8.0.100-preview.6.23319.37/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.Sdk.targets(267,5):
error NETSDK1206: (NETCORE_ENGINEERING_TELEMETRY=Build)
Found version-specific or distribution-specific runtime identifier(s): alpine-arm, alpine-arm64, alpine-x64, browser-wasm, maccatalyst-arm64, maccatalyst-x64.
Affected libraries: SQLitePCLRaw.lib.e_sqlite3.
In .NET 8.0 and higher, assets for version-specific and distribution-specific runtime identifiers will not be found by default. See https://aka.ms/dotnet/rid-usage for details.

I filed an issue on SQLitePCLRaw at ericsink/SQLitePCL.raw#543, but in the meantime, I think we're going to need to set the System.Runtime.Loader.UseRidGraph compatibility switch to true to take this SDK update in a reasonable amount of time. @elinor-fung

SteveSandersonMS · 2023-06-22T09:36:56Z

src/Middleware/Spa/SpaProxy/src/SpaHostingStartup.cs

@@ -31,7 +31,7 @@ public void Configure(IWebHostBuilder builder)
        });

        [UnconditionalSuppressMessageAttribute("Trimming", "IL2026", Justification = "Configuration object's public properties are preserved.")]
-        static void ConfigureOptions<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties)] T>(IServiceCollection services, IConfigurationSection section)
+        static void ConfigureOptions<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] T>(IServiceCollection services, IConfigurationSection section)


The upstream annotation (in Configure<TOptions>) now specifies DynamicallyAccessedMemberTypes.All, so we have to broaden the annotation here to match.

The weird thing is the underlying API hasn't change in the last 10 months.

https://github.com/dotnet/runtime/blob/dee36a1ea369517c1253dd560eabecc7ddcf987c/src/libraries/Microsoft.Extensions.Options.ConfigurationExtensions/src/OptionsConfigurationServiceCollectionExtensions.cs

Was there a linker or anlalyzer change?

Good question

@sbomer @agocke @vitek-karas - any thoughts on what could have fixed this between 8.0.100-preview.6.23305.3 and 8.0.100-preview.7.23321.23? We must have had a bug here in the analyzer?

Maybe the configuration binder source generator? I seem to see something like the Configure call below to be handled by the source generated at least in some cases. So it's possible that it's effectively started to call a different overload than before? Just guessing.

/home/vsts/work/1/s/src/Middleware/Spa/SpaProxy/src/SpaHostingStartup.cs(37,13): error IL2091: 'TOptions' generic argument does not satisfy 'DynamicallyAccessedMemberTypes.All' in 'Microsoft.Extensions.DependencyInjection.OptionsConfigurationServiceCollectionExtensions.Configure<TOptions>(IServiceCollection, IConfiguration)'. The generic parameter 'T' of 'ConfigureOptions<T>(IServiceCollection, IConfigurationSection)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. [/home/vsts/work/1/s/src/Middleware/Spa/SpaProxy/src/Microsoft.AspNetCore.SpaProxy.csproj] ##[error]src/Middleware/Spa/SpaProxy/src/SpaHostingStartup.cs(37,13): error IL2091: (NETCORE_ENGINEERING_TELEMETRY=Build) 'TOptions' generic argument does not satisfy 'DynamicallyAccessedMemberTypes.All' in 'Microsoft.Extensions.DependencyInjection.OptionsConfigurationServiceCollectionExtensions.Configure<TOptions>(IServiceCollection, IConfiguration)'. The generic parameter 'T' of 'ConfigureOptions<T>(IServiceCollection, IConfigurationSection)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.

The IL2091 error specifically references the call to Microsoft.Extensions.DependencyInjection.OptionsConfigurationServiceCollectionExtensions.Configure<TOptions>(IServiceCollection, IConfiguration), so I don't think the call is getting intercepted by the source generator.

SteveSandersonMS · 2023-06-22T09:38:13Z

I filed an issue on SQLitePCLRaw at ericsink/SQLitePCL.raw#543, but in the meantime, I think we're going to need to set the System.Runtime.Loader.UseRidGraph compatibility switch to true to take this SDK update in a reasonable amount of time.

@halter73 Thanks for tracking that down. I've added 47f5966 as suggested.

SteveSandersonMS · 2023-06-22T17:19:36Z

Unfortunately this is still stuck on the new RID resolution warning because it also shows up in our template tests, and we don't have such an easy way to override the build config for those. I did try one possible workaround but it didn't work. Some other approach will be needed for setting System.Runtime.Loader.UseRidGraph on the projects generated during template tests.

SteveSandersonMS · 2023-06-23T19:14:07Z

src/ProjectTemplates/Web.ProjectTemplates/RazorPagesWeb-CSharp.csproj.in

+  <!-- Temporarily ignore RID-related warnings. -->
+  <ItemGroup>
+    <RuntimeHostConfigurationOption Include="System.Runtime.Loader.UseRidGraph" Value="true" />
+  </ItemGroup>


This will actually show up in people's real apps when they create them, won't it? Is that OK?

Yes, it will. I gave this a try largely to see if this is the only remaining issue with this SDK update or if we might have to onion peel on another item.

As to whether or not it is OK, it appears that the right thing to do is resolve the underlying issue is resolve ericsink/SQLitePCL.raw#543.

Given that we've branched for preview7 already, I was assuming that it was OK to take this in as we have some runaway to land a fix in preview7.

@bricelam Thoughts on feasibility of getting a fix for the underlying issue?

It looks like we'll also need to fix templates in the spa-templates repo. :/

I'm fine to pursue this assuming we think we can land a permanent fix for preview7 or there isn't a way to address this that is more hidden from the user.

captainsafia · 2023-06-26T16:13:52Z

eng/tools/GenerateFiles/Directory.Build.props.in

+  <!-- Ignore warning about RID resolution rules changing in .NET 8 Preview 6 -->
+  <!-- Needed until https://github.com/ericsink/SQLitePCL.raw/issues/543 is fixed -->
+  <!-- See https://github.com/dotnet/aspnetcore/pull/48908#issuecomment-1601894643 -->
+  <ItemGroup>


How does this work for templates?

I thought it might work for the templates because of these lines:

aspnetcore/src/ProjectTemplates/TestInfrastructure/PrepareForTest.targets

Lines 85 to 88 in 9f4b8b4

<GenerateFileFromTemplate

TemplateFile="$(MSBuildThisFileDirectory)..\TestInfrastructure\Directory.Build.props.in"

Properties="RepoRoot=$(RepoRoot);ArtifactsBinDir=$(ArtifactsBinDir)"

OutputPath="$(TestTemplateCreationFolder)Directory.Build.props" />

aspnetcore/src/ProjectTemplates/TestInfrastructure/Directory.Build.props.in

Line 8 in 9f4b8b4

<Import Project="${ArtifactsBinDir}GenerateFiles\Directory.Build.props" />

But I was just trying thing, and I'm a bit surprised it worked too. @wtgodbe Is there a better way to do this? I saw that @mitchdenny updated this a couple months ago as part of #48014. Do we still need to hardcode SuppressGenerateILCompilerExplicitPackageReferenceWarning?

Yeah, this is the right way to do it, at least for in-repo stuff. Template tests will get the workaround, but template projects may run into issues in real world scenarios (since the workaround won't be injected into them). I don't know enough about this specific issue to know if it's something that would actually manifest.

@eerhardt do we still need the SuppressGenerateILCompilerExplicitPackageReferenceWarning workaround?

Yeah, this is the right way to do it, at least for in-repo stuff. Template tests will get the workaround, but template projects may run into issues in real world scenarios (since the workaround won't be injected into them). I don't know enough about this specific issue to know if it's something that would actually manifest.

Ah, that makes sense. Hopefully, this is resolved for preview7 and we don't have to issue a known issues warning. 🤞🏽

@eerhardt do we still need the SuppressGenerateILCompilerExplicitPackageReferenceWarning workaround?

I believe so. I don't know of any changes here that would cause us to no longer need the workaround. We are running our tests against a specific version of the runtime, and not the runtime that comes with the SDK. See dotnet/runtime#84372.

Ah, that makes sense. Hopefully, this is resolved for preview7 and we don't have to issue a known issues warning. 🤞🏽

I doubt this will be fixed by preview7. Even if ericsink/SQLitePCL.raw#543 is resolved by then and uploaded to nuget, EF still needs to update its dependencies. Anyone using Microsoft.Data.Sqlite will likely see warnings until rc1 at the earliest unless they manually upgrade, so we should probably get ahead of the issues. @dotnet/efteam

I wonder if it would be reasonable to add a mechanism to tell the SDK that it should not warn on some particular package or particular versions of packages - for example, the consumer has checked that they are not affected by the non-portable RID in that package, as would be the case here. @dsplaisted @vitek-karas @agocke - thoughts?

It would go kind of counter to making a hard push for packages to fix their RID usage, but it would provide an option between suppressing the warning entirely and switching to the old behaviour - neither of which we really want people to do.

Is there a way to suppress the warning by its warning code - I don't know if NoWarn works on SDK warnings. If that works, then I'd probably not add any new support and ask the affected people to suppress the warning code instead.

Basically we want packages to fix this problem, and there are not that many packages which are affected by this and consequently not that many teams/customers will see the warning either. I'd like to avoid adding production code into the SDK unless there's a strong reason for it - and this doesn't feel that strong - assuming the warning can be suppressed already.

<NoWarn>NETSDK1206</NoWarn> works.

Basically we want packages to fix this problem, and there are not that many packages which are affected by this and consequently not that many teams/customers will see the warning either.

That's fair. The main concern is that the affected package is brought in as a dependency under certain template configurations to there is a chance that users might run into the issue with this particular package at a higher rate than other packages.

Update SDK

2f76362

RussKie requested review from a team and wtgodbe as code owners June 20, 2023 02:22

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

SteveSandersonMS added 2 commits June 22, 2023 10:07

Fix trimming annotation error

8b17dee

Supress RID resolution warning

47f5966

SteveSandersonMS requested review from Tratcher and BrennanConroy as code owners June 22, 2023 09:34

SteveSandersonMS approved these changes Jun 22, 2023

View reviewed changes

SteveSandersonMS reviewed Jun 22, 2023

View reviewed changes

SteveSandersonMS enabled auto-merge (squash) June 22, 2023 09:37

SteveSandersonMS added 3 commits June 22, 2023 11:30

Update SDK further to get fix dotnet/runtime#87659

3755745

Attempted further workaround

56d8310

Revert attempted workaround that didn't work

db226bb

Disable RID check in templates with SQLite

9db37bd

SteveSandersonMS reviewed Jun 23, 2023

View reviewed changes

Try different location for RID workaround

719fc18

SteveSandersonMS merged commit 85f7685 into main Jun 24, 2023

SteveSandersonMS deleted the update_sdk branch June 24, 2023 01:54

ghost added this to the 8.0-preview7 milestone Jun 24, 2023

captainsafia reviewed Jun 26, 2023

View reviewed changes

vitek-karas mentioned this pull request Jul 18, 2023

Microsoft.AspNetCore.App.runtimeconfig.json specifies runtime properties which values should not bet set by a shared framework #49486

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update SDK #48908

Update SDK #48908

RussKie commented Jun 20, 2023

RussKie commented Jun 20, 2023

halter73 commented Jun 22, 2023

SteveSandersonMS Jun 22, 2023

halter73 Jun 22, 2023 •

edited

Loading

SteveSandersonMS Jun 22, 2023

eerhardt Jun 26, 2023

vitek-karas Jun 26, 2023

halter73 Jun 26, 2023

SteveSandersonMS commented Jun 22, 2023

SteveSandersonMS commented Jun 22, 2023

SteveSandersonMS Jun 23, 2023

captainsafia Jun 23, 2023

captainsafia Jun 23, 2023

captainsafia Jun 26, 2023

halter73 Jun 26, 2023

wtgodbe Jun 26, 2023

captainsafia Jun 26, 2023

eerhardt Jun 26, 2023

halter73 Jun 27, 2023

elinor-fung Jun 27, 2023

vitek-karas Jul 10, 2023

elinor-fung Jul 10, 2023

captainsafia Jul 10, 2023

	<GenerateFileFromTemplate
	TemplateFile="$(MSBuildThisFileDirectory)..\TestInfrastructure\Directory.Build.props.in"
	Properties="RepoRoot=$(RepoRoot);ArtifactsBinDir=$(ArtifactsBinDir)"
	OutputPath="$(TestTemplateCreationFolder)Directory.Build.props" />

Update SDK #48908

Update SDK #48908

Conversation

RussKie commented Jun 20, 2023

RussKie commented Jun 20, 2023

halter73 commented Jun 22, 2023

Choose a reason for hiding this comment

halter73 Jun 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SteveSandersonMS commented Jun 22, 2023

SteveSandersonMS commented Jun 22, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

halter73 Jun 22, 2023 •

edited

Loading