Skip to content

Don't pass duplicate items to conflict resolution if CopyLocalLockFileAssemblies is true - #1256

Merged
dsplaisted merged 1 commit into
dotnet:release/2.0.0from
dsplaisted:1135-extra-conflicts
May 26, 2017
Merged

dsplaisted merged 1 commit into
dotnet:release/2.0.0from
dsplaisted:1135-extra-conflicts

Conversation

@dsplaisted

Copy link
Copy Markdown
Member

Fixes #1135

@dsplaisted

Copy link
Copy Markdown
Member Author

@dotnet-bot

test OSX10.12 Release
test Windows_NT_FullFramework Debug
test Windows_NT_FullFramework Release

@dsplaisted
dsplaisted requested review from livarcocc and nguerrera May 26, 2017 01:41
@dsplaisted

Copy link
Copy Markdown
Member Author

@ericstj for review

will always be included in ReferenceCopyLocalPaths.
-->
<_LockFileAssemblies Include="@(AllCopyLocalItems->WithMetadataValue('Type', 'assembly'))" />
<_LockFileAssemblies Include="@(AllCopyLocalItems->WithMetadataValue('Type', 'assembly'))"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Today, CopyLocalLockFileAssemblies for netcoreapp include shared framework and don't respect other publish filtering mechanisms either. I think that's what you want here, but I also want to fix CopyLocalLockFileAssemblies to be more like "publish in place". See #933

I'm annoyed by the fact that the log can be filled with items called CopyLocal that are not in fact copied anywhere. I'm not pushing back on this change because we're already in that boat, but it's something that's been on my mind for a while.

@nguerrera

Copy link
Copy Markdown
Contributor

@livarcocc Did you turn on the "this branch is out-of-date with the base branch" check? I think it does more harm than good. On several occasions, folks have done incorrect merges from release to master after submitting PR straight from release to master and pushing the big button to update release to master! It also adds extra merge commits to the history that we generally don't need. Can we turn it back off please?

@dsplaisted

Copy link
Copy Markdown
Member Author

@MattGertz for approval

Customer scenario

Build a project targeting .NET Framework which references a NuGet package. Without this fix, there will be errors in the log indicating there is a conflict when there shouldn't be.

Bugs this fixes:

#1135

Workarounds, if any

Ignore log file.

Risk

Low

Performance impact

Low - This will probably improve perf as it will mean the conflict resolver doesn't have to examine as many assemblies.

Is this a regression from a previous update?

It's a regression from the version of conflict resolution that was originally shipped in preview versions of the 2.0 NETStandard.Library package.

Root cause analysis:

A case was missed while moving and refactoring the code.

How was the bug found?

Bug report from @ericstj

@dsplaisted
dsplaisted merged commit e853bf9 into dotnet:release/2.0.0 May 26, 2017
mmitche pushed a commit to mmitche/sdk that referenced this pull request Jun 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants