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

Duplicate items in _ResolvedCopyLocalPublishAssets #3007

Closed
sbomer opened this issue Mar 4, 2019 · 1 comment · Fixed by #3021
Closed

Duplicate items in _ResolvedCopyLocalPublishAssets #3007

sbomer opened this issue Mar 4, 2019 · 1 comment · Fixed by #3021
Assignees
Milestone

Comments

@sbomer
Copy link
Member

sbomer commented Mar 4, 2019

  • When ResolveCopyLocalAssets runs during self-contained publish (_UseBuildDependencyFile is false), it outputs _ResolvedCopyLocalPublishAssets.
  • When _ComputeResolvedCopyLocalPublishAssets runs, it adds to _ResolvedCopyLocalPublishAssets the contents of (ReferenceCopyLocalPaths \ _ResolvedCopyLocalBuildAssets):
    <_ResolvedCopyLocalPublishAssets Include="@(ReferenceCopyLocalPaths)"
    Exclude="@(_ResolvedCopyLocalBuildAssets)"

_ResolvedCopyLocalBuildAssets is empty, so it gets a duplicate copy of items in ReferenceCopyLocalPaths (which comes from RuntimePackAsset), for example System.Private.CoreLib.dll.

Before #2646, ResolvedAssembliesToPublish only had one Item for System.Private.CoreLib.dll. I noticed this while trying to update https://github.com/mono/linker/blob/master/src/ILLink.Tasks/ILLink.Tasks.targets#L451 to work with the newest SDK. _ManagedAssembliesToLink was computed from ResolvedAssembliesToPublish, but that line breaks when there are multiple files with the same Filename in the input:

C:\Users\svbomer\.nuget\packages\illink.tasks\0.1.6\build\ILLink.Tasks.targets(451,34): error MSB4094: "C:\Users\svbomer\.nuget\packages\runtime.win-x64.microsoft.netcore.app\3.0.0-preview-27324-5\runtimes\win-x64\native\System.Private.CoreLib.dll;C:\Users\svbomer\.nuget\packages\runtime.win-x64.microsoft.netcore.app\3.0.0-preview-27324-5\runtimes\win-x64\native\System.Private.CoreLib.dll" is an invalid value for the "AssemblyPath" parameter of the "CheckEmbeddedRootDescriptor" task. Multiple items cannot be passed into a parameter of type "Microsoft.Build.Framework.ITaskItem". [D:\linker\test\ILLink.Tasks.Tests\bin\debug\netcoreapp3.0\helloworld\helloworld.csproj]

#2666 may be relevant.

/cc @peterhuene @nguerrera

@peterhuene peterhuene self-assigned this Mar 5, 2019
@peterhuene peterhuene added this to the 3.0.1xx milestone Mar 5, 2019
sbomer added a commit to sbomer/linker that referenced this issue Mar 5, 2019
sbomer added a commit to sbomer/linker that referenced this issue Mar 8, 2019
@peterhuene
Copy link
Contributor

The simple fix would be to just filter out RuntimePackAsset when adding items from ReferenceCopyLocalPaths.

Unfortunately it also seems that the runtime pack changes also introduced package references that are publish=false by default (i.e. the implicitly defined packages that are from the runtime packs which have their assets available in the RuntimePackAsset items).

As the set of packages now differs from the build, this causes the publish targets to fallback to resolving (non-platform) assets and generating a different deps file, which you can see just by diffing the deps file from build vs. publish; the former has entries for the packages excluded from publish and the latter does not.

This logic is getting difficult to reason because data is coming from many different places into the _ResolvedCopyLocalPublishAssets items. I'm working on a better solution than simply filtering out RuntimePackAsset from ReferenceCopyLocalPaths when adding to _ResolvedCopyLocalPublishAssets.

marek-safar pushed a commit to dotnet/linker that referenced this issue Mar 8, 2019
* Use 3.0 preview SDK to build illink

* Respond to SDK changes in conflict resolution targets

* Respond to SDK changes to publish assembly resolution

* Work around duplicate items introduced by an sdk change

See dotnet/sdk#3007

* Remove dependency on GetRuntimeLibraries

Instead, use RuntimePackAssets resolved by the SDK
peterhuene pushed a commit to peterhuene/sdk that referenced this issue Mar 14, 2019
When `CopyLocalLockFileAssemblies` was true, `ReferenceCopyLocalPaths`
contained the set of `RuntimePackAsset` items.

When resolving assets to copy local for publish, the `RuntimePackAsset` items
were added twice: once explicitly and again via `ReferenceCopyLocalPaths`.

This commit fixes this by only adding to the resolved copy local assets for
publish when `CopyLocalLockFileAssemblies` is false.

Fixes dotnet#3007.
peterhuene pushed a commit to peterhuene/sdk that referenced this issue Mar 14, 2019
When `CopyLocalLockFileAssemblies` was true, `ReferenceCopyLocalPaths`
contained the set of `RuntimePackAsset` items.

When resolving assets to copy local for publish, the `RuntimePackAsset` items
were added twice: once explicitly and again via `ReferenceCopyLocalPaths`.

This commit fixes this by only adding to the resolved copy local assets for
publish when `CopyLocalLockFileAssemblies` is false.

Fixes dotnet#3007.
wli3 pushed a commit that referenced this issue Feb 7, 2020
* Update dependencies from https://github.com/dotnet/cli build 20190930.6

- Microsoft.DotNet.Cli.Runtime - 5.0.100-alpha1.19480.6

* Update dependencies from https://github.com/dotnet/cli build 20191001.3

- Microsoft.DotNet.Cli.Runtime - 5.0.100-alpha1.19501.3

* Update dependencies from https://github.com/dotnet/cli build 20191001.6

- Microsoft.DotNet.Cli.Runtime - 5.0.100-alpha1.19501.6
tkapin pushed a commit to tkapin/runtime that referenced this issue Jan 31, 2023
* Use 3.0 preview SDK to build illink

* Respond to SDK changes in conflict resolution targets

* Respond to SDK changes to publish assembly resolution

* Work around duplicate items introduced by an sdk change

See dotnet/sdk#3007

* Remove dependency on GetRuntimeLibraries

Instead, use RuntimePackAssets resolved by the SDK


Commit migrated from dotnet/linker@5e9d86c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants