Optionally disable .NET Framework's WinFX targets

[Nirmal4G]

Optionally disable .NET Framework's WinFX targets i.e. Allow for op-out.
This change ensures the projects that were using Microsoft.NET.Sdk directly OR via their own customized WindowsDesktop Sdk to work as before (Regression).

Note: It will be permanently disabled if we use WindowsDesktop SDK (directly/indirectly via .NET5 workload resolution)

Reference: dotnet/wpf#2976

[marcpopMSFT]

@benvillalobos looks like you fixed this on the msbuild side. Is this needed anymore or can we close it out?

[benvillalobos]

@marcpopMSFT this is slightly different. It's allowing the option to disable it rather than forcing it disabled like my change does. It also shifts the definition of the property over to wpf, which I'm not against as long as it works. My main question is what's the use case here? I thought Importing it while on sdk style projects broke projects? But it looks like it imports then on net core and net standard projects.

[rainersigwald]

@Nirmal4G Can you please elaborate on the scenario that you think regressed? As I understood it, setting the property unconditionally in the SDK fixed a bug that allowed an odd mixture of .NET Core SDK and .NET Framework targets resulting in questionable, unreliable access to WPF build functionality.

[Nirmal4G]

This is just to ensure that those who have only NETFX WPF projects can use the sdk-style with their WinFX targets instead. I too have customized build logic that depends on it.

I'm trying to use WindowsDesktop SDK where possible but when it does not work, it's just safe to have the NETFX fallback.

Making it overridable allows just that.

[vatsan-madhavan]

It's has been intentional decision to not to support NetFx based WinFx.targets (and PresentationBuildTasks) in NET.Sdk.WindowsDesktop (and in general, in SDK style projects).

In fact, it has also been an intentional principle (speaking only wrt WindowsDesktop development) that NET.Sdk.WindowsDesktop - and SDK style projects in general - are only minimally supported for .NET Framework development insofar as it enables migration to .NET Core. Put simply, SDK style projects' support for .NET Framework based WindowsDesktop (or anything not(.NET Core)) development is focused on one use-case - namely migration to .NET Core.

Note the emphasis on support - which is different from saying what works etc.; netfx scenarios generally work well for WindowsDesktop development using SDK style projects. See [1] below which will create future challenges for maintaining this compatibility indefinitely..

I'm trying to use WindowsDesktop SDK where possible but when it does not work, it's just safe to have the NETFX fallback.

When it doesn't work, the preferred approach is to fix it/improve it so that it does work at par with NetFx version of WinFx.targets/PresentationBuildTasks.

Please help us understand your problem and you could get workarounds or fixes, or we could show you how to contribute to fixes. Falling back to older-tech and taking dependencies on it creates a forever-burden of sustaining that tech, which hampers our path forward.

[1] This is especially not sustainable in WinFX.targets because its functionality relies on the WPF runtime codebase significantly. The key reason NetFx/WinFx.targets+PresentationBuildTasks and WindowsDesktop/WinFx.targets+PresetationBuildTasks seem inter-compatible today is because the two codebases haven't (yet) diverged very much from one another.

Also, these solutions only apply when MSBuildRuntimeType==Full (msbuild, visual studio); When MSBuildRuntimeType==Core (dotnet.exe), NetFx/WinFx.targets never gets imported in the first place. This means that these proposals are already just partial band-aids. OTOH, identifying the failing scenario more precisely and making the right fix in the WindowsDesktop/WinFx.targets would apply equally to all build scenarios.

IMO suppressing NetFx/WinFx.targets import by default is the right approach. Instead of diluting this and providing an opt-in, we should see how to solve problems that arise as a side effect of this.

[Nirmal4G]

I agree. but at least allow opt-out than forcing it. I've changed it to be default on all targets. Also added comment to clarify it's intent.

Hope this is OK now?

[vatsan-madhavan]

allow opt-out than forcing it

Allowing opt-out implies enabling optional/opt-in use. Right? If so, then that's something I don't support for reasons I've explained. This is not limited to WindowsDesktop SDK - the rationale applies to all SDK style projects.

More importantly, I don't understand why this opt-in/opt-out capability is important. In other words, I don't understand what you can achieve with NetFx/WinFx.targets+PresentationBuildTasks that is broken in WindowsDesktop/WinFx.targets+PresentationBuildTasks.

If there is a need to have WinFX.targets functionality in non-WindowsDesktop scenarios, that's a good discussion to have and explore how to enable separately. The answer IMO is not perpetuating the legacy imports.

[Nirmal4G]

If there is a need to have WinFX.targets functionality in non-WindowsDesktop scenarios

There are some projects which uses Workflow, WCF and WPF (mixed) within the same project (the original authors achieved it some how) that I've been given the task to maintain and migrate to .NET (Core) by parts.

I have recently converted them to sdk-style but the projects which have WPF+Workflow or WPF+WCF has problems building. When I use the inbox targets it builds fine. I can't pinpoint where the problem lies. I'm trying to repro it. When I have a working repro, I'll share it here.

But at least for now this workaround will suffice. Please consider this.

[marcpopMSFT]

@Nirmal4G it looks like you simplified the fix and it's now just a conditional when setting the ImportFrameworkWinFXTargets property. Could you update the change back so it's the single line change as reording properties can have side affects that are unexpected?

[Nirmal4G]

@marcpopMSFT All Done. What about the whitespace changes (trimmed trailing whitespace)? Can I leave it in this PR?

[vatsan-madhavan]

functionally lgtm 👍

[marcpopMSFT]

@Nirmal4G Thanks for making the change. Looks good now and we'll do a final review/merge in our Wednesday PR review meeting.

[marcpopMSFT]

Thanks for the change