Skip to content
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 #29406

Merged
merged 18 commits into from
Dec 19, 2022

Conversation

dotnet-maestro[bot]
Copy link
Contributor

@dotnet-maestro dotnet-maestro bot commented Dec 6, 2022

This pull request updates the following dependencies

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

  • Subscription: aa69f164-2492-460a-3914-08d8e9750bf8
  • Build: 20221219.2
  • Date Produced: December 19, 2022 7:35:52 PM UTC
  • Commit: 62013d86608b1f8923bf9bd901bb4c8ced7b20a9
  • Branch: refs/heads/main

…1205.5

Microsoft.Extensions.DependencyModel , Microsoft.NET.HostModel , Microsoft.NETCore.App.Host.win-x64 , Microsoft.NETCore.App.Ref , Microsoft.NETCore.App.Runtime.win-x64 , Microsoft.NETCore.DotNetHostResolver , Microsoft.NETCore.Platforms , System.CodeDom , System.Reflection.MetadataLoadContext , System.Resources.Extensions , System.Security.Cryptography.ProtectedData , System.Text.Encoding.CodePages , VS.Redist.Common.NetCore.SharedFramework.x64.8.0 , VS.Redist.Common.NetCore.TargetingPack.x64.8.0
 From Version 8.0.0-alpha.1.22605.1 -> To Version 8.0.0-alpha.1.22605.5
@dotnet-maestro
Copy link
Contributor Author

dotnet-maestro bot commented Dec 6, 2022

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.

  • This pull request contains changes from your source repo (https://github.com/dotnet/runtime) and seems to have failed checks in this PR. Please take a peek at the failures and comment if they seem relevant to your changes.
  • If you're being tagged in this comment it is due to an entry in the related Maestro Subscription of the Build Asset Registry. If you feel this entry has added your GitHub login or your GitHub team in error, please update the subscription to reflect this.
  • For more details, please read the Arcade Darc documentation

@ViktorHofer
Copy link
Member

ViktorHofer commented Dec 6, 2022

So, here's the net8.0 runtime ingestion PR. cc @joeloff @marcpopMSFT

It looks like there are some hardcoded values that need to be changed, i.e. here a "net7.0" assembly in the 8.0 runtime pack isn't found because the assembly is now in the corresponding net8.0 directory.

/private/tmp/helix/working/B3F209A4/p/d/packs/Microsoft.NET.Runtime.WebAssembly.Sdk/8.0.0-alpha.1.22559.2/Sdk/WasmApp.Native.targets(293,5): error MSB4018: The "ManagedToNativeGenerator" task failed unexpectedly. [/private/tmp/helix/working/B3F209A4/w/AE50091F/e/testExecutionDirectory/AoT_Publish_H---E44E9AE0/blazorwasm/blazorwasm.csproj]
/private/tmp/helix/working/B3F209A4/p/d/packs/Microsoft.NET.Runtime.WebAssembly.Sdk/8.0.0-alpha.1.22559.2/Sdk/WasmApp.Native.targets(293,5): error MSB4018: System.IO.DirectoryNotFoundException: Could not find a part of the path '/private/tmp/helix/working/B3F209A4/w/AE50091F/e/testExecutionDirectory/.nuget/packages/microsoft.netcore.app.runtime.mono.browser-wasm/8.0.0-alpha.1.22605.5/runtimes/browser-wasm/lib/net7.0/mscorlib.dll'

This seems to be specific to dotnet/sdk but if there's anything wrong on our side, please let us know.

@ViktorHofer
Copy link
Member

ViktorHofer commented Dec 6, 2022

cc @radical as this is failing in the WasmApp.Native.targets. Also, this is a +1 of dotnet/runtime#79109

EDIT 1:
The version of the Microsoft.NET.Runtime.WebAssembly.Sdk is quite old which is why this fails in CI (because it contains a net7.0 hardcoded tfm in it). We need to update that component.

EDIT 2:
I'm relatively sure that this is a dotnet/sdk specific issue. It looks like the "Install wasm-tools Workload" yml step brings in the older WebAssembly.Sdk.

sdk/eng/build.yml

Lines 291 to 293 in 1924edc

- script: $(Build.SourcesDirectory)/artifacts/bin/redist/$(_BuildConfig)/dotnet/dotnet workload install wasm-tools --skip-manifest-update
displayName: Install wasm-tools Workload
continueOnError: false

…1206.7

Microsoft.Extensions.DependencyModel , Microsoft.NET.HostModel , Microsoft.NETCore.App.Host.win-x64 , Microsoft.NETCore.App.Ref , Microsoft.NETCore.App.Runtime.win-x64 , Microsoft.NETCore.DotNetHostResolver , Microsoft.NETCore.Platforms , System.CodeDom , System.Reflection.MetadataLoadContext , System.Resources.Extensions , System.Security.Cryptography.ProtectedData , System.Text.Encoding.CodePages , VS.Redist.Common.NetCore.SharedFramework.x64.8.0 , VS.Redist.Common.NetCore.TargetingPack.x64.8.0
 From Version 8.0.0-alpha.1.22605.1 -> To Version 8.0.0-alpha.1.22606.7
@radical
Copy link
Member

radical commented Dec 7, 2022

@ViktorHofer is this still failing? Anything I can do to help?

@ViktorHofer
Copy link
Member

Yeah it's still failing. Based on git blame, @marcpopMSFT was the last one who touched the code in the yml that causes the wrong version of the workload to come in. Marc, can you please take a look?

@joeloff
Copy link
Member

joeloff commented Dec 7, 2022

Is there a build for the failure? I don't see anything in Checks.

@marcpopMSFT
Copy link
Member

@joeloff the latest codeflow didn't trigger a build for some reason but the prior failed build is here: https://dev.azure.com/dnceng-public/public/_build/results?buildId=103129&view=results

@marcpopMSFT
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@marcpopMSFT
Copy link
Member

marcpopMSFT commented Dec 7, 2022

Yeah it's still failing. Based on git blame, @marcpopMSFT was the last one who touched the code in the yml that causes the wrong version of the workload to come in. Marc, can you please take a look?

I see two sets of errors that need to be investigated.
The first is loading system.text.json:
C:\h\w\AE1A098B\p\d\sdk\8.0.100-ci\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.Sdk.targets(654,5): error MSB4018: The "GenerateClsidMap" task failed unexpectedly. [C:\h\w\AE1A098B\t\dotnetSdkTests\xahsmptc.ow5\It_embeds_sin---38A77651\ComServerWithTypeLibs.csproj] C:\h\w\AE1A098B\p\d\sdk\8.0.100-ci\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.Sdk.targets(654,5): error MSB4018: System.IO.FileNotFoundException: Could not load file or assembly 'System.Text.Json, Version=7.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51' or one of its dependencies. The system cannot find the file specified. [C:\h\w\AE1A098B\t\dotnetSdkTests\xahsmptc.ow5\It_embeds_sin---38A77651\ComServerWithTypeLibs.csproj]

The second is what Viktor is raising in the blazorwasm aot tests. I see in the install logs that an 8.0 version of that file is getting installed: Installing pack Microsoft.NET.Runtime.WebAssembly.Sdk version 8.0.0-alpha.1.22559.2 @lewing @steveisok does the runtime workload need to be updated to pull in the right version of the runtime? /private/tmp/helix/working/B3F209A4/p/d/packs/Microsoft.NET.Runtime.WebAssembly.Sdk/8.0.0-alpha.1.22559.2/Sdk/WasmApp.Native.targets(293,5): error MSB4018: System.IO.DirectoryNotFoundException: Could not find a part of the path '/private/tmp/helix/working/B3F209A4/w/AE50091F/e/testExecutionDirectory/.nuget/packages/microsoft.netcore.app.runtime.mono.browser-wasm/8.0.0-alpha.1.22605.5/runtimes/browser-wasm/lib/net7.0/mscorlib.dll'

@radical
Copy link
Member

radical commented Dec 7, 2022

The second is what Viktor is raising in the blazorwasm aot tests. I see in the install logs that an 8.0 version of that file is getting installed: Installing pack Microsoft.NET.Runtime.WebAssembly.Sdk version 8.0.0-alpha.1.22559.2 @lewing @steveisok does the runtime workload need to be updated to pull in the right version of the runtime? /private/tmp/helix/working/B3F209A4/p/d/packs/Microsoft.NET.Runtime.WebAssembly.Sdk/8.0.0-alpha.1.22559.2/Sdk/WasmApp.Native.targets(293,5): error MSB4018: System.IO.DirectoryNotFoundException: Could not find a part of the path '/private/tmp/helix/working/B3F209A4/w/AE50091F/e/testExecutionDirectory/.nuget/packages/microsoft.netcore.app.runtime.mono.browser-wasm/8.0.0-alpha.1.22605.5/runtimes/browser-wasm/lib/net7.0/mscorlib.dll'

  1. Viktor's fix for the path is included in runtime packages version 8.0.0-alpha.1.22606.7, like it is being updated in this PR.
  2. global.json has:
$ cat global.json
{
  "tools": {
    "dotnet": "8.0.100-alpha.1.22579.5",
  1. The sdk above has .dotnet/sdk-manifests/8.0.100/microsoft.net.workload.mono.toolchain/WorkloadManifest.json which references:
    "Microsoft.NET.Runtime.WebAssembly.Sdk": {
      "kind": "Sdk",
      "version": "8.0.0-alpha.1.22559.2"
    },
  1. artifacts/bin/redist/Release/dotnet/dotnet workload install wasm-tools --skip-manifest-update
  • this is not updating the manifest, and thus uses the version from (3), which is older than (1), and is missing the required fix.

Updating the sdk version in global.json would fix this issue.

But is that the correct fix? Shouldn't the manifest be updated to match the versions received in this PR?

@marcpopMSFT
Copy link
Member

If there is an sdk with the updated baseline, we should just update the global.json. We specifically do a --skip-manifest update as we don't want to install a random version of the workloads (which is kind of what we'd get). We found previously that we would pick up the newest build from the feeds which sometimes had compatibility problems and were unrelated to the code flowing so we switched. We could potentially switch to a rollback file instead to try to match the runtime version that's flowing but we haven't had a need for that till now.

@radical
Copy link
Member

radical commented Dec 7, 2022

We could potentially switch to a rollback file instead to try to match the runtime version that's flowing but we haven't had a need for that till now.

Since you haven't seen the need for this yet, the current case seems like a rare one, so I'll update global.json here.

@radical
Copy link
Member

radical commented Dec 7, 2022

  • Um ok, so I tried 8.0.100-alpha.1.22607.4, which references WebAssembly.Sdk version 8.0.0-alpha.1.22605.1 in .dotnet/sdk-manifests/8.0.100/microsoft.net.workload.mono.toolchain/WorkloadManifest.json.

    • And that version is right behind the one containing the fix, and what we are getting in this PR!
    • And I guess, that makes sense - a new sdk with the updated manifest doesn't exist, because the updated manifest would be generated once this PR is merged!
  • Looking at the manifest in the generated sdk - ./artifacts/bin/redist/Release/dotnet/sdk-manifests/8.0.100/microsoft.net.workload.mono.toolchain/WorkloadManifest.json, even that has the same version. How is this manifest updated with the one generated by dotnet/runtime? Is the Microsoft.NET.Workload.Mono.ToolChain.Manifest-8.0.100*nupkg consumed to get that?

@marcpopMSFT
Copy link
Member

Codeflow to the installer repo should update the baseline manifest that comes with the sdk. I believe the test environment for the sdk repo will pull that manifest from the stage0 sdk since it's not built in the repo itself. So is this a chicken/egg problem where we need a manifest from installer with this runtime build but it's blocked by this PR?

@radical
Copy link
Member

radical commented Dec 7, 2022

So is this a chicken/egg problem where we need a manifest from installer with this runtime build but it's blocked by this PR?

Yeah, that's my understanding.

@marcpopMSFT
Copy link
Member

Try removing the --skip-manifest-update for now and then we can add it back once we have a new SDK? That'll have some risk as changes flow if there are bad builds out of runtime that break workloads though.

@radical
Copy link
Member

radical commented Dec 7, 2022

Removing it gets the latest package with the fix.

In general:

  • Using --skip-manifest-update makes sense
  • Maybe for testing, we should consider overwriting the manifests with those from the manifest nugets?

That'll have some risk as changes flow if there are bad builds out of runtime that break workloads though.

There is some protection at least, as we run lot of wasm tests using the workloads in dotnet/runtime, with the generated manifests, and other nugets, installed on top of the latest 8.0 sdk.

…1207.6

Microsoft.Extensions.DependencyModel , Microsoft.NET.HostModel , Microsoft.NETCore.App.Host.win-x64 , Microsoft.NETCore.App.Ref , Microsoft.NETCore.App.Runtime.win-x64 , Microsoft.NETCore.DotNetHostResolver , Microsoft.NETCore.Platforms , System.CodeDom , System.Reflection.MetadataLoadContext , System.Resources.Extensions , System.Security.Cryptography.ProtectedData , System.Text.Encoding.CodePages , VS.Redist.Common.NetCore.SharedFramework.x64.8.0 , VS.Redist.Common.NetCore.TargetingPack.x64.8.0
 From Version 8.0.0-alpha.1.22605.1 -> To Version 8.0.0-alpha.1.22607.6
@joeloff joeloff enabled auto-merge (squash) December 8, 2022 20:26
@dreddy-work
Copy link
Member

Seems tests assets still referencing .NET 7.0 and couldn't find binaries. @marcpopMSFT , is it time to change

public const string CurrentTargetFramework = "net7.0";
?

…1209.1

Microsoft.Extensions.DependencyModel , Microsoft.NET.HostModel , Microsoft.NETCore.App.Host.win-x64 , Microsoft.NETCore.App.Ref , Microsoft.NETCore.App.Runtime.win-x64 , Microsoft.NETCore.DotNetHostResolver , Microsoft.NETCore.Platforms , System.CodeDom , System.Reflection.MetadataLoadContext , System.Resources.Extensions , System.Security.Cryptography.ProtectedData , System.Text.Encoding.CodePages , VS.Redist.Common.NetCore.SharedFramework.x64.8.0 , VS.Redist.Common.NetCore.TargetingPack.x64.8.0
 From Version 8.0.0-alpha.1.22605.1 -> To Version 8.0.0-alpha.1.22609.1
@marcpopMSFT
Copy link
Member

Seems tests assets still referencing .NET 7.0 and couldn't find binaries. @marcpopMSFT , is it time to change

public const string CurrentTargetFramework = "net7.0";

?

Noah has started on that here: #29316 This is the first release we've had this mechanism but I don't think there is a sudden rush to update it. I had still been planning on transitioning after all the templates had been updated as I'm not sure we'll be able to have this work for any dotnet new tests until that time. At the moment, there are nearly a dozen templates still starting net7.0: https://github.com/dotnet/installer/blob/main/test/EndToEnd/ProjectBuildTests.cs#L399.
Pinging @vlada-shubina @wtgodbe to comment on any progress there since it seems to be web templates, core templates, and angular/react that are left.

@marcpopMSFT
Copy link
Member

For this PR the error is confusing as runtime already moved to 8.0 System.Text.Json in the host model weeks ago so not sure why it's looking for the 7.0 version still. https://github.com/dotnet/runtime/blame/main/eng/Versions.props#L139
C:\h\w\AA4B0966\p\d\sdk\8.0.100-ci\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.Sdk.targets(654,5): error MSB4018: The "GenerateClsidMap" task failed unexpectedly. [C:\h\w\AA4B0966\t\dotnetSdkTests\mtsim5cu.s2g\win10-x64_Giv---2747ACF9\ComServer.csproj] C:\h\w\AA4B0966\p\d\sdk\8.0.100-ci\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.Sdk.targets(654,5): error MSB4018: System.IO.FileNotFoundException: Could not load file or assembly 'System.Text.Json, Version=7.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51' or one of its dependencies. The system cannot find the file specified. [C:\h\w\AA4B0966\t\dotnetSdkTests\mtsim5cu.s2g\win10-x64_Giv---2747ACF9\ComServer.csproj] C:\h\w\AA4B0966\p\d\sdk\8.0.100-ci\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.Sdk.targets(654,5): error MSB4018: File name: 'System.Text.Json, Version=7.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51' [C:\h\w\AA4B0966\t\dotnetSdkTests\mtsim5cu.s2g\win10-x64_Giv---2747ACF9\ComServer.csproj]

@dsplaisted
Copy link
Member

@agocke @jtschuster @LakshanF @sbomer @vitek-karas This is now failing some single-file tests (looks like only targeting .NET 8) on Mac and Linux, with:

Exit Code: 137
StdOut:

StdErr:
Failed to create CoreCLR, HRESULT: 0x80131524

Can someone investigate? Should we disable those tests in the meantime to unblock the runtime flow?

@agocke
Copy link
Member

agocke commented Dec 15, 2022

@VSadov for single file

@kasperk81
Copy link
Contributor

@agocke those tests weren't failing until this update came along dotnet/runtime@c635ae2...322f20c

can it be dotnet/runtime@866f3eb?

@agocke
Copy link
Member

agocke commented Dec 15, 2022

Seems unlikely, that should have just avoided copying in the case that we aren't building the singlefilehost, which shouldn't happen in official builds where we build everything.

Even if it was missing, this is a strange failure case. I would expect it to have a missing file, not create an error.

…1215.5

Microsoft.Extensions.DependencyModel , Microsoft.NET.HostModel , Microsoft.NETCore.App.Host.win-x64 , Microsoft.NETCore.App.Ref , Microsoft.NETCore.App.Runtime.win-x64 , Microsoft.NETCore.DotNetHostResolver , Microsoft.NETCore.Platforms , System.CodeDom , System.Reflection.MetadataLoadContext , System.Resources.Extensions , System.Security.Cryptography.ProtectedData , System.Text.Encoding.CodePages , VS.Redist.Common.NetCore.SharedFramework.x64.8.0 , VS.Redist.Common.NetCore.TargetingPack.x64.8.0
 From Version 8.0.0-alpha.1.22605.1 -> To Version 8.0.0-alpha.1.22615.5
@dsplaisted
Copy link
Member

@agocke Would it be OK / a good idea to disable these failing tests and file a bug for them so that the runtime update can flow? This is blocking some other PRs.

@agocke
Copy link
Member

agocke commented Dec 16, 2022

I'll take a quick look and see if I can find anything interesting, and disable the test if I can't find anything.

@VSadov
Copy link
Member

VSadov commented Dec 16, 2022

It looks like something in net8.0 breaks singlefile apps on non-windows platforms. We do not see that in runtime repo though.
It can also be a result of some tool/libraries inconsistency and will be resolved once everything 8.0 is indeed 8.0

It seems ok to disable the test to make progress.

…1216.6

Microsoft.Extensions.DependencyModel , Microsoft.NET.HostModel , Microsoft.NETCore.App.Host.win-x64 , Microsoft.NETCore.App.Ref , Microsoft.NETCore.App.Runtime.win-x64 , Microsoft.NETCore.DotNetHostResolver , Microsoft.NETCore.Platforms , System.CodeDom , System.Reflection.MetadataLoadContext , System.Resources.Extensions , System.Security.Cryptography.ProtectedData , System.Text.Encoding.CodePages , VS.Redist.Common.NetCore.SharedFramework.x64.8.0 , VS.Redist.Common.NetCore.TargetingPack.x64.8.0
 From Version 8.0.0-alpha.1.22605.1 -> To Version 8.0.0-alpha.1.22616.6
…1217.4

Microsoft.Extensions.DependencyModel , Microsoft.NET.HostModel , Microsoft.NETCore.App.Host.win-x64 , Microsoft.NETCore.App.Ref , Microsoft.NETCore.App.Runtime.win-x64 , Microsoft.NETCore.DotNetHostResolver , Microsoft.NETCore.Platforms , System.CodeDom , System.Reflection.MetadataLoadContext , System.Resources.Extensions , System.Security.Cryptography.ProtectedData , System.Text.Encoding.CodePages , VS.Redist.Common.NetCore.SharedFramework.x64.8.0 , VS.Redist.Common.NetCore.TargetingPack.x64.8.0
 From Version 8.0.0-alpha.1.22605.1 -> To Version 8.0.0-alpha.1.22617.4
ViktorHofer added a commit to dotnet/runtime that referenced this pull request Dec 19, 2022
97a51cc regressed the inclusion of source generators in the targeting pack as the `netstandard2.0` inner build was never chosen by the sfx-gen.proj traversal project. Noticed in the SDK consumption PR: dotnet/sdk#29406
@ViktorHofer
Copy link
Member

The latest error will be fixed with dotnet/runtime#79810

ViktorHofer added a commit to dotnet/runtime that referenced this pull request Dec 19, 2022
97a51cc regressed the inclusion of source generators in the targeting pack as the `netstandard2.0` inner build was never chosen by the sfx-gen.proj traversal project. Noticed in the SDK consumption PR: dotnet/sdk#29406
…1218.1

Microsoft.Extensions.DependencyModel , Microsoft.NET.HostModel , Microsoft.NETCore.App.Host.win-x64 , Microsoft.NETCore.App.Ref , Microsoft.NETCore.App.Runtime.win-x64 , Microsoft.NETCore.DotNetHostResolver , Microsoft.NETCore.Platforms , System.CodeDom , System.Reflection.MetadataLoadContext , System.Resources.Extensions , System.Security.Cryptography.ProtectedData , System.Text.Encoding.CodePages , VS.Redist.Common.NetCore.SharedFramework.x64.8.0 , VS.Redist.Common.NetCore.TargetingPack.x64.8.0
 From Version 8.0.0-alpha.1.22605.1 -> To Version 8.0.0-alpha.1.22618.1
…1219.1

Microsoft.Extensions.DependencyModel , Microsoft.NET.HostModel , Microsoft.NETCore.App.Host.win-x64 , Microsoft.NETCore.App.Ref , Microsoft.NETCore.App.Runtime.win-x64 , Microsoft.NETCore.DotNetHostResolver , Microsoft.NETCore.Platforms , System.CodeDom , System.Reflection.MetadataLoadContext , System.Resources.Extensions , System.Security.Cryptography.ProtectedData , System.Text.Encoding.CodePages , VS.Redist.Common.NetCore.SharedFramework.x64.8.0 , VS.Redist.Common.NetCore.TargetingPack.x64.8.0
 From Version 8.0.0-alpha.1.22605.1 -> To Version 8.0.0-alpha.1.22619.1
@kasperk81
Copy link
Contributor

    Microsoft.NET.Build.Tests.GivenThatWeWantToUseAnalyzers.It_resolves_analyzers_correctly(language: "C#", testAssetName: "AppWithLibrary") [FAIL]
      Expected root to be a collection with 10 item(s), but
      "{(microsoft.net.sdk, , analyzers/Microsoft.CodeAnalysis.CSharp.NetAnalyzers.dll), (microsoft.net.sdk, , analyzers/Microsoft.CodeAnalysis.NetAnalyzers.dll), (microsoft.codequality.analyzers, 2.6.0, analyzers/dotnet/cs/Microsoft.CodeQuality.Analyzers.dll), (microsoft.codequality.analyzers, 2.6.0, analyzers/dotnet/cs/Microsoft.CodeQuality.CSharp.Analyzers.dll), (microsoft.dependencyvalidation.analyzers, 0.9.0, analyzers/dotnet/Microsoft.DependencyValidation.Analyzers.dll), (microsoft.netcore.app.ref, , analyzers/dotnet/cs/Microsoft.Interop.JavaScript.JSImportGenerator.dll), (microsoft.netcore.app.ref, , analyzers/dotnet/cs/Microsoft.Interop.LibraryImportGenerator.dll), (microsoft.netcore.app.ref, , analyzers/dotnet/cs/Microsoft.Interop.SourceGeneration.dll), (microsoft.netcore.app.ref, , analyzers/dotnet/cs/System.Text.RegularExpressions.Generator.dll)}"
      "contains 1 item(s) less than"
      "{(microsoft.net.sdk, , analyzers/Microsoft.CodeAnalysis.CSharp.NetAnalyzers.dll), (microsoft.net.sdk, , analyzers/Microsoft.CodeAnalysis.NetAnalyzers.dll), (microsoft.netcore.app.ref, , analyzers/dotnet/cs/System.Text.Json.SourceGeneration.dll), (microsoft.netcore.app.ref, , analyzers/dotnet/cs/System.Text.RegularExpressions.Generator.dll), (microsoft.codequality.analyzers, 2.6.0, analyzers/dotnet/cs/Microsoft.CodeQuality.Analyzers.dll), (microsoft.codequality.analyzers, 2.6.0, analyzers/dotnet/cs/Microsoft.CodeQuality.CSharp.Analyzers.dll), (microsoft.dependencyvalidation.analyzers, 0.9.0, analyzers/dotnet/Microsoft.DependencyValidation.Analyzers.dll), (microsoft.netcore.app.ref, , analyzers/dotnet/cs/Microsoft.Interop.LibraryImportGenerator.dll), (microsoft.netcore.app.ref, , analyzers/dotnet/cs/Microsoft.Interop.JavaScript.JSImportGenerator.dll), (microsoft.netcore.app.ref, , analyzers/dotnet/cs/Microsoft.Interop.SourceGeneration.dll)}.

@kasperk81
Copy link
Contributor

nowadays, multiple (7?) batch builds are created from dotnet/runtime daily but darc is configured to pick once per day (around 13:00-14:00 utc). apparently this policy was set long time ago when runtime used to have one official batch build a day. given the update blockers we frequently see (say in past year or two), are we happy with this frequency? all other repos participating in codeflow push their updates many times a day.

problem is when something breaks in runtime, by the time fix is merged, if it misses the last batch build before the darc update runs it dealys the update by 24 hours during which something else can (and does) easily break. then it continues to pile up issues as we can see in this pr and other dotnet/runtime updates in the past year.

(previous discussion: dotnet/installer#13376 (comment))

@ViktorHofer
Copy link
Member

ViktorHofer commented Dec 19, 2022

You can see that we got multiple dependency flow updates today because I triggered the subscription manually. We can of course change the interval but from my perspective that isn't necessary as whenever I need to ingest a fix, I just retrigger the subscription.

…1219.2

Microsoft.Extensions.DependencyModel , Microsoft.NET.HostModel , Microsoft.NETCore.App.Host.win-x64 , Microsoft.NETCore.App.Ref , Microsoft.NETCore.App.Runtime.win-x64 , Microsoft.NETCore.DotNetHostResolver , Microsoft.NETCore.Platforms , System.CodeDom , System.Reflection.MetadataLoadContext , System.Resources.Extensions , System.Security.Cryptography.ProtectedData , System.Text.Encoding.CodePages , VS.Redist.Common.NetCore.SharedFramework.x64.8.0 , VS.Redist.Common.NetCore.TargetingPack.x64.8.0
 From Version 8.0.0-alpha.1.22605.1 -> To Version 8.0.0-alpha.1.22619.2
@kasperk81
Copy link
Contributor

kasperk81 commented Dec 19, 2022

what are the pros and cons of fast intervals automatically triggering darc update for each batch build?

pros: less manual work, no need for human action, if something is failing we know it sooner, short window means less chances of something else regressing, aligns runtime update interval with other repos

cons: ?

@joeloff joeloff merged commit ac93651 into main Dec 19, 2022
@joeloff joeloff deleted the darc-main-b48c83eb-437c-499e-be77-c6d84240e7ba branch December 19, 2022 22:14
@ViktorHofer
Copy link
Member

cons: ?

One problem that we had with per commit batched subscriptions is that they were constantly adding to the PR which then never had a chance to finish validation before another commit came in.

@kasperk81
Copy link
Contributor

per commit batched subscriptions

per commit'd be too much. batch builds are triggered in runtime repo every 4 or 6 hours and not per commit, how was it colliding?

what we see is aspnetcore, msbuild, winforms and every repo but runtime have set their subscription so when there's a new batch build (every few hours), darc is free to push an update to sdk. it's enough time for ci to complete.

@akoeplinger
Copy link
Member

akoeplinger commented Dec 21, 2022

Every 4 or 6 hours is still quite often when you're trying to get a dependency update through and need to rerun jobs to deal with flaky tests, infrastructure issues etc and it's very annoying for the PR builds to be restarted just because darc pushed an update.

Having to manually trigger an update when you really need to get a change in sooner has not been an issue in my experience.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.