Skip to content

[Regression] .NET 6 SDK will no longer copy the netstandard facades to the publish directory for net461 apps #18101

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
joperezr opened this issue Jun 4, 2021 · 11 comments · Fixed by #19731
Assignees
Labels
Area-NetSDK untriaged Request triage from a team member
Milestone

Comments

@joperezr
Copy link
Member

joperezr commented Jun 4, 2021

cc: @dsplaisted @marcpopMSFT @andschwa @ericstj

Repro Steps

  1. Create a new netstandard lib
<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>netstandard2.0</TargetFramework>
  </PropertyGroup>

</Project>
  1. Create a new net461 Console app that references it:
<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net461</TargetFramework>
  </PropertyGroup>

  <ItemGroup>
    <ProjectReference Include="..\reproLib\reproLib.csproj" />
  </ItemGroup>

</Project>
  1. Publish the project with dotnet publish

Expected
Because the net461 project is referencing a netstandard library, tooling should inject all of the netstandard facades onto the publish directory to ensure the application will be able to run as expected. This is what happens with .NET 5.0 and previous sdks.

Actual
bin directory does get the facades injected, but the publish directory won't when you use a 6.0 SDK, which means that if the app tries to run in a machine that only has .NET 461 installed, the app will likely crash at runtime.

Problem

I took a look and the issue here seems to be caused by this change which basically changed the way we calculate the items to be copied to the publish directory and added an additional filter to check if the ResolveCopyLocal item had the Private metadata set to false. When we are injecting the netstandard facades to resolveCopyLocal item, we explicitly set Private to false, which is why they are now all getting excludded from the publish directory. Here is where we set the metadata to false and the explanation of why we do it:

<ItemGroup Condition="'@(_NETStandardLibraryNETFrameworkLib)' != ''">
<!-- Put each facade assembly in two separate items: Reference with Private set to false, which means it won't be
copied locally, and ReferenceCopyLocalPaths, which will be copied locally. The reason for this split is to
workaround https://github.com/dotnet/core-setup/issues/2981 by ensuring that the facades are written
to the deps.json with a type of "referenceassembly" instead of "reference".
The exception is netfx.force.conflicts.dll, which shouldn't be copied local. So it isn't included in
ReferenceCopyLocalPaths.
When we add the Reference items, we use the simple name as the ItemSpec, and refer to the full path
to the DLL via the HintPath metadata. This is so that if we're replacing a simple Reference,
the OriginalItemSpec of the resolved reference will match to the Reference that was replaced,
and VS won't show a warning icon on the reference. See https://github.com/dotnet/sdk/issues/1499
-->
<ReferenceCopyLocalPaths Include="@(_NETStandardLibraryNETFrameworkLib)" Condition="'%(FileName)' != 'netfx.force.conflicts'">
<Private>false</Private>
</ReferenceCopyLocalPaths>

This regression has a potential of great impact, so we should consider how we can fix it before 6.0 ships, whether the solution is to not set Private item to false or if the solution is instead to change the new filter on the publish items.

@ghost
Copy link

ghost commented Jun 4, 2021

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@joperezr
Copy link
Member Author

joperezr commented Jun 4, 2021

After taking a closer look, seems like the right fix for this should not be to change the new filter on publish items, but instead to not set Private=false on the Facades as that seems like a bug we had in the SDK from the beginning. @dsplaisted am I correct in saying that you should only set Private=false on things that are expected to be inbox/GAC, and therefore these facades shoud not have that set since the whole reason we inject them is that they are not inbox?

@dsplaisted
Copy link
Member

@marcpopMSFT Looks like this is a regression in .NET 6 that we need to look at

@marcpopMSFT
Copy link
Member

Moved it into the net6 backlog

@andyleejordan
Copy link
Member

@marcpopMSFT May I please ask, is there a timeline for it getting addressed? "Backlog" can mean many things, and I just want to be clear this is a blocking bug for the Visual Studio Code PowerShell extension that either needs to be fixed or worked around until then. (And my current workaround isn't working so I may need help to figure out something else.)

@marcpopMSFT
Copy link
Member

That is a fair question as the backlog milestone is different than the .NET 6 backlog pipeline. The .NET 6 backlog is for the list of items we should look at once we've completed our committed list for .NET 6. Basically, it's above all the other issues that have been filed but below our high priority committed work so I'd expect a fix by around RC most likely.

Is the PowerShell extension planning on adopting the .net 6 SDK for production prior to our go live release later this year?

@joperezr
Copy link
Member Author

@andschwa I haven't fully tested it, but you can try to see if the following target works for your needs:

  <Target Name="InjectNetStandardFacadesToPublishDirectory"
          BeforeTargets="ComputeResolvedFilesToPublishList"
          Condition="'@(_NETStandardLibraryNETFrameworkLib)' != ''">
    <ItemGroup>
      <_ResolvedCopyLocalPublishAssets Include="@(_NETStandardLibraryNETFrameworkLib)" 
                                      Condition="'%(_NETStandardLibraryNETFrameworkLib.FileName)' != 'netfx.force.conflicts'" />
    </ItemGroup>
  </Target>

I'm sure it is not perfect and it depends on internal items and targets so it is fragile and easy to get broken, but it may work while you wait for the final fix to make it to the SDK. From the testing I did, this should be injecting correctly all of the facades that your app requires into your publish directory.

@andyleejordan
Copy link
Member

andyleejordan commented Jun 11, 2021

Is the PowerShell extension planning on adopting the .net 6 SDK for production prior to our go live release later this year?

Yes and no. We've started installing .NET 6 SDK channel in addition to 3.1 and 5 for two reasons: 1) compatibility with Apple M1 and 2) PowerShell 7.2.x (still in preview) adopted .NET 6 SDK (as it targets the net6.0 runtime), which we need to test against. So, yes, we install .NET 6 SDK when building our production bits, and that brings along the dotnet CLI 6 as well as this regression. But also no, as the PowerShell 7.2.x bits are only built for testing, and the shipped bits target netcoreapp3.1, net461 (where this bug appears), and netstandard2.0; that is, we're not shipping anything targeting the .NET 6 SDK, but its presence introduces the bug.

@rjmholt suggested one workaround of disabling the 7.2 tests and excluding the 6.0 channel when building a release, which we could do and would work around the bug by only bringing in dotnet 5 since it's on a clean CI machine each time, but @joperezr's workaround seems to be working like a charm (thank you!). About to provide an insiders build for users to test and see if their bug is resolved.

@andyleejordan
Copy link
Member

Just curious, is there any update to a solution for this issue? I noticed a couple other issues have been traced back to the same root cause.

@joperezr
Copy link
Member Author

@dsplaisted is there an ETA for when this fix will land? I suppose that before shipping 6.0? If we don't fix this in time I'm afraid a lot of customers will start hitting this as soon as we ship a VS that contains this SDK

@andyleejordan
Copy link
Member

Thanks all!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-NetSDK untriaged Request triage from a team member
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants