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.

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

Download chromedriver binary for Mac M1 #327

Merged
merged 1 commit into from Feb 1, 2022
Merged

Conversation

johnathafelix
Copy link
Contributor

@johnathafelix johnathafelix commented Sep 20, 2021

I noticed that using Node.js emulated with Rosetta made the script download the Intel version of the chromedriver instead of the Apple Silicon one.

I've changed it the platform check so it can see what's the real processor architecture and download the chromedriver accordingly.

I run browsers without emulation but not Node.js since some packages are still not compatible.

We are in need of someone to test this

You need to be on an M1 Mac, this is how you do it:

Clone this PR:

git clone https://github.com/johnathafelix/node-chromedriver.git
cd node-chromedriver
npm install

Then, check if you can run Chromedriver:

./bin/chromedriver

If it shows output correctly, then it worked. This is the expected output (somewhat similar is ok):

Starting ChromeDriver 97.0.4692.71 (adefa7837d02a07a604c1e6eff0b3a09422ab88d-refs/branch-heads/4692@{#1247}) on port 9515
Only local connections are allowed.
Please see https://chromedriver.chromium.org/security-considerations for suggestions on keeping ChromeDriver safe.
ChromeDriver was started successfully.

If it works, please comment bellow.

@johnathafelix
Copy link
Contributor Author

A link for reference: https://developer.apple.com/documentation/apple-silicon/about-the-rosetta-translation-environment#3616845

Basically, process.arch returns x86_64 if Node.js is running under Rosetta.

}

function isEmulatedRosettaEnvironment() {
const archName = child_process.spawnSync('uname', ['-m']).stdout.toString().trim();
Copy link
Owner

Choose a reason for hiding this comment

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

I'm not sure you need this. Can't we just check for process.arch === 'x64'?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @giggio! In my tests, if I'm running Node.js under Rosetta 2, process.arch will return x86_64 and will return arm64 if running natively on Apple Silicon.

So the only way I've found to get the real architecture of the processor is by running uname -m and then sysctl -in sysctl.proc_translated.

Let me know if it's not clear 😃

Copy link
Owner

Choose a reason for hiding this comment

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

Ok, I understand. So, what if we make the function like this:

function isEmulatedRosettaEnvironment() {
  const processTranslated = child_process.spawnSync('sysctl', ['-in', 'sysctl.proc_translated'])
      .stdout.toString()
      .trim();
  return processTranslated === '1';
}

And then call it only from:

  if (process.arch === 'x64') {
    if (isEmulatedRosettaEnvironment())
      return 'mac64_m1';
    return 'mac64';
  }

Would that work? If yes, we have one less process spawning.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @giggio!
So sysctl -in sysctl.proc_translated returns an empty string on an Intel Mac, so I guess you're right, we don't need to check for uname here. I'll change it. Thanks!

Copy link
Owner

Choose a reason for hiding this comment

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

Hi @johnathafelix! When do you expect to update this? I'd like to ask a friend to verify it after um change it.

@giggio
Copy link
Owner

giggio commented Sep 27, 2021

@johnathafelix It looks good. I don't have an M1 Mac to test, so I need somebody else to validate.
I added only one comment, please review it.
I was not aware that the Chrome team was now releasing mac64_m1 binary. That is new, and your PR is welcome.

@johnathafelix
Copy link
Contributor Author

Thanks @giggio! Let's wait until someone else with a M1 Mac validates this.

@stale stale bot added the wontfix label Oct 6, 2021
@giggio giggio removed the wontfix label Oct 6, 2021
@stale stale bot added the wontfix label Oct 22, 2021
@giggio giggio removed the wontfix label Oct 22, 2021
Repository owner deleted a comment from stale bot Oct 22, 2021
Repository owner deleted a comment from stale bot Oct 22, 2021
@igor9silva
Copy link

This is the output I have, seems to be working properly:
image
image

@rodrigograudo
Copy link

Captura de Tela 2022-01-31 às 12 55 04

Tested on M1 Pro

@ricmello
Copy link

It seems to be working properly here too:

image

@samuelramox
Copy link

It's working here:
Screenshot 2022-01-31 às 14 15 18

But you might need to update a package with vulnerability:
Screenshot 2022-01-31 às 14 26 40

@RodrigoRVieira
Copy link

Well... It seems some folks already replied, but... here it goes!

image

@nailsonlinux
Copy link

Here's my output and system specs.

Screen Shot 2022-02-01 at 07 40 43

Screen Shot 2022-02-01 at 07 41 09

@giggio giggio merged commit 516990f into giggio:main Feb 1, 2022
@giggio
Copy link
Owner

giggio commented Feb 1, 2022

Great, I merged it! Thanks for everyone that helped testing it!
I'll release a new version to NPM in a moment.
Thanks for the great PR @johnathafelix!

@giggio
Copy link
Owner

giggio commented Feb 1, 2022

@samuelramox this branch was a little behind, the latest on main already has the vulnerabilities fixed. Thanks for reporting it.

@giggio
Copy link
Owner

giggio commented Feb 1, 2022

Published as 97.0.4. :shipit: 🚀

@github-actions
Copy link

github-actions bot commented Feb 9, 2022

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants