8000 Enable building PowerShell for Apple M1 runtime by SteveL-MSFT · Pull Request #14923 · PowerShell/PowerShell · GitHub
[go: up one dir, main page]

Skip to content

Conversation

@SteveL-MSFT
Copy link
Member
@SteveL-MSFT SteveL-MSFT commented Feb 28, 2021

PR Summary

Add support to Start-PSBuild with runtime osx-arm64. Requires PowerShell/PowerShell-Native#64 to be published first. A local build of both works. When looking for crossgen, there appears to be one found under x64-arm64 folder which fails, but the one in the parent tools folder works so need to filter that out for when finding crossgen.

PR Context

PR Checklist

@jborean93
Copy link
Collaborator

So does this build a universal binary or just one compiled for arm64? Does it require any work from .NET, I remember reading something about M1 support in the new preview so assumed PowerShell had to wait for that.

@SteveL-MSFT
Copy link
Member Author
SteveL-MSFT commented Mar 1, 2021

@jborean93 This PR leverages dotnet to build M1 optimized binaries. It depends on PowerShell/PowerShell-Native#64 which builds universal native binaries for pwsh. For "universal binaries" we could just have a MSIL non-crossgen'd package, but startup speed is greatly affected.

@jborean93
Copy link
Collaborator

Awesome thanks for the info, will have to get a move on updating the mi fork to produce arm64 libraries so powershell can use them.

build.psm1 Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this 'tools' path filter just for macOS, or for all runtimes? I assume the latter, but it is not clear to me. Maybe add a comment to reassure.

Copy link
Member Author

Choose a reason for hiding this comment

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

The tools folder is for all runtimes, but for some reason, x64-arm64 for macOS has a crossgen in that additional runtime folder which is discovered, but doesn't actually work. The one in tools does work.

@TravisEz13 TravisEz13 marked this pull request as draft March 2, 2021 00:38
@TravisEz13
Copy link
Member

Would you submit another PR to update the release build or add that to this PR?

@SteveL-MSFT
Copy link
Member Author

@TravisEz13 I can update this PR to also do the build, was using this to test locally

@ghost ghost added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Mar 5, 2021
@ghost ghost removed the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Mar 6, 2021
@SteveL-MSFT
Copy link
Member Author

@TravisEz13 I spent some time looking at the yml and psm1 that does the build, but it's much more complicated now than the last time I made changes. I would suggest having it integrated with the build as a separate PR.

@SteveL-MSFT SteveL-MSFT marked this pull request as ready for review March 6, 2021 01:31
@TravisEz13
Copy link
Member

@SteveL-MSFT is this ready to merge? Don't we need the updated native nuget?

@adityapatwardhan
Copy link
Member

The description says it is dependent on the PR in PS-Native repo. The NuGet package after merging that that PR has to be released too.

@SteveL-MSFT
Copy link
Member Author

For the full package to work, it requires the universal binary from powershell-native, however, this PR itself can be merged. @anmenaga can you do a separate PR to enable building of the M1 pkg?

@TravisEz13 TravisEz13 merged commit 727785c into PowerShell:master Mar 9, 2021
@SteveL-MSFT SteveL-MSFT deleted the build-m1 branch March 10, 2021 22:51
@TravisEz13 TravisEz13 added the CL-BuildPackaging Indicates that a PR should be marked as a build or packaging change in the Change Log label Mar 16, 2021
@TravisEz13 TravisEz13 added this to the 7.2.0-preview.4 milestone Mar 16, 2021
@ghost
Copy link
ghost commented Mar 16, 2021

🎉v7.2.0-preview.4 has been released which incorporates this pull request.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CL-BuildPackaging Indicates that a PR should be marked as a build or packaging change in the Change Log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

0