From db95f553941d7fb430e84c36f4b72819b6d68ce4 Mon Sep 17 00:00:00 2001 From: Austin Schuh Date: Wed, 30 Jul 2025 20:07:06 -0700 Subject: [PATCH] [bazel] Handle debug symbols and debug builds like gradle (#8118) This follows the gradle build accurately. Gradle copies debug symbols into a second file (libfoo.so.debug) and links it back into the .so file. Disable this behavior when gradle doesn't do it today. Also, name everything correctly. When building debug builds, most libraries get a 'd' at the end of them. Do that here too. --- .github/workflows/bazel.yml | 2 +- apriltag/BUILD.bazel | 1 + datalog/BUILD.bazel | 1 + hal/BUILD.bazel | 1 + ntcore/BUILD.bazel | 1 + shared/bazel/compiler_flags/windows_flags.rc | 3 + shared/bazel/rules/BUILD.bazel | 47 ++++ shared/bazel/rules/cc_rules.bzl | 215 ++++++++++++++++++- wpimath/BUILD.bazel | 1 + wpinet/BUILD.bazel | 3 +- wpiutil/BUILD.bazel | 1 + 11 files changed, 270 insertions(+), 6 deletions(-) diff --git a/.github/workflows/bazel.yml b/.github/workflows/bazel.yml index 2f2806a68b..b6caf58538 100644 --- a/.github/workflows/bazel.yml +++ b/.github/workflows/bazel.yml @@ -12,7 +12,7 @@ jobs: fail-fast: false matrix: include: - - { name: "Windows (native)", os: windows-2022, action: "test", config: "--config=windows", } + - { name: "Windows (native)", os: windows-2022, action: "test", config: "", } - { name: "Windows (arm)", os: windows-2022, action: "build", config: "--config=windows_arm", } name: "Build ${{ matrix.name }}" diff --git a/apriltag/BUILD.bazel b/apriltag/BUILD.bazel index 2dccf6290b..51f79e0b07 100644 --- a/apriltag/BUILD.bazel +++ b/apriltag/BUILD.bazel @@ -113,6 +113,7 @@ wpilib_cc_shared_library( "//wpimath:shared/wpimath", "//wpiutil:shared/wpiutil", ], + use_debug_name = False, visibility = ["//visibility:public"], deps = [":apriltagjni"], ) diff --git a/datalog/BUILD.bazel b/datalog/BUILD.bazel index 09cd0834b1..0b10834dcd 100644 --- a/datalog/BUILD.bazel +++ b/datalog/BUILD.bazel @@ -60,6 +60,7 @@ wpilib_cc_shared_library( ":shared/datalog", "//wpiutil:shared/wpiutil", ], + use_debug_name = False, visibility = ["//visibility:public"], deps = [":datalogjni"], ) diff --git a/hal/BUILD.bazel b/hal/BUILD.bazel index 9a06a08c3d..00c16860c6 100644 --- a/hal/BUILD.bazel +++ b/hal/BUILD.bazel @@ -163,6 +163,7 @@ wpilib_cc_shared_library( ":shared/wpiHal", "//wpiutil:shared/wpiutil", ], + use_debug_name = False, visibility = ["//visibility:public"], deps = [ ":wpiHaljni", diff --git a/ntcore/BUILD.bazel b/ntcore/BUILD.bazel index 5e1a6f0bb3..9165a3aeb3 100644 --- a/ntcore/BUILD.bazel +++ b/ntcore/BUILD.bazel @@ -143,6 +143,7 @@ wpilib_cc_shared_library( ":shared/ntcore", "//wpiutil:shared/wpiutil", ], + use_debug_name = False, visibility = ["//visibility:public"], deps = [":ntcorejni"], ) diff --git a/shared/bazel/compiler_flags/windows_flags.rc b/shared/bazel/compiler_flags/windows_flags.rc index e0a327eaf5..0276777b8b 100644 --- a/shared/bazel/compiler_flags/windows_flags.rc +++ b/shared/bazel/compiler_flags/windows_flags.rc @@ -27,3 +27,6 @@ build:windows_arm --platforms="@rules_bzlmodrio_toolchains//platforms/windows_ar build:windows --host_copt=/wd4141 # Ignore utf8 warning in tools build:windows --host_copt=/wd4715 + +# Disable the C++17 feature in the windows compiler +build:windows --features=-default_cpp_std --host_features=-default_cpp_std diff --git a/shared/bazel/rules/BUILD.bazel b/shared/bazel/rules/BUILD.bazel index 63efbb4b06..8721fbb6aa 100644 --- a/shared/bazel/rules/BUILD.bazel +++ b/shared/bazel/rules/BUILD.bazel @@ -19,3 +19,50 @@ pkg_files( prefix = "META-INF", visibility = ["//visibility:public"], ) + +config_setting( + name = "compilation_mode_windows_fastbuild", + constraint_values = [ + "@platforms//os:windows", + ], + values = { + "compilation_mode": "fastbuild", + }, +) + +config_setting( + name = "compilation_mode_windows_dbg", + constraint_values = [ + "@platforms//os:windows", + ], + values = { + "compilation_mode": "dbg", + }, +) + +config_setting( + name = "compilation_mode_dbg", + values = { + "compilation_mode": "dbg", + }, +) + +config_setting( + name = "linux_compilation_mode_dbg", + constraint_values = [ + "@platforms//os:linux", + ], + values = { + "compilation_mode": "dbg", + }, +) + +config_setting( + name = "osx_compilation_mode_dbg", + constraint_values = [ + "@platforms//os:osx", + ], + values = { + "compilation_mode": "dbg", + }, +) diff --git a/shared/bazel/rules/cc_rules.bzl b/shared/bazel/rules/cc_rules.bzl index c364744593..743c2d3149 100644 --- a/shared/bazel/rules/cc_rules.bzl +++ b/shared/bazel/rules/cc_rules.bzl @@ -1,5 +1,5 @@ load("@build_bazel_apple_support//rules:universal_binary.bzl", "universal_binary") -load("@rules_cc//cc:action_names.bzl", "CPP_LINK_STATIC_LIBRARY_ACTION_NAME") +load("@rules_cc//cc:action_names.bzl", "CPP_LINK_STATIC_LIBRARY_ACTION_NAME", "OBJ_COPY_ACTION_NAME", "STRIP_ACTION_NAME") load("@rules_cc//cc:cc_shared_library.bzl", "cc_shared_library") load("@rules_cc//cc:defs.bzl", "CcInfo", "cc_library") load("@rules_cc//cc:find_cc_toolchain.bzl", "CC_TOOLCHAIN_ATTRS", "find_cpp_toolchain", "use_cc_toolchain") @@ -32,6 +32,150 @@ def generate_def_file(ctx, def_parser, object_files, dll_name): ) return def_file +def _split_debug_symbols_impl(ctx): + """This splits debug symbols out from the main shared library where supported. + + WPILIB doesn't split them out for osx or Windows, so just pass them through. + For some builds, like for the systemcore, we don't want them split. + + Args: + copy: If true, pass the file right on through, though make sure the name + is right. + use_debug_name: If true, the file should be named 'libfood.so' when done + rather than 'libfoo.so'. This only makes sense on Linux. + shared_library: The library to split apart. + """ + + label = ctx.attr.shared_library.label + target_name = label.name + if label.package: + target_name = label.package + "/" + label.name + folder, lib_name = _folder_prefix(target_name) + + # For Windows and OSX, we just want to pass it all on through. Don't be clever. + if (ctx.target_platform_has_constraint(ctx.attr._darwin_constraint[platform_common.ConstraintValueInfo]) or + ctx.target_platform_has_constraint(ctx.attr._windows_constraint[platform_common.ConstraintValueInfo])): + files = ctx.attr.shared_library[OutputGroupInfo].main_shared_library_output + return [ + DefaultInfo(files = files), + OutputGroupInfo( + default = files, + ), + ] + + # Linux + debug_suffix = "d" if ctx.attr.use_debug_name else "" + + if not ctx.target_platform_has_constraint(ctx.attr._linux_constraint[platform_common.ConstraintValueInfo]): + fail("Unsupported platform") + + lib = ctx.actions.declare_file(folder + "/split/lib" + lib_name + debug_suffix + ".so") + + if ctx.attr.copy: + files = ctx.attr.shared_library[OutputGroupInfo].main_shared_library_output.to_list() + if len(files) != 1: + fail("Wrong number of files", files) + + ctx.actions.run_shell( + command = " ".join([ + "cp", + files[0].path, + lib.path, + ]), + inputs = depset( + direct = files, + ), + outputs = [lib], + ) + + return [ + DefaultInfo(files = depset([lib])), + OutputGroupInfo( + default = depset([lib]), + ), + ] + else: + debug = ctx.actions.declare_file(folder + "/split/lib" + lib_name + debug_suffix + ".so.debug") + cc_toolchain = find_cpp_toolchain(ctx) + + feature_configuration = cc_common.configure_features( + ctx = ctx, + cc_toolchain = cc_toolchain, + requested_features = ctx.features, + unsupported_features = ctx.disabled_features, + ) + + objcopy = cc_common.get_tool_for_action( + feature_configuration = feature_configuration, + action_name = OBJ_COPY_ACTION_NAME, + ) + + strip = cc_common.get_tool_for_action( + feature_configuration = feature_configuration, + action_name = STRIP_ACTION_NAME, + ) + + # This is the set of commands we want to implement to strip debug symbols out and link them back together: + # objcopy --only-keep-debug libmy-library.so libmy-library.so.debug + # strip --strip-debug libmy-library.so + # objcopy --strip-debug libmy-library.so + + files = ctx.attr.shared_library[OutputGroupInfo].main_shared_library_output.to_list() + if len(files) != 1: + fail("Wrong number of files", files) + shared_library = files[0] + + ctx.actions.run_shell( + command = " ".join([ + "cp", + shared_library.path, + lib.path, + "&& chmod u+w", + lib.path, + "&&", + objcopy, + "--only-keep-debug", + lib.path, + debug.path, + "&&", + strip, + "--strip-debug", + lib.path, + "&&", + objcopy, + "--strip-debug", + lib.path, + ]), + inputs = depset( + direct = files, + transitive = [ + cc_toolchain.all_files, + ], + ), + outputs = [lib, debug], + ) + + return [ + DefaultInfo(files = depset([lib, debug])), + OutputGroupInfo( + default = depset([lib, debug]), + ), + ] + +_split_debug_symbols = rule( + implementation = _split_debug_symbols_impl, + attrs = { + "copy": attr.bool(mandatory = True), + "shared_library": attr.label(mandatory = True), + "use_debug_name": attr.bool(mandatory = True), + "_darwin_constraint": attr.label(default = "@platforms//os:osx"), + "_linux_constraint": attr.label(default = "@platforms//os:linux"), + "_windows_constraint": attr.label(default = "@platforms//os:windows"), + } | CC_TOOLCHAIN_ATTRS, + fragments = ["cpp"], + toolchains = use_cc_toolchain(), +) + def _folder_prefix(name): if "/" in name: last_slash = name.rfind("/") @@ -179,17 +323,62 @@ def wpilib_cc_library( def wpilib_cc_shared_library( name, auto_export_windows_symbols = True, + user_link_flags = None, + visibility = None, + use_debug_name = True, + features = None, win_def_file = None, **kwargs): + """Builds a cc_shared_library with some wpilib conventions applied. + + The main workhorse of this rule is cc_shared_library and a pkg_files on the + back end to collect up the outputs. wpilib has some extra conventions we + can commonalize. + + Args: + auto_export_windows_symbols: If true, export all the symbols found in the + shared library on Windows. + user_link_flags: User link flags to add to the linking process. Note: + this gets augmented with extra flags to produce libfoo.so + or libfood.so if in debug mode. + use_debug_name: If true, (default): when in debug mode, produce + libfood.so, otherwise produce libfoo.so. This matches the + wpilib convention for debug library naming. JNI libraries + though want to be loaded with the same name for all builds, + which necesitates turning this off. + win_def_file: The .def file used to specify symbols used in linking on + Windows. This is selected automatically such that it is + only used on Windows. + """ + folder, lib = _folder_prefix(name) - features = [] + if not features: + features = [] + if auto_export_windows_symbols: features.append("windows_export_all_symbols") + if use_debug_name: + user_link_flags = (user_link_flags or []) + select({ + "//shared/bazel/rules:linux_compilation_mode_dbg": ["-Wl,-soname,lib" + lib + "d.so"], + "//shared/bazel/rules:osx_compilation_mode_dbg": ["-Wl,-install_name,lib" + lib + "d.so"], + "@platforms//os:linux": ["-Wl,-soname,lib" + lib + ".so"], + "@platforms//os:osx": ["-Wl,-install_name,lib" + lib + ".so"], + "//conditions:default": [], + }) + else: + user_link_flags = (user_link_flags or []) + select({ + "@platforms//os:linux": ["-Wl,-soname,lib" + lib + ".so"], + "@platforms//os:osx": ["-Wl,-install_name,lib" + lib + ".so"], + "//conditions:default": [], + }) + cc_shared_library( name = name, + user_link_flags = user_link_flags, features = features, + visibility = visibility, # Only include a .def file on windows. This makes it so we can mark # the .def file as only compatible with windows. win_def_file = select({ @@ -208,18 +397,32 @@ def wpilib_cc_shared_library( ], ) + _split_debug_symbols( + name = name + "-symbolsplit", + copy = select({ + "@rules_bzlmodrio_toolchains//conditions:linux_x86_64": False, + "//conditions:default": True, + }), + use_debug_name = select({ + "//shared/bazel/rules:compilation_mode_dbg": True, + "//conditions:default": False, + }) if use_debug_name else False, + shared_library = name, + ) + pkg_files( name = folder + "/lib" + lib + "-shared-files", srcs = select({ "@rules_bzlmodrio_toolchains//conditions:osx": [universal_name], "//conditions:default": [ - ":" + name, + ":" + name + "-symbolsplit", ], }), strip_prefix = select({ "@rules_bzlmodrio_toolchains//conditions:osx": "universal", - "//conditions:default": folder, + "//conditions:default": None, }), + visibility = visibility, ) CcStaticLibraryInfo = provider( @@ -412,6 +615,8 @@ def wpilib_cc_static_library( if not static_lib_name: folder, lib = _folder_prefix(name) static_lib_name = select({ + "//shared/bazel/rules:compilation_mode_dbg": folder + "/lib" + lib + "d.a", + "//shared/bazel/rules:compilation_mode_windows_dbg": folder + "/" + lib + ".lib", "@bazel_tools//src/conditions:windows": folder + "/" + lib + ".lib", "//conditions:default": folder + "/lib" + lib + ".a", }) @@ -423,6 +628,8 @@ def wpilib_cc_static_library( ) def _generate_def_windows_impl(ctx): + # Generate the .def file for Windows. Do this by finding the .obj files + # specified, and then passing those into the def file generator. deps = ctx.attr.deps cc_toolchain = find_cpp_toolchain(ctx) diff --git a/wpimath/BUILD.bazel b/wpimath/BUILD.bazel index a4f3b22e34..7a5ae9254b 100644 --- a/wpimath/BUILD.bazel +++ b/wpimath/BUILD.bazel @@ -206,6 +206,7 @@ wpilib_cc_shared_library( ":shared/wpimath", "//wpiutil:shared/wpiutil", ], + use_debug_name = False, visibility = ["//visibility:public"], deps = [":wpimathjni"], ) diff --git a/wpinet/BUILD.bazel b/wpinet/BUILD.bazel index 82a6ab3171..abb966eac6 100644 --- a/wpinet/BUILD.bazel +++ b/wpinet/BUILD.bazel @@ -189,9 +189,10 @@ wpilib_cc_shared_library( name = "shared/wpinetjni", auto_export_windows_symbols = False, dynamic_deps = [ - ":shared/wpinet", "//wpiutil:shared/wpiutil", + ":shared/wpinet", ], + use_debug_name = False, visibility = ["//visibility:public"], deps = [":wpinetjni"], ) diff --git a/wpiutil/BUILD.bazel b/wpiutil/BUILD.bazel index a151f2042b..503c6d4d12 100644 --- a/wpiutil/BUILD.bazel +++ b/wpiutil/BUILD.bazel @@ -209,6 +209,7 @@ wpilib_cc_shared_library( dynamic_deps = [ ":shared/wpiutil", ], + use_debug_name = False, visibility = ["//visibility:public"], deps = [":wpiutiljni"], )