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

Import WinFX Targets Only When Targeting .NETFramework #12789

Merged

Conversation

benvillalobos
Copy link
Member

Description

MSBuild was seeing different behavior between msbuild and dotnet builds with sdk style projects, so we stopped importing these targets for sdk style projects. The mistake we made when we implemented this fix was that we should have continued to import them for sdk style WPF projects that targeted .NETFramework specifically.

Customer Impact

This impacts all customers using SDK style WPF projects that target .NETFramework. They will import the incorrect Microsoft.WinFx.targets file and see failed builds unless they set the ImportFrameworkWinFXTargets property to any non false value. A couple of internal teams already have reported not being able to compile their projects using the 5.0 sdk including VSTest and project system.

Regression?

Yes, by #10998

Risk

Low risk.

Test changes in this PR

No test changes.

Additional Notes

Fixes #12324 by importing framework winfx.targets only when TargetFrameworkIdentifier is .NETFramework and UseWPF is false. Otherwise, ImportFrameworkWinFXTargets will be set to false and thus not import.

Moved the import of winfx.targets after Microsoft.NET.TargetFrameworkInference.targets, so we know what the TargetFrameworkIdentifier is by the time we need to check it.

I wrote the same property twice with two separate conditions because of how we implemented ImportFrameworkWinFXTargets on the msbuild side. So I wrote this in a way that'seasier easier to read, although there's an extra line.

Note that any non false value is interpreted as "yes, import framework winfx targets".

Side note, if TargetFrameworkIdentifier is netstandard, what should happen here?

Equivalent PR to master: #12786

@benvillalobos
Copy link
Member Author

I just verified with @davkean with his repro that this fix resolves the issue.

@dsplaisted project-system will not set UseWPF to true in their projects. So that test would be irrelevant. Unless directed otherwise I'll continue to test setting UseWPF to true on a framework-to-sdk-style converted WPF project.

It's also worth noting that I've verified through various binlogs that setting UseWPF to true guarantees ImportFrameworkWinFXTargets being set to false.

@mmitche mmitche merged commit 597a7a0 into dotnet:release/5.0.1xx-preview8 Aug 7, 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