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
3 changes: 3 additions & 0 deletions foreign_cc/cmake.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ load("//foreign_cc/private:transitions.bzl", "foreign_cc_rule_variant")
load(
"//foreign_cc/private/framework:platform.bzl",
"os_name",
"target_arch_name",
"target_os_name",
)
load(
Expand Down Expand Up @@ -253,6 +254,8 @@ def _create_configure_script(configureParameters):
configure_script = create_cmake_script(
workspace_name = ctx.workspace_name,
target_os = target_os_name(ctx),
target_arch = target_arch_name(ctx),
host_os = os_name(ctx),
generator = attrs.generator,
cmake_path = attrs.cmake_path,
tools = tools,
Expand Down
37 changes: 30 additions & 7 deletions foreign_cc/private/cmake_script.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,30 @@ def _escape_dquote_bash_crosstool(text):
# We use a starlark raw string to prevent the need to escape backslashes for starlark as well.
return text.replace('"', r'\\\\\\\"')

_TARGET_OS_PARAMS = {
"android": {
"ANDROID": "YES",
"CMAKE_SYSTEM_NAME": "Linux",
},
"linux": {
"CMAKE_SYSTEM_NAME": "Linux",
},
}

_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.

},
"x86_64": {
"CMAKE_SYSTEM_PROCESSOR": "x86_64",
},
}

def create_cmake_script(
workspace_name,
target_os,
target_arch,
host_os,
generator,
cmake_path,
tools,
Expand All @@ -39,6 +60,8 @@ def create_cmake_script(
Args:
workspace_name: current workspace name
target_os: The target OS for the build
target_arch: The target arch for the build
host_os: The execution OS for the build
generator: The generator target for cmake to use
cmake_path: The path to the cmake executable
tools: cc_toolchain tools (CxxToolsInfo)
Expand Down Expand Up @@ -93,13 +116,13 @@ def create_cmake_script(
if not params.cache.get("CMAKE_RANLIB"):
params.cache.update({"CMAKE_RANLIB": ""})

# Avoid cmake passing wrong linker flags when targeting android on macOS
# https://github.com/bazelbuild/rules_foreign_cc/issues/289
if target_os == "android":
params.cache.update({
"ANDROID": "YES",
"CMAKE_SYSTEM_NAME": "Linux",
})
# Avoid CMake passing the wrong linker flags when cross compiling
# 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.

params.cache.update(_TARGET_OS_PARAMS.get(target_os, {}))
params.cache.update(_TARGET_ARCH_PARAMS.get(target_arch, {}))

set_env_vars = [
"export {}=\"{}\"".format(key, _escape_dquote_bash(params.env[key]))
Expand Down
3 changes: 3 additions & 0 deletions foreign_cc/private/framework.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ CC_EXTERNAL_RULE_ATTRIBUTES = {
cfg = "exec",
default = [],
),
"_aarch64_constraint": attr.label(default = Label("@platforms//cpu:aarch64")),
"_android_constraint": attr.label(default = Label("@platforms//os:android")),
# we need to declare this attribute to access cc_toolchain
"_cc_toolchain": attr.label(
Expand All @@ -219,6 +220,8 @@ CC_EXTERNAL_RULE_ATTRIBUTES = {
cfg = "exec",
default = Label("@rules_foreign_cc//foreign_cc/private/framework:platform_info"),
),
"_linux_constraint": attr.label(default = Label("@platforms//os:linux")),
"_x86_64_constraint": attr.label(default = Label("@platforms//cpu:x86_64")),
}

# A list of common fragments required by rules using this framework
Expand Down
19 changes: 18 additions & 1 deletion foreign_cc/private/framework/platform.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,23 @@ def os_name(ctx):

return platform_info[ForeignCcPlatformInfo].os

def target_arch_name(ctx):
"""A helper function for getting the target architecture name based on the constraints

Args:
ctx (ctx): The current rule's context object

Returns:
str: The string of the current platform
"""
archs = ["x86_64", "aarch64"]
for arch in archs:
constraint = getattr(ctx.attr, "_{}_constraint".format(arch))
if constraint and ctx.target_platform_has_constraint(constraint[platform_common.ConstraintValueInfo]):
return arch

return "unknown"

def target_os_name(ctx):
"""A helper function for getting the target operating system name based on the constraints

Expand All @@ -75,7 +92,7 @@ def target_os_name(ctx):
Returns:
str: The string of the current platform
"""
operating_systems = ["android"]
operating_systems = ["android", "linux"]
for os in operating_systems:
constraint = getattr(ctx.attr, "_{}_constraint".format(os))
if constraint and ctx.target_platform_has_constraint(constraint[platform_common.ConstraintValueInfo]):
Expand Down
85 changes: 84 additions & 1 deletion test/cmake_text_tests.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,8 @@ def _merge_flag_values_no_toolchain_file_test(ctx):
script = create_cmake_script(
"ws",
"unknown",
"unknown",
"unknown",
"Unix Makefiles",
"cmake",
tools,
Expand Down Expand Up @@ -293,6 +295,8 @@ def _create_min_cmake_script_no_toolchain_file_test(ctx):
script = create_cmake_script(
"ws",
"unknown",
"unknown",
"unknown",
"Ninja",
"cmake",
tools,
Expand Down Expand Up @@ -347,6 +351,8 @@ def _create_min_cmake_script_wipe_toolchain_test(ctx):
script = create_cmake_script(
"ws",
"unknown",
"unknown",
"unknown",
"Ninja",
"cmake",
tools,
Expand Down Expand Up @@ -397,6 +403,8 @@ def _create_min_cmake_script_toolchain_file_test(ctx):
script = create_cmake_script(
"ws",
"unknown",
"unknown",
"unknown",
"Ninja",
"cmake",
tools,
Expand Down Expand Up @@ -475,6 +483,8 @@ def _create_cmake_script_no_toolchain_file_test(ctx):
script = create_cmake_script(
"ws",
"unknown",
"unknown",
"unknown",
"Ninja",
"cmake",
tools,
Expand Down Expand Up @@ -540,6 +550,75 @@ def _create_cmake_script_android_test(ctx):
script = create_cmake_script(
"ws",
"android",
"x86_64",
"unknown",
"Ninja",
"cmake",
tools,
flags,
"test_rule",
"external/test_rule",
True,
user_cache,
user_env,
["--debug-output", "-Wdev"],
cmake_commands = [],
)
expected = r"""export CC="sink-cc-value"
export CXX="sink-cxx-value"
export CFLAGS="-cc-flag -gcc_toolchain cc-toolchain --from-env --additional-flag"
export CXXFLAGS="--quoted=\\\"abc def\\\" --sysroot=/abc/sysroot --gcc_toolchain cxx-toolchain"
export ASMFLAGS="assemble assemble-user"
export CUSTOM_ENV="YES"
##enable_tracing##
cmake -DCMAKE_AR="/cxx_linker_static" -DCMAKE_CXX_LINK_EXECUTABLE="became" -DCMAKE_SHARED_LINKER_FLAGS="shared1 shared2" -DCMAKE_EXE_LINKER_FLAGS="executable" -DCMAKE_BUILD_TYPE="user_type" -DCUSTOM_CACHE="YES" -DCMAKE_INSTALL_PREFIX="test_rule" -DCMAKE_PREFIX_PATH="$$EXT_BUILD_DEPS$$" -DCMAKE_RANLIB="" -DANDROID="YES" -DCMAKE_SYSTEM_NAME="Linux" -DCMAKE_SYSTEM_PROCESSOR="x86_64" --debug-output -Wdev -G 'Ninja' $$EXT_BUILD_ROOT$$/external/test_rule
##disable_tracing##
"""
asserts.equals(env, expected.splitlines(), script)

return unittest.end(env)

def _create_cmake_script_linux_test(ctx):
env = unittest.begin(ctx)

tools = CxxToolsInfo(
cc = "/some-cc-value",
cxx = "external/cxx-value",
cxx_linker_static = "/cxx_linker_static",
cxx_linker_executable = "ws/cxx_linker_executable",
)
flags = CxxFlagsInfo(
cc = ["-cc-flag", "-gcc_toolchain", "cc-toolchain"],
cxx = [
"--quoted=\"abc def\"",
"--sysroot=/abc/sysroot",
"--gcc_toolchain",
"cxx-toolchain",
],
cxx_linker_shared = ["shared1", "shared2"],
cxx_linker_static = ["static"],
cxx_linker_executable = ["executable"],
assemble = ["assemble"],
)
user_env = {
"CC": "sink-cc-value",
"CFLAGS": "--from-env",
"CUSTOM_ENV": "YES",
"CXX": "sink-cxx-value",
}
user_cache = {
"CMAKE_ASM_FLAGS": "assemble-user",
"CMAKE_BUILD_TYPE": "user_type",
"CMAKE_CXX_LINK_EXECUTABLE": "became",
"CMAKE_C_FLAGS": "--additional-flag",
"CUSTOM_CACHE": "YES",
}

script = create_cmake_script(
"ws",
"linux",
"aarch64",
"unknown",
"Ninja",
"cmake",
tools,
Expand All @@ -559,7 +638,7 @@ export CXXFLAGS="--quoted=\\\"abc def\\\" --sysroot=/abc/sysroot --gcc_toolchain
export ASMFLAGS="assemble assemble-user"
export CUSTOM_ENV="YES"
##enable_tracing##
cmake -DCMAKE_AR="/cxx_linker_static" -DCMAKE_CXX_LINK_EXECUTABLE="became" -DCMAKE_SHARED_LINKER_FLAGS="shared1 shared2" -DCMAKE_EXE_LINKER_FLAGS="executable" -DCMAKE_BUILD_TYPE="user_type" -DCUSTOM_CACHE="YES" -DCMAKE_INSTALL_PREFIX="test_rule" -DCMAKE_PREFIX_PATH="$$EXT_BUILD_DEPS$$" -DCMAKE_RANLIB="" -DANDROID="YES" -DCMAKE_SYSTEM_NAME="Linux" --debug-output -Wdev -G 'Ninja' $$EXT_BUILD_ROOT$$/external/test_rule
cmake -DCMAKE_AR="/cxx_linker_static" -DCMAKE_CXX_LINK_EXECUTABLE="became" -DCMAKE_SHARED_LINKER_FLAGS="shared1 shared2" -DCMAKE_EXE_LINKER_FLAGS="executable" -DCMAKE_BUILD_TYPE="user_type" -DCUSTOM_CACHE="YES" -DCMAKE_INSTALL_PREFIX="test_rule" -DCMAKE_PREFIX_PATH="$$EXT_BUILD_DEPS$$" -DCMAKE_RANLIB="" -DCMAKE_SYSTEM_NAME="Linux" -DCMAKE_SYSTEM_PROCESSOR="aarch64" --debug-output -Wdev -G 'Ninja' $$EXT_BUILD_ROOT$$/external/test_rule
##disable_tracing##
"""
asserts.equals(env, expected.splitlines(), script)
Expand Down Expand Up @@ -604,6 +683,8 @@ def _create_cmake_script_toolchain_file_test(ctx):
script = create_cmake_script(
"ws",
"unknown",
"unknown",
"unknown",
"Ninja",
"cmake",
tools,
Expand Down Expand Up @@ -664,6 +745,7 @@ create_min_cmake_script_toolchain_file_test = unittest.make(_create_min_cmake_sc
create_cmake_script_no_toolchain_file_test = unittest.make(_create_cmake_script_no_toolchain_file_test)
create_cmake_script_toolchain_file_test = unittest.make(_create_cmake_script_toolchain_file_test)
create_cmake_script_android_test = unittest.make(_create_cmake_script_android_test)
create_cmake_script_linux_test = unittest.make(_create_cmake_script_linux_test)
merge_flag_values_no_toolchain_file_test = unittest.make(_merge_flag_values_no_toolchain_file_test)
create_min_cmake_script_wipe_toolchain_test = unittest.make(_create_min_cmake_script_wipe_toolchain_test)

Expand All @@ -682,6 +764,7 @@ def cmake_script_test_suite():
create_cmake_script_no_toolchain_file_test,
create_cmake_script_toolchain_file_test,
create_cmake_script_android_test,
create_cmake_script_linux_test,
merge_flag_values_no_toolchain_file_test,
create_min_cmake_script_wipe_toolchain_test,
)