-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Change _BlockWinMDsOnUnsupportedTFMs dependencies to fix WPF temp targets ordering #12104
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
Change _BlockWinMDsOnUnsupportedTFMs dependencies to fix WPF temp targets ordering #12104
Conversation
…gets ordering The introduction of _BlockWinMDsOnUnsupportedTFMs broke the WPF temp project generation, which broke PowerShell Core. This PR changes the target ordering so it doesn't destabilize the WPF targets. I've validated that this change doesn't cause this target to be skipped on incremental builds. Fixes #12093 cc: @rlander @AaronRobinsonMSFT @ryalanms @wli3 @dsplaisted
BeforeTargets="CoreCompile" | ||
AfterTargets="PreBuildEvent" | ||
DependsOnTargets="ResolveReferences" | ||
Condition="'$(TargetFrameworkIdentifier)' == '.NETCoreApp' and $([MSBuild]::VersionGreaterThanOrEquals($(TargetFrameworkVersion), '5.0'))"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chsienki another case where work is predicated on netcoreapp
versions. Should sync up to see if this is the preferred style and if so consider using in our code.
test? |
To my understanding @ryalanms validated this change. Since the repro for this uses the WindowsDesktop SDK and requires a strong-named assembly, I'm not sure of the best way to add a test. |
I performed these steps:
Errors:
<Target Name="_BlockWinMDsOnUnsupportedTFMs"
BeforeTargets="CoreCompile"
DependsOnTargets="ResolveReferences" <Target Name="_BlockWinMDsOnUnsupportedTFMs"
AfterTargets="PreBuildEvent"
DependsOnTargets="ResolveReferences"
The build now succeeds. |
It should be possible to write a test. Tests in this repo can use the WindowsDesktop SDK, and should also be able to create a strong name. |
Is this ready to go? I needs to be ASAP or we will miss p6. |
It's ready. Daniel agreed to let me add a test in master after we merge this in. |
Did this fix ship in preview 7? I'm trying to track down an issue that broke WPF compilation* for me after I installed .NET 5 preview 7. If I uninstall it, it works again. *) in an Sdk-style project, but in .NET Framework 4.x, using MSBuild.Sdk.Extras |
Yes, it got merged into Preview 7. |
The introduction of _BlockWinMDsOnUnsupportedTFMs broke the WPF temp project generation, which broke PowerShell Core. This PR changes the target ordering so it doesn't destabilize the WPF targets.
I've validated that this change doesn't cause this target to be skipped on incremental builds.
Fixes #12093
cc: @rlander @AaronRobinsonMSFT @ryalanms @wli3 @dsplaisted