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.

Sign up for GitHub

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

Jump to bottom

Create runner layout for osx-arm64 (Apple M1) platform. #1618

Merged
merged 3 commits into from
May 20, 2022
Merged

Create runner layout for osx-arm64 (Apple M1) platform. #1618

merged 3 commits into from
May 20, 2022

Conversation

TingluoHuang
Copy link
Member

@TingluoHuang TingluoHuang commented Jan 20, 2022 •
edited
Loading

Create a runner package that is able to run on macOS M1.

https://github.com/github/c2c-actions-runtime/issues/1565

#805

Test summary:

  • All L0 tests pass on the M1 machine.
  • Configure and start the runner as an interactive process.
  • Configure and start the runner as a service.
  • Run workflow with run step, node12 actions, and node16 actions
  • Runner self-update with double hops for both interactive and service runner, ex: 2.290.0 -> 2.290.1-> 2.290.2

@TingluoHuang TingluoHuang requested a review from a team as a code owner January 20, 2022 02:30
thboop
thboop reviewed Jan 24, 2022
View reviewed changes
src/Misc/layoutbin/update.sh.template Outdated
@@ -135,7 +135,7 @@ if [[ "$currentplatform" == 'darwin' && restartinteractiverunner -eq 0 ]]; then
then
# inspect the open file handles to find the node process
# we can't actually inspect the process using ps because it uses relative paths and doesn't follow symlinks
path=$(lsof -a -g "$procgroup" -F n | grep node12/bin/node | grep externals | tail -1 | cut -c2-)
path=$(lsof -a -g "$procgroup" -F n | grep node16/bin/node | grep externals | tail -1 | cut -c2-)
Copy link
Collaborator

Choose a reason for hiding this comment

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

should the node16 changes wait on fhammerl's pr?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, i will rebase on his change once it's merged.

Copy link

Choose a reason for hiding this comment

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

Depends on #1621

Copy link

Choose a reason for hiding this comment

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

Looks like #1621 has been merged. Can this PR now be merged? Thanks. This is important to us.

Copy link
Contributor

@ethomson ethomson Feb 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

We're still working on the dependencies for this - here's some more information. #805 (comment) Thanks everybody for your patience!

Copy link

@TomBeckett TomBeckett Mar 30, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I can see a lot of activity in Github org about nodejs 16 which is exciting!

@ethomson Is that list of blockers still up to date?

@themarpe
Copy link

themarpe commented Jan 31, 2022 •
edited
Loading

Hi @TingluoHuang
Wondering what is the current projected time to have this mainlined, approximately?
Looking forward to having macOS ARM64 runners:)

@stephanosio stephanosio mentioned this pull request Feb 10, 2022
Closed
@ethomson ethomson mentioned this pull request Feb 16, 2022
Closed
@hannesa2 hannesa2 mentioned this pull request Feb 17, 2022
Closed
@mateuszkwiecinski mateuszkwiecinski mentioned this pull request Mar 2, 2022
Closed
@TomBeckett
Copy link

@TingluoHuang Is there anything I can help with to get this merged? I can see there are some merge conflicts now?

@Bo98
Copy link

Bo98 commented Apr 12, 2022 •
edited
Loading

If you want extremely early feedback from a custom build, all works well for me apart from an upstream .NET issue: dotnet/runtime#64103, which looks like will be fixed in 6.0.5 (May) and has a workaround available.

@TomBeckett
Copy link

TomBeckett commented Apr 14, 2022 •
edited
Loading

@ethomson This looks ready for another review I think?

I've looked over it and it LGTM (though I'm not as familiar with codebase)

TingluoHuang
TingluoHuang commented May 19, 2022
View reviewed changes
src/Test/L0/Listener/SelfUpdaterL0.cs
@@ -1,4 +1,5 @@
using System;
#if !(OS_OSX && ARM64)
Copy link
Member Author

Choose a reason for hiding this comment

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

we don't have any osx-arm64 release publish yet, so all self-update L0 tests are failing because it can't find a package that matches the platform.

thboop
thboop approved these changes May 20, 2022
View reviewed changes
Copy link
Collaborator

@thboop thboop left a comment

Choose a reason for hiding this comment

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

lgtm

@TingluoHuang TingluoHuang merged commit 0398f57 into main May 20, 2022
@TingluoHuang TingluoHuang deleted the users/tihuang/osxarm branch May 20, 2022 15:00
@hassila
Copy link

hassila commented May 20, 2022

Awesome, now just waiting for a release, thanks!

@gabimoncha
Copy link

gabimoncha commented May 20, 2022 •
edited
Loading

So what's the timeline to actually having m1 github actions now? :D

@TingluoHuang
Copy link
Member Author

@gabimoncha the runner package will be created in the release of this repo next Monday (🤞 ) for preview. You can configure your self-hosted macOS-arm64 runner after that.

@Nantris Nantris mentioned this pull request May 21, 2022
Closed
@hannesa2
Copy link

hannesa2 commented May 27, 2022 •
edited
Loading

Please have a look on the build times
https://github.com/gitx/gitx/actions/runs/2396371412
image

This is the runner
image

@Luxonis-Brandon
Copy link

(Just wanted to chime in to say how excited I am for this to exist! It will be very very useful for us. Thank you to all who are working on it!)

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

@ethomson ethomson ethomson left review comments

@TomBeckett TomBeckett TomBeckett left review comments

@stephanosio stephanosio stephanosio left review comments

@mh-bmckee mh-bmckee mh-bmckee left review comments

@thboop thboop thboop approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.