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

[libraries] Restrict ILLink Suppression xml to mobile platforms #58519

Conversation

mdh1418
Copy link
Member

@mdh1418 mdh1418 commented Sep 1, 2021

@ghost ghost added the linkable-framework Issues associated with delivering a linker friendly framework label Sep 1, 2021
@ghost
Copy link

ghost commented Sep 1, 2021

Tagging subscribers to 'linkable-framework': @eerhardt, @vitek-karas, @LakshanF, @sbomer, @joperezr
See info in area-owners.md if you want to be subscribed.

Issue Details

Follow up to https://github.com/dotnet/runtime/pull/58456/files#r700368913

Author: mdh1418
Assignees: -
Labels:

linkable-framework

Milestone: -

@@ -30,7 +30,7 @@
<_RuntimePackSuppressionsXmls Include="$(ILLinkTrimAssemblyRuntimePackSuppressionsXmlsDir)*.xml" />

<!-- Collate CoreLib suppression XML files not bin-placed in earlier per-library linker run. CoreLib doesn't use bin-place logic. -->
<_RuntimePackSuppressionsXmls Include="$(CoreLibSharedDir)ILLink\ILLink.Suppressions.LibraryBuild.xml" />
<_RuntimePackSuppressionsXmls Condition="'$(TargetsMobile)' == 'true'" Include="$(CoreLibSharedDir)ILLink\ILLink.Suppressions.LibraryBuild.xml" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't going to work. This line is for CoreLib, but the file in question is for System.Net.Http.

My thinking was to rename https://github.com/dotnet/runtime/blob/main/src/libraries/System.Net.Http/src/ILLink/ILLink.Suppressions.LibraryBuild.xml to https://github.com/dotnet/runtime/blob/main/src/libraries/System.Net.Http/src/ILLink/ILLink.Suppressions.Mobile.LibraryBuild.xml

And then in System.Net.Http/src/System.Net.Http.csproj add:

<ItemGroup>
  <ILLinkSuppressionsXmls Condition="'$(TargetsMobile)' == 'true'" Include="$(ILLinkDirectory)ILLink.Suppressions.Mobile.LibraryBuild.xml" />

Copy link
Member

@eerhardt eerhardt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@marek-safar marek-safar merged commit 791a0d8 into dotnet:main Sep 6, 2021
@marek-safar marek-safar deleted the condition_xml_suppression_system_net_http_to_mobile_only branch September 6, 2021 09:04
mdh1418 added a commit to mdh1418/runtime that referenced this pull request Sep 9, 2021
…et#58519)

Co-authored-by: Mitchell Hwang <mitchell.hwang@microsoft.com>
steveisok pushed a commit that referenced this pull request Sep 13, 2021
…t and test fixes for Android (#58841)

Manual backport of #57208, #58519, #58562, #58210, #57732, #58428, #58586, #58745, #57687 to release/6.0

Numerous test suites have been failing for iOS/tvOS/MacCatalyst consistently on CI without useful logs as to why. Moreover, some of these suites pass locally.

This PR looks to reduce the failures on CI by skipping the problematic suites
Skips test suites logged in #53624

ActiveIssues
#58440
#58418
#58367
#58584

Co-authored-by: Mitchell Hwang <mitchell.hwang@microsoft.com>
Co-authored-by: Alexander Köplinger <alex.koeplinger@outlook.com>
Co-authored-by: Jo Shields <directhex@apebox.org>
@ghost ghost locked as resolved and limited conversation to collaborators Oct 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Infrastructure-libraries linkable-framework Issues associated with delivering a linker friendly framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants