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

Fix target override as it doesn't work when the target import order changes. #977

Merged
merged 3 commits into from
Jul 8, 2021

Conversation

jlaanstra
Copy link
Contributor

@jlaanstra jlaanstra commented Jul 8, 2021

Current C++/WinRT override the GetResolvedWinMD target. However if the import order changes this override can easily break, like it does when NuGet imports the targets as part of PackageReference.

This PR removes the override.

Validation:

  • OS
  • WinUI
  • Terminal
  • YourPhone
  • React-Native

@sylveon
Copy link
Contributor

sylveon commented Jul 8, 2021

I thought PackageReference was not supported for C++?

@jlaanstra
Copy link
Contributor Author

There seems to be ways to enable it, see dotnet/project-system#2491

@kennykerr
Copy link
Collaborator

Kicked off a build.

@jlaanstra
Copy link
Contributor Author

jlaanstra commented Jul 8, 2021

Normal validation passed. Since the react-native team is also experimenting with PackageReferences, tagging @JunielKatarn to see if he can test this for react-native. Probably don't have to block on that.

To validate this, copy the Microsoft.Windows.CppWinRT.targets over the one in the NuGet package you currently use (I would recommend doing this without any PackageReference changes) and run a build locally.

@jlaanstra jlaanstra requested a review from kennykerr July 8, 2021 18:24
@jlaanstra jlaanstra merged commit 54c1097 into master Jul 8, 2021
@jlaanstra jlaanstra deleted the user/jlaans/package-reference-fix branch July 8, 2021 19:26
jlaanstra added a commit that referenced this pull request Jul 13, 2021
#977 removed an override of a built-in CX target that wasn't working reliably. Because of this, various targets to resolve the WinMD would no longer find the WinMD file. This never worked reliably but it turns out the Xaml designer uses one of these targets. This PR adds a BeforeTargets to ensure the WinMD will be resolved.

Fixes #981
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