Omissions in types supported by both Desktop and Core - System.Security.AccessControl.AuthorizationRuleCollection #17604
Conversation
ViktorHofer
force-pushed
the
CannotRemoveBaseTypeOrInterface-ReadOnlyCollectionBase
branch
from
6d79c52
to
8d74fa8
Compare
ViktorHofer
added
area-Meta
area-System.Security
blocking-release
tenet-compatibility
| @@ -34,6 +34,7 @@ | |||
| <Compile Include="System\IO\FileSystemAclExtensions.net46.cs" /> | |||
| </ItemGroup> | |||
| <ItemGroup Condition="'$(TargetGroup)'!='netfx'"> | |||
| <Reference Include="System.Collections.NonGeneric" /> | |||
| @@ -120,6 +120,7 @@ | |||
| </ItemGroup> | |||
| <ItemGroup> | |||
| <Reference Include="System.Collections" /> | |||
| <Reference Include="System.Collections.NonGeneric" /> | |||
There was a problem hiding this comment.
Before we merge this -- @joperezr @weshaggard
S.S.AccessControl builds for netstandard but S.C.NonGeneric builds for netcoreapp and uap specifically. So assume this reference is prohibited. But why does NonGenreic not build for netstandard only?
There was a problem hiding this comment.
Oh, it's because it depends on System.Runtime and S.R.Extensions and S.Globalization and those don't have netstandard configurations. Can we unravel this somehow?
If I change S.C.NG to netstandard I get C:\git\corefx\buildvertical.targets(153,5): error : Could not find a configuration for ProjectReference '..\..\System.Runtime\ref\System.Runtime.csproj' from configurations 'netcoreapp;uap' when building 'System.Collections.NonGeneric' for configuration 'netstandard-Debug' . [C:\git\corefx\src\System.Collections.NonG eneric\ref\System.Collections.NonGeneric.csproj]
There was a problem hiding this comment.
I'm missing something conceptually -- I guess types in netstandard can be defined in libraries that don't have a netstandard configuration. That makes sense I guess. So is this new reference legal?
There was a problem hiding this comment.
S.S.AccessControl doesn't build for netstandard, at least not the implementation. The implementation is also platform-specific (uap, netcoreapp) which is why this is a valid reference.
There was a problem hiding this comment.
That's what confused me -- the ref is built for netstandard. Why's that? Do we just do that for convenience whenever it happens to compile successfully for netstandard?
There was a problem hiding this comment.
We should actually consider switching this library to be netstandard but if we do that then we may need to set some explicit configurations to Generate platform not supported exceptions (i.e. unix and maybe uap).
There was a problem hiding this comment.
should we merge the PR for now and track these ideas in a separate issue?
There was a problem hiding this comment.
I'm fine with merging this once CI is green. We do need to file an issue about all the AccessControl APIs about whether or not we want to support then for netstandard or not. @joperezr was going to file a similar issue, so chat with him about it.
|
@ViktorHofer it looks like you didn't correctly merge with the latest from master as your PR shows a lot of files which aren't relevant to your changes. |
|
@weshaggard yes, alread on it... |
ViktorHofer
force-pushed
the
CannotRemoveBaseTypeOrInterface-ReadOnlyCollectionBase
branch
from
8ae1649
to
505b5b9
Compare
|
@weshaggard Tests should finish soon. Please take another look. As stated above if you agree I would merge this PR and open a new issue with what you suggested. |
ViktorHofer
deleted the
CannotRemoveBaseTypeOrInterface-ReadOnlyCollectionBase
branch
Adjustments for System.Security.AccessControl.AuthorizationRuleCollection to correctly inherit from System.Collections.NonGeneric.ReadOnlyCollectionBase. For more info see here: https://github.com/dotnet/corefx/issues/15255#issuecomment-289550875