From 4520018bc27dbd78b84d839cbe27a614ca3931f0 Mon Sep 17 00:00:00 2001 From: James Sharpe Date: Wed, 24 Feb 2021 22:45:36 +0000 Subject: [PATCH] Make generate_crosstool=True the default for cmake. (#523) This was previously setting `CMAKE_SYSTEM_NAME` in the generated toolchain file but setting this explicitly enables `CMAKE_CROSSCOMPILING` (see https://cmake.org/cmake/help/v3.19/variable/CMAKE_CROSSCOMPILING.html#variable:CMAKE_CROSSCOMPILING) which breaks projects that use `TRY_RUN` --- README.md | 3 --- docs/README.md | 2 +- examples/cmake_crosstool/BUILD | 1 - examples/cmake_crosstool/static/BUILD | 3 --- examples/cmake_hello_world_lib/static/BUILD | 3 --- examples/cmake_hello_world_variant/BUILD | 1 - examples/cmake_synthetic/BUILD | 8 -------- examples/cmake_with_data/BUILD | 4 ---- examples/with_prebuilt_ninja_artefact/BUILD | 1 - test/cmake_text_tests.bzl | 14 +------------- tools/build_defs/cmake.bzl | 10 +++------- tools/build_defs/cmake_script.bzl | 21 +++------------------ 12 files changed, 8 insertions(+), 63 deletions(-) diff --git a/README.md b/README.md index 469e44d0..2da9b5c1 100644 --- a/README.md +++ b/README.md @@ -194,7 +194,6 @@ cmake_external( name = "hello", # This option can be omitted cmake_options = ["-G \"Visual Studio 15 2017 Win64\""], - generate_crosstool_file = True, lib_source = ":srcs", # .vcxproj or .sln file must be specified argument, as multiple files are generated by CMake make_commands = ["MSBuild.exe INSTALL.vcxproj"], @@ -206,7 +205,6 @@ cmake_external( lib_name = "hello", # explicitly specify the generator cmake_options = ["-GNinja"], - generate_crosstool_file = True, lib_source = ":srcs", # specify to call ninja after configuring make_commands = [ @@ -219,7 +217,6 @@ cmake_external( name = "hello_nmake", # explicitly specify the generator cmake_options = ["-G \"NMake Makefiles\""], - generate_crosstool_file = True, lib_source = ":srcs", # specify to call nmake after configuring make_commands = [ diff --git a/docs/README.md b/docs/README.md index 5de16b0f..a3f3ceca 100644 --- a/docs/README.md +++ b/docs/README.md @@ -91,7 +91,7 @@ Rule for building external library with CMake. | deps | Optional dependencies to be copied into the directory structure. Typically those directly required for the external building of the library/binaries. (i.e. those that the external buidl system will be looking for and paths to which are provided by the calling rule) | List of labels | optional | [] | | env | Environment variables to set during the build. $(execpath) macros may be used to point at files which are listed as data deps, tools_deps, or additional_tools, but unlike with other rules, these will be replaced with absolute paths to those files, because the build does not run in the exec root. No other macros are supported. | Dictionary: String -> String | optional | {} | | env_vars | CMake environment variable values to join with toolchain-defined. For example, additional CXXFLAGS. | Dictionary: String -> String | optional | {} | -| generate_crosstool_file | When True, CMake crosstool file will be generated from the toolchain values, provided cache-entries and env_vars (some values will still be passed as -Dkey=value and environment variables). If CMAKE_TOOLCHAIN_FILE cache entry is passed, specified crosstool file will be used When using this option, it makes sense to specify CMAKE_SYSTEM_NAME in the cache_entries - the rule makes only a poor guess about the target system, it is better to specify it manually. | Boolean | optional | False | +| generate_crosstool_file | When True, CMake crosstool file will be generated from the toolchain values, provided cache-entries and env_vars (some values will still be passed as -Dkey=value and environment variables). If CMAKE_TOOLCHAIN_FILE cache entry is passed, specified crosstool file will be used When using this option to cross-compile, it is required to specify CMAKE_SYSTEM_NAME in the cache_entries | Boolean | optional | True | | headers_only | Flag variable to indicate that the library produces only headers | Boolean | optional | False | | install_prefix | Relative install prefix to be passed to CMake in -DCMAKE_INSTALL_PREFIX | String | optional | "" | | interface_libraries | Optional names of the resulting interface libraries. | List of strings | optional | [] | diff --git a/examples/cmake_crosstool/BUILD b/examples/cmake_crosstool/BUILD index 471228ed..79bb3c54 100644 --- a/examples/cmake_crosstool/BUILD +++ b/examples/cmake_crosstool/BUILD @@ -14,7 +14,6 @@ cc_library( cmake_external( name = "hello_cmake", - generate_crosstool_file = True, lib_source = "//static:srcs", out_include_dir = "include/version123", static_libraries = ["libhello.a"], diff --git a/examples/cmake_crosstool/static/BUILD b/examples/cmake_crosstool/static/BUILD index 5114fe29..239fa17e 100644 --- a/examples/cmake_crosstool/static/BUILD +++ b/examples/cmake_crosstool/static/BUILD @@ -24,7 +24,6 @@ cmake_external( cmake_external( name = "libhello_win", cmake_options = ["-G \"Visual Studio 15 2017\" -A x64"], - generate_crosstool_file = True, lib_name = "libhello", lib_source = ":srcs", make_commands = ["MSBuild.exe INSTALL.vcxproj"], @@ -36,7 +35,6 @@ cmake_external( cmake_external( name = "libhello_win_ninja", cmake_options = ["-GNinja"], - generate_crosstool_file = True, lib_source = ":srcs", make_commands = [ "ninja", @@ -50,7 +48,6 @@ cmake_external( cmake_external( name = "libhello_win_nmake", cmake_options = ["-G \"NMake Makefiles\""], - generate_crosstool_file = True, lib_source = ":srcs", make_commands = [ "nmake", diff --git a/examples/cmake_hello_world_lib/static/BUILD b/examples/cmake_hello_world_lib/static/BUILD index 76b030af..d33cee3d 100644 --- a/examples/cmake_hello_world_lib/static/BUILD +++ b/examples/cmake_hello_world_lib/static/BUILD @@ -20,7 +20,6 @@ cmake_external( cmake_external( name = "libhello_win", cmake_options = ["-G \"Visual Studio 15 2017\" -A x64"], - generate_crosstool_file = True, lib_name = "libhello", lib_source = ":srcs", make_commands = ["MSBuild.exe INSTALL.vcxproj"], @@ -32,7 +31,6 @@ cmake_external( cmake_external( name = "libhello_win_ninja", cmake_options = ["-GNinja"], - generate_crosstool_file = True, lib_source = ":srcs", make_commands = [ "ninja", @@ -46,7 +44,6 @@ cmake_external( cmake_external( name = "libhello_win_nmake", cmake_options = ["-G \"NMake Makefiles\""], - generate_crosstool_file = True, lib_source = ":srcs", make_commands = [ "nmake", diff --git a/examples/cmake_hello_world_variant/BUILD b/examples/cmake_hello_world_variant/BUILD index a282e35d..91847002 100644 --- a/examples/cmake_hello_world_variant/BUILD +++ b/examples/cmake_hello_world_variant/BUILD @@ -8,7 +8,6 @@ cmake_external( "//conditions:default": ["CMakeHelloWorld"], }), cmake_options = ["-GNinja"], - generate_crosstool_file = True, lib_source = "@cmake_hello_world_variant_src//:all", make_commands = [ "ninja", diff --git a/examples/cmake_synthetic/BUILD b/examples/cmake_synthetic/BUILD index dca2f943..35f207ce 100644 --- a/examples/cmake_synthetic/BUILD +++ b/examples/cmake_synthetic/BUILD @@ -2,15 +2,9 @@ load("@bazel_skylib//rules:build_test.bzl", "build_test") load("@rules_cc//cc:defs.bzl", "cc_test") load("@rules_foreign_cc//tools/build_defs:cmake.bzl", "cmake_external") -generate_crosstool = select({ - "//:windows": True, - "//conditions:default": False, -}) - cmake_external( name = "liba", cmake_options = ["-GNinja"], - generate_crosstool_file = generate_crosstool, # Demonstrate non-alphanumeric name lib_name = "liba++", lib_source = "//cmake_synthetic/liba:a_srcs", @@ -24,7 +18,6 @@ cmake_external( cmake_external( name = "libb", cmake_options = ["-GNinja"], - generate_crosstool_file = generate_crosstool, lib_source = "//cmake_synthetic/libb:b_srcs", make_commands = [ "ninja", @@ -40,7 +33,6 @@ cmake_external( "LIBB_DIR": "$$EXT_BUILD_DEPS$$", }, cmake_options = ["-GNinja"], - generate_crosstool_file = generate_crosstool, lib_name = "libc", lib_source = "//cmake_synthetic/libc:c_srcs", make_commands = [ diff --git a/examples/cmake_with_data/BUILD b/examples/cmake_with_data/BUILD index e7b02213..5abbe05b 100644 --- a/examples/cmake_with_data/BUILD +++ b/examples/cmake_with_data/BUILD @@ -19,10 +19,6 @@ cmake_external( "cmake_with_data.txt", "//cmake_with_data/lib_b", ], - generate_crosstool_file = select({ - "@bazel_tools//src/conditions:windows": True, - "//conditions:default": False, - }), lib_source = "//cmake_with_data/lib_a:srcs", make_commands = select({ "@bazel_tools//src/conditions:windows": ["MSBuild.exe INSTALL.vcxproj"], diff --git a/examples/with_prebuilt_ninja_artefact/BUILD b/examples/with_prebuilt_ninja_artefact/BUILD index f1f9040a..c4b32083 100644 --- a/examples/with_prebuilt_ninja_artefact/BUILD +++ b/examples/with_prebuilt_ninja_artefact/BUILD @@ -22,7 +22,6 @@ toolchain( cmake_external( name = "hello_cmake", cmake_options = ["-GNinja"], - generate_crosstool_file = True, lib_source = "@examples//cmake_hello_world_lib/static:srcs", make_commands = [ "ninja", diff --git a/test/cmake_text_tests.bzl b/test/cmake_text_tests.bzl index b6ea86fd..cd7087fa 100644 --- a/test/cmake_text_tests.bzl +++ b/test/cmake_text_tests.bzl @@ -85,10 +85,7 @@ def _fill_crossfile_from_toolchain_test(ctx): assemble = ["assemble"], ) - res = export_for_test.fill_crossfile_from_toolchain("ws", "linux", tools, flags) - - system = res.pop("CMAKE_SYSTEM_NAME") - asserts.true(env, system != None) + res = export_for_test.fill_crossfile_from_toolchain("ws", tools, flags) expected = { "CMAKE_AR": "/cxx_linker_static", @@ -172,7 +169,6 @@ def _reverse_descriptor_dict_test(ctx): "CMAKE_EXE_LINKER_FLAGS_INIT": struct(value = "CMAKE_EXE_LINKER_FLAGS", replace = False), "CMAKE_SHARED_LINKER_FLAGS_INIT": struct(value = "CMAKE_SHARED_LINKER_FLAGS", replace = False), "CMAKE_STATIC_LINKER_FLAGS_INIT": struct(value = "CMAKE_STATIC_LINKER_FLAGS", replace = False), - "CMAKE_SYSTEM_NAME": struct(value = "CMAKE_SYSTEM_NAME", replace = True), } for key in expected: @@ -241,7 +237,6 @@ def _merge_flag_values_no_toolchain_file_test(ctx): script = create_cmake_script( "ws", - "linux", "cmake", tools, flags, @@ -282,7 +277,6 @@ def _create_min_cmake_script_no_toolchain_file_test(ctx): script = create_cmake_script( "ws", - "linux", "cmake", tools, flags, @@ -326,7 +320,6 @@ def _create_min_cmake_script_wipe_toolchain_test(ctx): script = create_cmake_script( "ws", - "linux", "cmake", tools, flags, @@ -366,7 +359,6 @@ def _create_min_cmake_script_toolchain_file_test(ctx): script = create_cmake_script( "ws", - "linux", "cmake", tools, flags, @@ -386,7 +378,6 @@ set(CMAKE_C_COMPILER "/usr/bin/gcc") set(CMAKE_C_FLAGS_INIT "-U_FORTIFY_SOURCE -fstack-protector -Wall") set(CMAKE_EXE_LINKER_FLAGS_INIT "-fuse-ld=gold -Wl -no-as-needed") set(CMAKE_SHARED_LINKER_FLAGS_INIT "-shared -fuse-ld=gold") -set(CMAKE_SYSTEM_NAME "Linux") EOF cmake -DNOFORTRAN="on" -DCMAKE_TOOLCHAIN_FILE="crosstool_bazel.cmake" -DCMAKE_BUILD_TYPE="Debug" -DCMAKE_INSTALL_PREFIX="test_rule" -DCMAKE_PREFIX_PATH="$EXT_BUILD_DEPS" -DCMAKE_RANLIB=\"\" -GNinja $EXT_BUILD_ROOT/external/test_rule""" @@ -432,7 +423,6 @@ def _create_cmake_script_no_toolchain_file_test(ctx): script = create_cmake_script( "ws", - "linux", "cmake", tools, flags, @@ -485,7 +475,6 @@ def _create_cmake_script_toolchain_file_test(ctx): script = create_cmake_script( "ws", - "osx", "cmake", tools, flags, @@ -509,7 +498,6 @@ set(CMAKE_C_FLAGS_INIT "-cc-flag -gcc_toolchain cc-toolchain --from-env --additi set(CMAKE_EXE_LINKER_FLAGS_INIT "executable") set(CMAKE_SHARED_LINKER_FLAGS_INIT "shared1 shared2") set(CMAKE_SYSROOT "/abc/sysroot") -set(CMAKE_SYSTEM_NAME "Apple") EOF CUSTOM_ENV=\"YES\" cmake -DCUSTOM_CACHE=\"YES\" -DCMAKE_TOOLCHAIN_FILE=\"crosstool_bazel.cmake\" -DCMAKE_BUILD_TYPE=\"Debug\" -DCMAKE_INSTALL_PREFIX=\"test_rule\" -DCMAKE_PREFIX_PATH=\"$EXT_BUILD_DEPS\" -DCMAKE_RANLIB=\"\" -GNinja $EXT_BUILD_ROOT/external/test_rule""" diff --git a/tools/build_defs/cmake.bzl b/tools/build_defs/cmake.bzl index 90e74a22..380fc1dc 100644 --- a/tools/build_defs/cmake.bzl +++ b/tools/build_defs/cmake.bzl @@ -1,7 +1,6 @@ """ Defines the rule for building external library with CMake """ -load("@rules_foreign_cc//tools/build_defs:shell_script_helper.bzl", "os_name") load( "//tools/build_defs:cc_toolchain_util.bzl", "get_flags_info", @@ -76,8 +75,6 @@ def _create_configure_script(configureParameters): define_install_prefix = "export INSTALL_PREFIX=\"" + _get_install_prefix(ctx) + "\"\n" configure_script = create_cmake_script( workspace_name = ctx.workspace_name, - # as default, pass execution OS as target OS - target_os = os_name(ctx), cmake_path = configureParameters.attrs.cmake_path, tools = tools, flags = flags, @@ -130,12 +127,11 @@ def _attrs(): "provided cache-entries and env_vars (some values will still be passed as -Dkey=value " + "and environment variables). " + "If CMAKE_TOOLCHAIN_FILE cache entry is passed, specified crosstool file will be used " + - "When using this option, it makes sense to specify CMAKE_SYSTEM_NAME in the " + - "cache_entries - the rule makes only a poor guess about the target system, " + - "it is better to specify it manually." + "When using this option to cross-compile, it is required to specify CMAKE_SYSTEM_NAME in the " + + "cache_entries" ), mandatory = False, - default = False, + default = True, ), "install_prefix": attr.string( doc = "Relative install prefix to be passed to CMake in -DCMAKE_INSTALL_PREFIX", diff --git a/tools/build_defs/cmake_script.bzl b/tools/build_defs/cmake_script.bzl index ec4a1566..77189a61 100644 --- a/tools/build_defs/cmake_script.bzl +++ b/tools/build_defs/cmake_script.bzl @@ -4,7 +4,6 @@ load(":cc_toolchain_util.bzl", "absolutize_path_in_str") def create_cmake_script( workspace_name, - target_os, cmake_path, tools, flags, @@ -20,8 +19,6 @@ def create_cmake_script( Args: workspace_name: current workspace name - target_os: OSInfo with target operating system information, used for CMAKE_SYSTEM_NAME in - CMake toolchain file cmake_path: The path to the cmake executable tools: cc_toolchain tools (CxxToolsInfo) flags: cc_toolchain flags (CxxFlagsInfo) @@ -40,7 +37,7 @@ def create_cmake_script( merged_prefix_path = _merge_prefix_path(user_cache, include_dirs) - toolchain_dict = _fill_crossfile_from_toolchain(workspace_name, target_os, tools, flags) + toolchain_dict = _fill_crossfile_from_toolchain(workspace_name, tools, flags) params = None keys_with_empty_values_in_user_cache = [key for key in user_cache if user_cache.get(key) == ""] @@ -120,7 +117,6 @@ _CMAKE_CACHE_ENTRIES_CROSSTOOL = { "CMAKE_RANLIB": struct(value = "CMAKE_RANLIB", replace = True), "CMAKE_SHARED_LINKER_FLAGS": struct(value = "CMAKE_SHARED_LINKER_FLAGS_INIT", replace = False), "CMAKE_STATIC_LINKER_FLAGS": struct(value = "CMAKE_STATIC_LINKER_FLAGS_INIT", replace = False), - "CMAKE_SYSTEM_NAME": struct(value = "CMAKE_SYSTEM_NAME", replace = True), } def _create_crosstool_file_text(toolchain_dict, user_cache, user_env): @@ -152,8 +148,6 @@ def _dict_copy(d): return out def _create_cache_entries_env_vars(toolchain_dict, user_cache, user_env): - toolchain_dict.pop("CMAKE_SYSTEM_NAME") # specify this only in a toolchain file - _move_dict_values(toolchain_dict, user_env, _CMAKE_ENV_VARS_FOR_CROSSTOOL) _move_dict_values(toolchain_dict, user_cache, _CMAKE_CACHE_ENTRIES_CROSSTOOL) @@ -211,17 +205,8 @@ def _move_dict_values(target, source, descriptor_map): else: target[existing.value] = target[existing.value] + " " + value -def _fill_crossfile_from_toolchain(workspace_name, target_os, tools, flags): - os_name = target_os - if target_os == "windows": - os_name = "Windows" - if target_os == "osx": - os_name = "Apple" - if target_os == "linux": - os_name = "Linux" - dict = { - "CMAKE_SYSTEM_NAME": os_name, - } +def _fill_crossfile_from_toolchain(workspace_name, tools, flags): + dict = {} _sysroot = _find_in_cc_or_cxx(flags, "sysroot") if _sysroot: