ResolvePackageFileConflicts is detecting unexpected conflicts

[MichaelSimons]

While building the runtime repo in source build for .NET 6.0 I have encountered an issue where ResolvePackageFileConflicts is detecting unexpected conflicts.

Here is a binlog for the issue - https://artprodcus3.artifacts.visualstudio.com/Ab55de4ed-4b5a-4215-a8e4-0a0a5f71e7d8/9ee6d478-d288-47f7-aacc-f6e6d082ae6d/_apis/artifact/cGlwZWxpbmVhcnRpZmFjdDovL2RuY2VuZy9wcm9qZWN0SWQvOWVlNmQ0NzgtZDI4OC00N2Y3LWFhY2MtZjZlNmQwODJhZTZkL2J1aWxkSWQvMTMzMTc1Ny9hcnRpZmFjdE5hbWUvU291cmNlLUJ1aWxkK0J1aWxkK1RhcmJhbGwrRmVkb3JhMzMtT25saW5lX0J1aWxkTG9nc19BdHRlbXB0Mg2/content?format=file&subPath=%2Fsrc%2Fruntime.566b53a66b0afa573f0dae33d07c8de9685aa5c8%2Fartifacts%2FbuildLogs%2Fsource-build%2Fself%2Fsrc%2Fartifacts%2Fsourcebuild.binlog.

_CreateR2RImages is failing for the Microsoft.NETCore.App.CrossGen2 project. The root issue is that System.Private.CoreLib.dll cannot be loaded. It appears that the ResolvePackageFileConflicts task is removing this assembly from the ReadyToRun implementation assemblies list as well as several others.

Encountered conflict between'CopyLocal:/repos/installer/artifacts/tarball/src/runtime.566b53a66b0afa573f0dae33d07c8de9685aa5c8/artifacts/source-build/self/src/artifacts/bin/microsoft.netcore.app.runtime.fedora.33-x64/Release/runtimes/fedora.33-x64/lib/net6.0/Microsoft.VisualBasic.Core.dll'and'CopyLocal:/repos/installer/artifacts/tarball/src/runtime.566b53a66b0afa573f0dae33d07c8de9685aa5c8/artifacts/source-build/self/src/artifacts/bin/coreclr/Linux.x64.Release/System.Private.CoreLib.dll'.Choosing'CopyLocal:/repos/installer/artifacts/tarball/src/runtime.566b53a66b0afa573f0dae33d07c8de9685aa5c8/artifacts/source-build/self/src/artifacts/bin/microsoft.netcore.app.runtime.fedora.33-x64/Release/runtimes/fedora.33-x64/lib/net6.0/Microsoft.VisualBasic.Core.dll'because AssemblyVersion '11.0.0.0' is greater than '6.0.0.0'.

I don't understand what the ResolvePackageFileConflicts task is doing and why it is picking Microsoft.VisualBasic.Core.dll over System.Private.CoreLib.dll. Can you help me understand what is going on here?

[MichaelSimons]

cc @ericstj, @dsplaisted

[MichaelSimons]

cc @crummel, @dseefeld

[ericstj]

That's not expected at all. Identifying conflicts happens by comparing item keys.

Since these are "CopyLocal" that would be here:
https://github.com/dotnet/sdk/blob/1fb85c4e1bc9f005ead912cb6b27635f5461ed74/src/Tasks/Common/ConflictResolution/ResolvePackageFileConflicts.cs#L117-L121

So the item key in this case is ItemUtilities.GetTargetPath
https://github.com/dotnet/sdk/blob/1fb85c4e1bc9f005ead912cb6b27635f5461ed74/src/Tasks/Common/ItemUtilities.cs#L123-L153

So that will honor "TargetPath" metadata as the item key if it exists

These items did set TargetPath, and for some reason, it doesn't contain filename.

/tarball/src/runtime.566b53a66b0afa573f0dae33d07c8de9685aa5c8/artifacts/source-build/self/src/artifacts/bin/microsoft.netcore.app.runtime.fedora.33-x64/Release/runtimes/fedora.33-x64/lib/net6.0/Microsoft.VisualBasic.Core.dll
    IsNative = 
    PostprocessAssembly = true
    RelativePath = Microsoft.VisualBasic.Core.dll
    CopyToPublishDirectory = Always
    TargetPath = tools/


