Skip to content
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

Conflict resolution doesn't drop copy local assemblies which were absorbed into .NETStandard2.1 #18129

Open
ericstj opened this issue Jun 7, 2021 · 4 comments
Milestone

Comments

@ericstj
Copy link
Member

ericstj commented Jun 7, 2021

Build the following project:

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <TargetFramework>netstandard2.1</TargetFramework>
    <CopyLocalLockFileAssemblies>true</CopyLocalLockFileAssemblies>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="System.Security.Cryptography.Pkcs" Version="5.0.1" />
  </ItemGroup>
</Project>

Expect:
System.Buffers.dll, System.Memory.dll, System.Numerics.Vectors.dll should be absent from the output directory.

Actual:
All are present.

These are all conflict resolved from the reference assets, but not runtime. It looks like netstandard is missing runtime handling for conflict resolution. Looks like the same thing happened during netstandard2.0 so this isn't new.

Perhaps we could avoid this by leveraging

/// <summary>
/// A collection of items that contain information of which packages get overridden
/// by which packages before doing any other conflict resolution.
/// </summary>
/// <remarks>
/// This is an optimization so AssemblyVersions, FileVersions, etc. don't need to be read
/// in the default cases where platform packages (Microsoft.NETCore.App) should override specific packages
/// (System.Console v4.3.0).
/// </remarks>
public ITaskItem[] PackageOverrides { get; set; }

Might improve perf on netstandard as well.

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged Request triage from a team member label Jun 7, 2021
@dotnet-issue-labeler
Copy link

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.

@ericstj
Copy link
Member Author

ericstj commented Jun 7, 2021

I was able to workaround this with the following:


  <Target Name="IncludeNetStandardConflictResolution" BeforeTargets="_HandlePackageFileConflicts">
    <ItemGroup>
      <_RuntimeAssetsForConflictResolution Include="@(Reference)" Condition="'%(Reference.NuGetPackageId)' == 'NETStandard.Library' or '%(Reference.NuGetPackageId)' == 'NETStandard.Library.Ref'" />
    </ItemGroup>
  </Target>

My suggestion of using PackageOverrides isn't good enough, since it looks like that's not considered unless there is a conflict. So the fix here would need to represent the runtime items expected by a .NETStandard-implementing framework. To avoid hitting files, it means we should probably have a platform manifest or framework list.

@dsplaisted
Copy link
Member

Is CopyLocalLockFileAssemblies really even valid for a .NET Standard project? Doesn't it basically mean the implementation assemblies should be copied to the output folder? But on .NET Standard there's no guarantee that there are implementation assemblies at all (for .NET Standard).

@dsplaisted dsplaisted added needs team triage Requires a full team discussion and removed untriaged Request triage from a team member labels Jul 29, 2021
@dsplaisted dsplaisted removed their assignment Jul 29, 2021
@ericstj
Copy link
Member Author

ericstj commented Jul 29, 2021

Right, but people still do it, and in some places we tell customers to do it (EG: roslyn analyzers). We could block it completely, but I suspect that's too breaking. If we allow it, then we should fix this.

@marcpopMSFT marcpopMSFT added Area-NetSDK and removed needs team triage Requires a full team discussion labels Aug 4, 2021
@marcpopMSFT marcpopMSFT added this to the Backlog milestone Aug 4, 2021
v-wuzhai pushed a commit that referenced this issue Apr 22, 2024
ViktorHofer added a commit that referenced this issue May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants