Skip to content

Commit

Permalink
Include cc configure headers in the cache key
Browse files Browse the repository at this point in the history
This PR adds a file containing builtin include directories paths as an input to every C++ compilation action. The result is that whenever these headers change (as a result of reconfiguring `cc_configure` repository rule), we include this file in the action cache key.

Fixes #9296.

Closes #9295.

PiperOrigin-RevId: 266783158
  • Loading branch information
hlopko authored and copybara-github committed Sep 2, 2019
1 parent 11a98a5 commit 8b0bfaf
Show file tree
Hide file tree
Showing 9 changed files with 84 additions and 31 deletions.
4 changes: 2 additions & 2 deletions src/main/res/winsdk_configure.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ Usage:
register_local_rc_exe_toolchains()
"""

load("//tools/cpp:windows_cc_configure.bzl", "find_vc_path", "setup_vc_env_vars")
load("//tools/cpp:cc_configure.bzl", "MSVC_ENVVARS")
load("@bazel_tools//tools/cpp:windows_cc_configure.bzl", "find_vc_path", "setup_vc_env_vars")
load("@bazel_tools//tools/cpp:cc_configure.bzl", "MSVC_ENVVARS")

# Keys: target architecture, as in <Windows-SDK-path>/<target-architecture>/bin/rc.exe
# Values: corresponding Bazel CPU value under @platforms//cpu:*
Expand Down
2 changes: 1 addition & 1 deletion tools/cpp/BUILD.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ filegroup(

filegroup(
name = "compiler_deps",
srcs = glob(["extra_tools/**"], allow_empty = True) + ["%{cc_compiler_deps}"],
srcs = glob(["extra_tools/**"], allow_empty = True) + [%{cc_compiler_deps}],
)

# This is the entry point for --crosstool_top. Toolchains are found
Expand Down
31 changes: 23 additions & 8 deletions tools/cpp/BUILD.windows.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,21 @@ filegroup(
srcs = [],
)

filegroup(
name = "mingw_compiler_files",
srcs = [":builtin_include_directory_paths_mingw"]
)

filegroup(
name = "clangcl_compiler_files",
srcs = [":builtin_include_directory_paths_clangcl"]
)

filegroup(
name = "msvc_compiler_files",
srcs = [":builtin_include_directory_paths_msvc"]
)

# Hardcoded toolchain, legacy behaviour.
cc_toolchain_suite(
name = "toolchain",
Expand All @@ -49,8 +64,8 @@ cc_toolchain(
toolchain_config = ":msys_x64",
all_files = ":empty",
ar_files = ":empty",
as_files = ":empty",
compiler_files = ":empty",
as_files = ":mingw_compiler_files",
compiler_files = ":mingw_compiler_files",
dwp_files = ":empty",
linker_files = ":empty",
objcopy_files = ":empty",
Expand Down Expand Up @@ -95,8 +110,8 @@ cc_toolchain(
toolchain_config = ":msys_x64_mingw",
all_files = ":empty",
ar_files = ":empty",
as_files = ":empty",
compiler_files = ":empty",
as_files = ":mingw_compiler_files",
compiler_files = ":mingw_compiler_files",
dwp_files = ":empty",
linker_files = ":empty",
objcopy_files = ":empty",
Expand Down Expand Up @@ -141,8 +156,8 @@ cc_toolchain(
toolchain_config = ":msvc_x64",
all_files = ":empty",
ar_files = ":empty",
as_files = ":empty",
compiler_files = ":empty",
as_files = ":msvc_compiler_files",
compiler_files = ":msvc_compiler_files",
dwp_files = ":empty",
linker_files = ":empty",
objcopy_files = ":empty",
Expand Down Expand Up @@ -206,8 +221,8 @@ cc_toolchain(
toolchain_config = ":clang_cl_x64",
all_files = ":empty",
ar_files = ":empty",
as_files = ":empty",
compiler_files = ":empty",
as_files = ":clangcl_compiler_files",
compiler_files = ":clangcl_compiler_files",
dwp_files = ":empty",
linker_files = ":empty",
objcopy_files = ":empty",
Expand Down
1 change: 1 addition & 0 deletions tools/cpp/cc_configure.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ cc_autoconf = repository_rule(
"BAZEL_DO_NOT_DETECT_CPP_TOOLCHAIN",
"BAZEL_USE_LLVM_NATIVE_COVERAGE",
"BAZEL_LLVM",
"BAZEL_IGNORE_SYSTEM_HEADERS_VERSIONS",
"USE_CLANG_CL",
"CC",
"CC_CONFIGURE_DEBUG",
Expand Down
24 changes: 24 additions & 0 deletions tools/cpp/lib_cc_configure.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ def build_flags(flags):
return "\n".join([" flag: '" + flag + "'" for flag in flags])

def get_starlark_list(values):
"""Convert a list of string into a string that can be passed to a rule attribute."""
if not values:
return ""
return "\"" + "\",\n \"".join(values) + "\""
Expand All @@ -217,3 +218,26 @@ def auto_configure_warning_maybe(repository_ctx, msg):
"""Output warning message when CC_CONFIGURE_DEBUG is enabled."""
if is_cc_configure_debug(repository_ctx):
auto_configure_warning(msg)

def write_builtin_include_directory_paths(repository_ctx, cc, directories, file_suffix = ""):
"""Generate output file named 'builtin_include_directory_paths' in the root of the repository."""
if get_env_var(repository_ctx, "BAZEL_IGNORE_SYSTEM_HEADERS_VERSIONS", "0", False) == "1":
repository_ctx.file(
"builtin_include_directory_paths" + file_suffix,
"""This file is generated by cc_configure and normally contains builtin include directories
that C++ compiler reported. But because BAZEL_IGNORE_SYSTEM_HEADERS_VERSIONS was set to 1,
header include directory paths are intentionally not put there.
""",
)
else:
repository_ctx.file(
"builtin_include_directory_paths" + file_suffix,
"""This file is generated by cc_configure and contains builtin include directories
that %s reported. This file is a dependency of every compilation action and
changes to it will be reflected in the action cache key. When some of these
paths change, Bazel will make sure to rerun the action, even though none of
declared action inputs or the action commandline changes.
%s
""" % (cc, "\n".join(directories)),
)
2 changes: 2 additions & 0 deletions tools/cpp/osx_cc_configure.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ load(
"@bazel_tools//tools/cpp:lib_cc_configure.bzl",
"escape_string",
"resolve_labels",
"write_builtin_include_directory_paths",
)
load(
"@bazel_tools//tools/cpp:unix_cc_configure.bzl",
Expand Down Expand Up @@ -129,6 +130,7 @@ def configure_osx_toolchain(repository_ctx, overriden_tools):
error_msg)

escaped_include_paths = _get_escaped_xcode_cxx_inc_directories(repository_ctx, cc, xcode_toolchains)
write_builtin_include_directory_paths(repository_ctx, cc, escaped_include_paths)
escaped_cxx_include_directories = []
for path in escaped_include_paths:
escaped_cxx_include_directories.append((" \"%s\"," % path))
Expand Down
41 changes: 22 additions & 19 deletions tools/cpp/unix_cc_configure.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ load(
"resolve_labels",
"split_escaped",
"which",
"write_builtin_include_directory_paths",
)

def _field(name, value):
Expand Down Expand Up @@ -388,15 +389,34 @@ def configure_unix_toolchain(repository_ctx, cpu_value, overriden_tools):
bin_search_flag = []

coverage_compile_flags, coverage_link_flags = _coverage_flags(repository_ctx, darwin)
builtin_include_directories = _uniq(
get_escaped_cxx_inc_directories(repository_ctx, cc, "-xc") +
get_escaped_cxx_inc_directories(repository_ctx, cc, "-xc++", cxx_opts) +
get_escaped_cxx_inc_directories(
repository_ctx,
cc,
"-xc",
_get_no_canonical_prefixes_opt(repository_ctx, cc),
) +
get_escaped_cxx_inc_directories(
repository_ctx,
cc,
"-xc++",
cxx_opts + _get_no_canonical_prefixes_opt(repository_ctx, cc),
),
)

write_builtin_include_directory_paths(repository_ctx, cc, builtin_include_directories)
repository_ctx.template(
"BUILD",
paths["@bazel_tools//tools/cpp:BUILD.tpl"],
{
"%{cc_toolchain_identifier}": cc_toolchain_identifier,
"%{name}": cpu_value,
"%{supports_param_files}": "0" if darwin else "1",
"%{cc_compiler_deps}": ":cc_wrapper" if darwin else ":empty",
"%{cc_compiler_deps}": get_starlark_list([":builtin_include_directory_paths"] + (
[":cc_wrapper"] if darwin else []
)),
"%{compiler}": escape_string(get_env_var(
repository_ctx,
"BAZEL_COMPILER",
Expand Down Expand Up @@ -442,24 +462,7 @@ def configure_unix_toolchain(repository_ctx, cpu_value, overriden_tools):
"%{tool_paths}": ",\n ".join(
['"%s": "%s"' % (k, v) for k, v in tool_paths.items()],
),
"%{cxx_builtin_include_directories}": get_starlark_list(
_uniq(
get_escaped_cxx_inc_directories(repository_ctx, cc, "-xc") +
get_escaped_cxx_inc_directories(repository_ctx, cc, "-xc++", cxx_opts) +
get_escaped_cxx_inc_directories(
repository_ctx,
cc,
"-xc",
_get_no_canonical_prefixes_opt(repository_ctx, cc),
) +
get_escaped_cxx_inc_directories(
repository_ctx,
cc,
"-xc++",
cxx_opts + _get_no_canonical_prefixes_opt(repository_ctx, cc),
),
),
),
"%{cxx_builtin_include_directories}": get_starlark_list(builtin_include_directories),
"%{compile_flags}": get_starlark_list(
[
# Security hardening requires optimization.
Expand Down
9 changes: 8 additions & 1 deletion tools/cpp/windows_cc_configure.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ load(
"escape_string",
"execute",
"resolve_labels",
"write_builtin_include_directory_paths",
)

def _get_path_env_var(repository_ctx, name):
Expand Down Expand Up @@ -476,6 +477,7 @@ def _get_msys_mingw_vars(repository_ctx):
"""Get the variables we need to populate the msys/mingw toolchains."""
tool_paths, tool_bin_path, inc_dir_msys = _get_escaped_windows_msys_starlark_content(repository_ctx)
tool_paths_mingw, tool_bin_path_mingw, inc_dir_mingw = _get_escaped_windows_msys_starlark_content(repository_ctx, use_mingw = True)
write_builtin_include_directory_paths(repository_ctx, "mingw", [inc_dir_mingw], file_suffix = "_mingw")
msys_mingw_vars = {
"%{cxx_builtin_include_directories}": inc_dir_msys,
"%{mingw_cxx_builtin_include_directories}": inc_dir_mingw,
Expand Down Expand Up @@ -514,6 +516,7 @@ def _get_msvc_vars(repository_ctx, paths):
)

if not vc_path or missing_tools:
write_builtin_include_directory_paths(repository_ctx, "msvc", [], file_suffix = "_msvc")
msvc_vars = {
"%{msvc_env_tmp}": "msvc_not_found",
"%{msvc_env_path}": "msvc_not_found",
Expand Down Expand Up @@ -569,6 +572,7 @@ def _get_msvc_vars(repository_ctx, paths):

support_debug_fastlink = _is_support_debug_fastlink(repository_ctx, link_path)

write_builtin_include_directory_paths(repository_ctx, "msvc", escaped_cxx_include_directories, file_suffix = "_msvc")
msvc_vars = {
"%{msvc_env_tmp}": escaped_tmp_dir,
"%{msvc_env_path}": escaped_paths,
Expand Down Expand Up @@ -615,6 +619,7 @@ def _get_clang_cl_vars(repository_ctx, paths, msvc_vars):
error_script = "clang_installation_error.bat"

if error_script:
write_builtin_include_directory_paths(repository_ctx, "clang-cl", [], file_suffix = "_clangcl")
clang_cl_vars = {
"%{clang_cl_env_tmp}": "clang_cl_not_found",
"%{clang_cl_env_path}": "clang_cl_not_found",
Expand All @@ -639,12 +644,14 @@ def _get_clang_cl_vars(repository_ctx, paths, msvc_vars):
clang_include_path = (clang_dir + "\\include").replace("\\", "\\\\")
clang_lib_path = (clang_dir + "\\lib\\windows").replace("\\", "\\\\")

clang_cl_include_directories = msvc_vars["%{msvc_cxx_builtin_include_directories}"] + (",\n \"%s\"" % clang_include_path)
write_builtin_include_directory_paths(repository_ctx, "clang-cl", [clang_cl_include_directories], file_suffix = "_clangcl")
clang_cl_vars = {
"%{clang_cl_env_tmp}": msvc_vars["%{msvc_env_tmp}"],
"%{clang_cl_env_path}": msvc_vars["%{msvc_env_path}"],
"%{clang_cl_env_include}": msvc_vars["%{msvc_env_include}"] + ";" + clang_include_path,
"%{clang_cl_env_lib}": msvc_vars["%{msvc_env_lib}"] + ";" + clang_lib_path,
"%{clang_cl_cxx_builtin_include_directories}": msvc_vars["%{msvc_cxx_builtin_include_directories}"] + (",\n \"%s\"" % clang_include_path),
"%{clang_cl_cxx_builtin_include_directories}": clang_cl_include_directories,
"%{clang_cl_cl_path}": clang_cl_path,
"%{clang_cl_link_path}": lld_link_path,
"%{clang_cl_lib_path}": llvm_lib_path,
Expand Down
1 change: 1 addition & 0 deletions tools/osx/crosstool/BUILD.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ cc_toolchain_suite(
filegroup(
name = "osx_tools_" + arch,
srcs = [
":builtin_include_directory_paths",
":cc_wrapper",
":libtool",
":make_hashed_objlist.py",
Expand Down

0 comments on commit 8b0bfaf

Please sign in to comment.