[release/3.1] WinFX casing and double-import fixes #4254
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ghost
requested review from
Nirmal4G
force-pushed
the
hotfix/winfx-v3.1
branch
5 times, most recently
from
fea11bb
to
c09f125
Compare
Nirmal4G
force-pushed
the
hotfix/winfx-v3.1
branch
from
c09f125
to
0144c0f
Compare
Nirmal4G
force-pushed
the
hotfix/winfx-v3.1
branch
from
0144c0f
to
bf22099
Compare
The 'MarkupCompiler' sources in 'PresentationBuildTasks' uses hardcoded backslash '\' which leads to projects failing build in non-Windows platforms. This patch replaces the hardcoded backslash to a cross-platform friendly replacement with 'Path.DirectorySeparator'. NOTE: This only fixes in PBT and not anywhere else.
'WinFX' is the correct casing as observed in the .NET CLR 2 frameworks
In project files, scripts, comments, etc... But not in sources (like GetWinFxCallback) 'WinFX' is the correct casing as observed in the .NET CLR 2 frameworks
When the SDK's implementation is being used.
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.
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.
Only in WinFX and Desktop SDK props/targets
Nirmal4G
force-pushed
the
hotfix/winfx-v3.1
branch
from
bf22099
to
8c101b8
Compare
|
Can someone/anyone (please) take a look at this patch at priority? 🧐 Been asking for months. Any feedback or response would be nice… 😔 |
pchaurasia14
added
the
Community Contribution
|
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!! |
|
@Nirmal4G - This PR got closed as a side effect of deleting the If the problem addressed by this PR is still relevant, you may want to open a PR against We apologize for the inconvenience. |
ghost
locked as resolved and limited conversation to collaborators
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #4253
Master PRs: #2975, #2976, #4630
Description
Casing of
WinFXis wrong in file name of the props/targets leading to file not found errors in case-sensitive file systems, leading to build break.When using the Desktop SDK, the build double imports both Full framework and Core
WinFXtargets, leading to build break.Also, there's no straight forward way to switch between either
WinFXtargets as needed. This patch provides such mechanism throughImportFrameworkWinFXTargets.Although, there are workarounds for the above issues, they require modifying the installation of the .NET SDKs. So, it's preferable to have them fixed at least for LTS (
v3.1) .NET SDK.Customer Impact
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
CoreandFullframework 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 exact casing and 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 those issues.