Skip to content

[main] Update dependencies from dotnet/efcore dotnet/runtime #33072

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
May 29, 2021
Merged

[main] Update dependencies from dotnet/efcore dotnet/runtime #33072

merged 5 commits into from
May 29, 2021

Conversation

dotnet-maestro[bot]
Copy link
Contributor

@dotnet-maestro dotnet-maestro bot commented May 27, 2021 •
edited
Loading

This pull request updates the following dependencies

From https://github.com/dotnet/runtime

  • Subscription: 32db3699-5666-45da-a1b7-08d8b804cd75
  • Build: 20210528.10
  • Date Produced: 5/29/2021 4:35 AM
  • Commit: 95a4476b91827e6f707744fab6e9d87df870407a
  • Branch: refs/heads/main

From https://github.com/dotnet/efcore

  • Subscription: 8ce5251e-8269-419d-3b2a-08d8b8050dda
  • Build: 20210528.1
  • Date Produced: 5/28/2021 10:21 PM
  • Commit: dd315f672b4c7f9b63b776afb432f5b3e585e7ea
  • Branch: refs/heads/main

@dotnet-maestro
Update dependencies from https://github.com/dotnet/runtime build 2021…
cefdd7b 
…0526.13

Microsoft.Win32.Registry , Microsoft.NETCore.Platforms , Microsoft.NETCore.App.Runtime.win-x64 , Microsoft.NETCore.BrowserDebugHost.Transport , 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.Extensions.Http , Microsoft.NET.Runtime.MonoAOTCompiler.Task , Microsoft.NET.Runtime.WebAssembly.Sdk , Microsoft.Extensions.Hosting.Abstractions , Microsoft.AspNetCore.Internal.Transport , 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.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.Configuration.Ini , Microsoft.NETCore.App.Runtime.AOT.win-x64.Cross.browser-wasm , Microsoft.NETCore.App.Ref , System.Net.Http.Json , System.Net.Http.WinHttpHandler , System.Reflection.Metadata , System.Resources.Extensions , System.Runtime.CompilerServices.Unsafe , System.Security.Cryptography.Pkcs , System.Security.Cryptography.Xml , System.Security.Principal.Windows , System.ServiceProcess.ServiceController , System.Text.Encodings.Web , System.Text.Json , System.Threading.Channels , System.IO.Pipelines , System.Diagnostics.DiagnosticSource , System.Diagnostics.EventLog , System.DirectoryServices.Protocols
 From Version 6.0.0-preview.6.21276.1 -> To Version 6.0.0-preview.6.21276.13
@dotnet-maestro dotnet-maestro bot requested a review from dougbu as a code owner May 27, 2021 12:09
@ghost ghost added area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework Type: Dependency Update 🔼 labels May 27, 2021
ghost
ghost approved these changes May 27, 2021
View reviewed changes
Copy link

@ghost ghost left a 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.

@ghost ghost added the tell-mode Indicates a PR which is being merged during tell-mode label May 27, 2021
@eerhardt
Copy link
Member

I re-ran the Helix x64 leg a 2nd time, and both times it failed with this exception:

Attempt by method 'System.Text.ValueStringBuilder.AppendSlow(System.String)' to access method 'System.String.CopyTo(System.Span`1<Char>)' failed.)
[xUnit.net 00:01:35.94]              ---> System.MethodAccessException: Attempt by method 'System.Text.ValueStringBuilder.AppendSlow(System.String)' to access method 'System.String.CopyTo(System.Span`1<Char>)' failed.
[xUnit.net 00:01:35.94]                at System.Text.ValueStringBuilder.AppendSlow(String s) in Microsoft.Extensions.Logging.Abstractions.dll:token 0x6000028+0x28
[xUnit.net 00:01:35.94]                at Microsoft.Extensions.Logging.LogValuesFormatter.FormatArgument(Object value) in Microsoft.Extensions.Logging.Abstractions.dll:token 0x60000a7+0x0
[xUnit.net 00:01:35.94]                at Microsoft.Extensions.Logging.LogValuesFormatter.Format(Object arg0) in Microsoft.Extensions.Logging.Abstractions.dll:token 0x60000a2+0x0
[xUnit.net 00:01:35.94]                at Microsoft.Extensions.Logging.LoggerMessage.LogValues`1.ToString() in Microsoft.Extensions.Logging.Abstractions.dll:token 0x60000ec+0x0
[xUnit.net 00:01:35.94]                at Microsoft.Extensions.Logging.LoggerMessage.LogValues`1.<>c.<.cctor>b__12_0(LogValues`1 state, Exception exception) in Microsoft.Extensions.Logging.Abstractions.dll:token 0x6000149+0x0
[xUnit.net 00:01:35.94]                at Microsoft.Extensions.Logging.Console.SimpleConsoleFormatter.Write[TState](LogEntry`1& logEntry, IExternalScopeProvider scopeProvider, TextWriter textWriter) in Microsoft.Extensions.Logging.Console.dll:token 0x6000093+0x0

It looks like this could have been caused by dotnet/runtime#53246 (cc @stephentoub). But I'm not sure how this could have happened. Are these tests running on an old runtime version?

@pranavkm @dougbu @BrennanConroy

@pranavkm
Copy link
Contributor

This is very similar to the #31795. It's super problematic that templates are using the runtime from the SDK.

@HaoK any chance you might have some time to look at this?

@HaoK
Copy link
Member

HaoK commented May 27, 2021

@dougbu I sort of recall you made a change that I thought was supposed to have fixed this? Did that PR get merged?

@HaoK
Copy link
Member

HaoK commented May 27, 2021 •
edited
Loading

07c6a62#diff-146dcb0c4b795775b3c2bfbd104df0b4dafec8d8b51c881dac18a7f3235b0dca appears to be the change, but I'm not quite sure I see what effect this has

context is here: #32510 (comment)

@dougbu
Copy link
Contributor

dougbu commented May 27, 2021

07c6a62#diff-146dcb0c4b795775b3c2bfbd104df0b4dafec8d8b51c881dac18a7f3235b0dca appears to be the change

Yes, $(TestDependsOnAspNetRuntime) controls whether the Directory.Build.props and Directory.Build.targets files placed in the helix work item root force use of the latest runtime. I debugged things when I made that change and confirmed the tests used the incoming Microsoft.NETCore.App as well as the just-built Microsoft.AspNetCore.App bits. But, something may have gone South since or in GitHub's final squashing.

Either way, can someone confirm the change is about running against an older Microsoft.NETCore.App and not about using a newer one❔

@pranavkm
Copy link
Contributor

I think the templates are being compiled against a newer targeting pack, but run against an older runtime. The stack trace is using an API that was fairly recently made public - dotnet/runtime@6c4f45c. It would follow that it's running against an older runtime, no?

@dougbu
Copy link
Contributor

dougbu commented May 27, 2021

I think the templates are being compiled against a newer targeting pack, but run against an older runtime. The stack trace is using an API that was fairly recently made public - dotnet/runtime@6c4f45c. It would follow that it's running against an older runtime, no?

It would follow but shouldn't happen in the project template tests on Helix (anymore). Something new and different may be busted. I'll poke around later today or tomorrow morning…

@dotnet-maestro
Update dependencies from https://github.com/dotnet/runtime build 2021…
8876e74 
…0528.2

