From bad4ab0c3dfb196d55f523c8c27f003f47e3c74a Mon Sep 17 00:00:00 2001 From: James Sharpe Date: Wed, 28 Jul 2021 18:12:53 +0100 Subject: [PATCH] Fix quoting for cmake (#703) --- examples/cmake_defines/BUILD.bazel | 2 + .../cmake_defines/nocrosstool/BUILD.bazel | 23 +++++++ foreign_cc/private/cmake_script.bzl | 52 ++++++++------- foreign_cc/private/framework.bzl | 14 +--- foreign_cc/private/framework/helpers.bzl | 6 ++ test/cmake_text_tests.bzl | 64 ++++++++++++------- 6 files changed, 105 insertions(+), 56 deletions(-) create mode 100644 examples/cmake_defines/nocrosstool/BUILD.bazel diff --git a/examples/cmake_defines/BUILD.bazel b/examples/cmake_defines/BUILD.bazel index 13e46225..99789812 100644 --- a/examples/cmake_defines/BUILD.bazel +++ b/examples/cmake_defines/BUILD.bazel @@ -26,6 +26,7 @@ filegroup( "CMakeLists.txt", "lib_a.cpp", ]], + visibility = ["//cmake_defines:__subpackages__"], ) filegroup( @@ -34,4 +35,5 @@ filegroup( "CMakeLists.txt", "lib_b.cpp", ]], + visibility = ["//cmake_defines:__subpackages__"], ) diff --git a/examples/cmake_defines/nocrosstool/BUILD.bazel b/examples/cmake_defines/nocrosstool/BUILD.bazel new file mode 100644 index 00000000..43edfac5 --- /dev/null +++ b/examples/cmake_defines/nocrosstool/BUILD.bazel @@ -0,0 +1,23 @@ +load("@rules_foreign_cc//foreign_cc:defs.bzl", "cmake") + +cmake( + name = "lib_a", + generate_crosstool_file = False, + lib_source = "//cmake_defines:lib_a_sources", + out_static_libs = select({ + "//:windows": ["lib_a.lib"], + "//conditions:default": ["liblib_a.a"], + }), + deps = [":lib_b"], +) + +cmake( + name = "lib_b", + defines = ["FOO"], + generate_crosstool_file = False, + lib_source = "//cmake_defines:lib_b_sources", + out_static_libs = select({ + "//:windows": ["lib_b.lib"], + "//conditions:default": ["liblib_b.a"], + }), +) diff --git a/foreign_cc/private/cmake_script.bzl b/foreign_cc/private/cmake_script.bzl index 0742c0b0..6f212455 100644 --- a/foreign_cc/private/cmake_script.bzl +++ b/foreign_cc/private/cmake_script.bzl @@ -2,6 +2,21 @@ load(":cc_toolchain_util.bzl", "absolutize_path_in_str") +def _escape_dquote_bash(text): + """ Escape double quotes in flag lists for use in bash strings that set environment variables """ + + # We use a starlark raw string to prevent the need to escape backslashes for starlark as well. + return text.replace('"', r'\\\"') + +def _escape_dquote_bash_crosstool(text): + """ Escape double quotes in flag lists for use in bash strings to be passed to a HEREDOC. + + This requires an additional level of indirection as CMake requires the variables to be escaped inside the crosstool file + """ + + # We use a starlark raw string to prevent the need to escape backslashes for starlark as well. + return text.replace('"', r'\\\\\\\"') + def create_cmake_script( workspace_name, generator, @@ -81,7 +96,7 @@ def create_cmake_script( for key in params.env ] str_cmake_cache_entries = " ".join([ - "-D{}=\"{}\"".format(key, _escape_dquote_bash(params.cache[key])) + "-D{}=\"{}\"".format(key, _escape_dquote_bash_crosstool(params.cache[key])) for key in params.cache ]) @@ -109,15 +124,6 @@ def create_cmake_script( return params.commands + script -def _escape_dquote_bash(text): - """ Escape double quotes in flag lists for use in bash strings. """ - - # Objective is to escape the quotes twice for bash. - # 1. \\\" -> \" - when set_env_vars' strings get evaluated. - # 2. \" -> " - when each flag containing a string is passed to the compiler. - # We use a starlark raw string to prevent the need to escape backslashes for starlark as well. - return text.replace('"', r'\\\"') - def _wipe_empty_values(cache, keys_with_empty_values_in_user_cache): for key in keys_with_empty_values_in_user_cache: if cache.get(key) != None: @@ -155,13 +161,6 @@ _CMAKE_CACHE_ENTRIES_CROSSTOOL = { "CMAKE_STATIC_LINKER_FLAGS": struct(value = "CMAKE_STATIC_LINKER_FLAGS_INIT", replace = False), } -def _escape_dquote_cmake(text): - """ Escape double quotes for use in bash heredoc and CMake crosstool. """ - - # Context: - # cat >> file.cmake < crosstool_bazel.cmake << EOF"] + sorted(lines) + ["EOF", ""], + commands = sorted(crosstool_vars) + ["cat > crosstool_bazel.cmake << EOF"] + sorted(lines) + ["EOF", ""], env = env_vars, cache = cache_entries, ) diff --git a/foreign_cc/private/framework.bzl b/foreign_cc/private/framework.bzl index fe628ebd..c7dca485 100644 --- a/foreign_cc/private/framework.bzl +++ b/foreign_cc/private/framework.bzl @@ -11,6 +11,7 @@ load( "//foreign_cc/private/framework:helpers.bzl", "convert_shell_script", "create_function", + "escape_dquote_bash", "script_extension", "shebang", ) @@ -259,17 +260,6 @@ dependencies.""", ), ) -def _escape_dquote(text): - """Escape double quotes for use in bash variable definitions - - Args: - text (str): The text to escape - - Returns: - str: text with escaped `"` characters. - """ - return text.replace('"', r'\"\\\\\\"') - def get_env_prelude(ctx, lib_name, data_dependencies, target_root): """Generate a bash snippet containing environment variable definitions @@ -308,7 +298,7 @@ def get_env_prelude(ctx, lib_name, data_dependencies, target_root): if "PATH" in user_var and cc_env.get(user_var): env.update({user_var: user_vars.get(user_var) + ":" + cc_env.get(user_var)}) - env_snippet.extend(["export {}=\"{}\"".format(key, _escape_dquote(val)) for key, val in env.items()]) + env_snippet.extend(["export {}=\"{}\"".format(key, escape_dquote_bash(val)) for key, val in env.items()]) return env_snippet diff --git a/foreign_cc/private/framework/helpers.bzl b/foreign_cc/private/framework/helpers.bzl index b26d6b1a..447dcdc6 100644 --- a/foreign_cc/private/framework/helpers.bzl +++ b/foreign_cc/private/framework/helpers.bzl @@ -135,6 +135,12 @@ def replace_var_ref(text, shell_context): return "".join(parts) +def escape_dquote_bash(text): + """ Escape double quotes in flag lists for use in bash strings. """ + + # We use a starlark raw string to prevent the need to escape backslashes for starlark as well. + return text.replace('"', r'\\\\\"') + # buildifier: disable=function-docstring def replace_exports(text, shell_context): text = text.strip(" ") diff --git a/test/cmake_text_tests.bzl b/test/cmake_text_tests.bzl index 35ec175b..e7cb8d4f 100644 --- a/test/cmake_text_tests.bzl +++ b/test/cmake_text_tests.bzl @@ -405,15 +405,23 @@ def _create_min_cmake_script_toolchain_file_test(ctx): ["--debug-output", "-Wdev"], cmake_commands = [], ) - expected = r"""cat > crosstool_bazel.cmake << EOF -set(CMAKE_AR "/usr/bin/ar" CACHE FILEPATH "Archiver") -set(CMAKE_ASM_FLAGS_INIT "-U_FORTIFY_SOURCE -fstack-protector -Wall") -set(CMAKE_CXX_COMPILER "/usr/bin/gcc") -set(CMAKE_CXX_FLAGS_INIT "-U_FORTIFY_SOURCE -fstack-protector -Wall") -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") + expected = r"""__var_CMAKE_AR="/usr/bin/ar" +__var_CMAKE_ASM_FLAGS_INIT="-U_FORTIFY_SOURCE -fstack-protector -Wall" +__var_CMAKE_CXX_COMPILER="/usr/bin/gcc" +__var_CMAKE_CXX_FLAGS_INIT="-U_FORTIFY_SOURCE -fstack-protector -Wall" +__var_CMAKE_C_COMPILER="/usr/bin/gcc" +__var_CMAKE_C_FLAGS_INIT="-U_FORTIFY_SOURCE -fstack-protector -Wall" +__var_CMAKE_EXE_LINKER_FLAGS_INIT="-fuse-ld=gold -Wl -no-as-needed" +__var_CMAKE_SHARED_LINKER_FLAGS_INIT="-shared -fuse-ld=gold" +cat > crosstool_bazel.cmake << EOF +set(CMAKE_AR "$$__var_CMAKE_AR$$" CACHE FILEPATH "Archiver") +set(CMAKE_ASM_FLAGS_INIT "$$__var_CMAKE_ASM_FLAGS_INIT$$") +set(CMAKE_CXX_COMPILER "$$__var_CMAKE_CXX_COMPILER$$") +set(CMAKE_CXX_FLAGS_INIT "$$__var_CMAKE_CXX_FLAGS_INIT$$") +set(CMAKE_C_COMPILER "$$__var_CMAKE_C_COMPILER$$") +set(CMAKE_C_FLAGS_INIT "$$__var_CMAKE_C_FLAGS_INIT$$") +set(CMAKE_EXE_LINKER_FLAGS_INIT "$$__var_CMAKE_EXE_LINKER_FLAGS_INIT$$") +set(CMAKE_SHARED_LINKER_FLAGS_INIT "$$__var_CMAKE_SHARED_LINKER_FLAGS_INIT$$") EOF ##enable_tracing## @@ -537,19 +545,31 @@ def _create_cmake_script_toolchain_file_test(ctx): ["--debug-output", "-Wdev"], cmake_commands = [], ) - expected = r"""cat > crosstool_bazel.cmake << EOF -set(CMAKE_AR "/cxx_linker_static" CACHE FILEPATH "Archiver") -set(CMAKE_ASM_FLAGS_INIT "assemble assemble-user") -set(CMAKE_CXX_COMPILER "sink-cxx-value") -set(CMAKE_CXX_COMPILER_EXTERNAL_TOOLCHAIN "cxx-toolchain") -set(CMAKE_CXX_FLAGS_INIT "--quoted=\"\\\\\\"abc def\"\\\\\\" --sysroot=/abc/sysroot --gcc_toolchain cxx-toolchain") -set(CMAKE_CXX_LINK_EXECUTABLE "became") -set(CMAKE_C_COMPILER "sink-cc-value") -set(CMAKE_C_COMPILER_EXTERNAL_TOOLCHAIN "cc-toolchain") -set(CMAKE_C_FLAGS_INIT "-cc-flag -gcc_toolchain cc-toolchain --from-env --additional-flag") -set(CMAKE_EXE_LINKER_FLAGS_INIT "executable") -set(CMAKE_SHARED_LINKER_FLAGS_INIT "shared1 shared2") -set(CMAKE_SYSROOT "/abc/sysroot") + expected = r"""__var_CMAKE_AR="/cxx_linker_static" +__var_CMAKE_ASM_FLAGS_INIT="assemble assemble-user" +__var_CMAKE_CXX_COMPILER="sink-cxx-value" +__var_CMAKE_CXX_COMPILER_EXTERNAL_TOOLCHAIN="cxx-toolchain" +__var_CMAKE_CXX_FLAGS_INIT="--quoted=\\\\\\\"abc def\\\\\\\" --sysroot=/abc/sysroot --gcc_toolchain cxx-toolchain" +__var_CMAKE_CXX_LINK_EXECUTABLE="became" +__var_CMAKE_C_COMPILER="sink-cc-value" +__var_CMAKE_C_COMPILER_EXTERNAL_TOOLCHAIN="cc-toolchain" +__var_CMAKE_C_FLAGS_INIT="-cc-flag -gcc_toolchain cc-toolchain --from-env --additional-flag" +__var_CMAKE_EXE_LINKER_FLAGS_INIT="executable" +__var_CMAKE_SHARED_LINKER_FLAGS_INIT="shared1 shared2" +__var_CMAKE_SYSROOT="/abc/sysroot" +cat > crosstool_bazel.cmake << EOF +set(CMAKE_AR "$$__var_CMAKE_AR$$" CACHE FILEPATH "Archiver") +set(CMAKE_ASM_FLAGS_INIT "$$__var_CMAKE_ASM_FLAGS_INIT$$") +set(CMAKE_CXX_COMPILER "$$__var_CMAKE_CXX_COMPILER$$") +set(CMAKE_CXX_COMPILER_EXTERNAL_TOOLCHAIN "$$__var_CMAKE_CXX_COMPILER_EXTERNAL_TOOLCHAIN$$") +set(CMAKE_CXX_FLAGS_INIT "$$__var_CMAKE_CXX_FLAGS_INIT$$") +set(CMAKE_CXX_LINK_EXECUTABLE "$$__var_CMAKE_CXX_LINK_EXECUTABLE$$") +set(CMAKE_C_COMPILER "$$__var_CMAKE_C_COMPILER$$") +set(CMAKE_C_COMPILER_EXTERNAL_TOOLCHAIN "$$__var_CMAKE_C_COMPILER_EXTERNAL_TOOLCHAIN$$") +set(CMAKE_C_FLAGS_INIT "$$__var_CMAKE_C_FLAGS_INIT$$") +set(CMAKE_EXE_LINKER_FLAGS_INIT "$$__var_CMAKE_EXE_LINKER_FLAGS_INIT$$") +set(CMAKE_SHARED_LINKER_FLAGS_INIT "$$__var_CMAKE_SHARED_LINKER_FLAGS_INIT$$") +set(CMAKE_SYSROOT "$$__var_CMAKE_SYSROOT$$") EOF export CUSTOM_ENV="YES"