From 0f103598bba32439355c94b2c226fb60625c94dd Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Wed, 13 Sep 2023 21:47:59 -0700 Subject: [PATCH] Only use `/showIncludes` if supported 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 --- .../rules/cpp/CppCompileActionBuilder.java | 4 ++- .../build/lib/rules/cpp/CppRuleClasses.java | 3 ++ tools/cpp/BUILD.windows.tpl | 6 ++++ tools/cpp/lib_cc_configure.bzl | 16 ++++++--- tools/cpp/windows_cc_configure.bzl | 34 +++++++++++++++++++ tools/cpp/windows_cc_toolchain_config.bzl | 29 +++++++++++++--- 6 files changed, 82 insertions(+), 10 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java index 6dad4d1a7bd39c..deeac6dd94b9ea 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java @@ -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() { diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java index 53377dc1cceb12..858d0e0a028c11 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java @@ -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. */ diff --git a/tools/cpp/BUILD.windows.tpl b/tools/cpp/BUILD.windows.tpl index 0151325835be06..3fcbe30f61a27e 100644 --- a/tools/cpp/BUILD.windows.tpl +++ b/tools/cpp/BUILD.windows.tpl @@ -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( @@ -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( @@ -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( @@ -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( @@ -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( @@ -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( diff --git a/tools/cpp/lib_cc_configure.bzl b/tools/cpp/lib_cc_configure.bzl index 497f25741613c8..67ee9ca5c6c639 100644 --- a/tools/cpp/lib_cc_configure.bzl +++ b/tools/cpp/lib_cc_configure.bzl @@ -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) @@ -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): diff --git a/tools/cpp/windows_cc_configure.bzl b/tools/cpp/windows_cc_configure.bzl index 760a8435f99a65..4c3e367b22b4f8 100644 --- a/tools/cpp/windows_cc_configure.bzl +++ b/tools/cpp/windows_cc_configure.bzl @@ -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.""" @@ -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 @@ -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, @@ -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", } @@ -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 @@ -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 diff --git a/tools/cpp/windows_cc_toolchain_config.bzl b/tools/cpp/windows_cc_toolchain_config.bzl index 2f8e2b37f0c55c..6d8e8af6d50e4a 100644 --- a/tools/cpp/windows_cc_toolchain_config.bzl +++ b/tools/cpp/windows_cc_toolchain_config.bzl @@ -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", @@ -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, @@ -187,7 +187,6 @@ def _impl(ctx): "compiler_output_flags", "nologo", "msvc_env", - "parse_showincludes", "user_compile_flags", "sysroot", ], @@ -202,7 +201,6 @@ def _impl(ctx): "default_compile_flags", "nologo", "msvc_env", - "parse_showincludes", "user_compile_flags", "sysroot", "unfiltered_compile_flags", @@ -217,7 +215,6 @@ def _impl(ctx): "compiler_output_flags", "nologo", "msvc_env", - "parse_showincludes", "user_compile_flags", "sysroot", ], @@ -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 = [ @@ -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( @@ -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, @@ -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], )