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

fix linux cross compiling on macos #1062

Merged

Conversation

jungleraptor
Copy link
Contributor

@jungleraptor jungleraptor commented Jun 20, 2023

Fixes cross-compiling from macos to linux.

see #997 for background.

I had to make a couple of extra changes to support this:

  • Setting CMAKE_SYSTEM_NAME manually causes CMAKE_SYSTEM_PROCESSOR to not be set. This breaks some builds that expect this variable to be set like libjpeg-turbo.
  • I made it so that the above variables are only set when cross-compilation is detected so that rules_foreign_cc only takes on responsibility of setting them when necessary.

@jungleraptor jungleraptor marked this pull request as ready for review June 20, 2023 18:44

_TARGET_ARCH_PARAMS = {
"aarch64": {
"CMAKE_SYSTEM_PROCESSOR": "aarch64",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's a little more nuance to it than this (mostly on mac) but I think this is a good enough start.

@jungleraptor
Copy link
Contributor Author

cc @keith @jsharpe sorry for the direct ping but don't know the etiquette for this project for requesting reviews.

Just wanted to get it on someones radar.

@keith
Copy link
Member

keith commented Jun 23, 2023

nice, looks reasonable to me, I don't have access here tho, @jsharpe I think is the right contact

# by setting CMAKE_SYSTEM_NAME and CMAKE_SYSTEM_PROCESSOR,
# see https://github.com/bazelbuild/rules_foreign_cc/issues/289,
# and https://github.com/bazelbuild/rules_foreign_cc/pull/1062
if target_os != host_os and target_os != "unknown":
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's more to it than this (i.e. linux-x86_64 -> linux-aarch64).

Probably should be a function like is_cross_compilation and handle this more rigorously.

@jsharpe
Copy link
Member

jsharpe commented Jun 26, 2023

This is on my radar to review - will try to take a look at some point this week!

@jsharpe jsharpe self-requested a review June 26, 2023 08:16
Copy link
Member

@jsharpe jsharpe left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@jsharpe jsharpe merged commit ea7ed42 into bazel-contrib:main Jun 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants