Enable build of ARM64 installers in ASP.NET Core by hoyosjs · Pull Request #25579 · dotnet/aspnetcore (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation30 Commits1 Checks0 Files changed

Conversation

This file contains hidden or 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

[ Show hidden characters]({{ revealButtonHref }})

Enable ARM64 installers build.

Changes WiX toolset used to 3.14 to support ARM64
Generates targeting pack from the x86/x64 leg, as it gets produced using a zip that gets generated there.
The ARM leg now produces all the necessary msi's, exe, and wixlib needed for the installer to generate a bundle.

@@ -11,6 +11,7 @@

Choose a reason for hiding this comment

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

I added this feed for testing. The main issue was that WiX doesn't have a package for the toolset that supports ARM64. Other repositories don't use wixproj, so we didn't need it before this. I don't know what feed this should go to, or if this is even a good idea.

Choose a reason for hiding this comment

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

Using general-testing is not a good idea because pretty much anything could be pushed there. Suggest working w/ core-eng to get the new WiX into dotnet-eng

Choose a reason for hiding this comment

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

Yeah, this was to verify this won't break in CI. I'm asking for some guidance from the engineering team here.

Choose a reason for hiding this comment

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

@dougbu I got the green light to move this as is to dotnet-eng. I've reverted all changes to NuGet.config

hoyosjs marked this pull request as ready for review

September 3, 2020 22:27

Do we want to target this to release/5.0-rc2?

@Pilchie, yes. I will port once this gets reviewed and goes in.

@Pilchie if we did, we might be able to unblock the SDK and get that part completed for RC2 as well, but it would be tight. I'll have a chat with other SDK folks. @marcpopMSFT as an FYI

@@ -11,6 +11,7 @@

Choose a reason for hiding this comment

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

Using general-testing is not a good idea because pretty much anything could be pushed there. Suggest working w/ core-eng to get the new WiX into dotnet-eng

build.ps1 Outdated Show resolved Hide resolved

I can't approve this until the general-testing feed is removed. @hoyosjs you've addressed my other comments well. (nit: "only" isn't spelled "inly" even in comments.)

Also suggest @joeloff or someone else w/ more Wix knowledge have a look at these changes. They follow the existing patterns but I may be missing something.

Choose a reason for hiding this comment

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

I'm fine with this but would really appreciate additional eyes on it @joeloff @dotnet/aspnet-build or anyone w/ more WiX experience than me, please chime in.

Looks good. I'm guessing that the hosting bundle will remain x86/x64 only for the time being. Unlike the other installers, the hosting bundle prefers to install all architectures by default and I think there will be a clash since for x64, there aren't new properties defined yet (at least none that I can see in Windows).

My big concern is the wixlib because we rely on that to share the package chaining across the shared framework, hosting and SDK installer bundles.

@joeloff any verification I can do to verify? I have official runs so I have signed installers and can test anything that's needed. However, this is becoming time sensitive.

@hoyosjs if the have the signed MSI, you can try them out on a VM. You can also double check the Product/Upgrade code for the x64/arm64 MSIs just to be sure they are different (you can use ORCA for that). The build seeds the GUIDs using the Platform property, so they should be different.

@joeloff I've tested this:

My big concern is the wixlib because we rely on that to share the package chaining across the shared framework, hosting and SDK installer bundles.

Do you have a particular concern I can check against?

Issues I keep hitting are #25623 and a gradle issue solved by #25707. Closing and reopening to create a rebase.

@hoyosjs the problem here is you're targeting the master branch and all the fixes mentioned have been checked into release/5.0-rc2. Need to either wait for #25650 to merge or target release/5.0-rc2 instead. Recommend doing the second because time for RC2 is getting short.

hoyosjs changed the base branch from master to release/5.0-rc2

September 10, 2020 06:20

Changes WiX toolset used to 3.14 to support ARM64 Generates targeting pack from the x86/x64 leg, as it gets produced using a zip that gets generated there. The ARM64 leg now produces all the necessary msi's, exe, and wixlib needed for the installer to generate a bundle.

@dougbu I didn't know there was an automated FI from RC2 to master. I've retargeted this to RC2.

Feel free to merge once it's green

@Pilchie can this please be considered for RC2 for the ARM64 installer?

Hello human! Please make sure you've included the Shiproom Template in a comment or (preferably) the PR description. Also, make sure this PR is not marked as a draft and is ready-to-merge.

All checks 🟢 Waiting on your word @Pilchie…

Just wanted to double check with Tactics - we're good to merge this now.

JunTaoLuo pushed a commit that referenced this pull request

Feb 25, 2021

JunTaoLuo pushed a commit that referenced this pull request

Mar 2, 2021

JunTaoLuo pushed a commit that referenced this pull request

Mar 3, 2021