Remove S.S.Permissions reference from S.C.ConfigurationManager #82259

steveharter · 2023-02-16T20:22:05Z

Contributes to #64592

This PR, if approved, will replace #80347 since it is less risky by not requiring removing public members. This is possible since it turns out S.S.P does not contain the implementation of the PermissionSet class (it's in corelib).

The use of PrivateAssets="all" allows the reference to the S.S.P package to be removed from the netstandard version of the S.C.CM package.

This will still be considered a breaking change, but only because a package reference was removed.

The System.Configuration.ConfigurationManager .nuspec goes from:

    <dependencies>
      <group targetFramework=".NETFramework4.6.2">
        <dependency id="System.Security.Permissions" version="8.0.0-dev" exclude="Build,Analyzers" />
      </group>
      <group targetFramework="net6.0">
        <dependency id="System.Security.Cryptography.ProtectedData" version="8.0.0-dev" exclude="Build,Analyzers" />
        <dependency id="System.Security.Permissions" version="8.0.0-dev" exclude="Build,Analyzers" />
      </group>
      <group targetFramework="net7.0">
        <dependency id="System.Diagnostics.EventLog" version="8.0.0-dev" exclude="Build,Analyzers" />
        <dependency id="System.Security.Cryptography.ProtectedData" version="8.0.0-dev" exclude="Build,Analyzers" />
        <dependency id="System.Security.Permissions" version="8.0.0-dev" exclude="Build,Analyzers" />
      </group>
      <group targetFramework="net8.0">
        <dependency id="System.Diagnostics.EventLog" version="8.0.0-dev" exclude="Build,Analyzers" />
        <dependency id="System.Security.Cryptography.ProtectedData" version="8.0.0-dev" exclude="Build,Analyzers" />
        <dependency id="System.Security.Permissions" version="8.0.0-dev" exclude="Build,Analyzers" />
      </group>
      <group targetFramework=".NETStandard2.0">
        <dependency id="System.Security.Cryptography.ProtectedData" version="8.0.0-dev" exclude="Build,Analyzers" />
        <dependency id="System.Security.Permissions" version="8.0.0-dev" exclude="Build,Analyzers" />
      </group>
    </dependencies>

to:

    <dependencies>
      <group targetFramework=".NETFramework4.6.2" />
      <group targetFramework="net6.0">
        <dependency id="System.Security.Cryptography.ProtectedData" version="8.0.0-dev" exclude="Build,Analyzers" />
      </group>
      <group targetFramework="net7.0">
        <dependency id="System.Diagnostics.EventLog" version="8.0.0-dev" exclude="Build,Analyzers" />
        <dependency id="System.Security.Cryptography.ProtectedData" version="8.0.0-dev" exclude="Build,Analyzers" />
      </group>
      <group targetFramework="net8.0">
        <dependency id="System.Diagnostics.EventLog" version="8.0.0-dev" exclude="Build,Analyzers" />
        <dependency id="System.Security.Cryptography.ProtectedData" version="8.0.0-dev" exclude="Build,Analyzers" />
      </group>
      <group targetFramework=".NETStandard2.0">
        <dependency id="System.Security.Cryptography.ProtectedData" version="8.0.0-dev" exclude="Build,Analyzers" />
      </group>
    </dependencies>

ghost · 2023-02-16T20:22:17Z

Tagging subscribers to this area: @dotnet/area-system-configuration
See info in area-owners.md if you want to be subscribed.

Issue Details

Contributes to #64592

This PR, if approved, will replace #80347 since it is less risky by not requiring removing public members. This is possible since it turns out S.S.P does not contain the implementation of the PermissionSet class (it's in corelib).

The use of PrivateAssets="all" allows the reference to the S.S.P package to be removed from the netstandard version of the package.

This will still be considered a breaking change, but only because a package reference was removed.

The System.Configuration.ConfigurationManager .nuspec goes from:

    <dependencies>
      <group targetFramework=".NETFramework4.6.2">
        <dependency id="System.Security.Permissions" version="8.0.0-dev" exclude="Build,Analyzers" />
      </group>
      <group targetFramework="net6.0">
        <dependency id="System.Security.Cryptography.ProtectedData" version="8.0.0-dev" exclude="Build,Analyzers" />
        <dependency id="System.Security.Permissions" version="8.0.0-dev" exclude="Build,Analyzers" />
      </group>
      <group targetFramework="net7.0">
        <dependency id="System.Diagnostics.EventLog" version="8.0.0-dev" exclude="Build,Analyzers" />
        <dependency id="System.Security.Cryptography.ProtectedData" version="8.0.0-dev" exclude="Build,Analyzers" />
        <dependency id="System.Security.Permissions" version="8.0.0-dev" exclude="Build,Analyzers" />
      </group>
      <group targetFramework="net8.0">
        <dependency id="System.Diagnostics.EventLog" version="8.0.0-dev" exclude="Build,Analyzers" />
        <dependency id="System.Security.Cryptography.ProtectedData" version="8.0.0-dev" exclude="Build,Analyzers" />
        <dependency id="System.Security.Permissions" version="8.0.0-dev" exclude="Build,Analyzers" />
      </group>
      <group targetFramework=".NETStandard2.0">
        <dependency id="System.Security.Cryptography.ProtectedData" version="8.0.0-dev" exclude="Build,Analyzers" />
        <dependency id="System.Security.Permissions" version="8.0.0-dev" exclude="Build,Analyzers" />
      </group>
    </dependencies>

to:

    <dependencies>
      <group targetFramework=".NETFramework4.6.2" />
      <group targetFramework="net6.0">
        <dependency id="System.Security.Cryptography.ProtectedData" version="8.0.0-dev" exclude="Build,Analyzers" />
      </group>
      <group targetFramework="net7.0">
        <dependency id="System.Diagnostics.EventLog" version="8.0.0-dev" exclude="Build,Analyzers" />
        <dependency id="System.Security.Cryptography.ProtectedData" version="8.0.0-dev" exclude="Build,Analyzers" />
      </group>
      <group targetFramework="net8.0">
        <dependency id="System.Diagnostics.EventLog" version="8.0.0-dev" exclude="Build,Analyzers" />
        <dependency id="System.Security.Cryptography.ProtectedData" version="8.0.0-dev" exclude="Build,Analyzers" />
      </group>
      <group targetFramework=".NETStandard2.0">
        <dependency id="System.Security.Cryptography.ProtectedData" version="8.0.0-dev" exclude="Build,Analyzers" />
      </group>
    </dependencies>

Author:	steveharter
Assignees:	steveharter
Labels:	`area-System.Configuration`
Milestone:	-

ghost · 2023-02-16T20:22:35Z

Added needs-breaking-change-doc-created label because this PR has the breaking-change label.

When you commit this breaking change:

Create and link to this PR and the issue a matching issue in the dotnet/docs repo using the breaking change documentation template, then remove this needs-breaking-change-doc-created label.
Ask a committer to mail the .NET Breaking Change Notification DL.

Tagging @dotnet/compat for awareness of the breaking change.

steveharter · 2023-02-17T16:34:48Z

@ViktorHofer there's a package test failure like this:

artifacts\bin\testPackages\packageTest.targets(50,5): error : (NETCORE_ENGINEERING_TELEMETRY=Build) Assembly 'System.Configuration.ConfigurationManager' is missing dependency 'System.Security.Permissions'

coming from Arcade's Microsoft.DotNet.PackageTesting I believe.

Do you know if there a suppression mechanism for that?

ViktorHofer · 2023-02-17T16:53:55Z

Do you know if there a suppression mechanism for that?

That usually indicates that package is incorrectly authored. @ericstj can you please take a look? I lack knowledge of what this PR tries to achieve and I currently don't have cycles to investigate further.

steveharter · 2023-02-17T18:16:37Z

That usually indicates that package is incorrectly authored. @ericstj can you please take a look? I lack knowledge of what this PR tries to achieve and I currently don't have cycles to investigate further.

I added another commit that should address this.

ViktorHofer · 2023-02-17T18:51:31Z

As Eric is out, please hold off merging until he is back and took a look. Just want to make sure that we understand all the implications of the change.

steveharter · 2023-02-17T23:06:16Z

This approach does leave a dangling reference to [System.Security.Permissions]System.Security.PermissionSet in the netstandard build of S.C.CM:

instance void  GetRestrictedPermissions(class System.Configuration.Internal.IInternalConfigRecord configRecord,
                                        [out] class [System.Security.Permissions]System.Security.PermissionSet& permissionSet,
                                        [out] bool& isHostReady) cil managed

This is because the netstandard build of S.S.P cross-compiles PermissionSet and friends since they are not part of netstandard.

As mentioned earlier, the Net8.0 and Net462 builds of S.C.CM have a runtime reference instead since those types are implemented in corelib: [System.Runtime]System.Security.PermissionSet with no dependency on S.S.P (before or after this PR).

src/libraries/testPackages/frameworkSettings/netstandard/settings.targets

This reverts commit f449cd3.

ericstj

LGTM

...stem.Configuration.ConfigurationManager/ref/System.Configuration.ConfigurationManager.csproj

ericstj · 2023-03-01T20:20:03Z

src/libraries/testPackages/packageSettings/System.Runtime.Caching/netstandard/settings.targets

+<Project>
+  <ItemGroup>
+    <!-- intentional dangling ref in System.Configuration.ConfigurationManager -->
+    <IgnoredReference Include="System.Security.Permissions" />


Interesting -- it looks like System.Runtime.Caching is the only library that retained it's control knob to from config file:

runtime/src/libraries/System.Runtime.Caching/src/System/Runtime/Caching/MemoryCacheStatistics.cs

Line 129 in 0a15c3b

MemoryCacheSection section = ConfigurationManager.GetSection("system.runtime.caching/memoryCache") as MemoryCacheSection;

Probably because it was viewed as a "legacy" package with a replacement in Microsoft.Extensions -- similar to System.Configuration.ConfiguratinManager itself.

steveharter · 2023-08-16T17:43:07Z

Breaking change issue: dotnet/docs#36720

Remove S.S.Permissions reference from S.C.ConfigurationManager

6782a0c

steveharter added the area-System.Configuration label Feb 16, 2023

steveharter self-assigned this Feb 16, 2023

steveharter requested a review from ViktorHofer February 16, 2023 20:22

steveharter requested a review from ericstj February 16, 2023 20:22

steveharter added the breaking-change Issue or PR that represents a breaking API or functional change over a prerelease. label Feb 16, 2023

ghost added the needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet label Feb 16, 2023

steveharter requested a review from buyaa-n February 16, 2023 20:32

build-analysis bot mentioned this pull request Feb 16, 2023

OSX infra issue - prereq check for 'pkg-config' missing #82240

Closed

Add suppression for missing S.S.Permissions

2d206c3

Add the appropriate [Obsolete] attributes

36465bb

steveharter mentioned this pull request Feb 21, 2023

Remove S.S.Permissions reference from S.DirectoryServices #82453

Merged

Update comment

c90da5b

build-analysis bot mentioned this pull request Feb 22, 2023

Tracking issue for CI build timeouts #76454

Closed

Add ref to S.S.P for test project

f449cd3

ericstj reviewed Feb 22, 2023

View reviewed changes

src/libraries/testPackages/frameworkSettings/netstandard/settings.targets Outdated Show resolved Hide resolved

steveharter added 2 commits February 22, 2023 12:47

Revert "Add ref to S.S.P for test project"

79a3ce7

This reverts commit f449cd3.

Use per-package suppression file for testPackages

fff2a36

steveharter added this to the 8.0.0 milestone Feb 23, 2023

steveharter mentioned this pull request Mar 1, 2023

Remove System.Security.Permissions from S.C.CM. #80347

Closed

ericstj approved these changes Mar 1, 2023

View reviewed changes

stephentoub approved these changes Mar 2, 2023

View reviewed changes

Add same comment to ref file

25506a9

steveharter merged commit 9d6e592 into dotnet:main Mar 8, 2023

steveharter deleted the CmPermissionSet branch March 8, 2023 14:02

steveharter mentioned this pull request Mar 16, 2023

[API Proposal]: remove dependency on System.Drawing.Common from System.Configuration.ConfigurationManager #64592

Closed

ghost locked as resolved and limited conversation to collaborators Apr 7, 2023

steveharter removed the needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet label Oct 16, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove S.S.Permissions reference from S.C.ConfigurationManager #82259

Remove S.S.Permissions reference from S.C.ConfigurationManager #82259

steveharter commented Feb 16, 2023 •

edited

Loading

ghost commented Feb 16, 2023

ghost commented Feb 16, 2023 •

edited by steveharter

Loading

steveharter commented Feb 17, 2023

ViktorHofer commented Feb 17, 2023 •

edited

Loading

steveharter commented Feb 17, 2023

ViktorHofer commented Feb 17, 2023

steveharter commented Feb 17, 2023 •

edited

Loading

ericstj left a comment

ericstj Mar 1, 2023

steveharter commented Aug 16, 2023

Remove S.S.Permissions reference from S.C.ConfigurationManager #82259

Remove S.S.Permissions reference from S.C.ConfigurationManager #82259

Conversation

steveharter commented Feb 16, 2023 • edited Loading

ghost commented Feb 16, 2023

ghost commented Feb 16, 2023 • edited by steveharter Loading

steveharter commented Feb 17, 2023

ViktorHofer commented Feb 17, 2023 • edited Loading

steveharter commented Feb 17, 2023

ViktorHofer commented Feb 17, 2023

steveharter commented Feb 17, 2023 • edited Loading

ericstj left a comment

Choose a reason for hiding this comment

ericstj Mar 1, 2023

Choose a reason for hiding this comment

steveharter commented Aug 16, 2023

steveharter commented Feb 16, 2023 •

edited

Loading

ghost commented Feb 16, 2023 •

edited by steveharter

Loading

ViktorHofer commented Feb 17, 2023 •

edited

Loading

steveharter commented Feb 17, 2023 •

edited

Loading