-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Bump versions of maintenance-packages dependencies consumed in machin… #7274
Bump versions of maintenance-packages dependencies consumed in machin… #7274
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7274 +/- ##
==========================================
+ Coverage 68.80% 68.90% +0.09%
==========================================
Files 1461 1467 +6
Lines 272400 274380 +1980
Branches 28176 28642 +466
==========================================
+ Hits 187436 189066 +1630
- Misses 77729 77983 +254
- Partials 7235 7331 +96
Flags with carried forward coverage won't be shown. Click here to find out more. |
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.
Update to latest versions
@michaelgsharp there are several dead-lettering failures. Can you take a look? Maybe the VM queues need updating? |
/azp run |
Azure Pipelines successfully started running 2 pipeline(s). |
The new version of |
I found these two lines hardcoding the package version, which should now be 4.6.0: machinelearning/test/Microsoft.ML.CodeAnalyzer.Tests/Helpers/AdditionalMetadataReferences.cs Lines 17 to 23 in 5c50319
|
Those are just for testing the analyzer. Here the problem is due to RemoteExecutor. The .dll.config has the correct redirect. If you download the helix payload and examine <dependentAssembly>
<assemblyIdentity name="System.Memory" publicKeyToken="cc7b13ffcd2ddd51" culture="neutral" />
<bindingRedirect oldVersion="0.0.0.0-4.0.2.0" newVersion="4.0.2.0" />
</dependentAssembly> But that .dll.config isn't being reused with RemoteExecutor. |
This same issue was hit here: dotnet/runtime#104647 @ViktorHofer mentions that it should be handled with https://github.com/dotnet/arcade/blob/fc2f7ce8372a55725aab7b48c25bad7327a9769d/src/Microsoft.DotNet.RemoteExecutor/src/build/Microsoft.DotNet.RemoteExecutor.targets#L8-L33 -- it looks like ML.NET doesn't use that remote executor, but still uses it's own which may not have the fix. So that's the "right fix" here -- make the ML.NET remoteexecutor honor the tests redirects. If I look at the assembly refs here, most are coming from the |
We don't use the RemoteExecutor in arcade because it doens't support NetFX, so we still have our custom one. Let me take a look at @ViktorHofer's fix and see what it would take to adopt it as well. |
@michaelgsharp RemoteExecutor in Arcade supports .NET Framework. A lot of the tests in runtime target framework and depend on RemoteExecutor. |
Closing as #7295 takes care of it and the remote executor update. |
dotnet/runtime depends on these packages that we are now publishing from dotnet/maintenance-packages:
Bumping their versions to consume the new preview versions, available in the dotnet-libraries feed: https://dnceng.visualstudio.com/public/_artifacts/feed/dotnet-libraries
Related PRs: