Skip to content
This repository has been archived by the owner on Jun 1, 2023. It is now read-only.

Ensure correct ngrok version is downloaded for Apple M1 #1831

Merged
merged 1 commit into from
Dec 8, 2021

Conversation

stevemar
Copy link
Contributor

@stevemar stevemar commented Dec 7, 2021

This patch adds support for installing the proper version of
ngrok when running shopify app serve on M1 macs.

Specifically, this patch updates the os property of the shop-cli
context with a new key mac_m1, not just mac, and updates the
DOWNLOAD_URL dictionary.

Fixes #1785

How to test your changes?

Pull down the branch, install it locally, create a new app and try to serve it..

bundle install
bundle exec shopify app create node
bundle exec shopify app serve

As discussed in the bug, previously the ngrok tunnel step would fail.

➜  test-m1 git:(m1-support) ✗ bundle exec shopify app serve    
⭑ You are running a development version of the CLI at:
  /Users/stevemartinelli/workspace/stevemar/shopify-cli/bin/shopify
✓ curl @ /usr/bin/curl
✓ tar @ /usr/bin/tar
✓ Installing ngrok…
✓ ngrok tunnel running at https://fd15-76-68-94-203.ngrok.io
⭑ To avoid tunnels that timeout, it is recommended to signup for a free ngrok
account at https://ngrok.com/signup. After you signup, install your
personalized authorization token using shopify [ node | rails ] tunnel auth <token>.
✓ .env saved to project root

Verifying change

The new version is arm64 based

➜  shopify-cli git:(m1-support) ✗ lipo -info ~/.cache/shopify/ngrok 
Non-fat file: /Users/stevemartinelli/.cache/shopify/ngrok is architecture: arm64

The old version was not

➜  test-m1 git:(m1-support) ✗ lipo -info ~/.cache/shopify_bak/ngrok
Non-fat file: /Users/stevemartinelli/.cache/shopify_bak/ngrok is architecture: x86_64

Update checklist

  • I've added a CHANGELOG entry for this PR (if the change is public-facing)
  • I've considered possible cross-platform impacts (Mac, Linux, Windows).
  • I've left the version number as is (we'll handle incrementing this when releasing).
  • I've included any post-release steps in the section above.

@stevemar stevemar requested review from a team, pepicrft and gonzaloriestra and removed request for a team December 7, 2021 16:42
@ghost ghost added the cla-needed label Dec 7, 2021
Copy link
Contributor

@pepicrft pepicrft left a comment

Choose a reason for hiding this comment

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

This is amazing @stevemar 👏🏼 . Would you mind updating the CHANGELOG.md to mention that we fixed an issue that causes Ngrok tunneling to fail? After that and if CI is green we can merge.

This patch adds support for installing the proper version of
ngrok when running `shopify app serve` on M1 macs.

Specifically, this patch updates the `os` property of the shop-cli
context with a new key `mac_m1`, not just `mac`, and updates the
DOWNLOAD_URL dictionary.
@stevemar
Copy link
Contributor Author

stevemar commented Dec 7, 2021

@pepicrft thanks for reviewing! As requested, I updated the CHANGELOG to note the change. In case you're wondering, I had to close and re-open the PR to clear the CLA label since I just joined the org.

Copy link
Contributor

@pepicrft pepicrft left a comment

Choose a reason for hiding this comment

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

Thanks a lot, @stevemar. Let's merge this and get it released in the next version that goes out next Monday, December 13th. Keep those great contributions landing 🚀

@pepicrft pepicrft merged commit a238178 into Shopify:main Dec 8, 2021
@hannachen hannachen mentioned this pull request Dec 13, 2021
@shopify-shipit shopify-shipit bot temporarily deployed to rubygems December 13, 2021 21:03 Inactive
@gonzaloriestra gonzaloriestra mentioned this pull request Jan 11, 2022
4 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use specific Ngrok binary for Apple Silicon
2 participants