Skip to content

Add support for Apple M1 chips. #10

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion scripts/postinstall.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ const releases = {
};

const platform = process.platform;
const arch = process.arch === "x64" ? "x86_64" : "x86_32";
const arch =
process.arch === "x64" || process.arch === "arm64" ? "x86_64" : "x86_32";
Copy link
Owner

Choose a reason for hiding this comment

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

I think the M1 chip uses the aarch_64 and not x86_64 (https://en.wikipedia.org/wiki/AArch64). I think we need to translate it to use protoc-${protoVersion}-osx-aarch_64.zip instead (needs to be added to the list of releases).

So the logic should be something like:

  • If arch is equal to x64 use x86_64
  • If arch is equal to arm64 use aarch_64
  • Otherwise use x86_32

Copy link
Author

@brunobely brunobely Nov 18, 2022

Choose a reason for hiding this comment

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

That sounds correct. When I saw the osx-aarch_64 release in v3.20.1 and v3.20.2 but absent in v3.20.3 I assumed they consolidated the macOS release names, but it looks like it was a different issue.

Those binaries were just copies of the x86_64 artifacts, and aarch_64 binaries are only available on or after v21.x: see this comment

I followed this commit to upgrade to v3.21.9 (looks like they're referring to it as just 21.9 now).

Looks like the JS implementation is now under https://github.com/protocolbuffers/protobuf-javascript, so the files in https://github.com/protocolbuffers/protobuf/tree/v3.20.3/js mentioned in this comment no longer exist under v21.9. Is that an issue?

As an aside, is there a reason for both Windows links pointing to the win32 release?

Copy link
Owner

Choose a reason for hiding this comment

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

It lookes like https://github.com/protocolbuffers/protobuf-javascript is needed as I get the following error:

'protoc-gen-js' is not recognized as an internal or external command,
operable program or batch file.
--js_out: protoc-gen-js: Plugin failed with status code 1.

I then downloaded protoc-gen-js.exe from https://github.com/protocolbuffers/protobuf-javascript/releases/tag/v3.21.2 put it in the same bin folder as protoc.exe. Made sure that the bin folder was added to the path. However, that just creates another error:

--js_out: protoc-gen-js: Plugin failed with status code 3221225781.

I'm not exactly sure what causes that issue. Perhaps it's because it needs a specific version or I'm missing some libraries. But maybe you can try and see whether you can get it to work.

I modified the protoc function in index.js to automatically add the folder to the PATH environment:

exports.protoc = function(args, options, callback) {
  if (typeof options === "function" && !callback) {
    callback = options;
    options = {};
  }

  try {
    cp.execFile(protoc, args, {
      env: {
        ...process.env,
        PATH: process.env.PATH + ";" + path.dirname(protoc)
      },
      ...options
    }, callback);
  } catch (err) {
    callback(err);
  }
};

If you're running something other than Windows you probably need to change the separator ; to : instead.

const release = platform + "_" + arch;
const protocDirectory = path.join(__dirname, "..", "protoc");

Expand Down