google / cel-cpp

Fast, portable, non-Turing complete expression evaluation (C++)
https://cel.dev
Apache License 2.0
168 stars 48 forks source link

FTBFS on AArch64 Darwin due to unknown linker options with Apple ld #832

Open jchadwick-buf opened 2 months ago

jchadwick-buf commented 2 months ago

Since 16ca259aeea2330998f3665c289267a82a738d46, cel-cpp fails to build correctly on AArch64 Darwin, due to using -Wl,--export-dynamic-symbol:

ld: unknown options: --export-dynamic-symbol=cel_common_internal_LegacyListValue_GetType --export-dynamic-symbol=cel_common_internal_LegacyListValue_DebugString --export-dynamic-symbol=cel_common_internal_LegacyListValue_GetSerializedSize --export-dynamic-symbol=cel_common_internal_LegacyListValue_SerializeTo --export-dynamic-symbol=cel_common_internal_LegacyListValue_ConvertToJsonArray --export-dynamic-symbol=cel_common_internal_LegacyListValue_IsEmpty --export-dynamic-symbol=cel_common_internal_LegacyListValue_Size --export-dynamic-symbol=cel_common_internal_LegacyListValue_Get --export-dynamic-symbol=cel_common_internal_LegacyListValue_ForEach --export-dynamic-symbol=cel_common_internal_LegacyListValue_NewIterator --export-dynamic-symbol=cel_common_internal_LegacyListValue_Contains --export-dynamic-symbol=cel_common_internal_LegacyMapValue_GetType --export-dynamic-symbol=cel_common_internal_LegacyMapValue_DebugString --export-dynamic-symbol=cel_common_internal_LegacyMapValue_GetSerializedSize --export-dynamic-symbol=cel_common_internal_LegacyMapValue_SerializeTo --export-dynamic-symbol=cel_common_internal_LegacyMapValue_ConvertToJsonObject --export-dynamic-symbol=cel_common_internal_LegacyMapValue_IsEmpty --export-dynamic-symbol=cel_common_internal_LegacyMapValue_Size --export-dynamic-symbol=cel_common_internal_LegacyMapValue_Find --export-dynamic-symbol=cel_common_internal_LegacyMapValue_Get --export-dynamic-symbol=cel_common_internal_LegacyMapValue_Has --export-dynamic-symbol=cel_common_internal_LegacyMapValue_ListKeys --export-dynamic-symbol=cel_common_internal_LegacyMapValue_ForEach --export-dynamic-symbol=cel_common_internal_LegacyMapValue_NewIterator --export-dynamic-symbol=cel_common_internal_LegacyStructValue_DebugString --export-dynamic-symbol=cel_common_internal_LegacyStructValue_GetSerializedSize --export-dynamic-symbol=cel_common_internal_LegacyStructValue_SerializeTo --export-dynamic-symbol=cel_common_internal_LegacyStructValue_GetType --export-dynamic-symbol=cel_common_internal_LegacyStructValue_GetTypeName --export-dynamic-symbol=cel_common_internal_LegacyStructValue_ConvertToJsonObject --export-dynamic-symbol=cel_common_internal_LegacyStructValue_GetFieldByName --export-dynamic-symbol=cel_common_internal_LegacyStructValue_GetFieldByNumber --export-dynamic-symbol=cel_common_internal_LegacyStructValue_HasFieldByName --export-dynamic-symbol=cel_common_internal_LegacyStructValue_HasFieldByNumber --export-dynamic-symbol=cel_common_internal_LegacyStructValue_Equal --export-dynamic-symbol=cel_common_internal_LegacyStructValue_ForEachField --export-dynamic-symbol=cel_common_internal_LegacyStructValue_Qualify --export-dynamic-symbol=cel_common_internal_LegacyTypeReflector_NewListValueBuilder --export-dynamic-symbol=cel_common_internal_LegacyTypeReflector_NewMapValueBuilder

Apple's system linker, ld, does not support this option, though I believe the option -exported_symbol may be equivalent.

Since Apple ld is OS-specific, it might be OK to just use a typical select statement here. Like this:

diff --git a/common/BUILD b/common/BUILD
index 8933cf5e..aad7cad9 100644
--- a/common/BUILD
+++ b/common/BUILD
@@ -553,7 +553,10 @@ cc_library(
     # use the dynamic linker to find the symbols at runtime. This only works if they are exported as
     # dynamic symbols. When static linking, our symbols are not exported as dynamic by default. We
     # workaround this by explicitly passing our symbols to the linker for exporting.
-    linkopts = ["-Wl,--export-dynamic-symbol=%s" % dynamic_symbol for dynamic_symbol in _DYNAMIC_SYMBOLS],
+    linkopts = select({
+        "@bazel_tools//src/conditions:darwin": ["-Wl,-exported_symbol -Wl,%s" % dynamic_symbol for dynamic_symbol in _DYNAMIC_SYMBOLS],
+        "//conditions:default": ["-Wl,--export-dynamic-symbol=%s" % dynamic_symbol for dynamic_symbol in _DYNAMIC_SYMBOLS],
+    }),
     deps = [
         ":casting",
         ":json",

Testing locally, this does seem to make the build go further.

P.S.: It seems like all of LLVM, GNU Binutils and Apple's linker have equivalent options where you can specify a file that contains a list of symbols to export; -exported_symbols_list on Apple LD and --export-dynamic-symbol-list on GNU Binutils and LLVM's linker. That said, the format is different between the two: the LLVM and GNU Binutils version seems to match the symbol versions file and look like { a; b; } whereas the Apple linker version appears to be one per-line, like:

a
b

Maybe still worth it to make the linker args more reasonable. (Eventually it's bound to breach the argv limit on some platform, I'm sure.)

P.P.S: The equivalent windows_msvc option might be a little trickier to nail down. Realistically the closest Win32 analogue I can think of is linker export definitions, which is yet another format that looks something like:

LIBRARY lib
EXPORTS
    a
    b

It's a little awkward that every platform has these little quirks with linking that can't be hidden away by Bazel somehow.