Remove dependencies on WindowsIdentity from Unix binaries #5576
Conversation
It looks like a couple libraries were using System.Security.Principal.Windows unintentionally in their Unix implementations. @dagood discovered this when trying to restore our tools-runtime project for unix RIDs. This fix removes the dependencies from those libs in the CoreFx repro. /cc @CIPop @saurabh500 It looks like WCF also has this problem as well, since it hasn't split System.Private.ServiceModel into windows specific packages. /cc @StephenBonikowsky
| @@ -2,7 +2,12 @@ | |||
| <Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003"> | |||
| <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" /> | |||
| <ItemGroup> | |||
| <Project Include="System.Data.SqlClient.csproj" /> | |||
| <Project Include="System.Data.SqlClient.csproj"> | |||
There was a problem hiding this comment.
@ericstj What do these changes do ?
There was a problem hiding this comment.
Those are specifying that you actually have 2 distinct builds of the library: one for windows and one for linux (actually, unix but we don't have a unix only convention /cc @weshaggard)
This makes it consistent with the package.
The way it was structured before was as if the library only had a single cross-platform build.
|
@ericstj The code changes look good to me. I want to understand the changes in System.Data.SqlClient.builds |
|
This change is just disabling/throwing on the code path that was hitting WindowsIdentity. You might need to instead replace with an appropriate impl on UNIX. |
| @@ -0,0 +1,18 @@ | |||
| // Copyright (c) Microsoft. All rights reserved. | |||
There was a problem hiding this comment.
I wonder if we should have a single file ContextAwareResult.WindowsIdentityNop.cs, or something like that, which we use for both the unix and netcore50 builds, rather than duplicating the same contents.
There was a problem hiding this comment.
Thought about that. My imppression was the networking team would want to fill in the real implementation for unix.
There was a problem hiding this comment.
My guess is these will remain nops, but ok.
|
LGTM. Thanks for fixing this, Eric. |
|
Awesome, thanks Eric! |
|
Thanks @ericstj! For the changes for ContextAwareResults: LGTM |
Remove dependencies on WindowsIdentity from Unix binaries
|
The reason this is even possible is that we cannot represent dependencies that differ based on RID, so we share the same project.json for Windows and Unix. We need NuGet/Home#1660 and https://github.com/dotnet/cli/issues/465 |
…dencies Remove dependencies on WindowsIdentity from Unix binaries Commit migrated from dotnet/corefx@c8818d1
It looks like a couple libraries were using
System.Security.Principal.Windows unintentionally in their
Unix implementations. @dagood discovered this when
trying to restore our tools-runtime project for unix RIDs.
This fix removes the dependencies from those libs
in the CoreFx repro.
/cc @CIPop @saurabh500
It looks like WCF also has this problem as well,
since it hasn't split System.Private.ServiceModel
into windows specific packages.
/cc @StephenBonikowsky