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

[release/5.0] Switch between either WinFX targets #4255

Closed

Conversation

Nirmal4G
Copy link
Contributor

@Nirmal4G Nirmal4G commented Mar 7, 2021

Closes #4253

Master PRs: #2976, #4629, #4630

Description

When using the Desktop SDK, the build double imports both Full framework and Core WinFX targets, leading to build break.
Also, there's no straight forward way to switch between either WinFX targets as needed. This patch provides such mechanism through ImportFrameworkWinFXTargets.

Although there is a workaround for the above issue, they require modifying the installation of the .NET SDKs. So, it's preferable to have them fixed at least for Current (v5.0) .NET SDK.

Customer Impact

We (and possibly many others like me) have a large set of NETCLR WPF projects that can't move to CoreCLR right now. I have recently converted them to sdk-style projects with the WindowsDesktop SDK which includes custom tasks that are meant for Full framework projects. We also have some silverlight projects to maintain and I recently converted them to sdk-style too. Seems they also need WinFX targets.

So, to use the official WindowsDesktop, we want to switch between either WinFX targets for our custom tasks to take effect. This patch is just allowing that. We're migrating our projects and tools to .NET CoreCLR and it'll take some time. Until then, we need this ability to maintain our projects.

Regression

Yes, Desktop SDK and WinFX targets were introduced since v3.0!

Testing

Manual testing and mostly with our WPF projects builds against both .NET Core and Full framework CLRs.

Risk

There should be minimal to no risk to most when taking this patch.
But, if you're building both Full framework and Core projects with heavily modified build like ours, depending on the behaviour of the double import, then Yes, the build will break.
However, if you already have your workarounds placed, then there should be no problem. You can also remove those workarounds when updating to the SDK with this patch as it fixes the issue.

@Nirmal4G Nirmal4G requested a review from a team as a code owner March 7, 2021 14:52
@ghost ghost added the PR metadata: Label to tag PRs, to facilitate with triage label Mar 7, 2021
@ghost ghost requested review from fabiant3, ryalanms and SamBent March 7, 2021 14:52
@Nirmal4G Nirmal4G changed the title [release/3.1] Don't double import WinFX targets [release/5.0] Don't double import WinFX targets Mar 7, 2021
@Nirmal4G Nirmal4G changed the title [release/5.0] Don't double import WinFX targets [release/5.0] Switch between either WinFX targets Mar 7, 2021
@Nirmal4G Nirmal4G force-pushed the hotfix/disable-netfx-winfx-v5.0 branch 3 times, most recently from d2c63da to bb5799a Compare March 12, 2021 17:55
@Nirmal4G Nirmal4G force-pushed the hotfix/disable-netfx-winfx-v5.0 branch from bb5799a to ff4a0cc Compare June 4, 2021 12:57
@Nirmal4G Nirmal4G force-pushed the hotfix/disable-netfx-winfx-v5.0 branch 2 times, most recently from 6c0f230 to 0a6051c Compare June 15, 2021 03:32
Only in Desktop SDK and WinFX props/targets
Respect EditorConfig preferences for whitespace and new-lines
The MSBuild items & targets that were defined after the WinFX targets import doesn't depend on anything from the WinFX targets.
So, it is safe and better to declare the import after those items and targets definitions.
When the SDK's implementation is being used.
Currently there's no way to properly switch between NETFX's and CoreCLR's WinFX targets.
This adds an opt-out, just in case, if we want to use the NETFX's WinFX targets instead.
@Nirmal4G Nirmal4G force-pushed the hotfix/disable-netfx-winfx-v5.0 branch from 0a6051c to cb0e626 Compare August 26, 2021 08:38
@Nirmal4G
Copy link
Contributor Author

@ryalanms @SamBent 🔔

@Nirmal4G
Copy link
Contributor Author

Can someone/anyone (please) take a look at this patch at priority? 🧐

Been asking for months. Any feedback or response would be nice… 😔

@pchaurasia14 pchaurasia14 added the Community Contribution A label for all community Contributions label Jul 20, 2022
@ghost ghost assigned Nirmal4G Jul 20, 2022
@singhashish-wpf singhashish-wpf deleted the branch dotnet:release/5.0 January 31, 2023 07:13
@Nirmal4G Nirmal4G deleted the hotfix/disable-netfx-winfx-v5.0 branch January 31, 2023 15:31
@Nirmal4G
Copy link
Contributor Author

This doesn't look good on you, M$FT! Not even taking a look at a patch and then closing it when it becomes irrelevant. Not a good open-source practice! Very disappointed here!!

@pchaurasia14
Copy link
Member

@Nirmal4G - Since release/3.1 and release/5.0 branches are deleted from the repository, this PR got closed as a consequence.

Also, we recommend all PRs to be made against main branch (instead of any release/* branches.

We apologize for the inconvenience you have experienced. If the PR addresses an issue that is still relevant, you may open the same against main branch, we will review it for our upcoming CTP(s).

@ghost ghost locked as resolved and limited conversation to collaborators Mar 2, 2023
@Nirmal4G Nirmal4G removed their assignment Jul 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Community Contribution A label for all community Contributions PR metadata: Label to tag PRs, to facilitate with triage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants