Fix a way of wiping CMake cache entry values defined by toolchain fro… (#293)

* Fix a way of wiping CMake cache entry values defined by toolchain from a command line, in case user specified empty values for these cache entries explicitly in cmake_external target

- it was a problem that we did not check that there was some value in the "toolchain" dict before we pop() - reported in comment in #292
- make code which does wiping empty values more explicit

* Additionally, if non-empty value for CMAKE_RANLIB was not specified by user or Bazel toolchain, put empty value for it to forbid CMake to auto detect it

context: https://github.com/bazelbuild/rules_foreign_cc/issues/252

* Add test for user's redefined value for CMAKE_BUILD_TYPE
This commit is contained in:
irengrig 2019-07-17 13:51:21 +02:00 committed by GitHub
parent 64e2cda959
commit 30f398dc40
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 79 additions and 23 deletions

View File

@ -131,8 +131,10 @@ def _move_dict_values_test(ctx):
"CMAKE_CXX_LINK_EXECUTABLE": "became",
"CUSTOM": "YES",
}
export_for_test.move_dict_values(target, source_env, export_for_test.CMAKE_ENV_VARS_FOR_CROSSTOOL)
export_for_test.move_dict_values(target, source_cache, export_for_test.CMAKE_CACHE_ENTRIES_CROSSTOOL)
export_for_test.move_dict_values(target, source_env,
export_for_test.CMAKE_ENV_VARS_FOR_CROSSTOOL)
export_for_test.move_dict_values(target, source_cache,
export_for_test.CMAKE_CACHE_ENTRIES_CROSSTOOL)
expected_target = {
"CMAKE_C_COMPILER": "sink-cc-value",
@ -162,8 +164,10 @@ def _reverse_descriptor_dict_test(ctx):
"CMAKE_C_FLAGS_INIT": struct(value = "CMAKE_C_FLAGS", replace = False),
"CMAKE_CXX_FLAGS_INIT": struct(value = "CMAKE_CXX_FLAGS", replace = False),
"CMAKE_ASM_FLAGS_INIT": struct(value = "CMAKE_ASM_FLAGS", replace = False),
"CMAKE_STATIC_LINKER_FLAGS_INIT": struct(value = "CMAKE_STATIC_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_SHARED_LINKER_FLAGS_INIT": struct(value = "CMAKE_SHARED_LINKER_FLAGS",
replace = False),
"CMAKE_EXE_LINKER_FLAGS_INIT": struct(value = "CMAKE_EXE_LINKER_FLAGS", replace = False),
}
@ -189,7 +193,8 @@ def _merge_toolchain_and_user_values_test(ctx):
"CUSTOM": "YES",
}
res = export_for_test.merge_toolchain_and_user_values(target, source_cache, export_for_test.CMAKE_CACHE_ENTRIES_CROSSTOOL)
res = export_for_test.merge_toolchain_and_user_values(target, source_cache,
export_for_test.CMAKE_CACHE_ENTRIES_CROSSTOOL)
expected_target = {
"CMAKE_C_FLAGS": "-cc-flag -gcc_toolchain cc-toolchain --additional-flag",
@ -224,12 +229,12 @@ def _merge_flag_values_no_toolchain_file_test(ctx):
user_env = {}
user_cache = {
"CMAKE_CXX_FLAGS": "-Fbat",
# test that the computed value is not used
"CMAKE_BUILD_TYPE": "",
"CMAKE_BUILD_TYPE": "RelWithDebInfo",
}
script = create_cmake_script("ws", "linux", "cmake", tools, flags, "test_rule", "external/test_rule", True, user_cache, user_env, [])
expected = """CC=\"/usr/bin/gcc\" CXX=\"/usr/bin/gcc\" CXXFLAGS=\"foo=\\\"bar\\\" -Fbat" cmake -DCMAKE_AR=\"/usr/bin/ar\" -DCMAKE_PREFIX_PATH=\"$EXT_BUILD_DEPS\" -DCMAKE_INSTALL_PREFIX=\"test_rule\" $EXT_BUILD_ROOT/external/test_rule"""
script = create_cmake_script("ws", "linux", "cmake", tools, flags, "test_rule",
"external/test_rule", True, user_cache, user_env, [])
expected = """CC=\"/usr/bin/gcc\" CXX=\"/usr/bin/gcc\" CXXFLAGS=\"foo=\\\"bar\\\" -Fbat" cmake -DCMAKE_AR=\"/usr/bin/ar\" -DCMAKE_BUILD_TYPE="RelWithDebInfo" -DCMAKE_PREFIX_PATH=\"$EXT_BUILD_DEPS\" -DCMAKE_INSTALL_PREFIX=\"test_rule\" -DCMAKE_RANLIB=\"\" $EXT_BUILD_ROOT/external/test_rule"""
asserts.equals(env, expected, script)
return unittest.end(env)
@ -257,8 +262,42 @@ def _create_min_cmake_script_no_toolchain_file_test(ctx):
"CMAKE_PREFIX_PATH": "/abc/def",
}
script = create_cmake_script("ws", "linux", "cmake", tools, flags, "test_rule", "external/test_rule", True, user_cache, user_env, ["-GNinja"])
expected = "CC=\"/usr/bin/gcc\" CXX=\"/usr/bin/gcc\" CFLAGS=\"-U_FORTIFY_SOURCE -fstack-protector -Wall\" CXXFLAGS=\"-U_FORTIFY_SOURCE -fstack-protector -Wall\" ASMFLAGS=\"-U_FORTIFY_SOURCE -fstack-protector -Wall\" cmake -DCMAKE_AR=\"/usr/bin/ar\" -DCMAKE_SHARED_LINKER_FLAGS=\"-shared -fuse-ld=gold\" -DCMAKE_EXE_LINKER_FLAGS=\"-fuse-ld=gold -Wl -no-as-needed\" -DNOFORTRAN=\"on\" -DCMAKE_PREFIX_PATH=\"$EXT_BUILD_DEPS;/abc/def\" -DCMAKE_INSTALL_PREFIX=\"test_rule\" -DCMAKE_BUILD_TYPE=\"Debug\" -GNinja $EXT_BUILD_ROOT/external/test_rule"
script = create_cmake_script("ws", "linux", "cmake", tools, flags, "test_rule",
"external/test_rule", True, user_cache, user_env, ["-GNinja"])
expected = "CC=\"/usr/bin/gcc\" CXX=\"/usr/bin/gcc\" CFLAGS=\"-U_FORTIFY_SOURCE -fstack-protector -Wall\" CXXFLAGS=\"-U_FORTIFY_SOURCE -fstack-protector -Wall\" ASMFLAGS=\"-U_FORTIFY_SOURCE -fstack-protector -Wall\" cmake -DCMAKE_AR=\"/usr/bin/ar\" -DCMAKE_SHARED_LINKER_FLAGS=\"-shared -fuse-ld=gold\" -DCMAKE_EXE_LINKER_FLAGS=\"-fuse-ld=gold -Wl -no-as-needed\" -DNOFORTRAN=\"on\" -DCMAKE_PREFIX_PATH=\"$EXT_BUILD_DEPS;/abc/def\" -DCMAKE_INSTALL_PREFIX=\"test_rule\" -DCMAKE_BUILD_TYPE=\"Debug\" -DCMAKE_RANLIB=\"\" -GNinja $EXT_BUILD_ROOT/external/test_rule"
asserts.equals(env, expected, script)
return unittest.end(env)
def _create_min_cmake_script_wipe_toolchain_test(ctx):
env = unittest.begin(ctx)
tools = CxxToolsInfo(
cc = "/usr/bin/gcc",
cxx = "/usr/bin/gcc",
cxx_linker_static = "/usr/bin/ar",
cxx_linker_executable = "/usr/bin/gcc",
)
flags = CxxFlagsInfo(
cc = ["-U_FORTIFY_SOURCE", "-fstack-protector", "-Wall"],
cxx = ["-U_FORTIFY_SOURCE", "-fstack-protector", "-Wall"],
cxx_linker_shared = ["-shared", "-fuse-ld=gold"],
cxx_linker_static = ["static"],
cxx_linker_executable = ["-fuse-ld=gold", "-Wl", "-no-as-needed"],
assemble = ["-U_FORTIFY_SOURCE", "-fstack-protector", "-Wall"],
)
user_env = {}
user_cache = {
"CMAKE_PREFIX_PATH": "/abc/def",
# These two flags/CMake cache entries must be wiped,
# but the second is not present in toolchain flags.
"CMAKE_SHARED_LINKER_FLAGS": "",
"WIPE_ME_IF_PRESENT": "",
}
script = create_cmake_script("ws", "linux", "cmake", tools, flags, "test_rule",
"external/test_rule", True, user_cache, user_env, ["-GNinja"])
expected = "CC=\"/usr/bin/gcc\" CXX=\"/usr/bin/gcc\" CFLAGS=\"-U_FORTIFY_SOURCE -fstack-protector -Wall\" CXXFLAGS=\"-U_FORTIFY_SOURCE -fstack-protector -Wall\" ASMFLAGS=\"-U_FORTIFY_SOURCE -fstack-protector -Wall\" cmake -DCMAKE_AR=\"/usr/bin/ar\" -DCMAKE_EXE_LINKER_FLAGS=\"-fuse-ld=gold -Wl -no-as-needed\" -DCMAKE_PREFIX_PATH=\"$EXT_BUILD_DEPS;/abc/def\" -DCMAKE_INSTALL_PREFIX=\"test_rule\" -DCMAKE_BUILD_TYPE=\"Debug\" -DCMAKE_RANLIB=\"\" -GNinja $EXT_BUILD_ROOT/external/test_rule"
asserts.equals(env, expected, script)
return unittest.end(env)
@ -285,7 +324,8 @@ def _create_min_cmake_script_toolchain_file_test(ctx):
"NOFORTRAN": "on",
}
script = create_cmake_script("ws", "linux", "cmake", tools, flags, "test_rule", "external/test_rule", False, user_cache, user_env, ["-GNinja"])
script = create_cmake_script("ws", "linux", "cmake", tools, flags, "test_rule",
"external/test_rule", False, user_cache, user_env, ["-GNinja"])
expected = """cat > crosstool_bazel.cmake <<EOF
set(CMAKE_SYSTEM_NAME "Linux")
set(CMAKE_C_COMPILER "/usr/bin/gcc")
@ -298,7 +338,7 @@ set(CMAKE_SHARED_LINKER_FLAGS_INIT "-shared -fuse-ld=gold")
set(CMAKE_EXE_LINKER_FLAGS_INIT "-fuse-ld=gold -Wl -no-as-needed")
EOF
cmake -DNOFORTRAN="on" -DCMAKE_TOOLCHAIN_FILE="crosstool_bazel.cmake" -DCMAKE_PREFIX_PATH="$EXT_BUILD_DEPS" -DCMAKE_INSTALL_PREFIX="test_rule" -DCMAKE_BUILD_TYPE=\"Debug\" -GNinja $EXT_BUILD_ROOT/external/test_rule"""
cmake -DNOFORTRAN="on" -DCMAKE_TOOLCHAIN_FILE="crosstool_bazel.cmake" -DCMAKE_PREFIX_PATH="$EXT_BUILD_DEPS" -DCMAKE_INSTALL_PREFIX="test_rule" -DCMAKE_BUILD_TYPE=\"Debug\" -DCMAKE_RANLIB=\"\" -GNinja $EXT_BUILD_ROOT/external/test_rule"""
asserts.equals(env, expected.splitlines(), script.splitlines())
return unittest.end(env)
@ -314,7 +354,8 @@ def _create_cmake_script_no_toolchain_file_test(ctx):
)
flags = CxxFlagsInfo(
cc = ["-cc-flag", "-gcc_toolchain", "cc-toolchain"],
cxx = ["--quoted=\"abc def\"", "--sysroot=/abc/sysroot", "--gcc_toolchain", "cxx-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"],
@ -334,8 +375,9 @@ def _create_cmake_script_no_toolchain_file_test(ctx):
"CMAKE_BUILD_TYPE": "user_type",
}
script = create_cmake_script("ws", "linux", "cmake", tools, flags, "test_rule", "external/test_rule", True, user_cache, user_env, ["-GNinja"])
expected = "CC=\"sink-cc-value\" CXX=\"sink-cxx-value\" CFLAGS=\"-cc-flag -gcc_toolchain cc-toolchain --from-env --additional-flag\" CXXFLAGS=\"--quoted=\\\"abc def\\\" --sysroot=/abc/sysroot --gcc_toolchain cxx-toolchain\" ASMFLAGS=\"assemble assemble-user\" CUSTOM_ENV=\"YES\" cmake -DCMAKE_AR=\"/cxx_linker_static\" -DCMAKE_CXX_LINK_EXECUTABLE=\"became\" -DCMAKE_SHARED_LINKER_FLAGS=\"shared1 shared2\" -DCMAKE_EXE_LINKER_FLAGS=\"executable\" -DCUSTOM_CACHE=\"YES\" -DCMAKE_BUILD_TYPE=\"user_type\" -DCMAKE_PREFIX_PATH=\"$EXT_BUILD_DEPS\" -DCMAKE_INSTALL_PREFIX=\"test_rule\" -GNinja $EXT_BUILD_ROOT/external/test_rule"
script = create_cmake_script("ws", "linux", "cmake", tools, flags, "test_rule",
"external/test_rule", True, user_cache, user_env, ["-GNinja"])
expected = "CC=\"sink-cc-value\" CXX=\"sink-cxx-value\" CFLAGS=\"-cc-flag -gcc_toolchain cc-toolchain --from-env --additional-flag\" CXXFLAGS=\"--quoted=\\\"abc def\\\" --sysroot=/abc/sysroot --gcc_toolchain cxx-toolchain\" ASMFLAGS=\"assemble assemble-user\" CUSTOM_ENV=\"YES\" cmake -DCMAKE_AR=\"/cxx_linker_static\" -DCMAKE_CXX_LINK_EXECUTABLE=\"became\" -DCMAKE_SHARED_LINKER_FLAGS=\"shared1 shared2\" -DCMAKE_EXE_LINKER_FLAGS=\"executable\" -DCUSTOM_CACHE=\"YES\" -DCMAKE_BUILD_TYPE=\"user_type\" -DCMAKE_PREFIX_PATH=\"$EXT_BUILD_DEPS\" -DCMAKE_INSTALL_PREFIX=\"test_rule\" -DCMAKE_RANLIB=\"\" -GNinja $EXT_BUILD_ROOT/external/test_rule"
asserts.equals(env, expected, script)
return unittest.end(env)
@ -351,7 +393,8 @@ def _create_cmake_script_toolchain_file_test(ctx):
)
flags = CxxFlagsInfo(
cc = ["-cc-flag", "-gcc_toolchain", "cc-toolchain"],
cxx = ["--quoted=\"abc def\"", "--sysroot=/abc/sysroot", "--gcc_toolchain", "cxx-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"],
@ -370,7 +413,8 @@ def _create_cmake_script_toolchain_file_test(ctx):
"CUSTOM_CACHE": "YES",
}
script = create_cmake_script("ws", "osx", "cmake", tools, flags, "test_rule", "external/test_rule", False, user_cache, user_env, ["-GNinja"])
script = create_cmake_script("ws", "osx", "cmake", tools, flags, "test_rule",
"external/test_rule", False, user_cache, user_env, ["-GNinja"])
expected = """cat > crosstool_bazel.cmake <<EOF
set(CMAKE_SYSTEM_NAME "Apple")
set(CMAKE_SYSROOT "/abc/sysroot")
@ -387,7 +431,7 @@ set(CMAKE_SHARED_LINKER_FLAGS_INIT "shared1 shared2")
set(CMAKE_EXE_LINKER_FLAGS_INIT "executable")
EOF
CUSTOM_ENV="YES" cmake -DCUSTOM_CACHE="YES" -DCMAKE_TOOLCHAIN_FILE="crosstool_bazel.cmake" -DCMAKE_PREFIX_PATH="$EXT_BUILD_DEPS" -DCMAKE_INSTALL_PREFIX="test_rule" -DCMAKE_BUILD_TYPE=\"Debug\" -GNinja $EXT_BUILD_ROOT/external/test_rule"""
CUSTOM_ENV="YES" cmake -DCUSTOM_CACHE="YES" -DCMAKE_TOOLCHAIN_FILE="crosstool_bazel.cmake" -DCMAKE_PREFIX_PATH="$EXT_BUILD_DEPS" -DCMAKE_INSTALL_PREFIX="test_rule" -DCMAKE_BUILD_TYPE=\"Debug\" -DCMAKE_RANLIB=\"\" -GNinja $EXT_BUILD_ROOT/external/test_rule"""
asserts.equals(env, expected.splitlines(), script.splitlines())
return unittest.end(env)
@ -404,6 +448,7 @@ create_min_cmake_script_toolchain_file_test = unittest.make(_create_min_cmake_sc
create_cmake_script_no_toolchain_file_test = unittest.make(_create_cmake_script_no_toolchain_file_test)
create_cmake_script_toolchain_file_test = unittest.make(_create_cmake_script_toolchain_file_test)
merge_flag_values_no_toolchain_file_test = unittest.make(_merge_flag_values_no_toolchain_file_test)
create_min_cmake_script_wipe_toolchain_test = unittest.make(_create_min_cmake_script_wipe_toolchain_test)
def cmake_script_test_suite():
unittest.suite(
@ -420,4 +465,5 @@ def cmake_script_test_suite():
create_cmake_script_no_toolchain_file_test,
create_cmake_script_toolchain_file_test,
merge_flag_values_no_toolchain_file_test,
create_min_cmake_script_wipe_toolchain_test,
)

View File

@ -34,6 +34,9 @@ def create_cmake_script(
toolchain_dict = _fill_crossfile_from_toolchain(workspace_name, target_os, tools, flags)
params = None
keys_with_empty_values_in_user_cache = [key for key in user_cache if user_cache.get(key) == ""]
if no_toolchain_file:
params = _create_cache_entries_env_vars(toolchain_dict, user_cache, user_env)
else:
@ -53,7 +56,14 @@ def create_cmake_script(
# or to suppress calculated CMAKE_BUILD_TYPE
# If the user passes "CMAKE_BUILD_TYPE": "" (empty string),
# CMAKE_BUILD_TYPE will not be passed to CMake
wipe_empty_values(params.cache, user_cache)
wipe_empty_values(params.cache, keys_with_empty_values_in_user_cache)
# However, if no CMAKE_RANLIB was passed, pass the empty value for it explicitly,
# as it is legacy and autodetection of ranlib made by CMake automatically
# breaks some cross compilation builds,
# see https://github.com/envoyproxy/envoy/pull/6991
if not params.cache.get("CMAKE_RANLIB"):
params.cache.update({"CMAKE_RANLIB": ""})
set_env_vars = " ".join([key + "=\"" + params.env[key] + "\"" for key in params.env])
str_cmake_cache_entries = " ".join(["-D" + key + "=\"" + params.cache[key] + "\"" for key in params.cache])
@ -67,9 +77,9 @@ def create_cmake_script(
return "\n".join(params.commands + [cmake_call])
def wipe_empty_values(cache, user_cache):
for key in user_cache:
if len(user_cache.get(key)) == 0:
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:
cache.pop(key)
# From CMake documentation: ;-list of directories specifying installation prefixes to be searched...