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

build: add darwin_arm64 support #429

Closed
wants to merge 1 commit into from
Closed

Conversation

jbedard
Copy link
Contributor

@jbedard jbedard commented Feb 23, 2022

May also require a fix or patch to the cc config: bazelbuild/bazel#13514 (comment)

I'm not sure if there's anything that should be added, or maybe we'd want to wait until that patch isn't required? But without this and the linked patch the build on an m1 doesn't work...

May also require a fix or patch to the cc config: bazelbuild/bazel#13514 (comment)
@jbedard jbedard changed the title chore(build): add darwin_arm64 support build: add darwin_arm64 support Feb 23, 2022
@devversion
Copy link
Member

@jbedard In what cases would you need this to be built with arm64? This is the remote execution platform code that will run as k8 anyway (see here). Building these targets with Arm64, or Windows, MacOS fails currently yes, but that package shouldn't be built unless actually used.

@gregmagolan
Copy link
Contributor

What's the error you're getting on M1 @jbedard ?

@gregmagolan
Copy link
Contributor

@devversion Jason is working at Aspect and taking over the work the Dylan Martin was working on

@gregmagolan
Copy link
Contributor

Perhaps it is time to bring in a proper cpp toolchain into this repo so we can avoid the k8 hack for remote execution on macos.

I have an example of using the grails toolchain up here https://github.com/gregmagolan/bazel-llvm-cgo-cross-compile-example/blob/main/WORKSPACE#L39

@jbedard
Copy link
Contributor Author

jbedard commented Feb 25, 2022

This is the error with the patch referenced in the commit message but without this PR:

ERROR: /.../angular/dev-infra/bazel/remote-execution/cpp/BUILD.bazel:10:19: Analysis of target '//bazel/remote-execution/cpp:cc_toolchain_suite' failed
ERROR: Analysis of target '//bazel/remote-execution/cpp:cc_toolchain_suite' failed; build aborted: 

@devversion
Copy link
Member

As stated though, the CC toolchain in bazel/remote-execution/cpp is k8-only since that is what our platform in bazel/remote-execution provides. There is no need for the toolchain to support arm64, win64, darwin64 etc.

I think this error just surfaces due to the use of something like bazel build //...? We should rather exclude these targets from the expansion, or use something like manual (not sure if it works though)

@devversion
Copy link
Member

@gregmagolan I don't 100% remember, but I don't think the cpp toolchain is a hack. It's supposed to be a copy of the default k8 toolchain in Bazel. We just need to define it ourselves/copy it since it's not exposed in bazel_tools or elsewhere. The larger question might be if we need the cpp toolchain at all (which seems unrealistic given the project scope)

@jbedard
Copy link
Contributor Author

jbedard commented Feb 25, 2022

Tagging bazel/remote-execution/cpp:cc_toolchain_suite as manual works as well. Is that preferred?

@devversion
Copy link
Member

Yeah, I would be good with that

@jbedard
Copy link
Contributor Author

jbedard commented Mar 1, 2022

See #429

@jbedard jbedard closed this Mar 1, 2022
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Apr 1, 2022
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.

3 participants