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.

Sign up for GitHub

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

Jump to bottom

Libraries are missing Unsupported platforms annotations when implementation exists for multiple RIDs #49278

Closed
marek-safar opened this issue Mar 7, 2021 · 10 comments
Closed

Libraries are missing Unsupported platforms annotations when implementation exists for multiple RIDs #49278

marek-safar opened this issue Mar 7, 2021 · 10 comments
Assignees
@jeffhandley
Labels
area-Infrastructure-libraries Priority:1 Work that is critical for the release, but we could probably ship without
Milestone
6.0.0

Comments

@marek-safar
Copy link
Contributor

marek-safar commented Mar 7, 2021 •
edited
Loading

Unsupported ref libraries are not correctly annotated or not annotated at all when complex build conditions exist.

I don't know what's the underlying condition but it seems to happen for at least a few in the current build

  • System.Security.Cryptography.Algorithms
    • No UnsupportedOSPlatform/SupportedOSPlatform attributes exist in ref assembly even though the assembly is not supported for example on Android
  • System.Net.Quic
    • No UnsupportedOSPlatform/SupportedOSPlatform attributes exist in ref assembly even though the assembly is not supported for example on Browser
  • System.IO.Ports
    • Only UnsupportedOSPlatform("browser") attribute exists in ref assembly but the assembly is also not supported for example on iOS

This also caused #49201 problem and I checked only a few libraries so not sure how widespread the problem is.

@jeffhandley @buyaa-n
@dotnet-issue-labeler dotnet-issue-labeler bot added area-Infrastructure-libraries untriaged New issue has not been triaged by the area owner labels Mar 7, 2021
@ghost
Copy link

ghost commented Mar 7, 2021

Tagging subscribers to this area: @safern, @ViktorHofer, @Anipik
See info in area-owners.md if you want to be subscribed.

Issue Details

Unsupported ref libraries are not correctly annotated or not annotated at all when complex build conditions exist.

I don't know what's the underlying condition but it seems to happen for at least a few in the current build

  • System.Security.Cryptography.Algorithms
    • No UnsupportedOSPlatform/SupportedOSPlatform attributes exist in ref assembly even though the assembly is not supported for example on Android
  • System.Net.Quic
    • No UnsupportedOSPlatform/SupportedOSPlatform attributes exist in ref assembly even though the assembly is not supported for example on Browser
  • System.IO.Ports
    • Only UnsupportedOSPlatform("browser") attribute exists in ref assembly but the assembly is also not supported for example on iOS

This is caused #49201 problem and I checked only a few libraries so not sure how widespread the problem is.

@jeffhandley @buyaa-n

Author: marek-safar
Assignees: -
Labels:

area-Infrastructure-libraries, untriaged
Milestone: -

@marek-safar marek-safar changed the title Libraries are missing Unsupported platforms annotations when build for multiple RIDs Libraries are missing Unsupported platforms annotations when implementation exists for multiple RIDs Mar 7, 2021
@buyaa-n
Copy link
Contributor

buyaa-n commented Mar 8, 2021 •
edited
Loading

I think in .NET 5.0, we only annotated Windows only and unsupported on Browser APIs completely. Very few annotations made for other OS in 5.0, we planned that work for 6.0 with #47228 and #44916

@marek-safar
Copy link
Contributor Author

I think in .NET 5.0

This is .NET6 P1 issue not 5.0.

@marek-safar marek-safar mentioned this issue Mar 9, 2021
Merged
@jeffhandley jeffhandley self-assigned this Mar 12, 2021
@jeffhandley jeffhandley added the Priority:1 Work that is critical for the release, but we could probably ship without label Mar 12, 2021
@buyaa-n buyaa-n removed the untriaged New issue has not been triaged by the area owner label Mar 12, 2021
@ViktorHofer ViktorHofer added this to the 6.0.0 milestone Mar 24, 2021
@marek-safar marek-safar mentioned this issue Apr 1, 2021
Closed
@buyaa-n
Copy link
Contributor

buyaa-n commented Apr 1, 2021

  • System.Security.Cryptography.Algorithms
    • No UnsupportedOSPlatform/SupportedOSPlatform attributes exist in ref assembly even though the assembly is not supported for example on Android

It does have several targets including Android, doesn't have a cross-platform target, for now, i could suggest only add SupportedOSPlatform for each target (Windows, Unix, Android, OSX, iOS, tvOS, Browser), i might have more information after PNSE analysis for targeted builds cc @bartonjs

  • System.Net.Quic
    • No UnsupportedOSPlatform/SupportedOSPlatform attributes exist in ref assembly even though the assembly is not supported for example on Browser

