Publish postprocessing logic doesn't account for conflicts

[sbomer]

The Publish logic that computes the assemblies to postprocess misses assemblies which were picked by conflict resolution during publish. This results for example in illink not getting a chance to process such assemblies.

To Reproduce

Add a reference to Microsoft.Diagnostics.Tracing.TraceEvent version 3.0.2:

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

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net9.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
    <SelfContained>true</SelfContained>
    <!-- <PublishTrimmed>true</PublishTrimmed> -->
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.Diagnostics.Tracing.TraceEvent" Version="3.0.2" />
  </ItemGroup>

</Project>

dotnet publish -v:d. Note that the output has:

Encountered conflict between 'CopyLocal:/home/svbomer/.nuget/packages/microsoft.diagnostics.tracing.traceevent/3.0.2/lib/netstandard2.0/Dia2Lib.dll' and 'CopyLocal:/home/svbomer/.nuget/packages/microsoft.diagnostics.tracing.traceevent/3.0.2/lib/netstandard1.6/Dia2Lib.dll'. Choosing 'CopyLocal:/home/svbomer/.nuget/packages/microsoft.diagnostics.tracing.traceevent/3.0.2/lib/netstandard1.6/Dia2Lib.dll' arbitrarily as both items are copy-local and have equal file and assembly versions.

There is a conflict between the netstandard1.6 and netstandard2.0 versions of Dia2Lib.dll, and the conflict resolution picks one and continues.

Now uncomment the <PublishTrimmed> setting in the project file and publish again. Notice that the illink.dll invocation in the log does not pass Dia2Lib.dll.

Further technical details

I see that RuntimeCopyLocalItems has a netstandard2.0 version of Dia2Lib, but ResolvedFileToPublish has a netstandard1.6 version, so the two don't unify in the logic which computes the set of assemblies to postprocess:

sdk/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Publish.targets

Lines 952 to 954 in be20b69

	<JoinItems Left="@(_ManagedRuntimeAssembly)" Right="@(ResolvedFileToPublish)" RightMetadata="*">
	<Output TaskParameter="JoinResult" ItemName="_AssemblyToPostprocessOnPublish" />
	</JoinItems>

_AssemblyToPostprocessOnPublish doesn't contain Dia2Lib.

Compare this to the Publish conflict resolution logic:

sdk/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Publish.targets

Lines 544 to 549 in 3786af2

	<ResolveOverlappingItemGroupConflicts ItemGroup1="@(_ResolvedCopyLocalPublishAssets->Distinct())"
	ItemGroup2="@(ResolvedFileToPublish->Distinct())"
	PreferredPackages="$(PackageConflictPreferredPackages)">
	<Output TaskParameter="RemovedItemGroup1" ItemName="_ResolvedCopyLocalPublishAssetsRemoved" />
	<Output TaskParameter="RemovedItemGroup2" ItemName="ResolvedFileToPublishRemoved" />
	</ResolveOverlappingItemGroupConflicts>

This compares ResolvedFileToPublish with _ResolvedCopyLocalPublishAssets (which is populated earlier from RuntimeCopyLocalItems), and chooses one to include in the output.

I think the root cause is this discrepancy between the filtering in these two places. Seems like the logic that computes assemblies to postprocess should operate on the previously filtered set, without doing its own different filtering.

[Forgind]

Thanks for a clear bug report! It sounds like you just want to delete lines 952-954 and replace them by filling _AssemblyToPostprocessOnPublish with the output from ResolveOverlappingItemGroupConflicts? That sounds like a pretty reasonable proposal to me.

[marcpopMSFT]

Triage: @sbomer if you want to make the change Forgind suggested, we can review. If not, we'll try to get to it eventually. We'll want to ensure the repro is included in a test.

[sbomer]

Not sure if directly filling it from the ResolveOverlappingItemGroupConflicts outputs is exactly right, since _ManagedRuntimeAssembly is computed from a variety of other inputs. But one of the inputs is RuntimeCopyLocalItems - maybe that one should instead be _ResolvedCopyLocalPublishAssets, here:

sdk/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Publish.targets

Line 932 in be20b69

<_ManagedRuntimeAssembly Include="@(RuntimeCopyLocalItems)" />

I'd rather someone on the SDK team makes the change because I'm not familiar with the inputs to _ManagedRuntimeAssembly and don't want to accidentally break something here:

sdk/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Publish.targets

Lines 930 to 944 in be20b69

	<ItemGroup>
	<!-- Assemblies from packages -->
	<_ManagedRuntimeAssembly Include="@(RuntimeCopyLocalItems)" />
	<!-- Assemblies from other references -->
	<_ManagedRuntimeAssembly Include="@(UserRuntimeAssembly)" />
	<!-- Assembly produced by this project -->
	<_ManagedRuntimeAssembly Include="@(IntermediateAssembly)" />
	</ItemGroup>
	<!-- Assemblies from runtime packs for self-contained apps -->
	<ItemGroup Condition="'$(SelfContained)' == 'true'">
	<_ManagedRuntimeAssembly Include="@(_ManagedRuntimePackAssembly)" />
	</ItemGroup>