/tarball/src/runtime.566b53a66b0afa573f0dae33d07c8de9685aa5c8/artifacts/source-build/self/src/artifacts/bin/coreclr/Linux.x64.Release/System.Private.CoreLib.dll
    PostprocessAssembly = true
    RelativePath = System.Private.CoreLib.dll
    CopyToPublishDirectory = Always
    TargetPath = tools/

So this explains why it's happening, but I'm not sure what changed between source build and the normal build, but perhaps this gives you enough breadcrumbs to compare and find out why TargetPath is wrong?

[MichaelSimons]

@ericstj - Thanks, I'll dig in based on that info.

[dseefeld]

@ericstj - Those items are added here with the TargetPath hardcoded to tools/.

runtime/src/installer/pkg/sfx/Microsoft.NETCore.App/Microsoft.NETCore.App.Crossgen2.sfxproj

Lines 52 to 65 in 462ab6e

	<Target Name="AddRuntimeFilesToPackage" DependsOnTargets="ResolveRuntimeFilesFromLocalBuild">
	<ItemGroup>
	<CrossgenFile Include="@(RuntimeFiles)" Condition="'%(Filename)' == 'crossgen'" />
	<OptimizationDataFile Include="@(RuntimeFiles)" Condition="'%(Filename)' == 'StandardOptimizationData'" />
	<ReferenceCopyLocalPaths Include="@(RuntimeFiles)" Exclude="@(CrossgenFile);@(OptimizationDataFile)" />
	<ReferenceCopyLocalPaths TargetPath="tools/" />
	</ItemGroup>
	</Target>
	<Target Name="AddFrameworkFilesToPackage" DependsOnTargets="ResolveLibrariesFromLocalBuild">
	<ItemGroup>
	<ReferenceCopyLocalPaths Include="@(LibrariesRuntimeFiles)" TargetPath="tools/" />
	</ItemGroup>
	</Target>

I have 3 scenarios: 1) failing source-build, 2) working source-build and 3) working standalone runtime. The working cases are the same. When calling ResolvePackageFilesConflicts, it reports a conflict for every file in ReferenceCopyLocalPaths similar to this:

Encountered conflict between 'CopyLocal:/tarball/src/runtime.566b53a66b0afa573f0dae33d07c8de9685aa5c8/artifacts/source-build/self/src/artifacts/bin/coreclr/Linux.x64.Release/sharedFramework/createdump' and 'CopyLocal:/tarball/src/runtime.566b53a66b0afa573f0dae33d07c8de9685aa5c8/artifacts/source-build/self/src/artifacts/bin/coreclr/Linux.x64.Release/System.Private.CoreLib.dll'. Could not determine a winner because 'CopyLocal:/tarball/src/runtime.566b53a66b0afa573f0dae33d07c8de9685aa5c8/artifacts/source-build/self/src/artifacts/bin/coreclr/Linux.x64.Release/sharedFramework/createdump' is not an assembly.

All files show a conflict to src/artifacts/bin/coreclr/Linux.x64.Release/sharedFramework/createdump which says it can't be resolved because createdump is not an assembly.

In the failing case, when calling ResolvePackageFilesConflicts, it also reports a conflict for every file in ReferenceCopyLocalPaths similar to this:

Encountered conflict between 'CopyLocal:/tarball/src/runtime.566b53a66b0afa573f0dae33d07c8de9685aa5c8/artifacts/source-build/self/src/artifacts/bin/microsoft.netcore.app.runtime.fedora.33-x64/Release/runtimes/fedora.33-x64/lib/net6.0/Microsoft.VisualBasic.Core.dll' and 'CopyLocal:/tarball/src/runtime.566b53a66b0afa573f0dae33d07c8de9685aa5c8/artifacts/source-build/self/src/artifacts/bin/coreclr/Linux.x64.Release/System.Private.CoreLib.dll'. Choosing 'CopyLocal:/tarball/src/runtime.566b53a66b0afa573f0dae33d07c8de9685aa5c8/artifacts/source-build/self/src/artifacts/bin/microsoft.netcore.app.runtime.fedora.33-x64/Release/runtimes/fedora.33-x64/lib/net6.0/Microsoft.VisualBasic.Core.dll' because AssemblyVersion '11.0.0.0' is greater than '6.0.0.0'.

In this case, all files show a conflict to src/artifacts/bin/microsoft.netcore.app.runtime.fedora.33-x64/Release/runtimes/fedora.33-x64/lib/net6.0/Microsoft.VisualBasic.Core.dll and it always chooses that one because the AssemblyVersion is greater. The result is that Microsoft.VisualBasic.Core.dll is the only assembly left, which causes issues with _CreateR2RImages later.

I have binlogs for all three cases. If you want to see them, please let me know.

[ericstj]

I see, so it's just a coincidence of ordering that it works at all in the shipping product. I think the fix here is to have the CrossGen2 package fix it's use of TargetPath. It's incorrect to use a directory for that metadata, as can be seen here:
https://github.com/dotnet/msbuild/blob/c88325c7859bcd282ff1fae14ed95936e051e1c3/src/Tasks/Microsoft.Common.CurrentVersion.targets#L6582
If those targets weren't intending to use the common metadata they should just create their own that doesn't have meaning in other parts of the existing toolchain. AFAICT the usage on ReferenceCopyLocalPaths here ends up being ignored later on, so it might not even be necessary, but I can't say I fully understand the scenario to know for sure. cc @jkoritzinsky

[hoyosjs]

Probably the fix here is to change that SFXPROJ section to be:

  <Target Name="AddRuntimeFilesToPackage" DependsOnTargets="ResolveRuntimeFilesFromLocalBuild">
    <ItemGroup>
      <CrossgenFile Include="@(RuntimeFiles)" Condition="'%(Filename)' == 'crossgen'" />
      <OptimizationDataFile Include="@(RuntimeFiles)" Condition="'%(Filename)' == 'StandardOptimizationData'" />
      <ReferenceCopyLocalPaths Include="@(RuntimeFiles)" Exclude="@(CrossgenFile);@(OptimizationDataFile)" />
        <TargetPath>tools/%(RuntimeFiles.Filename)%(RuntimeFiles.Extension)</TargetPath>
      <ReferenceCopyLocalPaths/>
      <ReferenceCopyLocalPaths DestinationSubDirectory="tools/" />
    </ItemGroup>
  </Target>

  <Target Name="AddFrameworkFilesToPackage" DependsOnTargets="ResolveLibrariesFromLocalBuild">
    <ItemGroup>
      <ReferenceCopyLocalPaths Include="@(LibrariesRuntimeFiles)" DestinationSubDirectory="tools/">
        <TargetPath>tools/%(LibrariesRuntimeFiles.Filename)%(LibrariesRuntimeFiles.Extension)</TargetPath>
      <ReferenceCopyLocalPaths/>
    </ItemGroup>
  </Target>

cc: @dotnet/crossgen-contrib

[MichaelSimons]

@hoyosjs, I applied your suggested fix to the tip of release/6.0 and encountered the following errors in _GenerateFrameworkList. Let me know if you want a binlog and I can share it with you.

  /repos/runtime/artifacts/source-build/self/package-cache/microsoft.dotnet.sharedframework.sdk/6.0.0-beta.21427.6/targets/sharedfx.targets(401,5): error : No public key token found for assembly /repos/runtime/artifacts/source-build/self/src/artifacts/obj/Microsoft.NETCore.App.Crossgen2/Release/net6.0/linux-x64/R2R/ILCompiler.DependencyAnalysisFramework.dll [/repos/runtime/artifacts/source-build/self/src/src/installer/pkg/sfx/Microsoft.NETCore.App/Microsoft.NETCore.App.Crossgen2.sfxproj]
  /repos/runtime/artifacts/source-build/self/package-cache/microsoft.dotnet.sharedframework.sdk/6.0.0-beta.21427.6/targets/sharedfx.targets(401,5): error : No public key token found for assembly /repos/runtime/artifacts/source-build/self/src/artifacts/obj/Microsoft.NETCore.App.Crossgen2/Release/net6.0/linux-x64/R2R/ILCompiler.Diagnostics.dll [/repos/runtime/artifacts/source-build/self/src/src/installer/pkg/sfx/Microsoft.NETCore.App/Microsoft.NETCore.App.Crossgen2.sfxproj]
  /repos/runtime/artifacts/source-build/self/package-cache/microsoft.dotnet.sharedframework.sdk/6.0.0-beta.21427.6/targets/sharedfx.targets(401,5): error : No public key token found for assembly /repos/runtime/artifacts/source-build/self/src/artifacts/obj/Microsoft.NETCore.App.Crossgen2/Release/net6.0/linux-x64/R2R/ILCompiler.ReadyToRun.dll [/repos/runtime/artifacts/source-build/self/src/src/installer/pkg/sfx/Microsoft.NETCore.App/Microsoft.NETCore.App.Crossgen2.sfxproj]
  /repos/runtime/artifacts/source-build/self/package-cache/microsoft.dotnet.sharedframework.sdk/6.0.0-beta.21427.6/targets/sharedfx.targets(401,5): error : No public key token found for assembly /repos/runtime/artifacts/source-build/self/src/artifacts/obj/Microsoft.NETCore.App.Crossgen2/Release/net6.0/linux-x64/R2R/ILCompiler.TypeSystem.ReadyToRun.dll [/repos/runtime/artifacts/source-build/self/src/src/installer/pkg/sfx/Microsoft.NETCore.App/Microsoft.NETCore.App.Crossgen2.sfxproj]
  /repos/runtime/artifacts/source-build/self/package-cache/microsoft.dotnet.sharedframework.sdk/6.0.0-beta.21427.6/targets/sharedfx.targets(401,5): error : No public key token found for assembly /repos/runtime/artifacts/source-build/self/src/artifacts/obj/Microsoft.NETCore.App.Crossgen2/Release/net6.0/linux-x64/R2R/crossgen2.dll [/repos/runtime/artifacts/source-build/self/src/src/installer/pkg/sfx/Microsoft.NETCore.App/Microsoft.NETCore.App.Crossgen2.sfxproj]
      0 Warning(s)
      5 Error(s)

[hoyosjs]

That normally means that such assemblies were not run through crossgen, so they are still delay signed. If you have the binlog handy, I could take a look.

[hoyosjs]

Looks like https://github.com/dotnet/arcade/blob/d70e912c6a60971d5281163dd0b2159fdfc643e3/src/Microsoft.DotNet.SharedFramework.Sdk/src/CreateFrameworkListFile.cs#L119 assumes that TargetPath is a folder target, and not DestinationSubDirectory, so the fix would likely also need to go there.

[hoyosjs]

And looks like that same assumption is made all over the SFX SDK: https://github.com/dotnet/arcade/blob/d70e912c6a60971d5281163dd0b2159fdfc643e3/src/Microsoft.DotNet.SharedFramework.Sdk/targets/sharedfx.targets#L113-L142 and some other places that seem to use a directory as well https://github.com/dotnet/arcade/blob/d70e912c6a60971d5281163dd0b2159fdfc643e3/src/Microsoft.DotNet.Build.Tasks.Packaging/src/build/Packaging.targets#L279, however usage in that file is inconsistent - sometimes used as a file, or a directory - with the following disclaimer. https://github.com/dotnet/arcade/blob/d70e912c6a60971d5281163dd0b2159fdfc643e3/src/Microsoft.DotNet.Build.Tasks.Packaging/src/build/Packaging.targets#L458-L461. Looks like this is just an inconsistent behavior all around, and that file path should be preferred but both are technically valid? Maybe @dsplaisted, @rainersigwald, or someone from NuGet have better guidance on what the correct behavior is so we can go an fix this? Either way, we'll need to update the SFX SDK and potentially runtime.

[ericstj]

What if you instead just move these targets to run after ResolveReferences (and thus after conflict resolution) or even disable conflict resolution entirely? That might avoid churning stuff too much at this point in the product. IMHO we shouldn't be relying on the sorts of things conflict resolution does in an installer project: we'd want to be explicit about which files are includeded.

[hoyosjs]

hmm I agree about not wanting to rely on that - strongly. But that is also a big change in behavior and I am not familiar enough with the ordering and dependencies of targets. I am not sure we have enough time to test. Looks like resolution took a dependency on a TargetPath being the file-named one (in my opinion the better option as some files don't have extensions).

Maybe a targeted unblock for now is in SFX changing that one line to something that does the following:

-string path = Path.Combine(f.TargetPath, f.Filename).Replace('\\', '/');
+string path = Path.GetExtension(f.TargetPath) == Path.GetExtension(f.FileName) ? f.TargetPath.Replace('\\', '/') : Path.Combine(f.TargetPath, f.Filename).Replace('\\', '/');

There are some edge cases around extensions and unix files on this. Any strong opinions?

[ericstj]

Interesting, it looks like my suggestion isn't even valid as it seems like the actual place where conflict resolution is running is not part of ResolveReferences. Instead the CrossGen target is calling it directly 😮
https://github.com/dotnet/sdk/blob/43c8f12b569f6bf163cc9679167c9c340290ac46/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.CrossGen.targets#L396-L399

There are some edge cases around extensions and unix files on this. Any strong opinions?

If possible isolate your change to the single project that needs it rather than touching the SDK since that SDK is used by many other projects which likely don't suffer from this problem.