Fix quoting for cmake (#703)

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__"],
 )

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"],
+    }),
+)

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 <<EOF set(CXXFLAGS "{text}")EOF
-    return text.replace('"', r'\"\\\\\\"')
-
 def _create_crosstool_file_text(toolchain_dict, user_cache, user_env):
     cache_entries = _dict_copy(user_cache)
     env_vars = _dict_copy(user_env)
@@ -169,21 +168,30 @@ def _create_crosstool_file_text(toolchain_dict, user_cache, user_env):
     _move_dict_values(toolchain_dict, cache_entries, _CMAKE_CACHE_ENTRIES_CROSSTOOL)
 
     lines = []
+    crosstool_vars = []
+
+    # The __var_* bash variables that are set here are a method to avoid
+    # having to quote the values when they are expanded in the HEREDOC.
+    # We could disable shell expansion by single quoting EOF in the HEREDOC
+    # but then we loose the ability to expand other variables such as
+    # $EXT_BUILD_DEPS and so we use this trick to leave expansion turned on in
+    # the HEREDOC for the crosstool
     for key in toolchain_dict:
+        crosstool_vars.append("__var_{}=\"{}\"".format(key, _escape_dquote_bash_crosstool(toolchain_dict[key])))
         if ("CMAKE_AR" == key):
-            lines.append('set({} "{}" {})'.format(
+            lines.append('set({} "$$__var_{}$$" {})'.format(
+                key,
                 key,
-                _escape_dquote_cmake(toolchain_dict[key]),
                 'CACHE FILEPATH "Archiver"',
             ))
-            continue
+        else:
-        lines.append('set({} "{}")'.format(key, _escape_dquote_cmake(toolchain_dict[key])))
+            lines.append('set({} "$$__var_{}$$")'.format(key, key))
 
     cache_entries.update({
         "CMAKE_TOOLCHAIN_FILE": "crosstool_bazel.cmake",
     })
     return struct(
-        commands = ["cat > 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,
     )

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
 

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(" ")

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
+    expected = r"""__var_CMAKE_AR="/usr/bin/ar"
-set(CMAKE_AR "/usr/bin/ar" CACHE FILEPATH "Archiver")
+__var_CMAKE_ASM_FLAGS_INIT="-U_FORTIFY_SOURCE -fstack-protector -Wall"
-set(CMAKE_ASM_FLAGS_INIT "-U_FORTIFY_SOURCE -fstack-protector -Wall")
+__var_CMAKE_CXX_COMPILER="/usr/bin/gcc"
-set(CMAKE_CXX_COMPILER "/usr/bin/gcc")
+__var_CMAKE_CXX_FLAGS_INIT="-U_FORTIFY_SOURCE -fstack-protector -Wall"
-set(CMAKE_CXX_FLAGS_INIT "-U_FORTIFY_SOURCE -fstack-protector -Wall")
+__var_CMAKE_C_COMPILER="/usr/bin/gcc"
-set(CMAKE_C_COMPILER "/usr/bin/gcc")
+__var_CMAKE_C_FLAGS_INIT="-U_FORTIFY_SOURCE -fstack-protector -Wall"
-set(CMAKE_C_FLAGS_INIT "-U_FORTIFY_SOURCE -fstack-protector -Wall")
+__var_CMAKE_EXE_LINKER_FLAGS_INIT="-fuse-ld=gold -Wl -no-as-needed"
-set(CMAKE_EXE_LINKER_FLAGS_INIT "-fuse-ld=gold -Wl -no-as-needed")
+__var_CMAKE_SHARED_LINKER_FLAGS_INIT="-shared -fuse-ld=gold"
-set(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
+    expected = r"""__var_CMAKE_AR="/cxx_linker_static"
-set(CMAKE_AR "/cxx_linker_static" CACHE FILEPATH "Archiver")
+__var_CMAKE_ASM_FLAGS_INIT="assemble assemble-user"
-set(CMAKE_ASM_FLAGS_INIT "assemble assemble-user")
+__var_CMAKE_CXX_COMPILER="sink-cxx-value"
-set(CMAKE_CXX_COMPILER "sink-cxx-value")
+__var_CMAKE_CXX_COMPILER_EXTERNAL_TOOLCHAIN="cxx-toolchain"
-set(CMAKE_CXX_COMPILER_EXTERNAL_TOOLCHAIN "cxx-toolchain")
+__var_CMAKE_CXX_FLAGS_INIT="--quoted=\\\\\\\"abc def\\\\\\\" --sysroot=/abc/sysroot --gcc_toolchain cxx-toolchain"
-set(CMAKE_CXX_FLAGS_INIT "--quoted=\"\\\\\\"abc def\"\\\\\\" --sysroot=/abc/sysroot --gcc_toolchain cxx-toolchain")
+__var_CMAKE_CXX_LINK_EXECUTABLE="became"
-set(CMAKE_CXX_LINK_EXECUTABLE "became")
+__var_CMAKE_C_COMPILER="sink-cc-value"
-set(CMAKE_C_COMPILER "sink-cc-value")
+__var_CMAKE_C_COMPILER_EXTERNAL_TOOLCHAIN="cc-toolchain"
-set(CMAKE_C_COMPILER_EXTERNAL_TOOLCHAIN "cc-toolchain")
+__var_CMAKE_C_FLAGS_INIT="-cc-flag -gcc_toolchain cc-toolchain --from-env --additional-flag"
-set(CMAKE_C_FLAGS_INIT "-cc-flag -gcc_toolchain cc-toolchain --from-env --additional-flag")
+__var_CMAKE_EXE_LINKER_FLAGS_INIT="executable"
-set(CMAKE_EXE_LINKER_FLAGS_INIT "executable")
+__var_CMAKE_SHARED_LINKER_FLAGS_INIT="shared1 shared2"
-set(CMAKE_SHARED_LINKER_FLAGS_INIT "shared1 shared2")
+__var_CMAKE_SYSROOT="/abc/sysroot"
-set(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"