Skip to content

Use ResolveProjectReferences target and limit WinMDs passed to mdmerge.exe#491

Merged
Scottj1s merged 15 commits into
microsoft:masterfrom
jlaanstra:jlaans/project-references
Aug 27, 2019
Merged

Use ResolveProjectReferences target and limit WinMDs passed to mdmerge.exe#491
Scottj1s merged 15 commits into
microsoft:masterfrom
jlaanstra:jlaans/project-references

Conversation

@jlaanstra

@jlaanstra jlaanstra commented Jun 30, 2019

Copy link
Copy Markdown
Contributor

This PR fixes a few issues with the way CppWinRT handles project references, that prevent certain project types from not being included in the reference projection or causes duplicate type errors because the winmd ends up being referenced twice:

  • When a Runtime Component and a Static Library reference the same NuGet package containing a WinMD file, it will be referenced twice because the WinMD will always be copied to the output folder.
  • A CX project referenced by a CppWinRT project won't have it's WinMD included in the reference projection.
  • You have to set project to project references to not copy the reference locally in the properties window to prevent duplicate type errors.

To fix those problems I made two changes.

  1. Hook into the ResolveProjectReferences target instead of ResolvedLinkLib.
  2. Pass the winmds directly to mdmerge, instead of using folders as input.

@jlaanstra jlaanstra requested a review from Scottj1s as a code owner June 30, 2019 20:39
@jlaanstra

Copy link
Copy Markdown
Contributor Author

Seems like I have some whitespace changes that I will revert.

@jlaanstra

Copy link
Copy Markdown
Contributor Author

@kennykerr probably want to wait for @Scottj1s to take a look before merging.

@kennykerr

Copy link
Copy Markdown
Contributor

Yes, Scott will be back in a week. This can wait.

Comment thread src/package/cppwinrt/nuget/Microsoft.Windows.CppWinRT.targets
Comment thread src/package/cppwinrt/nuget/Microsoft.Windows.CppWinRT.targets Outdated
Comment thread src/package/cppwinrt/nuget/Microsoft.Windows.CppWinRT.targets Outdated
Comment thread src/package/cppwinrt/nuget/Microsoft.Windows.CppWinRT.targets
@Scottj1s

Scottj1s commented Jul 9, 2019

Copy link
Copy Markdown
Member

Lots of tweaks (thanks!) - how has this been tested? Both VS 2017 and 2019, with various p2p reference scenarios?

@jlaanstra

Copy link
Copy Markdown
Contributor Author

I tested this on the YourPhone solution with 34 projects. It has:

  • Static lib -> Nuget package
  • Dynamic lib -> NuGet package
  • Dynamic lib -> Static lib
  • Dynamic lib -> Dynamic lib
  • CX -> WinRT
  • WinRT -> CX

@Scottj1s

Scottj1s commented Jul 9, 2019

Copy link
Copy Markdown
Member

Great - thank you!

One remaining test scenario - dynamic projects with multiple winmds (see comment)


In reply to: 509732892 [](ancestors = 509732892)

Comment thread src/package/cppwinrt/nuget/Microsoft.Windows.CppWinRT.targets Outdated
Comment thread src/package/cppwinrt/nuget/Microsoft.Windows.CppWinRT.targets
Comment thread src/test/package/nuget/NuGetTest/TestRuntimeComponent2/module.cpp
Comment thread src/test/package/nuget/NuGetTest/Directory.Build.props
Comment thread src/test/package/nuget/NuGetTest/TestApp/packages.config Outdated
@Scottj1s

Copy link
Copy Markdown
Member

great test collateral - just need to point to props/targets in repo instead of using last published Nuget.

I was just discussing with Kenny scripting the creation of something like this from project templates (also testing the VSIX). Seems doable with dotnet CLI, which can create projects, references, etc - but we can do that later.

@jlaanstra jlaanstra force-pushed the jlaans/project-references branch from 0679bfc to 9702ada Compare July 31, 2019 05:02
@jlaanstra jlaanstra force-pushed the jlaans/project-references branch from a2f1819 to 0585d74 Compare August 14, 2019 03:52
Comment thread src/test/package/nuget/NuGetTest/Directory.Build.props Outdated
Comment thread src/test/package/nuget/NuGetTest/Directory.Build.targets Outdated
Comment thread src/test/package/nuget/NuGetTest/NuGetTest.sln
@Scottj1s

Copy link
Copy Markdown
Member

Looks Good!

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 this pull request may close these issues.

3 participants