-
Notifications
You must be signed in to change notification settings - Fork 1.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 #17948
[main] Update dependencies from dotnet/runtime #17948
Conversation
…0528.2 Microsoft.NETCore.Platforms , Microsoft.NETCore.App.Runtime.win-x64 , Microsoft.NETCore.DotNetHostResolver , Microsoft.NET.HostModel , Microsoft.Extensions.DependencyModel , Microsoft.NETCore.App.Host.win-x64 , Microsoft.NETCore.App.Ref , System.Reflection.MetadataLoadContext , System.Resources.Extensions , System.Security.Cryptography.ProtectedData , System.Text.Encoding.CodePages , System.CodeDom , VS.Redist.Common.NetCore.SharedFramework.x64.6.0 From Version 6.0.0-preview.6.21276.13 -> To Version 6.0.0-preview.6.21278.2
I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label. |
Notification for subscribed users from https://github.com/dotnet/runtime:@dnr-codeflow Action requested: Please take a look at this failing automated dependency-flow pull request's checks; failures may be related to changes which originated in your repo.
|
@adamsitnik - this looks like it is caused by dotnet/runtime#53231. Two different legs are failing with:
|
@sfoslund @dsplaisted @wli3 - is it possible that the tests are using 2 different versions of the runtime? Building against one and running against the older SDK? |
In this case, essentially yes. In our build we update the FrameworkReference to use the version coming from dependency flow instead of the one in stage 0: Lines 38 to 46 in 3c50a7d
The test projects themselves run on stage 0, which doesn't include the latest runtime. Many of the tests create projects and then launch a separate process to build or run the project, and the separate project uses "stage 2" which is the just-built SDK which includes the latest runtime. However, it looks like these BrowserRefresh tests are running in-process. If the BrowserRefresh tests are the only ones failing, we may need to disable them until we can flow the runtime update through and update stage 0. If there are other test projects impacted, then we may need to disable the FrameworkReference update until we can complete that loop. |
This seems like it could be an issue any time we do refactoring in the runtime. Can the owner of these tests fix it so they don't build and run against different versions of the runtime? |
OK, I dug into the history a bit more, and it actually is supposed to be running the tests on the same updated runtime. Here's the original PR for reference: dotnet/cli#11639 The global.json specifies the same version that is used to compile against as a runtime, and the arcade tooling downloads that runtime and it gets used. However, what appears to be going on here is that it's only downloading the Microsoft.NETCore.App shared framework. So when a test uses the ASP.NET Core shared framework in-process, it loads the stage 0 version of that shared framework, which loads the stage 0 version of Microsoft.NETCore.App, since that's the closest match to the version specified in Microsoft.AspNetCore.App.runtimeconfig.json. Ideally it would roll-forward to the latest version of Microsoft.NETCore.App, but I'm not sure we have a way of doing that. Or maybe we should say that we shouldn't be testing other shared frameworks in-process in the tests. Right now I'm inclined to disable the tests until the update flows. Thoughts? |
That's a fine solution for now. I'll see if there's a way to move those tests out of this repo since we had some trouble with them earlier too. |
I think I have a fix for this, though I don't have enough time to finish it today. If the test lists both shared frameworks in its runtimeconfig file (which normally doesn't happen), then the test project will load the right version of Microsoft.NETCore.App. Here's the proof of concept target I added to the BrowserRefresh tests: <Target Name="PatchRuntimeConfig" AfterTargets="GenerateBuildRuntimeConfigurationFiles">
<PropertyGroup>
<PatchedRuntimeConfigContents>
<![CDATA[
{
"runtimeOptions": {
"tfm": "net6.0",
"frameworks": [
{
"name": "Microsoft.NETCore.App",
"version": "$(VSRedistCommonNetCoreSharedFrameworkx6460PackageVersion)"
},
{
"name": "Microsoft.AspNetCore.App",
"version": "6.0.0-preview.6.21277.6"
}
],
"rollForwardOnNoCandidateFx": 2
}
}
]]>
</PatchedRuntimeConfigContents>
</PropertyGroup>
<WriteLinesToFile File="$(ProjectRuntimeConfigFilePath)"
Lines="$(PatchedRuntimeConfigContents)"
Overwrite="true"/>
</Target> At a minimum it needs to be updated so that the version for Microsoft.AspNetCore.App is not hard-coded. |
…0528.10 Microsoft.NETCore.Platforms , Microsoft.NETCore.App.Runtime.win-x64 , Microsoft.NETCore.DotNetHostResolver , Microsoft.NET.HostModel , Microsoft.Extensions.DependencyModel , Microsoft.NETCore.App.Host.win-x64 , Microsoft.NETCore.App.Ref , System.Reflection.MetadataLoadContext , System.Resources.Extensions , System.Security.Cryptography.ProtectedData , System.Text.Encoding.CodePages , System.CodeDom , VS.Redist.Common.NetCore.SharedFramework.x64.6.0 From Version 6.0.0-preview.6.21276.13 -> To Version 6.0.0-preview.6.21278.10
…0529.3 Microsoft.NETCore.Platforms , Microsoft.NETCore.App.Runtime.win-x64 , Microsoft.NETCore.DotNetHostResolver , Microsoft.NET.HostModel , Microsoft.Extensions.DependencyModel , Microsoft.NETCore.App.Host.win-x64 , Microsoft.NETCore.App.Ref , System.Reflection.MetadataLoadContext , System.Resources.Extensions , System.Security.Cryptography.ProtectedData , System.Text.Encoding.CodePages , System.CodeDom , VS.Redist.Common.NetCore.SharedFramework.x64.6.0 From Version 6.0.0-preview.6.21276.13 -> To Version 6.0.0-preview.6.21279.3
I think this should be fixed with 7bb178a |
…0531.1 Microsoft.NETCore.Platforms , Microsoft.NETCore.App.Runtime.win-x64 , Microsoft.NETCore.DotNetHostResolver , Microsoft.NET.HostModel , Microsoft.Extensions.DependencyModel , Microsoft.NETCore.App.Host.win-x64 , Microsoft.NETCore.App.Ref , System.Reflection.MetadataLoadContext , System.Resources.Extensions , System.Security.Cryptography.ProtectedData , System.Text.Encoding.CodePages , System.CodeDom , VS.Redist.Common.NetCore.SharedFramework.x64.6.0 From Version 6.0.0-preview.6.21276.13 -> To Version 6.0.0-preview.6.21281.1
6160b78
to
0cd40b6
Compare
….NETCore.App shared framework
0cd40b6
to
23ca01c
Compare
@@ -8,7 +8,8 @@ | |||
</PropertyGroup> | |||
|
|||
<ItemGroup> | |||
<PackageReference Include="Microsoft.Extensions.DependencyModel" Version="$(MicrosoftExtensionsDependencyModelPackageVersion)" /> | |||
<!-- .NET 6 versions of this package no longer support .NET Core 2.2, so hard-code the version here to 5.0 --> |
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'm not sure what this means. The package supports netstandard2.0
, why wouldn't that asset be used?
cc @ViktorHofer
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.
It looks like the latest build of DependencyModel now depends on System.Runtime.CompilerServices.Unsafe, and that package has a target in it blocking the build:
<Project InitialTargets="NETStandardCompatError_System_Runtime_CompilerServices_Unsafe_netcoreapp3_1">
<Target Name="NETStandardCompatError_System_Runtime_CompilerServices_Unsafe_netcoreapp3_1"
Condition="'$(SuppressTfmSupportBuildWarnings)' == ''">
<Error Text="System.Runtime.CompilerServices.Unsafe doesn't support $(TargetFramework). Consider updating your TargetFramework to netcoreapp3.1 or later." />
</Target>
</Project>
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.
OK, that explains it. Thanks.
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.
You're the first one that encounters these recent changes. I plan to send out a breaking change notice later this week after I synced up with @terrajobst on the format.
Is there a reason for dotnet/sdk to still have tests that build against an unsupported version of .NETCoreApp?
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.
@ViktorHofer Late reply: Generally we want the SDK to continue to build projects successfully even if they're out of support. IE when a runtime goes out of support it means it won't get updates anymore, not that suddenly the tools stop working for that runtime.
This pull request updates the following dependencies
From https://github.com/dotnet/runtime