Update SDK #48908
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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.All)] T>(IServiceCollection services, IConfigurationSection section) | ||
|
There was a problem hiding this comment. The upstream annotation (in There was a problem hiding this comment. The weird thing is the underlying API hasn't change in the last 10 months. Was there a linker or anlalyzer change? There was a problem hiding this comment. @sbomer @agocke @vitek-karas - any thoughts on what could have fixed this between There was a problem hiding this comment. 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. There was a problem hiding this comment.
The IL2091 error specifically references the call to |
||
| where T : class | ||
| { | ||
| services.Configure<T>(section); | ||
|
|
||
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.
How does this work for templates?
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 thought it might work for the templates because of these lines:
aspnetcore/src/ProjectTemplates/TestInfrastructure/PrepareForTest.targets
Lines 85 to 88 in 9f4b8b4
aspnetcore/src/ProjectTemplates/TestInfrastructure/Directory.Build.props.in
Line 8 in 9f4b8b4
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?
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.
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
SuppressGenerateILCompilerExplicitPackageReferenceWarningworkaround?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.
Ah, that makes sense. Hopefully, this is resolved for preview7 and we don't have to issue a known issues warning. 🤞🏽
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 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.
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 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
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 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.
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.
Is there a way to suppress the warning by its warning code - I don't know if
NoWarnworks 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.
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.
<NoWarn>NETSDK1206</NoWarn>works.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.
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.