Skip to content

SDK not deploying facades correctly #1552

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

Closed
jaredpar opened this issue Aug 30, 2017 · 17 comments
Closed

SDK not deploying facades correctly #1552

jaredpar opened this issue Aug 30, 2017 · 17 comments

Comments

@jaredpar
Copy link
Member

Repo steps:

The expected steps is that System.Security.Cryptography.Algorithms 4.1.0.0 appears in the directory Binaries\Debug\UnitTests\CSharpCompilerSemanticTest. The actual result is that it does not and this prevents the tests from running.

The lib is clearly needed in this scenario. The most recent commit includes the following code definition:

public sealed class FixBuild
{
  public IncrementalHash GetIncrementalHash() => throw new NotSupportedException();
}

This is important because IncrementalHash is not a type forward inside the lib version of S.S.C.Algorithms. Instead it's an actual type definition. Lacking the assembly the code will fail to run.

Interestingly enough though a binding redirect is generated for S.S.C.Algorithms and it correctly merges 4.0.0.0-4.1.0.0 to 4.1.0.0.

This is currently blocking roslyn from moving to 2.0SDK. Is there any work around here? Some way to force this DLL to get deployed?

@dsplaisted
Copy link
Member

dsplaisted commented Aug 30, 2017

I've made some progress figuring out what is going on here. A simplified repro is the following project:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net461</TargetFramework>
    <PlatformTarget>AnyCPU</PlatformTarget>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="System.Security.Cryptography.Algorithms" Version="4.3.0" />
  </ItemGroup>

</Project>

Setting the PlatformTarget property to AnyCPU causes the runtime items from the NuGet package to be included in the conflict resolution and win, even though they aren't deployed.

Encountered conflict between 'CopyLocal:C:\Users\daplaist.nuget\packages\system.security.cryptography.algorithms\4.3.0\lib\net461\System.Security.Cryptography.Algorithms.dll' and 'Runtime:C:\Users\daplaist.nuget\packages\system.security.cryptography.algorithms\4.3.0\runtimes\osx\lib\netstandard1.6\System.Security.Cryptography.Algorithms.dll'. Choosing 'Runtime:C:\Users\daplaist.nuget\packages\system.security.cryptography.algorithms\4.3.0\runtimes\osx\lib\netstandard1.6\System.Security.Cryptography.Algorithms.dll' because AssemblyVersion '4.2.1.0' is greater than '4.1.0.0'.

cc @ericstj

@ericstj
Copy link
Member

ericstj commented Aug 30, 2017

Looks the same as #1510.

@jaredpar
Copy link
Member Author

@ericstj if that is the case what is the work around? I don't see one listed in the bug. This is completely blocking our 2.0 transition.

@clairernovotny
Copy link

@jaredpar
Copy link
Member Author

@onovotny some of our project that hit this bug use <RuntimeIdentifiers>. Not sure how to best apply that patch to that case.

@clairernovotny
Copy link

What project is it? Could you just test for an empty RuntimeIdentifier and set it?

@jaredpar
Copy link
Member Author

@onovotny there are a few. Most notably at the moment is src\Compilers\CSharp\Test\Semantic\CSharpCompilerSemanticTest.csproj. Trying out your idea right now.

@jaredpar
Copy link
Member Author

@onovotny getting a lot of the following now

C:\Program Files\dotnet\sdk\2.0.0-preview3-006923\Sdks\Microsoft.NET.Sdk\build\Microsoft.NET.RuntimeIdentifierInference.targets(125,5): error : The RuntimeIdenti fier platform and the PlatformTarget must match. [E:\code\roslyn\src\Interactive\HostTest\InteractiveHostTest.csproj]

@clairernovotny
Copy link

That's getting beyond me :( That property worked in my case, but it may be something similar to unblock you.

@dsplaisted
Copy link
Member

dsplaisted commented Aug 30, 2017

@jaredpar Try adding the following to work around this issue:

  <Target Name="WorkaroundConflictResolution" AfterTargets="_ComputeActiveTFMFileDependencies"
          Condition="'$(PlatformTarget)' == 'AnyCPU'">
    <ItemGroup>
      <_ActiveTFMFileDependencies Remove="@(_ActiveTFMFileDependencies->WithMetadataValue('FileGroup', 'RuntimeTarget'))" />
    </ItemGroup>
  </Target>

EDIT: You might want to also condition it on whether you're targeting .NET Framework

@jaredpar
Copy link
Member Author

@dsplaisted thanks! A quick local check seems to confirm that works. Testing out in Jenkins now.

@jaredpar
Copy link
Member Author

@dsplaisted that's leading to the following errors

14:08:16 Unhandled Exception: System.IO.FileNotFoundException: Could not load file or assembly 'System.Runtime.InteropServices.RuntimeInformation, Version=4.0.2.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a'. The system cannot find the file specified.

@dsplaisted
Copy link
Member

@jaredpar Is this in the same project or elsewhere? And do you see the failure locally or just in Jenkins?

@jaredpar
Copy link
Member Author

@dsplaisted it's in other projects. Essentially the state of the world is the following with respect to your work around:

  • Before: desktop tests all broken, coreclr working like a champ
  • After: desktop tests working like a champ, coreclr broken

😦

@dsplaisted
Copy link
Member

@jaredpar If you're applying the target to all projects, then update the condition on it to only apply to projects targeting .NET Framework:

  <Target Name="WorkaroundConflictResolution" AfterTargets="_ComputeActiveTFMFileDependencies"
          Condition="'$(PlatformTarget)' == 'AnyCPU' And '$(TargetFrameworkIdentifier)' == '.NETFramework'">
    <ItemGroup>
      <_ActiveTFMFileDependencies Remove="@(_ActiveTFMFileDependencies->WithMetadataValue('FileGroup', 'RuntimeTarget'))" />
    </ItemGroup>
  </Target>

jaredpar added a commit to jaredpar/roslyn that referenced this issue Aug 30, 2017
@jaredpar
Copy link
Member Author

@dsplaisted great! That seems to be working now. Thanks for the help here.

@nguerrera
Copy link
Contributor

Closing as duplicate of #1510.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants