-
Notifications
You must be signed in to change notification settings - Fork 10.1k
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] Update dependencies from dotnet/runtime #47778
[main] Update dependencies from dotnet/runtime #47778
Conversation
…0419.2 Microsoft.Bcl.AsyncInterfaces , Microsoft.Extensions.Caching.Abstractions , Microsoft.Extensions.Caching.Memory , Microsoft.Extensions.Configuration , Microsoft.Extensions.Configuration.Abstractions , Microsoft.Extensions.Configuration.Binder , Microsoft.Extensions.Configuration.CommandLine , Microsoft.Extensions.Configuration.EnvironmentVariables , Microsoft.Extensions.Configuration.FileExtensions , Microsoft.Extensions.Configuration.Ini , Microsoft.Extensions.Configuration.Json , Microsoft.Extensions.Configuration.UserSecrets , Microsoft.Extensions.Configuration.Xml , Microsoft.Extensions.DependencyInjection , Microsoft.Extensions.DependencyInjection.Abstractions , Microsoft.Extensions.DependencyModel , Microsoft.Extensions.FileProviders.Abstractions , Microsoft.Extensions.FileProviders.Composite , Microsoft.Extensions.FileProviders.Physical , Microsoft.Extensions.FileSystemGlobbing , Microsoft.Extensions.HostFactoryResolver.Sources , Microsoft.Extensions.Hosting , Microsoft.Extensions.Hosting.Abstractions , Microsoft.Extensions.Http , Microsoft.Extensions.Logging , Microsoft.Extensions.Logging.Abstractions , Microsoft.Extensions.Logging.Configuration , Microsoft.Extensions.Logging.Console , Microsoft.Extensions.Logging.Debug , Microsoft.Extensions.Logging.EventLog , Microsoft.Extensions.Logging.EventSource , Microsoft.Extensions.Logging.TraceSource , Microsoft.Extensions.Options , Microsoft.Extensions.Options.ConfigurationExtensions , Microsoft.Extensions.Options.DataAnnotations , Microsoft.Extensions.Primitives , Microsoft.Internal.Runtime.AspNetCore.Transport , Microsoft.NET.Runtime.MonoAOTCompiler.Task , Microsoft.NET.Runtime.WebAssembly.Sdk , Microsoft.NETCore.App.Ref , Microsoft.NETCore.App.Runtime.AOT.win-x64.Cross.browser-wasm , Microsoft.NETCore.App.Runtime.win-x64 , Microsoft.NETCore.BrowserDebugHost.Transport , Microsoft.NETCore.Platforms , System.Configuration.ConfigurationManager , System.Diagnostics.DiagnosticSource , System.Diagnostics.EventLog , System.DirectoryServices.Protocols , System.IO.Pipelines , System.Net.Http.Json , System.Net.Http.WinHttpHandler , System.Reflection.Metadata , System.Resources.Extensions , System.Security.Cryptography.Pkcs , System.Security.Cryptography.Xml , System.Security.Permissions , System.ServiceProcess.ServiceController , System.Text.Encodings.Web , System.Text.Json , System.Threading.AccessControl , System.Threading.Channels , System.Threading.RateLimiting From Version 8.0.0-preview.4.23218.5 -> To Version 8.0.0-preview.4.23219.2
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.
Auto-approving dependency update.
Hey @dotnet/aspnet-build, looks like this PR is something you want to take a look at. |
@layomia this should bring in dotnet/runtime#84436. If all works correctly, we should see |
Be on the lookout for any part of ASP.NET that might want to use/not-use |
What does it do? We typically turn on analyzers/generators manually project-by-project. I can sync with folks on our team who might be motivated to do so |
Well that depends on what ASPNET.Core does with the transport package. I grabbed a binlog to have a look. It does look like it ends up passing the analyzer to most (all?) of your projects. So they are all getting the source generator even though we probably don't want that.
This doesn't appear to be happening right now, but for better or worse the build is succeeding. You might want to check the cases that use the config binder to ensure they're behaving correctly. |
Hi @ericstj. 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. |
I examined all the ASP.NET runtime binaries in the runtime pack from this build and determined that none of them generated real usage of the binder source generator. I did notice that all got an empty type, for which I filed dotnet/runtime#85057. I'll leave it to you if you'd like to disable this generator yourself until you can update to an SDK with dotnet/sdk#31654. CC @captainsafia |
Hi @ericstj. 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. |
Sorry, I meant that it's not obvious to me what the purpose of this source generator is, or what types of projects would want to use it. Once I understand that I can talk with people on the team about where we do/don't want this enabled. |
The generator provides a reflection-free version of configuration binding using a source generator. I think it's fine to not disable it for now and wait until the SDK update that disables it by default. Worst case scenario, we'll find some bugs in the generator that we can report to the team. Best case scenario, there's no net-negative impact and we'll pick up the bits to disable it by default soon. |
This pull request updates the following dependencies
From https://github.com/dotnet/runtime