Skip to content

Commit

Permalink
Only use /showIncludes if supported
Browse files Browse the repository at this point in the history
The auto-configured Windows toolchain now only enables `/showIncludes` if the English language pack for VS is installed and the compiler language can thus be forced to be English. Other languages may use encodings other than ASCII or UTF-8 and thus fail to be recognized by Bazel's `ShowInputFilter`. A warning with a suggested fix is shown if this fails.

See #19451
Fixes #19439

RELNOTES: Compilation actions using the auto-configured MSVC toolchain are forced to emit error messages in English if the English language pack for Visual Studio is installed.

Closes #19497.

PiperOrigin-RevId: 565251294
Change-Id: I926c484c21310ee4d7b5e8d5756cb61e45d6e6ac
  • Loading branch information
fmeum authored and copybara-github committed Sep 14, 2023
1 parent 4ddb739 commit 0f10359
Show file tree
Hide file tree
Showing 6 changed files with 82 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,9 @@ public boolean useDotdFile(Artifact sourceFile) {
}

public boolean dotdFilesEnabled() {
return cppSemantics.needsDotdInputPruning(configuration) && !shouldParseShowIncludes();
return cppSemantics.needsDotdInputPruning(configuration)
&& !shouldParseShowIncludes()
&& !featureConfiguration.isEnabled(CppRuleClasses.NO_DOTD_FILE);
}

public boolean serializedDiagnosticsFilesEnabled() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,9 @@ public static ToolchainTypeRequirement ccToolchainTypeRequirement(RuleDefinition
/** A string constant for /showIncludes parsing feature, should only be used for MSVC toolchain */
public static final String PARSE_SHOWINCLUDES = "parse_showincludes";

/** A string constant for a feature that, if enabled, disables .d file handling. */
public static final String NO_DOTD_FILE = "no_dotd_file";

/*
* A string constant for the fdo_instrument feature.
*/
Expand Down
6 changes: 6 additions & 0 deletions tools/cpp/BUILD.windows.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@ cc_toolchain_config(
default_link_flags = ["/MACHINE:X64"],
dbg_mode_debug_flag = "%{dbg_mode_debug_flag_x64}",
fastbuild_mode_debug_flag = "%{fastbuild_mode_debug_flag_x64}",
supports_parse_showincludes = %{msvc_parse_showincludes_x64},
)

toolchain(
Expand Down Expand Up @@ -295,6 +296,7 @@ cc_toolchain_config(
default_link_flags = ["/MACHINE:X86"],
dbg_mode_debug_flag = "%{dbg_mode_debug_flag_x86}",
fastbuild_mode_debug_flag = "%{fastbuild_mode_debug_flag_x86}",
supports_parse_showincludes = %{msvc_parse_showincludes_x86},
)

toolchain(
Expand Down Expand Up @@ -361,6 +363,7 @@ cc_toolchain_config(
default_link_flags = ["/MACHINE:ARM"],
dbg_mode_debug_flag = "%{dbg_mode_debug_flag_arm}",
fastbuild_mode_debug_flag = "%{fastbuild_mode_debug_flag_arm}",
supports_parse_showincludes = %{msvc_parse_showincludes_arm},
)

toolchain(
Expand Down Expand Up @@ -427,6 +430,7 @@ cc_toolchain_config(
default_link_flags = ["/MACHINE:ARM64"],
dbg_mode_debug_flag = "%{dbg_mode_debug_flag_arm64}",
fastbuild_mode_debug_flag = "%{fastbuild_mode_debug_flag_arm64}",
supports_parse_showincludes = %{msvc_parse_showincludes_arm64},
)

toolchain(
Expand Down Expand Up @@ -493,6 +497,7 @@ cc_toolchain_config(
default_link_flags = ["/MACHINE:X64"],
dbg_mode_debug_flag = "%{clang_cl_dbg_mode_debug_flag_x64}",
fastbuild_mode_debug_flag = "%{clang_cl_fastbuild_mode_debug_flag_x64}",
supports_parse_showincludes = %{clang_cl_parse_showincludes_x64},
)

toolchain(
Expand Down Expand Up @@ -560,6 +565,7 @@ cc_toolchain_config(
default_link_flags = ["/MACHINE:ARM64"],
dbg_mode_debug_flag = "%{clang_cl_dbg_mode_debug_flag_arm64}",
fastbuild_mode_debug_flag = "%{clang_cl_fastbuild_mode_debug_flag_arm64}",
supports_parse_showincludes = %{clang_cl_parse_showincludes_arm64},
)

toolchain(
Expand Down
16 changes: 11 additions & 5 deletions tools/cpp/lib_cc_configure.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,8 @@ def execute(
repository_ctx,
command,
environment = None,
expect_failure = False):
expect_failure = False,
expect_empty_output = False):
"""Execute a command, return stdout if succeed and throw an error if it fails. Doesn't %-escape the result!"""
if environment:
result = repository_ctx.execute(command, environment = environment)
Expand All @@ -171,10 +172,15 @@ def execute(
),
)
stripped_stdout = result.stdout.strip()
if not stripped_stdout:
auto_configure_fail(
"empty output from command %s, stderr: (%s)" % (command, result.stderr),
)
if expect_empty_output != (not stripped_stdout):
if expect_empty_output:
auto_configure_fail(
"non-empty output from command %s, stdout: (%s), stderr: (%s)" % (command, result.stdout, result.stderr),
)
else:
auto_configure_fail(
"empty output from command %s, stderr: (%s)" % (command, result.stderr),
)
return stripped_stdout

def get_cpu_value(repository_ctx):
Expand Down
34 changes: 34 additions & 0 deletions tools/cpp/windows_cc_configure.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,28 @@ def _is_support_debug_fastlink(repository_ctx, linker):
result = execute(repository_ctx, [linker], expect_failure = True)
return result.find("/DEBUG[:{FASTLINK|FULL|NONE}]") != -1

def _is_support_parse_showincludes(repository_ctx, cl, env):
repository_ctx.file(
"main.cpp",
"#include \"bazel_showincludes.h\"\nint main(){}\n",
)
repository_ctx.file(
"bazel_showincludes.h",
"\n",
)
result = execute(
repository_ctx,
[cl, "/nologo", "/showIncludes", "/c", "main.cpp", "/out", "main.exe", "/Fo", "main.obj"],
# Attempt to force English language. This may fail if the language pack isn't installed.
environment = env | {"VSLANG": "1033"},
)
for file in ["main.cpp", "bazel_showincludes.h", "main.exe", "main.obj"]:
execute(repository_ctx, ["cmd", "/C", "del", file], expect_empty_output = True)
return any([
line.startswith("Note: including file:") and line.endswith("bazel_showincludes.h")
for line in result.split("\n")
])

def find_llvm_path(repository_ctx):
"""Find LLVM install path."""

Expand Down Expand Up @@ -635,6 +657,7 @@ def _get_msvc_vars(repository_ctx, paths, target_arch = "x64", msvc_vars_x64 = N
"%{msvc_lib_path_" + target_arch + "}": "vc_installation_error_" + target_arch + ".bat",
"%{dbg_mode_debug_flag_" + target_arch + "}": "/DEBUG",
"%{fastbuild_mode_debug_flag_" + target_arch + "}": "/DEBUG",
"%{msvc_parse_showincludes_" + target_arch + "}": repr(False),
}
return msvc_vars

Expand Down Expand Up @@ -679,6 +702,13 @@ def _get_msvc_vars(repository_ctx, paths, target_arch = "x64", msvc_vars_x64 = N
support_debug_fastlink = _is_support_debug_fastlink(repository_ctx, build_tools["LINK"])
write_builtin_include_directory_paths(repository_ctx, "msvc", escaped_cxx_include_directories, file_suffix = "_msvc")

support_parse_showincludes = _is_support_parse_showincludes(repository_ctx, build_tools["CL"], env)
if not support_parse_showincludes:
auto_configure_warning("""
Header pruning has been disabled since Bazel failed to recognize the output of /showIncludes.
This can result in unnecessary recompilation.
Fix this by installing the English language pack for the Visual Studio installation at {} and run 'bazel sync --configure'.""".format(vc_path))

msvc_vars = {
"%{msvc_env_tmp_" + target_arch + "}": escaped_tmp_dir,
"%{msvc_env_include_" + target_arch + "}": escaped_include_paths,
Expand All @@ -689,6 +719,7 @@ def _get_msvc_vars(repository_ctx, paths, target_arch = "x64", msvc_vars_x64 = N
"%{msvc_ml_path_" + target_arch + "}": build_tools.get("ML", "msvc_arm_toolchain_does_not_support_ml"),
"%{msvc_link_path_" + target_arch + "}": build_tools["LINK"],
"%{msvc_lib_path_" + target_arch + "}": build_tools["LIB"],
"%{msvc_parse_showincludes_" + target_arch + "}": repr(support_parse_showincludes),
"%{dbg_mode_debug_flag_" + target_arch + "}": "/DEBUG:FULL" if support_debug_fastlink else "/DEBUG",
"%{fastbuild_mode_debug_flag_" + target_arch + "}": "/DEBUG:FASTLINK" if support_debug_fastlink else "/DEBUG",
}
Expand Down Expand Up @@ -738,6 +769,7 @@ def _get_clang_cl_vars(repository_ctx, paths, msvc_vars, target_arch):
"%{clang_cl_dbg_mode_debug_flag_" + target_arch + "}": "/DEBUG",
"%{clang_cl_fastbuild_mode_debug_flag_" + target_arch + "}": "/DEBUG",
"%{clang_cl_cxx_builtin_include_directories_" + target_arch + "}": "",
"%{clang_cl_parse_showincludes_" + target_arch + "}": repr(False),
}
return clang_cl_vars

Expand Down Expand Up @@ -765,6 +797,8 @@ def _get_clang_cl_vars(repository_ctx, paths, msvc_vars, target_arch):
# LLVM's lld-link.exe doesn't support /DEBUG:FASTLINK.
"%{clang_cl_dbg_mode_debug_flag_" + target_arch + "}": "/DEBUG",
"%{clang_cl_fastbuild_mode_debug_flag_" + target_arch + "}": "/DEBUG",
# clang-cl always emits the English language version of the /showIncludes prefix.
"%{clang_cl_parse_showincludes_" + target_arch + "}": repr(True),
}
return clang_cl_vars

Expand Down
29 changes: 25 additions & 4 deletions tools/cpp/windows_cc_toolchain_config.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

"""A Starlark cc_toolchain configuration rule for Windows"""

load("@bazel_tools//tools/build_defs/cc:action_names.bzl", "ACTION_NAMES")
load(
"@bazel_tools//tools/cpp:cc_toolchain_config_lib.bzl",
"action_config",
Expand All @@ -28,7 +29,6 @@ load(
"variable_with_value",
"with_feature_set",
)
load("@bazel_tools//tools/build_defs/cc:action_names.bzl", "ACTION_NAMES")

all_compile_actions = [
ACTION_NAMES.c_compile,
Expand Down Expand Up @@ -187,7 +187,6 @@ def _impl(ctx):
"compiler_output_flags",
"nologo",
"msvc_env",
"parse_showincludes",
"user_compile_flags",
"sysroot",
],
Expand All @@ -202,7 +201,6 @@ def _impl(ctx):
"default_compile_flags",
"nologo",
"msvc_env",
"parse_showincludes",
"user_compile_flags",
"sysroot",
"unfiltered_compile_flags",
Expand All @@ -217,7 +215,6 @@ def _impl(ctx):
"compiler_output_flags",
"nologo",
"msvc_env",
"parse_showincludes",
"user_compile_flags",
"sysroot",
],
Expand Down Expand Up @@ -789,6 +786,7 @@ def _impl(ctx):

parse_showincludes_feature = feature(
name = "parse_showincludes",
enabled = ctx.attr.supports_parse_showincludes,
flag_sets = [
flag_set(
actions = [
Expand All @@ -802,6 +800,27 @@ def _impl(ctx):
flag_groups = [flag_group(flags = ["/showIncludes"])],
),
],
env_sets = [
env_set(
actions = [
ACTION_NAMES.preprocess_assemble,
ACTION_NAMES.c_compile,
ACTION_NAMES.linkstamp_compile,
ACTION_NAMES.cpp_compile,
ACTION_NAMES.cpp_module_compile,
ACTION_NAMES.cpp_header_parsing,
],
# Force English (and thus a consistent locale) output so that Bazel can parse
# the /showIncludes output without having to guess the encoding.
env_entries = [env_entry(key = "VSLANG", value = "1033")],
),
],
)

# MSVC does not emit .d files.
no_dotd_file_feature = feature(
name = "no_dotd_file",
enabled = True,
)

treat_warnings_as_errors_feature = feature(
Expand Down Expand Up @@ -1101,6 +1120,7 @@ def _impl(ctx):
external_include_paths_feature,
preprocessor_defines_feature,
parse_showincludes_feature,
no_dotd_file_feature,
generate_pdb_file_feature,
shared_flag_feature,
linkstamps_feature,
Expand Down Expand Up @@ -1417,6 +1437,7 @@ cc_toolchain_config = rule(
"dbg_mode_debug_flag": attr.string(),
"fastbuild_mode_debug_flag": attr.string(),
"tool_bin_path": attr.string(default = "not_found"),
"supports_parse_showincludes": attr.bool(),
},
provides = [CcToolchainConfigInfo],
)

0 comments on commit 0f10359

Please sign in to comment.