fix: CMAKE_MODULE_LINKER_FLAGS_INIT breaks macos builds

The PR bazel-contrib#1308, added `CMAKE_MODULE_LINKER_FLAGS_INIT` with the value of cxx_linker_shared to the generated toolchain file in CMAKE. This works for most use cases, however, it broke some macos builds that build bundles. This is because the toolchain usually sets "-shared" or "-dynamiclib" while CMAKE sets "-bundle" for Apple platforms. "-bundle" cannot co-exist with the previous two flags.
jsun-splunk · Dec 17, 2024 · 148e04b · 148e04b
1 parent 21e463e
commit 148e04b
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 30 deletions.
diff --git a/foreign_cc/private/cmake_script.bzl b/foreign_cc/private/cmake_script.bzl
@@ -90,7 +90,7 @@ def create_cmake_script(
 
     merged_prefix_path = _merge_prefix_path(user_cache, include_dirs, ext_build_dirs)
 
-    toolchain_dict = _fill_crossfile_from_toolchain(workspace_name, tools, flags)
+    toolchain_dict = _fill_crossfile_from_toolchain(workspace_name, tools, flags, target_os)
     params = None
 
     keys_with_empty_values_in_user_cache = [key for key in user_cache if user_cache.get(key) == ""]
@@ -307,7 +307,7 @@ def _move_dict_values(target, source, descriptor_map):
             else:
                 target[existing.value] = target[existing.value] + " " + value
 
-def _fill_crossfile_from_toolchain(workspace_name, tools, flags):
+def _fill_crossfile_from_toolchain(workspace_name, tools, flags, target_os):
     dict = {}
 
     _sysroot = _find_in_cc_or_cxx(flags, "sysroot")
@@ -360,7 +360,17 @@ def _fill_crossfile_from_toolchain(workspace_name, tools, flags):
     #        lines += [_set_list(ctx, "CMAKE_STATIC_LINKER_FLAGS_INIT", flags.cxx_linker_static)]
     if flags.cxx_linker_shared:
         dict["CMAKE_SHARED_LINKER_FLAGS_INIT"] = _join_flags_list(workspace_name, flags.cxx_linker_shared)
-        dict["CMAKE_MODULE_LINKER_FLAGS_INIT"] = _join_flags_list(workspace_name, flags.cxx_linker_shared)
+
+        # cxx_linker_shared will contain '-shared' or '-dynamiclib' on macos. This flag conflicts with "-bundle"
+        # that is set by CMAKE based on platform. e.g.
+        # https://gitlab.kitware.com/cmake/cmake/-/blob/master/Modules/Platform/Apple-Intel.cmake#L11
+        # Therefore, for modules aka bundles we want to remove these flags.
+        module_linker_flags = []
+        if target_os == "macos":
+            module_linker_flags = [flag for flag in flags.cxx_linker_shared if flag not in ["-shared", "-dynamiclib"]]
+        else:
+            module_linker_flags = flags.cxx_linker_shared
+        dict["CMAKE_MODULE_LINKER_FLAGS_INIT"] = _join_flags_list(workspace_name, module_linker_flags)
     if flags.cxx_linker_executable:
         dict["CMAKE_EXE_LINKER_FLAGS_INIT"] = _join_flags_list(workspace_name, flags.cxx_linker_executable)
 

diff --git a/test/cmake_text_tests.bzl b/test/cmake_text_tests.bzl
@@ -80,36 +80,44 @@ def _fill_crossfile_from_toolchain_test(ctx):
         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"],
-    )
 
-    res = export_for_test.fill_crossfile_from_toolchain("ws", tools, flags)
-
-    expected = {
-        "CMAKE_AR": "/cxx_linker_static",
-        "CMAKE_ASM_FLAGS_INIT": "assemble",
-        "CMAKE_CXX_COMPILER": "$${EXT_BUILD_ROOT//\\\\//}$$/external/cxx-value",
-        "CMAKE_CXX_COMPILER_EXTERNAL_TOOLCHAIN": "cxx-toolchain",
-        # Quoted args are escaped when crossfile is written to a script in create_cmake_script
-        "CMAKE_CXX_FLAGS_INIT": "--quoted=\"abc def\" --sysroot=/abc/sysroot --gcc_toolchain cxx-toolchain",
-        "CMAKE_CXX_LINK_EXECUTABLE": "$${EXT_BUILD_ROOT//\\\\//}$$/ws/cxx_linker_executable <FLAGS> <CMAKE_CXX_LINK_FLAGS> <LINK_FLAGS> <OBJECTS> -o <TARGET> <LINK_LIBRARIES>",
-        "CMAKE_C_COMPILER": "/some-cc-value",
-        "CMAKE_C_COMPILER_EXTERNAL_TOOLCHAIN": "cc-toolchain",
-        "CMAKE_C_FLAGS_INIT": "-cc-flag -gcc_toolchain cc-toolchain",
-        "CMAKE_EXE_LINKER_FLAGS_INIT": "executable",
-        "CMAKE_MODULE_LINKER_FLAGS_INIT": "shared1 shared2",
-        "CMAKE_SHARED_LINKER_FLAGS_INIT": "shared1 shared2",
-        "CMAKE_SYSROOT": "/abc/sysroot",
+    cases = {
+        # format: target_os: (input_flags, expected_flags)
+        "unknown": (["shared1", "shared2"], ["shared1", "shared2"]),
+        "macos": (["-shared", "-dynamiclib", "-bundle"], ["-bundle"]),
     }
 
-    for key in expected:
-        asserts.equals(env, expected[key], res[key])
+    for target_os, inputs in cases.items():
+        flags = CxxFlagsInfo(
+            cc = ["-cc-flag", "-gcc_toolchain", "cc-toolchain"],
+            cxx = ["--quoted=\"abc def\"", "--sysroot=/abc/sysroot", "--gcc_toolchain", "cxx-toolchain"],
+            cxx_linker_shared = inputs[0],
+            cxx_linker_static = ["static"],
+            cxx_linker_executable = ["executable"],
+            assemble = ["assemble"],
+        )
+
+        res = export_for_test.fill_crossfile_from_toolchain("ws", tools, flags, target_os)
+
+        expected = {
+            "CMAKE_AR": "/cxx_linker_static",
+            "CMAKE_ASM_FLAGS_INIT": "assemble",
+            "CMAKE_CXX_COMPILER": "$${EXT_BUILD_ROOT//\\\\//}$$/external/cxx-value",
+            "CMAKE_CXX_COMPILER_EXTERNAL_TOOLCHAIN": "cxx-toolchain",
+            # Quoted args are escaped when crossfile is written to a script in create_cmake_script
+            "CMAKE_CXX_FLAGS_INIT": "--quoted=\"abc def\" --sysroot=/abc/sysroot --gcc_toolchain cxx-toolchain",
+            "CMAKE_CXX_LINK_EXECUTABLE": "$${EXT_BUILD_ROOT//\\\\//}$$/ws/cxx_linker_executable <FLAGS> <CMAKE_CXX_LINK_FLAGS> <LINK_FLAGS> <OBJECTS> -o <TARGET> <LINK_LIBRARIES>",
+            "CMAKE_C_COMPILER": "/some-cc-value",
+            "CMAKE_C_COMPILER_EXTERNAL_TOOLCHAIN": "cc-toolchain",
+            "CMAKE_C_FLAGS_INIT": "-cc-flag -gcc_toolchain cc-toolchain",
+            "CMAKE_EXE_LINKER_FLAGS_INIT": "executable",
+            "CMAKE_MODULE_LINKER_FLAGS_INIT": " ".join(inputs[1]),
+            "CMAKE_SHARED_LINKER_FLAGS_INIT": " ".join(inputs[0]),
+            "CMAKE_SYSROOT": "/abc/sysroot",
+        }
+
+        for key in expected:
+            asserts.equals(env, expected[key], res[key])
 
     return unittest.end(env)