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

Mark DirectoryServices CAS APIs as Obsolete #40756

Merged
merged 5 commits into from
Aug 14, 2020
Merged

Mark DirectoryServices CAS APIs as Obsolete #40756

merged 5 commits into from
Aug 14, 2020

Conversation

jeffhandley
Copy link
Member

Fixes #39413. Per dotnet/designs#139, we want to mark System.DirectoryServices.DirectoryServicesPermission and System.DirectoryServices.DirectoryServicesPermissionAttribute as obsolete in .NET 5 (as part of obsoleting the Code Access Security APIs).

These two typeswere not marked in #39269 because System.DirectoryServices does not have a build specific to .NET 5.0, as was reported in #39401. The discussion there landed on a solution for these APIs of using an internal ObsoleteAttribute within System.DirectoryServices, so long as the tooling would respect the internal attribute--it does.

The approach taken here is to:

  1. Link ObsoleteAttribute.cs into System.DirectoryServices (src and ref projects)
  2. Conditionally use internal for ObsoleteAttribute when not built as part of corelib
  3. Suppress the warning about ObsoleteAttribute being used locally instead of from the reference
  4. Link Obsoletions.cs into System.DirectoryServices (src)
  5. Mark the APIs as obsolete
  6. Because System.DirectoryServices hasn't yet been annotated for nullability, we also need to include #nullable enable in the ObsoleteAttribute.cs within the #ifdef.

@jeffhandley jeffhandley requested review from jkotas and safern August 13, 2020 06:48
@Dotnet-GitSync-Bot
Copy link
Collaborator

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@Dotnet-GitSync-Bot
Copy link
Collaborator

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@jeffhandley
Copy link
Member Author

There are a bunch of failures like:

src/libraries/shims/ApiCompat.proj(116,5): error : (NETCORE_ENGINEERING_TELEMETRY=Build) CannotRemoveAttribute : Attribute 'System.ComponentModel.TypeConverterAttribute' exists on 'System.ComponentModel.MarshalByValueComponent' in the contract but not the implementation.

I'm unsure if/how this PR could have caused that; can someone with more infrastructure and/or ApiCompat knowledge assess that?

@jkotas
Copy link
Member

jkotas commented Aug 13, 2020

There are a bunch of failures like:

This is #40416 . It is taking down a large share of PRs.

Copy link
Member

@joperezr joperezr left a comment

Choose a reason for hiding this comment

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

Other than addressing @jkotas comments this looks good.

Copy link
Member

@safern safern left a comment

Choose a reason for hiding this comment

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

Other than @jkotas feedback and probably adding a shared hook to include the ObsoleteAttributes (like we do for PlatformAttributes), looks good.

@jeffhandley jeffhandley merged commit f0461cb into dotnet:master Aug 14, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
@jeffhandley jeffhandley deleted the obsolete-directoryservices branch January 30, 2021 21:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mark DirectoryServicesPermission and DirectoryServicesPermissionAttribute as Obsolete
5 participants