Could add assembly level SupportedOSPlaform("windows"), SupportedOSPlaform("Linux"), SupportedOSPlaform("OSX"), SupportedOSPlaform("FreeBSD") attributes

  • System.IO.Ports
    • Only UnsupportedOSPlatform("browser") attribute exists in ref assembly but the assembly is also not supported for example on iOS

Could add assembly level SupportedOSPlaform("windows"), SupportedOSPlaform("Linux"), SupportedOSPlaform("OSX"), SupportedOSPlaform("FreeBSD") attributs

@marek-safar
Copy link
Contributor Author

I'm not following the suggestion. Are you saying we can no longer rely on UnsupportedOSPlatform/SupportedOSPlatform attribute to be automatically generated from implementation libraries and we have to add them manually for each library? How are we going to keep them in-sync?

@buyaa-n
Copy link
Contributor

buyaa-n commented Apr 1, 2021 •
edited
Loading

It will not generated in the Cross-platform target for example for Quic $(NetCoreAppCurrent)-windows;$(NetCoreAppCurrent)-Linux;$(NetCoreAppCurrent)-OSX;$(NetCoreAppCurrent)-FreeBSD; targets will have their own Supported attributes but there will be no attribute for $(NetCoreAppCurrent) build which is i guess the official one, the ref assembly of Quic only have $(NetCoreAppCurrent) so it also will not have any attribute

@buyaa-n
Copy link
Contributor

buyaa-n commented Apr 1, 2021 •
edited
Loading

Are you saying we can no longer rely on UnsupportedOSPlatform/SupportedOSPlatform attribute to be automatically generated from implementation libraries and we have to add them manually for each library

As far as i know, we have added assembly-level Supported attribute only for Windows-specific projects:

runtime/eng/versioning.targets

Lines 28 to 33 in c1b3d56

<!-- Adds SupportedOSPlatform attribute for Windows Specific libraries -->
<ItemGroup Condition="'$(IsWindowsSpecific)' == 'true' and '$(IsTestProject)' != 'true' and '$(IncludePlatformAttributes)' != 'false'">
<AssemblyAttribute Include="System.Runtime.Versioning.SupportedOSPlatform">
<_Parameter1>windows</_Parameter1>
</AssemblyAttribute>
</ItemGroup>

and for Unsupported attribute only for Browser unsupport:

runtime/eng/versioning.targets

Lines 69 to 76 in c1b3d56

<Target Name="AddUnsupportedOSPlatformAttribute" BeforeTargets="GenerateAssemblyInfo" AfterTargets="PrepareForBuild"
Condition="'@(_unsupportedOSPlatforms)' != '' and '$(TargetsAnyOS)' == 'true' and '$(IsTestProject)' != 'true' and '$(TargetFrameworkIdentifier)' != '.NETFramework'">
<ItemGroup>
<AssemblyAttribute Include="System.Runtime.Versioning.UnsupportedOSPlatform">
<_Parameter1>%(_unsupportedOSPlatforms.Identity)</_Parameter1>
</AssemblyAttribute>
</ItemGroup>
</Target>

well here unsupported attribute not only for Browser but we used it only for browser (only one windows case), those are still working as expected

@marek-safar
Copy link
Contributor Author

Got it, so the information needs to be added to both .props and .csproj files for each project

@buyaa-n
Copy link
Contributor

buyaa-n commented Apr 1, 2021

Not sure how exactly we would add them, could do as you mentioned add the attributes for each project, probably that is better for now as we don't have so many projects to cover.

Or we could add a general rule as we did for Unsupported attributes and add corresponding property into .props for each project

runtime/eng/versioning.targets

Lines 69 to 76 in c1b3d56

<Target Name="AddUnsupportedOSPlatformAttribute" BeforeTargets="GenerateAssemblyInfo" AfterTargets="PrepareForBuild"
Condition="'@(_unsupportedOSPlatforms)' != '' and '$(TargetsAnyOS)' == 'true' and '$(IsTestProject)' != 'true' and '$(TargetFrameworkIdentifier)' != '.NETFramework'">
<ItemGroup>
<AssemblyAttribute Include="System.Runtime.Versioning.UnsupportedOSPlatform">
<_Parameter1>%(_unsupportedOSPlatforms.Identity)</_Parameter1>
</AssemblyAttribute>
</ItemGroup>
</Target>

@marek-safar
Copy link
Contributor Author

I added them manually and there are not that many but we should think about how to keep them in sync.

@ghost ghost locked as resolved and limited conversation to collaborators May 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@jeffhandley jeffhandley

Labels
area-Infrastructure-libraries Priority:1 Work that is critical for the release, but we could probably ship without
Projects
None yet
Milestone
6.0.0
Development

No branches or pull requests
4 participants
@marek-safar @jeffhandley @ViktorHofer @buyaa-n