Microsoft.Win32.Registry , Microsoft.NETCore.Platforms , Microsoft.NETCore.App.Runtime.win-x64 , Microsoft.NETCore.BrowserDebugHost.Transport , 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.Extensions.Http , Microsoft.NET.Runtime.MonoAOTCompiler.Task , Microsoft.NET.Runtime.WebAssembly.Sdk , Microsoft.Extensions.Hosting.Abstractions , Microsoft.AspNetCore.Internal.Transport , 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.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.Configuration.Ini , Microsoft.NETCore.App.Runtime.AOT.win-x64.Cross.browser-wasm , Microsoft.NETCore.App.Ref , System.Net.Http.Json , System.Net.Http.WinHttpHandler , System.Reflection.Metadata , System.Resources.Extensions , System.Runtime.CompilerServices.Unsafe , System.Security.Cryptography.Pkcs , System.Security.Cryptography.Xml , System.Security.Principal.Windows , System.ServiceProcess.ServiceController , System.Text.Encodings.Web , System.Text.Json , System.Threading.Channels , System.IO.Pipelines , System.Diagnostics.DiagnosticSource , System.Diagnostics.EventLog , System.DirectoryServices.Protocols
 From Version 6.0.0-preview.6.21276.1 -> To Version 6.0.0-preview.6.21278.2
@eerhardt
Copy link
Member

Either way, can someone confirm the change is about running against an older Microsoft.NETCore.App and not about using a newer one❔

Looking at the change I linked above, and the error coming from the tests, all signs point to the following situation:

  1. .NET 6 introduced a new string.CopyTo(Span) API in the runtime
  2. This new API is being used by a Microsoft.Extensions library that isn't in the NetCoreApp shared framework, but is included in the ASP.NET App shared framework
  3. This test is using the newer version of the Microsoft.Extensions library (which requires this new string API) on an older version of the runtime which doesn't have this new string API

@dougbu
Copy link
Contributor

dougbu commented May 28, 2021

Thanks for the information @eerhardt. Very interesting to hear the tests are somehow using the just-built Microsoft.AspNetCore.App.Runtime but not using the latest incoming Microsoft.NETCore.App.Runtime. I'll look deeper but will likely need to build locally…

@dougbu
Copy link
Contributor

dougbu commented May 28, 2021

I don't see anything obvious in the job submission. The right files are copied into the Helix work item root and they list the expected runtime versions. Looking more closely at the test infrastructure now.

@dotnet-maestro
Update dependencies from https://github.com/dotnet/efcore build 20210…
b42a795 
…528.1

Microsoft.EntityFrameworkCore.Tools , dotnet-ef , Microsoft.EntityFrameworkCore , Microsoft.EntityFrameworkCore.SqlServer , Microsoft.EntityFrameworkCore.InMemory , Microsoft.EntityFrameworkCore.Relational , Microsoft.EntityFrameworkCore.Sqlite , Microsoft.EntityFrameworkCore.Design
 From Version 6.0.0-preview.6.21274.4 -> To Version 6.0.0-preview.6.21278.1
@dotnet-maestro dotnet-maestro bot changed the title [main] Update dependencies from dotnet/runtime [main] Update dependencies from dotnet/runtime dotnet/efcore May 28, 2021
@dougbu
Copy link
Contributor

dougbu commented May 29, 2021

❕ Found it ❕

At some point, %(KnownFrameworkReference.TargetFramework) metadata for recent TFMs changed from netcoreapp5.0 and netcoreapp6.0 to net5.0 and net6.0. We have a workaround for the unusual format previously used in this metadata and it's broken now. @dsplaisted was that considered a breaking change❔ Is the metadata going to change back in the future❔

As long as this will remain consistent when using 'net5.0` or later TFMs and a current SDK, I'll remove the special case…

@dougbu
Remove workaround for %(TargetFramework) format
a368920 
- remove `$(KnownAppHostPackOrFrameworkReferenceTfm)` workaround
  - `%(KnownAppHostPack.TargetFramework)` metadata now matches `$(TargetFramework)` format
  - `%(KnownFrameworkReference.TargetFramework)` metadata now matches `$(TargetFramework)` format
@dotnet-maestro dotnet-maestro bot requested review from Pilchie and a team as code owners May 29, 2021 05:48
@dougbu
Copy link
Contributor

dougbu commented May 29, 2021

New "Remove workaround for %(TargetFramework) format" commit should fix build issues. @dsplaisted @dotnet/aspnet-build please have a look…

@dougbu
Copy link
Contributor

dougbu commented May 29, 2021

Oh, it was actually @ViktorHofer who introduced the workaround I removed in the "Remove workaround for %(TargetFramework) format" commit. See #21630 for more historical context.

@ViktorHofer please confirm that workaround should not be needed going forward from your perspective.

@dougbu dougbu requested review from dsplaisted and ViktorHofer May 29, 2021 05:56
@dotnet-maestro
Update dependencies from https://github.com/dotnet/runtime build 2021…
b405f40 
…0528.10

Microsoft.Win32.Registry , Microsoft.NETCore.Platforms , Microsoft.NETCore.App.Runtime.win-x64 , Microsoft.NETCore.BrowserDebugHost.Transport , 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.Extensions.Http , Microsoft.NET.Runtime.MonoAOTCompiler.Task , Microsoft.NET.Runtime.WebAssembly.Sdk , Microsoft.Extensions.Hosting.Abstractions , Microsoft.AspNetCore.Internal.Transport , 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.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.Configuration.Ini , Microsoft.NETCore.App.Runtime.AOT.win-x64.Cross.browser-wasm , Microsoft.NETCore.App.Ref , System.Net.Http.Json , System.Net.Http.WinHttpHandler , System.Reflection.Metadata , System.Resources.Extensions , System.Runtime.CompilerServices.Unsafe , System.Security.Cryptography.Pkcs , System.Security.Cryptography.Xml , System.Security.Principal.Windows , System.ServiceProcess.ServiceController , System.Text.Encodings.Web , System.Text.Json , System.Threading.Channels , System.IO.Pipelines , System.Diagnostics.DiagnosticSource , System.Diagnostics.EventLog , System.DirectoryServices.Protocols
 From Version 6.0.0-preview.6.21276.1 -> To Version 6.0.0-preview.6.21278.10
@dotnet-maestro dotnet-maestro bot changed the title [main] Update dependencies from dotnet/runtime dotnet/efcore [main] Update dependencies from dotnet/efcore dotnet/runtime May 29, 2021
@dotnet-maestro dotnet-maestro bot merged commit df0feef into main May 29, 2021
@dotnet-maestro dotnet-maestro bot deleted the darc-main-fcb03cba-352f-48ea-8e02-6ba7d6ec3099 branch May 29, 2021 13:34
@ghost ghost added this to the 6.0-preview6 milestone May 29, 2021
@ViktorHofer
Copy link
Member

@ViktorHofer please confirm that workaround should not be needed going forward from your perspective.

It has been a long time since I applied that fix so I can't answer for sure but we will see when aspnetcore again branches from NET6 to NET7 :)

@ghost
Copy link

ghost commented May 30, 2021

Hi @ViktorHofer. 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.

@dsplaisted
Copy link
Member

@dougbu That looks fine. You'll need to update the "previous" version for .NET 7 (and 8, etc.), but we don't expect the format to change again. Ideally people consuming this metadata would be parsing the values into target framework identifier and version, and it wouldn't matter what format it was in. But that's hard to do.

@ghost
Copy link

ghost commented Jun 1, 2021

Hi @dsplaisted. 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
Copy link
Contributor

dougbu commented Jun 1, 2021

Thanks @ViktorHofer and @dsplaisted❕

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dougbu dougbu Awaiting requested review from dougbu

@Pilchie Pilchie Awaiting requested review from Pilchie

@dsplaisted dsplaisted Awaiting requested review from dsplaisted

@ViktorHofer ViktorHofer Awaiting requested review from ViktorHofer

Assignees
No one assigned
Labels
area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework tell-mode Indicates a PR which is being merged during tell-mode Type: Dependency Update 🔼
Projects
None yet
Milestone
6.0-preview6
Development

Successfully merging this pull request may close these issues.
6 participants
@eerhardt @pranavkm @HaoK @dougbu @ViktorHofer @dsplaisted