Fix publishing .NET Standard facades #19731
Conversation
ghost
added
the
| <ReferenceCopyLocalPaths Include="@(_NETStandardLibraryNETFrameworkLib)" Condition="'%(FileName)' != 'netfx.force.conflicts'"> | ||
| <Private>false</Private> | ||
| </ReferenceCopyLocalPaths> | ||
| <ReferenceCopyLocalPaths Include="@(_NETStandardLibraryNETFrameworkLib)" Condition="'%(FileName)' != 'netfx.force.conflicts'"/> |
There was a problem hiding this comment.
Perhaps we should also update the comment above since it still says that Private is set to false?
There was a problem hiding this comment.
Private is still set to false on the Reference items which are created in the AddFacadesToReferences task. That's what the comment is supposed to be referring to, as far as I understand.
There was a problem hiding this comment.
Small nit, but other than updating the comment this LGTM. Thanks for fixing this @dsplaisted!
| // We should choose the system.runtime.interopservices.runtimeinformation file from Microsoft.NET.Build.Extensions as it has a higher AssemblyVersion (4.0.2.0 compared to 4.0.1.0) | ||
| files.FirstOrDefault().Contains(@"Microsoft.NET.Build.Extensions\net461\lib\System.Runtime.InteropServices.RuntimeInformation.dll").Should().BeTrue(); |
There was a problem hiding this comment.
@joperezr @ericstj I had to change this test to expect the file from Microsoft.NET.Build.Extensions to win instead of the one from the package. This seems legit, here's the message from the binlog:
Encountered conflict between 'Reference:C:\Users\daplaist.nuget\packages\system.runtime.interopservices.runtimeinformation\4.3.0\ref\netstandard1.1\System.Runtime.InteropServices.RuntimeInformation.dll' and 'Reference:System.Runtime.InteropServices.RuntimeInformation'. Choosing 'Reference:System.Runtime.InteropServices.RuntimeInformation' because AssemblyVersion '4.0.2.0' is greater than '4.0.1.0'.
I'm not sure why the test originally expected the other file to win, maybe it was always a bug.
Does this seem OK?
There was a problem hiding this comment.
How was the test even passing before this change? I'm seeing the same behavior locally as the test after your change as well, and also for the copyLocal item file:
Encountered conflict between 'CopyLocal:C:\Users\joe_p\.nuget\packages\system.runtime.interopservices.runtimeinformation\4.3.0\runtimes\win\lib\net45\System.Runtime.InteropServices.RuntimeInformation.dll' and 'CopyLocal:C:\Program Files\dotnet\sdk\5.0.100-rc.2.20479.15\Microsoft\Microsoft.NET.Build.Extensions\net461\lib\System.Runtime.InteropServices.RuntimeInformation.dll'. Choosing 'CopyLocal:C:\Program Files\dotnet\sdk\5.0.100-rc.2.20479.15\Microsoft\Microsoft.NET.Build.Extensions\net461\lib\System.Runtime.InteropServices.RuntimeInformation.dll' because AssemblyVersion '4.0.2.0' is greater than '4.0.1.0'.
Given this, I'm vine with fixing the test as well as I agree that it must have been a bug before unless this was written before we serviced the Microsoft.NET.Build.Extensions facades a long time ago and we were actually expecting the package to win.
Fixes #18101