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/linker #24024

Merged
merged 11 commits into from
Feb 25, 2022

Conversation

dotnet-maestro[bot]
Copy link
Contributor

@dotnet-maestro dotnet-maestro bot commented Feb 19, 2022

This pull request updates the following dependencies

From /~https://github.com/dotnet/linker

  • Subscription: 4a865c13-5e52-41f5-3916-08d8e9750bf8
  • Build: 20220224.4
  • Date Produced: February 25, 2022 2:17:03 AM UTC
  • Commit: 24373afb86622c2c4130699766518d4950b7f827
  • Branch: refs/heads/main

…218.2

Microsoft.NET.ILLink.Analyzers , Microsoft.NET.ILLink.Tasks
 From Version 7.0.100-1.22118.1 -> To Version 7.0.100-1.22118.2
@dotnet-maestro
Copy link
Contributor Author

Notification for subscribed users from /~https://github.com/dotnet/linker:

@marek-safar

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/linker) 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

…221.1

Microsoft.NET.ILLink.Analyzers , Microsoft.NET.ILLink.Tasks
 From Version 7.0.100-1.22118.1 -> To Version 7.0.100-1.22121.1
…221.2

Microsoft.NET.ILLink.Analyzers , Microsoft.NET.ILLink.Tasks
 From Version 7.0.100-1.22118.1 -> To Version 7.0.100-1.22121.2
…222.1

Microsoft.NET.ILLink.Analyzers , Microsoft.NET.ILLink.Tasks
 From Version 7.0.100-1.22118.1 -> To Version 7.0.100-1.22122.1
…223.1

Microsoft.NET.ILLink.Analyzers , Microsoft.NET.ILLink.Tasks
 From Version 7.0.100-1.22118.1 -> To Version 7.0.100-1.22123.1
…223.2

Microsoft.NET.ILLink.Analyzers , Microsoft.NET.ILLink.Tasks
 From Version 7.0.100-1.22118.1 -> To Version 7.0.100-1.22123.2
@dsplaisted
Copy link
Member

@vitek-karas @AntonLapounov It looks like there may be a lot of flaky errors, but this one seems real. Could you take a look?

Microsoft.NET.Publish.Tests.GivenThatWeWantToRunILLink.ILLink_verify_analysis_warnings_hello_world_app_trim_mode_copyused

Target framework from test: net7.0
Runtime identifier: osx.10.15-x64
Runtime Assembly Informational Version: 7.0.0-preview.2.22116.9+793185a23907b941787fa5e9dab1524d0063df1f
The execution of a hello world app generated a diff in the number of warnings the app produces

Test output contained the following extra linker warnings:

ILLink : Trim analysis warning IL2026: System.Resources.ManifestBasedResourceGroveler.CreateResourceSet(Stream, Assembly): Using member 'System.Resources.ManifestBasedResourceGroveler.InternalGetResourceSetFromSerializedData(Stream, String, String, ResourceManager.ResourceManagerMediator)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. The CustomResourceTypesSupport feature switch has been enabled for this app which is being trimmed. Custom readers as well as custom objects on the resources file are not observable by the trimmer and so required assemblies, types and members may be removed. [/private/tmp/helix/working/AE230964/w/AC78094D/e/testExecutionDirectory/ILLink_verify---50627DEE/AnalysisWarningsOnHelloWorldApp/AnalysisWarningsOnHelloWorldApp.csproj]
ILLink : Trim analysis warning IL2063: System.RuntimeType.GetInterface(String, Boolean): Value returned from method 'System.RuntimeType.GetInterface(String, Boolean)' can not be statically determined and may not meet 'DynamicallyAccessedMembersAttribute' requirements. [/private/tmp/helix/working/AE230964/w/AC78094D/e/testExecutionDirectory/ILLink_verify---50627DEE/AnalysisWarningsOnHelloWorldApp/AnalysisWarningsOnHelloWorldApp.csproj]
Expected: True
Actual: False

The new version of ILLink slightly changes the formatting of warning messages (we add space after comma in a method signature). This has very low probablity of actually breaking anybody, but the test infra compares strings directly, so it noticed.
This change updates the expected warnings with the new format.

It also removes an expected warning which is no longer generated.
@vitek-karas
Copy link
Member

I pushed a commit which fixes some of the warnings (we changed the text format of the warning message slightly).
This leaves two warnings which are not expected. These were already addressed in runtime in dotnet/runtime#65580.
We will need to wait for the runtime with that change in it to make its way to the SDK repo. So once #24009 merges this should be fixed as well.

@sbomer, @tlakollo could you please review the test fix in 3f4ffad?

@tlakollo
Copy link
Contributor

Looks good to me, although the runtime build is failing, we might want to add the additional warnings to unblock sdk. We allow missing warnings so eventually we would only comeback and remove them once runtime has the fix

dotnet-maestro bot and others added 4 commits February 24, 2022 19:42
…224.1

Microsoft.NET.ILLink.Analyzers , Microsoft.NET.ILLink.Tasks
 From Version 7.0.100-1.22118.1 -> To Version 7.0.100-1.22124.1
…224.3

Microsoft.NET.ILLink.Analyzers , Microsoft.NET.ILLink.Tasks
 From Version 7.0.100-1.22118.1 -> To Version 7.0.100-1.22124.3
…224.4

Microsoft.NET.ILLink.Analyzers , Microsoft.NET.ILLink.Tasks
 From Version 7.0.100-1.22118.1 -> To Version 7.0.100-1.22124.4
…going to be supressed from runtime and they can be removed from the test
@tlakollo
Copy link
Contributor

I added the additional warnings, seems like the runtime issue is still unresolved and won't be able to merge for now. This PR is blocking already #24091 so better to get it going

@tlakollo tlakollo enabled auto-merge (squash) February 25, 2022 08:42
@tlakollo tlakollo merged commit ee1eafa into main Feb 25, 2022
@dotnet-maestro dotnet-maestro bot deleted the darc-main-45c076ff-2891-4b00-bb15-0ce8a5b075f4 branch February 25, 2022 08:56
@sbomer
Copy link
Member

sbomer commented Feb 25, 2022

Thanks @tlakollo!

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.

4 participants