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

Add arm64 as a possible WinForms arch #7457

Merged
merged 4 commits into from
May 18, 2020

Conversation

hoyosjs
Copy link
Member

@hoyosjs hoyosjs commented May 13, 2020

Makes it possible to build/publish ARM64 WinForms apps.

  • If an app tries anything requiring UseWPF it will fail as a build time scenario as there's no available runtime pack.
  • Anything using UsingWindowsForms will correctly compile and run natively as long as all the references are in the assemblies produced by winforms.

cc: @merriemcgaw, @tommcdon

Makes it possible to build/publish arm64 winforms app. WPF/Full Winforms+WPF should still fail as a build time scenario.
@tommcdon
Copy link
Member

FYI @richlander

@dsplaisted
Copy link
Member

Do we have a runtime pack for this yet?

Ideally I'd like to also add a basic test that this works.

@richlander
Copy link
Member

There are at least three pieces we need:

  • Ref pack for Windows Desktop. This is required to build, and should be in-box.
  • Runtime for Windows desktop @ C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App. This is required for fx-dependent apps to run and should be in-box.
  • Runtime pack for Windows desktop. This is required for self-contained publishing and should be referenced as a URL (not in box).

Will we have all of these after this PR merges?

@hoyosjs
Copy link
Member Author

hoyosjs commented May 13, 2020

Do we have a runtime pack for this yet?

https://dnceng.visualstudio.com/public/_packaging?_a=package&feed=dotnet5&package=Microsoft.WindowsDesktop.App.Runtime.win-arm64&protocolType=NuGet&version=5.0.0-preview.6.20261.2

Ideally I'd like to also add a basic test that this works.

Yes, this I have to look how to do. Tom and I confirmed manually it worked but it would be good to actually confirm it works as expected. I'll take a look at the tree tomorrow to see where they are.

Ideally I could test these two:

  • Winforms works on arm64
  • Wpf errors as expected

Should I also open an issue to track the cleanup?

@wli3
Copy link

wli3 commented May 13, 2020

Are you blocked by this change? If not, I think we should checkin until the feature can work.

Yes, you should open an issue to track everything that is required for this feature to function.

@hoyosjs
Copy link
Member Author

hoyosjs commented May 13, 2020

There's some loose ends here:

  • Ref pack for Windows Desktop. This is required to build, and should be in-box.
  • Runtime for Windows desktop @ C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App. This is required for fx-dependent apps to run and should be in-box.
  • Runtime pack for Windows desktop. This is required for self-contained publishing and should be referenced as a URL (not in box).
    Will we have all of these after this PR merges?

All three are done to some extent:

  • The ARM64 zipped SDK has the WD ref pack under DOTNET_ROOT\packs\Microsoft.WindowsDesktop.App.Ref\<VERSION>\ref. @dsplaisted, is there a way to solve or warn about Error out properly when building WPF on ARM64 app until support is added. #7467 is targeting refs in the Wpf profile?
  • The ARM64 zipped SDK has DOTNET_ROOT\shared\Microsoft.WindowsDesktop.App\<VERSION> after this. The assets contained are only the winforms ones. This causes one of the issues above.
  • The runtime pack, albeit with the issues mentioned, is up in the dotnet5 feed. A win-arm64 publish on x64 pulls that package and I can marshal the self contained app and use it on a cobalt device and it runs as an ARM64 app and not under emulation.

@dsplaisted
Copy link
Member

I would like to see a simple test case that we can build a self-contained Windows Forms app for ARM64 before merging this.

I don't think we need to address #7467 before merging this though.

@richlander
Copy link
Member

I validated installing and using PowerShell today with this build: https://gist.github.com/richlander/d3521d2f8798405e1b5eb1d8e5805fbd

I did more than what is demonstrated there, but it still makes the point.

@vatsan-madhavan
Copy link
Member

@fabiant3 please review.

@vatsan-madhavan
Copy link
Member

/cc @dotnet/wpf-developers

@vatsan-madhavan
Copy link
Member

  • Ref pack for Windows Desktop. This is required to build, and should be in-box.

This should be easy enough to construct today in dotnet/windowsdeskop.

WPF produces ref-assemblies from x86 legs for both x86 and x64. In a future arm64 scenario, I would expect that WPF would continue shipping ref-assemblies from its x86 build-leg (ref-assemblies are built from special projects built as AnyCPU). The Microsoft.DotNet.Wpf.GitHub transport package contains everything needed for this already today (on .NET 5 only).

@vatsan-madhavan
Copy link
Member

change itself lgtm 👍

@vatsan-madhavan vatsan-madhavan removed their request for review May 14, 2020 17:49
@vatsan-madhavan
Copy link
Member

#7467 (comment)

That gave me an an idea about another potential missing piece for completing #7457 - an arm64 compatible version of Microsoft.NET.Sdk.WindowsDesktop - which is needed to even build WinForms apps.

Here by "compatible" I mean two different things - (i) can target arm64 successfully and (ii) can run on arm64 successfully. When scoped to UseWindowsForms==true && UseWpf==false No. (i) might be free, but needs validation/testing. No. (ii) may also be free (i.e., the SDK might just work though I'm less sure of it). When UseWpf==true=, no. (i) might just work for free, but (ii) would need some work (PresentationBuildTasks.dll` may or may not need arch-specific work).

@hoyosjs
Copy link
Member Author

hoyosjs commented May 15, 2020

Added a test doing rid specific publishes, where WinForms succeeds but WPF fails

@hoyosjs
Copy link
Member Author

hoyosjs commented May 15, 2020

Also tested a simple WinForms app from the samples repo as self-contained and framework dependent on a win-arm64 device:

image

@richlander
Copy link
Member

2020-05-15

Copy link
Member

@merriemcgaw merriemcgaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@wli3
Copy link

wli3 commented May 16, 2020

Although we need a separate change in dotnet/SDK to generate a better error message. This current error message is cryptic for the customer. And they should know it is a restriction from the platform, and there is nothing they can do to make it work.

@wli3
Copy link

wli3 commented May 16, 2020

Let me know if you don't have the access to merge this PR. I can merge it

@hoyosjs
Copy link
Member Author

hoyosjs commented May 16, 2020

I don't have access to it. However someone from @dotnet/wpf-developers (I think Vatsan said @fabiant3) should make sure they know how this can impact. Once this is done, I'm happy to merge. Thanks @wli3

@richlander
Copy link
Member

My ask: Can you merge this PR by EOD Monday, if at all possible? That would help me a lot.

@hoyosjs
Copy link
Member Author

hoyosjs commented May 18, 2020

@msftbot please merge this in an hour.

Talked with @richlander and this has been acknowledged by the WPF team. Giving some time in case anyone in their team wants to add a review.

@hoyosjs hoyosjs merged commit f1564ea into dotnet:master May 18, 2020
hoyosjs added a commit that referenced this pull request May 19, 2020
* Add Winforms RT assets to shared and WindowsDesktop refs to ARM64 SDK
* Add arm64 as a possible WinForms arch
hoyosjs added a commit to hoyosjs/core-sdk that referenced this pull request Sep 10, 2020
This reverts commit f1564ea, reversing
changes made to 0adcdc9.
hoyosjs added a commit to hoyosjs/core-sdk that referenced this pull request Sep 10, 2020
This reverts commit f1564ea, reversing
changes made to 0adcdc9.
hoyosjs added a commit to hoyosjs/core-sdk that referenced this pull request Sep 11, 2020
This reverts commit f1564ea, reversing
changes made to 0adcdc9.
hoyosjs added a commit to hoyosjs/core-sdk that referenced this pull request Oct 15, 2020
hoyosjs added a commit that referenced this pull request Oct 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants