Skip to content

Commit 8aaf1f4

Browse files
authored
fix: various mac build fixes and improvements [DIP-311] (#112)
The recent macos sdk v15 update has caused a number of issues with our builds that require various tweaks to our build settings to fix. ## Switch to ninja for cmake generator The `cmake` build process involves bootstrapping a gnu make binary from source. The update has caused some strange build errors during this process. More info can be found in the ticket I've created here: bazel-contrib/rules_foreign_cc#1099 In the meantime we can just switch to using the ninja generator. ## Drop support for shared linking We make liberal use of "weak linking" for various purposes across the codebase - usually implemented using function pointers for (I assume) portability. However, this means that most of our libraries can't really be built as shared libraries due to resulting "undefined reference" errors. In bazel the build system will by default always try to build the shared library unless `linkstatic` is set to `True`. So on mac this was causing errors during the normal build. Not sure why linux was not having problems (all our CMake builds fail when `-DBUILD_SHARED_LIBS=TRUE`). To fix this I added `-undefined dynamic_lookup` to the link flags on mac to tell the linker we'll supply the symbols later at the final link step. Now with the update, for whatever reason these flags appear to be causing the following crash on m1 macs when the linker tries to link a binary: ``` Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging 0 0x102277648 __assert_rtn + 72 1 0x10224c6c8 ld::AtomFileConsolidator::addAtomFile(ld::AtomFile const*, ld::AtomFile const*, bool) + 4860 2 0x1022581d0 ld::AtomFileConsolidator::addAtomFile(ld::AtomFile const*) + 148 3 0x102275b20 ld::pass::stubs(ld::Options const&, ld::AtomFileConsolidator&) + 1644 4 0x10225f84c ld::AtomFileConsolidator::resolve() + 12232 5 0x1021ea1a8 main + 9104 ld: Assertion failed: (slot < _sideTableBuffer.size()), function addAtom, file AtomFileConsolidator.cpp, line 2158. clang: error: linker command failed with exit code 1 (use -v to see invocation) INFO: Elapsed time: 190.377s, Critical Path: 36.73s INFO: 1484 processes: 210 internal, 1274 darwin-sandbox. FAILED: Build did NOT complete successfully ``` Since we've never really supported shared linking except in some exceptional cases, I've disabled shared library builds by default across all our code and thus can remove the problematic flags (and probably get a perf boost as well). ## Make the minimum macos target consistent Another annoyance with the mac builds was that the bazel code was getting compiled with a different macos deployment target than the cmake libraries, which resulting in the build output getting spammed with linker warnings referencing this difference. On my system the bazel target was `13.0`, while the cmake value was `13.6`. I'm not sure how (as of our current version of bazel 6.2) this value is computed. In CMake it's interpolated from the system. To fix this I've wired up our toolchain to respect the [macos_minimum_os](https://bazel.build/reference/command-line-reference#flag--macos_minimum_os) flags and add `-mmacos-version-min` to our build. This is just taken from the most recent `HEAD` of `bazel` which is not in 6.x. The CMake side requires explicitly setting [CMAKE_OSX_DEPLOYMENT_TARGET](https://cmake.org/cmake/help/latest/variable/CMAKE_OSX_DEPLOYMENT_TARGET.html) variable to the same value. It would be better if `rules_foreign_cc` also respected the `macos_minimum_os` flag so the value would not have to be hardcoded. We can perhaps open a ticket to request this. `13.0` seems like a reasonable minimum deployment target since these builds are only intended for development machines. ## Testing I've tested these changes on m1 and intel macs as well as the following test PR's: - https://github.com/swift-nav/starling/pull/8677 - https://github.com/swift-nav/orion-engine/pull/7239
1 parent 9924b81 commit 8aaf1f4

File tree

6 files changed

+65
-12
lines changed

6 files changed

+65
-12
lines changed

cc/BUILD.bazel

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ exports_files(
1616
visibility = ["//visibility:public"],
1717
)
1818

19-
# Disable tests and test libraries with --@rules_swiftnav//:disable_test=true
19+
# Disable tests and test libraries with --@rules_swiftnav///cc:disable_test=true
2020
bool_flag(
2121
name = "disable_tests",
2222
build_setting_default = False,
@@ -29,7 +29,7 @@ config_setting(
2929
visibility = ["//visibility:public"],
3030
)
3131

32-
# Enable exceptions with --@rules_swiftnav//:enable_exceptions=true
32+
# Enable exceptions with --@rules_swiftnav//cc:enable_exceptions=true
3333
bool_flag(
3434
name = "enable_exceptions",
3535
build_setting_default = False,
@@ -42,7 +42,7 @@ config_setting(
4242
visibility = ["//visibility:public"],
4343
)
4444

45-
# Enable rtti with --@rules_swiftnav//:enable_exceptions=true
45+
# Enable rtti with --@rules_swiftnav//cc:enable_exceptions=true
4646
bool_flag(
4747
name = "enable_rtti",
4848
build_setting_default = False,
@@ -55,6 +55,19 @@ config_setting(
5555
visibility = ["//visibility:public"],
5656
)
5757

58+
# Enable shared linking with --@rules_swiftnav//cc:enable_shared=true
59+
bool_flag(
60+
name = "enable_shared",
61+
build_setting_default = False,
62+
visibility = ["//visibility:public"],
63+
)
64+
65+
config_setting(
66+
name = "_enable_shared",
67+
flag_values = {":enable_shared": "true"},
68+
visibility = ["//visibility:public"],
69+
)
70+
5871
# Disable warnings as errors with --@rules_swiftnav//cc:warnings_as_errors=false
5972
bool_flag(
6073
name = "warnings_as_errors",
@@ -75,21 +88,21 @@ string_flag(
7588
visibility = ["//visibility:public"],
7689
)
7790

78-
# Enable with --@rules_swiftnav//:cxx_standard=17
91+
# Enable with --@rules_swiftnav//cc:cxx_standard=17
7992
config_setting(
8093
name = "cxx17",
8194
flag_values = {":cxx_standard": "17"},
8295
visibility = ["//visibility:public"],
8396
)
8497

85-
# Enable with --@rules_swiftnav//:cxx_standard=20
98+
# Enable with --@rules_swiftnav//cc:cxx_standard=20
8699
config_setting(
87100
name = "cxx20",
88101
flag_values = {":cxx_standard": "20"},
89102
visibility = ["//visibility:public"],
90103
)
91104

92-
# Enable with --@rules_swiftnav//:cxx_standard=23
105+
# Enable with --@rules_swiftnav//cc:cxx_standard=23
93106
config_setting(
94107
name = "cxx23",
95108
flag_values = {":cxx_standard": "23"},

cc/defs.bzl

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,12 @@ def _common_cxx_opts(exceptions = False, rtti = False, standard = None):
7777
def _construct_local_includes(local_includes):
7878
return [construct_local_include(path) for path in local_includes]
7979

80+
def _link_static(linkstatic = True):
81+
return select({
82+
Label("//cc:_enable_shared"): False,
83+
"//conditions:default": linkstatic,
84+
})
85+
8086
# Disable building when --//:disable_tests=true or when building on windows
8187
def _test_compatible_with():
8288
return select({
@@ -220,6 +226,8 @@ def swift_c_library(**kwargs):
220226

221227
kwargs["tags"] = [LIBRARY] + kwargs.get("tags", [])
222228

229+
kwargs["linkstatic"] = _link_static(kwargs.get("linkstatic", True))
230+
223231
native.cc_library(**kwargs)
224232

225233
def swift_cc_library(**kwargs):
@@ -271,6 +279,8 @@ def swift_cc_library(**kwargs):
271279

272280
kwargs["tags"] = [LIBRARY] + kwargs.get("tags", [])
273281

282+
kwargs["linkstatic"] = _link_static(kwargs.get("linkstatic", True))
283+
274284
native.cc_library(**kwargs)
275285

276286
def swift_c_tool_library(**kwargs):
@@ -313,6 +323,8 @@ def swift_c_tool_library(**kwargs):
313323

314324
kwargs["copts"] = copts + c_standard + kwargs.get("copts", [])
315325

326+
kwargs["linkstatic"] = _link_static(kwargs.get("linkstatic", True))
327+
316328
native.cc_library(**kwargs)
317329

318330
def swift_cc_tool_library(**kwargs):
@@ -358,6 +370,8 @@ def swift_cc_tool_library(**kwargs):
358370

359371
kwargs["copts"] = copts + cxxopts + kwargs.get("copts", [])
360372

373+
kwargs["linkstatic"] = _link_static(kwargs.get("linkstatic", True))
374+
361375
native.cc_library(**kwargs)
362376

363377
def swift_c_binary(**kwargs):
@@ -404,6 +418,8 @@ def swift_c_binary(**kwargs):
404418

405419
kwargs["tags"] = [BINARY] + kwargs.get("tags", [])
406420

421+
kwargs["linkstatic"] = _link_static(kwargs.get("linkstatic", True))
422+
407423
native.cc_binary(**kwargs)
408424

409425
def swift_cc_binary(**kwargs):
@@ -568,6 +584,8 @@ def swift_cc_test_library(**kwargs):
568584

569585
kwargs["target_compatible_with"] = kwargs.get("target_compatible_with", []) + _test_compatible_with()
570586

587+
kwargs["linkstatic"] = _link_static(kwargs.get("linkstatic", True))
588+
571589
native.cc_library(**kwargs)
572590

573591
def swift_cc_test(name, type, **kwargs):

cc/toolchains/llvm/cc_toolchain_config.bzl

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -108,12 +108,6 @@ def cc_toolchain_config(
108108
use_lld = False
109109
link_flags.extend([
110110
"-headerpad_max_install_names",
111-
# This will issue a warning on macOS ventura; see:
112-
# https://github.com/python/cpython/issues/97524
113-
# https://developer.apple.com/forums/thread/719961
114-
"-undefined",
115-
"dynamic_lookup",
116-
"-Wl,-no_fixup_chains",
117111
])
118112
else:
119113
use_lld = True

cc/toolchains/llvm/unix_cc_toolchain_config.bzl

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,11 @@ load(
2929
)
3030
load("@bazel_tools//tools/build_defs/cc:action_names.bzl", "ACTION_NAMES")
3131

32+
def _target_os_version(ctx):
33+
platform_type = ctx.fragments.apple.single_arch_platform.platform_type
34+
xcode_config = ctx.attr._xcode_config[apple_common.XcodeVersionConfig]
35+
return xcode_config.minimum_os_for_platform_type(platform_type)
36+
3237
def layering_check_features(compiler):
3338
if compiler != "clang":
3439
return []
@@ -1283,6 +1288,17 @@ def _impl(ctx):
12831288
],
12841289
)
12851290

1291+
macos_minimum_os_feature = feature(
1292+
name = "macos_minimum_os",
1293+
enabled = True,
1294+
flag_sets = [
1295+
flag_set(
1296+
actions = all_compile_actions + all_link_actions,
1297+
flag_groups = [flag_group(flags = ["-mmacos-version-min={}".format(_target_os_version(ctx))])],
1298+
),
1299+
],
1300+
)
1301+
12861302
is_linux = ctx.attr.target_libc != "macosx"
12871303
libtool_feature = feature(
12881304
name = "libtool",
@@ -1365,6 +1381,7 @@ def _impl(ctx):
13651381
asan_feature,
13661382
tsan_feature,
13671383
ubsan_feature,
1384+
macos_minimum_os_feature,
13681385
] + (
13691386
[
13701387
supports_start_end_lib_feature,
@@ -1431,6 +1448,11 @@ cc_toolchain_config = rule(
14311448
"coverage_link_flags": attr.string_list(),
14321449
"supports_start_end_lib": attr.bool(),
14331450
"builtin_sysroot": attr.string(),
1451+
"_xcode_config": attr.label(default = configuration_field(
1452+
fragment = "apple",
1453+
name = "xcode_config_label",
1454+
)),
14341455
},
1456+
fragments = ["apple"],
14351457
provides = [CcToolchainConfigInfo],
14361458
)

third_party/check.BUILD

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,10 @@ cmake(
2222
"CMAKE_INSTALL_LIBDIR": "lib",
2323
"HAVE_SUBUNIT": "0",
2424
},
25+
generate_args = [
26+
"-GNinja",
27+
"-DCMAKE_OSX_DEPLOYMENT_TARGET=13.0",
28+
],
2529
lib_source = ":srcs",
2630
linkopts = select({
2731
"@bazel_tools//src/conditions:darwin": ["-lpthread"],

third_party/nlopt.BUILD

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ cmake(
2626
"CMAKE_INSTALL_LIBDIR": "lib",
2727
},
2828
generate_args = [
29+
"-GNinja",
30+
"-DCMAKE_OSX_DEPLOYMENT_TARGET=13.0",
2931
"-DBUILD_SHARED_LIBS=OFF",
3032
"-DNLOPT_PYTHON=OFF",
3133
"-DNLOPT_OCTAVE=OFF",

0 commit comments

Comments
 (0)