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
feat: add support for Apple Silicon chromium builds #7546
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
I got 3 tests failed, but not sure that it's related to this PR
|
42bde9c
to
bce68ad
Compare
src/node/BrowserFetcher.ts
Outdated
if (platform === 'darwin') this._platform = 'mac'; | ||
if (platform === 'darwin' && productFromOptions === 'chrome') | ||
this._platform = os.arch() === 'arm64' ? 'mac_arm' : 'mac'; | ||
else if (productFromOptions === 'firefox') this._platform = 'mac'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it doesn't feel right for me, but firefox ships universal build, so in downloadURLs
we can't use the separate url for firefox for MacOS ARM because of string interpolation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as workaround we can pass platform as query parameter in firefox case and remove this hack
I can confirm that the three test failures you described are related to Chromium changes that we haven't rolled into Puppeteer yet. |
The change looks good to me from a technical standpoint but we'll have to wait for the next Chromium roll to land it. |
Sure! Feel free to ping me. Thanks |
Any chance to merge this PR? Related issue - #7632 |
Would love to see this land. As an alternative approach, would anyone be opposed to this function searching |
any progress ? |
@jschfflr as I understand, we are not blocked anymore. Should I update PR? https://commondatastorage.googleapis.com/chromium-browser-snapshots/index.html?prefix=Mac/938248/ |
Also, as I understand, it would be nice to test against MacOS ARM? But it's currently blocked by actions/runner-images#2187 |
Let's wait for GitHub Actions to support it or we can we land it as an experimental opt-in feature? |
@OrKoN honestly I prefer to release it as an opt-in because we don't really have any timeline for GitHub Actions release. Maybe we can download the ARM version only if env variables set? Something like |
yes, I think an env var would work. Perhaps |
I will update PR on this weekend or next week |
@OrKoN added some info to |
Great to see this close to landing. I'm guessing this won't help people running puppeteer in Docker on Apple Silicon? Are there Arm builds of Chromium for Debian running in Docker? |
Pupeteer does not play well with M1 and ARM in general. While they [just released](puppeteer/puppeteer#7546) experimental support for ARM macs, they still don't support [ARM machines for Debian/Docker](puppeteer/puppeteer#7546 (comment)). This PR switches from puppeteer to [zombie](https://github.com/assaf/zombie). Ideally I would like to move back, as Zombie does not appear to be actively maintained.
* Dev environment runs on M1 mac Pupeteer does not play well with M1 and ARM in general. While they [just released](puppeteer/puppeteer#7546) experimental support for ARM macs, they still don't support [ARM machines for Debian/Docker](puppeteer/puppeteer#7546 (comment)). This PR switches from puppeteer to [zombie](https://github.com/assaf/zombie). Ideally I would like to move back, as Zombie does not appear to be actively maintained. * Replaced puppeteer mock with zombie + removed commented out puppeteer code
Was this one supposed to fix #7916 ? |
@artyil nope. It's not related to any Linux (inside docker too) at all. Just native M1. |
@artyil this PR is related only to M1. This #7546 issue is about docker so it's about chromium and puppeteer on Linux. Also I see this comment #7916 (comment) and looks like in this case it's about native run (without docker) too, but I'm not sure. WIll try to run example from it. |
- puppeteer/puppeteer#7546 fixes chrome build issue on apple silicon machines Included in v14.0.0
Google has published Chromium builds for Apple Silicon so we can fetch it now
Related to #6622