Use .NET Standard facades without reference assembly bit set

[dsplaisted]

Bring back the fixes from #1582, which were reverted in #1610, this time without regressing ASP.NET Core apps on .NET Framework.

This does this by using only the "lib" and not the "ref" files from the .NET Standard facades, but putting each facade in two separate MSBuild items: Reference with Private set to false, which means they won't be copied locally, and ReferenceCopyLocalPaths which will be copied locally. This will result in the deps.json file will have those items listed with type referenceassembly instead of reference, which means this won't run afoul of https://github.com/dotnet/core-setup/issues/2981.

This also uses the simple name as the identity for the Reference items, and uses the HintPath metadata for the full path. This should resolve at least some cases of #1499.

[eerhardt]

Why use the HintPath? Why not just <Reference Include="full path"?

[dsplaisted]

This also uses the simple name as the identity for the Reference items, and uses the HintPath metadata for the full path. This should resolve at least some cases of #1499.

[eerhardt]

Ah, I see it in your description now - for #1499.

[ericstj]

Yeah, I wonder if we want NuGet to do the same thing 🤔 I can't recall why the NuGet task originally used RawFileName given that was inconsistent with what the project system was doing, @jasonmalinowski might know.

[eerhardt]

Note, this will cause the same assemblies to be copied twice to the output folder in ASP.NET apps - once in the output folder and once under $outputfolder\refs. I think this is a fine approach for now, but we may want to go back to the original implementation once we ship the fix for dotnet/core-setup#2981.

[eerhardt]

Thanks for fixing up the test!

[ericstj]

Thank you very much for getting this back in place so quickly.

[nguerrera]

Can this be conditioned differently? Why didn't we need to know about netfx.force.conflicts here before, but now we do?

[nguerrera]

I see now. This is just porting what was there before the revert to the new scheme.

[dsplaisted]

@eerhardt

I think this is a fine approach for now, but we may want to go back to the original implementation once we ship the fix for dotnet/core-setup#2981.

Even once that is fixed we can't break projects that haven't updated to the new package. We might be able to put a special case workaround in the deps.json generation code that would be based on what version of the DependencyModel library was in the graph (which you had suggested as a possible solution in IM).

[dsplaisted]

@MattGertz for approval (also we should discuss what release this should go in)

Customer scenario

.NET Framework projects targeting .NET Framework 4.6.1 or higher, and using a library that targets .NET Standard 1.5 or higher. A wide variety of tools will fail in this scenario, including debugging web projects (non ASP.NET Core), Workflow XAML projects, license generation (lc.exe / .licx), Sandcastle Help File Builder, and probably more.

Bugs this fixes

#1509

Related Issues: https://github.com/dotnet/corefx/issues/23229, #1522, https://github.com/dotnet/corefx/issues/23505, dotnet/corefx#23711

Workarounds, if any

To all desktop project files add the following:

  <Target Name="ReplaceNetFxNetStandardRefWithLib" AfterTargets="ImplicitlyExpandNETStandardFacades">
    <ItemGroup>
      <Reference Remove="@(_NETStandardLibraryNETFrameworkReference)" Condition="'%(FileName)' != 'netfx.force.conflicts'"/>
      <Reference Include="@(_NETStandardLibraryNETFrameworkLib)">
        <Private>true</Private>
      </Reference>
    </ItemGroup>
  </Target>
  <Target Name="RemoveNetFxForceConflicts" AfterTargets="ResolveAssemblyReferences">
    <ItemGroup>
      <ReferencePath Remove="@(ReferencePath)" Condition="'%(FileName)' == 'netfx.force.conflicts'" />
    </ItemGroup>
  </Target>

Risk

Low

Performance impact

Improves performance by removing a number of items and wildcard expansion (further changes aim to improve more).

Root cause analysis

Web project system ignores CopyLocal metadata on references and will copy them to the output directory outside of the build if the file is absent. This breaks if any assembly is intentionally CopyLocal=false (AKA: Private=false), and a ReferenceAssembly without a corresponding implementation in the GAC. It will be copied to the output directory and later loaded for execution by ASP.NET.

XAML compiler attempts to load all references for execution and fails for any that are ReferenceAssemblies without a corresponding implementation in the GAC.

The previous fix to this bug (in #1582) broke Razor on ASP.NET Core on .NET Framework, due to a bug in the library Razor uses to read deps.json: dotnet/core-setup#2981

How was the bug found?

Customer reports / Internal testing

[MattGertz]

Approved for Preview 2.

[livarcocc]

Please, re-target to release/15.5

[dsplaisted]

@livarcocc Rebasing this onto release/15.5 also brought in the change from #1610. We do want this change, it just means that we haven't merged the latest version of release/2.0-vs into 15.5. Should we do that separately?

[livarcocc]

@dsplaisted I think it is fine to do that together. They appear as separate commits anyways. My only concern is if git will know what to do when we merge this branch back.

[dsplaisted]

@dotnet-bot test this please