Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Moving part of System.Security.Permissions down to System.Runtime.Extensions#13476

Merged
joperezr merged 2 commits intodotnet:masterfrom
joperezr:SystemSecurityPermissions
Nov 11, 2016
Merged

Moving part of System.Security.Permissions down to System.Runtime.Extensions#13476
joperezr merged 2 commits intodotnet:masterfrom
joperezr:SystemSecurityPermissions

Conversation

@joperezr
Copy link
Member

@joperezr joperezr commented Nov 8, 2016

Fixes #13289

cc: @ianhays @weshaggard @danmosemsft

Moving netstandard 2.0 members of System.Security.Permissions down to System.Runtime.Extensions so that they are part of NETStandard.Library

@joperezr
Copy link
Member Author

joperezr commented Nov 8, 2016

@dotnet-bot test Innerloop Linux ARM Emulator SoftFP Release Cross Build please

</ProjectReference>
<!-- Do not remove this P2P reference since part of the implementation of NonGeneric has moved to Runtime.Extensions -->
<ProjectReference Include="..\..\..\System.Runtime.Extensions\pkg\System.Runtime.Extensions.pkgproj" />
<!-- ToDo: Remove this P2P reference once packages are updated -->
Copy link
Member

Choose a reason for hiding this comment

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

Why does it need S.Security.Permissions reference?

Copy link
Member

Choose a reason for hiding this comment

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

Why does it need S.Security.Permissions reference?

I assume it's because the test projects now automatically reference everything, which means any time you move a type from assembly A to assembly B, you need to include P2P references for both A and B so that the test project doesn't see the type duplicated between the old package of A and the new assembly of B. I've run into that recently any time I've moved a type.

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually the reason is because Security.Permissions was still being referenced as part of the closure, but it was an older version that still had Security Attributes defined there, so need to reference both in order to have the compiler not to complain. Once we update packages, we can remove this.

Copy link
Member

@stephentoub stephentoub left a comment

Choose a reason for hiding this comment

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

We also need to move the tests for all types being moved.

@joperezr
Copy link
Member Author

We also need to move the tests for all types being moved.

Actually we don't want to do that, because if we do then we loose coverage for older configurations like netcoreapp1.0. That is why the paradigm we have been using so far is to keep the tests where they used to live, and just have that old project reference the live version of where the code now lives so that we keep testing live implementation.

@joperezr joperezr force-pushed the SystemSecurityPermissions branch from 864c741 to 75594fa Compare November 11, 2016 18:25
@danmoseley danmoseley modified the milestone: 1.2.0 Nov 11, 2016
@joperezr
Copy link
Member Author

The OSX Debug build succeeded, but it failed when archiving results.

@joperezr joperezr merged commit 768e18c into dotnet:master Nov 11, 2016
@joperezr joperezr deleted the SystemSecurityPermissions branch February 8, 2017 17:45
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.

6 participants