Replace hardcoded backslashes with Path.DirectorySeparatorChar and fix WinFX casing (daviddenis-stx, Nirmal4G, PaulEremeeff) #3101

ryalanms · 2020-06-08T23:29:55Z

This includes @daviddenis-stx's Path.DirectorySeparatorChar fixes and the first two commits from @Nirmal4G and @PaulEremeeff to fix WinFX casing. (Thank you!)

The issue is described in more detail in PRs #2877 and #2975.

'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)

vatsan-madhavan · 2020-06-08T23:40:07Z

I highly recommend taking a look at this patch by @daviddenis-stx and integrating some of the changes related to path-separator chars into this PR:

https://github.com/systemathics/wpf/commit/a9b28ed2998f13ee3c9bce37f405ff820cf43b08

ryalanms · 2020-06-08T23:46:42Z

Thanks, @vatsan-madhavan. I wasn't aware of @daviddenis-stx's change.

ryalanms · 2020-06-09T00:17:34Z

I will be testing this today and tomorrow. Thanks.

lindexi · 2020-06-09T00:22:14Z

...Microsoft.DotNet.Wpf/src/PresentationBuildTasks/MS/Internal/MarkupCompiler/MarkupCompiler.cs

@@ -50,7 +50,7 @@ internal sealed class MarkupCompiler
        public string TargetPath
        {
            get { return _targetPath; }
-            set { _targetPath = value; }
+            set { _targetPath = value;}


Format issues

@lindexi: Thank you again for the review. These formatting changes will be taken care of with the other half of @Nirmal4G's PR. Thanks.

lindexi · 2020-06-09T00:26:09Z

...Microsoft.DotNet.Wpf/src/PresentationBuildTasks/MS/Internal/MarkupCompiler/MarkupCompiler.cs

@@ -712,7 +712,7 @@ private SourceFileInfo OnSourceFileResolve(FileUnit file)
                if (sourceFileInfo.IsXamlFile)
                {
                    int fileExtIndex = file.Path.LastIndexOf(DOT, StringComparison.Ordinal);
-
+                    


Extra space

lindexi · 2020-06-09T00:27:37Z

...ft.DotNet.Wpf/src/PresentationBuildTasks/Microsoft/Build/Tasks/Windows/MarkupCompilePass1.cs

-                int pathEndIndex = fullFilePath.LastIndexOf("\\", StringComparison.Ordinal);
-
+                int pathEndIndex = fullFilePath.LastIndexOf(string.Empty + Path.DirectorySeparatorChar, StringComparison.Ordinal);
+                


Extra space

lindexi · 2020-06-09T00:27:44Z

...ft.DotNet.Wpf/src/PresentationBuildTasks/Microsoft/Build/Tasks/Windows/MarkupCompilePass1.cs

@@ -1466,7 +1466,7 @@ private void GenerateOutputItems( )

                    codeItem = new TaskItem();
                    codeItem.ItemSpec = genLangFilePath;
-
+                    


Extra space

vatsan-madhavan · 2020-06-09T02:00:09Z

With these changes in place, can you now build from within WSL?

vatsan-madhavan · 2020-06-09T02:01:38Z

I know this is obvious but I’ll say it just so it doesn’t get forgotten - please don’t accidentally squash the PR 😄

daviddenis-stx · 2020-06-09T18:55:07Z

Hello all :) If you read the thread thoroughly you'll find that cross compilation support was found to be something dotnet team wasn't eager to support in the short term (or not at all ?) The message "Windows is required... " was added after this small commit I made merely to test and see. In my experiments it produced working WPF assemblies but in the thread you'll find that some corner cases were expected to hurt (some of them depending on windows executables handling resx and/or resources if I recollect well). It's probably one of the reasons why the target stops with this message ever since. Otherwise it would potentially have created mangled outputs and the team probably didn't want to add more problems than solutions. Long story short cross compiling was not broken just because of case sensitivity in targets files. I still wish cross compilation was supported, in our current CI setup, we build WPF repos using docker on windows with nanoserver 2004 base images, because we have no, other choice. Each and every other repo we have builds in both Ubuntu and Windows containers and overall the time to completion is 2x to 3x longer on Windows for the exact same code ... painful. Also, we cannot run the tests that reach inside WPF parts (view models and stuff) in nano server containers, we even have to use servercore images for that. 5GB nightmare. Docker on Windows is far less efficient than on Linux, alas, the extra time wasting seems to be occurring when docker publishes to layers, you wait 2 minutes after a stage containing a simple "dotnet build" finishes to run (that is, you're not out of trouble when dotnet build returns as a lot of copy operations are occurring between some temp hcs to a windowsfilter sub folder, and it's 33.6 modem old times again... ) Cross compilation please :) Get Outlook for Android<https://aka.ms/ghei36>

…

________________________________ From: Ryland <[email protected]> Sent: Tuesday, June 9, 2020 7:23:12 PM To: dotnet/wpf <[email protected]> Cc: David Denis <[email protected]>; Mention <[email protected]> Subject: Re: [dotnet/wpf] Replace hardcoded backslashes with Path.DirectorySeparatorChar and fix WinFX casing (daviddenis-stx, Nirmal4G, PaulEremeeff) (#3101) @vatsan-madhavan<https://github.com/vatsan-madhavan>: It looks like more changes are required for WSL. /usr/share/dotnet/sdk/5.0.100-preview.4.20258.7/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.Sdk.FrameworkReferenceResolution.targets(59,5): error NETSDK1100: Windows is required to build Windows desktop applications. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#3101 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AMB4IIXSWTODA5MIGF5DGKLRVZ4ZBANCNFSM4NY3KTQA>.

vatsan-madhavan · 2020-06-10T03:23:14Z

Yup, Microsoft.NET.Sdk.FrameworkReferenceResolution.targets has NETSDK1100 error - “Windows is required to build Windows desktop applications”.

The existing toolchain will probably work largely fine under cross-compilation - module problems like path-separators etc. Somebody should remove that error and try it to be sure. Any minor fixups needed from there can probably be figured out.

Earlier in .NET Core 3.0, there were still pieces of the toolchain that were still msbuild only and hadn’t been ported to ‘dotnet’. IIRC an example was the WinForms ResXFileCodeGenerator.

The one thing I’d pay attention to is the BAML generated under cross compilation. I’d suggest comparing it against reference from Windows based builds.

vatsan-madhavan · 2020-06-10T03:25:20Z

Otherwise it would potentially have created mangled outputs

I don’t recall that part of the history. Would you mind elaborating?

daviddenis-stx · 2020-06-10T05:43:57Z

I meant incomplete or different than what comes out of a Windows build. In my tests I remember that BAML was not binary identical between Windows build and Linux build, but decompiled to identical XAML with ILSPY. If I remember well, there were some binary values here and there in the binary stream (guids?) that were random (that would be useful to also compare 2 rebuilds from the same platfom to confirm). We could remove the switch that stops the build when non Windows is detected and try again the same recipe to see where it lands :) D Get Outlook for Android<https://aka.ms/ghei36>

…

________________________________ From: Vatsan Madhavan <[email protected]> Sent: Wednesday, June 10, 2020 4:25:33 AM To: dotnet/wpf <[email protected]> Cc: David Denis <[email protected]>; Mention <[email protected]> Subject: Re: [dotnet/wpf] Replace hardcoded backslashes with Path.DirectorySeparatorChar and fix WinFX casing (daviddenis-stx, Nirmal4G, PaulEremeeff) (#3101) Otherwise it would potentially have created mangled outputs I don’t recall that part of the history. Would you mind elaborating? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#3101 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AMB4IITLNR4FZUZ7OQ44HUTRV34K3ANCNFSM4NY3KTQA>.

dsplaisted · 2020-06-10T17:23:27Z

If you read the thread thoroughly you'll find that cross compilation support was found to be something dotnet team wasn't eager to support in the short term (or not at all ?)

I think we would like to support it. There are probably a variety of things that would need to be fixed, not all of which we may be aware of, but this fix looks like a step in the right direction.

vatsan-madhavan · 2020-06-10T22:41:59Z

@dsplaisted are you planning on removing NETSDK1100 from the sdk targets?

dsplaisted · 2020-06-10T22:50:13Z

We'd like to remove it eventually. It's there right now because we wanted to avoid shipping the targeting packs (or downloading them during restore) for non-Windows versions of the SDK. When we support optional SDK workloads we could have the Windows support be a workload and remove the error.

dsplaisted · 2020-06-10T23:47:20Z

Also FYI you can suppress the NETSDK1100 error with something like this:

<ItemGroup>
  <FrameworkReference Update="@(FrameworkReference)" IsWindowsOnly="false" />
</ItemGroup>

vatsan-madhavan · 2020-06-11T01:31:44Z

You should consider making it so that cross-compilation can be tried out more easily than that (for e.g., a propertygroup that can be set in commandline).

Nirmal4G · 2020-08-06T02:11:42Z

@ryalanms @vatsan-madhavan Will this be back ported into 3.1 as it is an LTS release?

Ref: dotnet/sdk#11108

vatsan-madhavan · 2020-08-06T03:32:10Z

No idea what the team would choose to do but I wouldn't recommend porting it back to 3.1.

This is a developer experience improvement, and "LTS" is a runtime quality/maintenance concept. NET 5 SDK can be used to target 3.1 runtime IIRC, and developers needing this fix could adopt a newer SDK. Besides, this hasn't been an adoption blocker for 3.1 in general.

worldbeater · 2020-08-18T20:25:11Z

Anyway, it's definitely disappointing that we are still unable to use MSBuild.Sdk.Extras with .NET Core 3.1 due to the issue which was fixed with this PR. The good news is, however, that a workaround suggested by Jerome finally made using MSBuild.Sdk.Extras possible on Linux, even with .NET Core 3.1.

Nirmal4G added 2 commits June 8, 2020 16:21

Rename 'Microsoft.WinFx.targets' to 'Microsoft.WinFX.targets'

bb16e23

'WinFX' is the correct casing as observed in the .NET CLR 2 frameworks

Fix 'WinFx' -> 'WinFX' everywhere else

333565f

In project files, scripts, comments, etc... But not in sources (like GetWinFxCallback)

ryalanms requested review from dsplaisted, clairernovotny and SamBent June 8, 2020 23:29

ryalanms requested a review from a team as a code owner June 8, 2020 23:29

ghost added the PR metadata: Label to tag PRs, to facilitate with triage label Jun 8, 2020

ghost requested a review from fabiant3 June 8, 2020 23:29

Enable the building of WinForms and WPF projects from Unix

41e0b78

ryalanms changed the title ~~Fix casing for Microsoft.WinFx.targets. (Nirmal4G, PaulEremeeff)~~ Enable building WPF from Unix (daviddenis-stx, Nirmal4G, PaulEremeeff) Jun 8, 2020

ryalanms changed the title ~~Enable building WPF from Unix (daviddenis-stx, Nirmal4G, PaulEremeeff)~~ Replace hardcoded backslashes with Path.DirectorySeparatorChar and fix WinFX casing (daviddenis-stx, Nirmal4G, PaulEremeeff) Jun 9, 2020

lindexi reviewed Jun 9, 2020

View reviewed changes

fabiant3 approved these changes Jun 9, 2020

View reviewed changes

Merge branch 'master' into Nirmal4g/WinFXFix

8910a31

ryalanms merged commit 734be70 into master Jun 10, 2020

ryalanms deleted the Nirmal4g/WinFXFix branch June 10, 2020 23:07

ryalanms mentioned this pull request Jul 2, 2020

Fix casing for Microsoft.WinFx.targets. #2877

Closed

ghost locked as resolved and limited conversation to collaborators Apr 12, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace hardcoded backslashes with Path.DirectorySeparatorChar and fix WinFX casing (daviddenis-stx, Nirmal4G, PaulEremeeff) #3101

Replace hardcoded backslashes with Path.DirectorySeparatorChar and fix WinFX casing (daviddenis-stx, Nirmal4G, PaulEremeeff) #3101

ryalanms commented Jun 8, 2020 •

edited

Loading

vatsan-madhavan commented Jun 8, 2020

ryalanms commented Jun 8, 2020

ryalanms commented Jun 9, 2020

lindexi Jun 9, 2020

ryalanms Jun 9, 2020

lindexi Jun 9, 2020

lindexi Jun 9, 2020

lindexi Jun 9, 2020

vatsan-madhavan commented Jun 9, 2020 •

edited

Loading

vatsan-madhavan commented Jun 9, 2020

daviddenis-stx commented Jun 9, 2020 via email

vatsan-madhavan commented Jun 10, 2020

vatsan-madhavan commented Jun 10, 2020

daviddenis-stx commented Jun 10, 2020 via email

dsplaisted commented Jun 10, 2020

vatsan-madhavan commented Jun 10, 2020

dsplaisted commented Jun 10, 2020

dsplaisted commented Jun 10, 2020

vatsan-madhavan commented Jun 11, 2020

Nirmal4G commented Aug 6, 2020

vatsan-madhavan commented Aug 6, 2020

worldbeater commented Aug 18, 2020 •

edited

Loading

		int pathEndIndex = fullFilePath.LastIndexOf("\\", StringComparison.Ordinal);

		int pathEndIndex = fullFilePath.LastIndexOf(string.Empty + Path.DirectorySeparatorChar, StringComparison.Ordinal);

		@@ -1466,7 +1466,7 @@ private void GenerateOutputItems( )

		codeItem = new TaskItem();
		codeItem.ItemSpec = genLangFilePath;

Replace hardcoded backslashes with Path.DirectorySeparatorChar and fix WinFX casing (daviddenis-stx, Nirmal4G, PaulEremeeff) #3101

Replace hardcoded backslashes with Path.DirectorySeparatorChar and fix WinFX casing (daviddenis-stx, Nirmal4G, PaulEremeeff) #3101

Conversation

ryalanms commented Jun 8, 2020 • edited Loading

vatsan-madhavan commented Jun 8, 2020

ryalanms commented Jun 8, 2020

ryalanms commented Jun 9, 2020

lindexi Jun 9, 2020

Choose a reason for hiding this comment

ryalanms Jun 9, 2020

Choose a reason for hiding this comment

lindexi Jun 9, 2020

Choose a reason for hiding this comment

lindexi Jun 9, 2020

Choose a reason for hiding this comment

lindexi Jun 9, 2020

Choose a reason for hiding this comment

vatsan-madhavan commented Jun 9, 2020 • edited Loading

vatsan-madhavan commented Jun 9, 2020

daviddenis-stx commented Jun 9, 2020 via email

vatsan-madhavan commented Jun 10, 2020

vatsan-madhavan commented Jun 10, 2020

daviddenis-stx commented Jun 10, 2020 via email

dsplaisted commented Jun 10, 2020

vatsan-madhavan commented Jun 10, 2020

dsplaisted commented Jun 10, 2020

dsplaisted commented Jun 10, 2020

vatsan-madhavan commented Jun 11, 2020

Nirmal4G commented Aug 6, 2020

vatsan-madhavan commented Aug 6, 2020

worldbeater commented Aug 18, 2020 • edited Loading

ryalanms commented Jun 8, 2020 •

edited

Loading

vatsan-madhavan commented Jun 9, 2020 •

edited

Loading

worldbeater commented Aug 18, 2020 •

edited

Loading