From 496585daf92f4bb7dee22725c9f3161bdde184f2 Mon Sep 17 00:00:00 2001 From: Joel Rosdahl Date: Fri, 31 Oct 2025 21:19:40 +0100 Subject: [PATCH] feat(msvc): Use /sourceDependencies to extract include file paths Extract include file paths from MSVC's /sourceDependencies output instead of parsing preprocessed output. This eliminates the need for the /utf-8 flag and msvc_utf8 config option, allowing ccache to work correctly with both Unicode and "ANSI" (non-Unicode) codebases without user configuration. This resolves the encoding issues described in issues #1619 and #1650 by using a more robust approach that doesn't depend on source file encoding. The change applies to both preprocessor mode and depend mode. For MSVC versions older than 2017 15.7 that don't support /sourceDependencies, ccache falls back to parsing preprocessed output. Other MSVC-like compilers (clang-cl, nvcc, etc.) continue using /showIncludes. --- doc/manual.adoc | 41 ++- src/ccache/argprocessing.cpp | 8 +- src/ccache/ccache.cpp | 409 +++++++++++++++++--------- src/ccache/compiler/msvc.cpp | 27 +- src/ccache/compiler/msvc.hpp | 7 +- src/ccache/config.cpp | 10 - src/ccache/config.hpp | 15 - src/ccache/context.hpp | 4 - src/ccache/core/resultretriever.cpp | 6 +- src/ccache/core/statistic.hpp | 3 +- src/ccache/core/statistics.cpp | 3 + test/msvc/test_basic_functionality.py | 123 ++++++++ unittest/test_compiler_msvc.cpp | 164 +++++++---- unittest/test_config.cpp | 5 - 14 files changed, 532 insertions(+), 293 deletions(-) diff --git a/doc/manual.adoc b/doc/manual.adoc index 44b32047..cc262ff4 100644 --- a/doc/manual.adoc +++ b/doc/manual.adoc @@ -898,19 +898,10 @@ file in `/etc/rsyslog.d`: [#config_msvc_dep_prefix] *msvc_dep_prefix* (*CCACHE_MSVC_DEP_PREFIX*):: - This option specifies the prefix of included files output for MSVC compiler. - The default prefix is "`Note: including file:`". If you use a localized - compiler, this should be set accordingly. - -[#config_msvc_utf8] -*msvc_utf8* (*CCACHE_MSVC_UTF8*):: - - This option adds `/utf-8` to the msvc command line when executing the preprocessor to - ensure that filenames are not garbled for non-ascii characters. - This implicitly enables `/validate-charset` and treats the source code as utf-8 which - may cause compilation errors if comments in your code have characters in the [128, 255] - range for a given Windows system codepage which results in an invalid utf-8 sequence. - The default is true. + This option specifies the prefix of included files in `/showIncludes` output + for clang-cl and other MSVC-like compilers that don't support + `/sourceDependencies`. The default prefix is "`Note: including file:`". If + you use a localized compiler, this should be set accordingly. [#config_namespace] *namespace* (*CCACHE_NAMESPACE*):: @@ -1584,6 +1575,9 @@ Preprocessing the source code using the compiler's `-E` option failed. Code like the assembler `.incbin` directive was found. This is not supported by ccache. +| Unsupported compiler | +Compiler type or version was not supported. + | Unsupported compiler option | A compiler option not supported by ccache was found. @@ -1591,8 +1585,7 @@ A compiler option not supported by ccache was found. An environment variable not supported by ccache was set. | Unsupported source encoding | -Source file (or an included header) has unsupported encoding. ccache currently -requires UTF-8-encoded source code for MSVC when `msvc_utf8` is true. +Source file (or an included header) has unsupported encoding. | Unsupported source language | A source language e.g. specified with `-x` was unsupported by ccache. @@ -1704,10 +1697,15 @@ The direct mode will be disabled if any of the following holds: === The depend mode If the depend mode is enabled, ccache will not use the preprocessor at all. The -hash used to identify results in the cache will be based on the direct mode -hash described above plus information about include files read from the -dependency list generated by MSVC with `/showIncludes`, or the dependency file -generated by other compilers with `-MD` or `-MMD`. +hash used to identify results in the cache will be based on the direct mode hash +described above plus information about include files read from: + +* the dependency list generated by MSVC with `/sourceDependencies` (for MSVC + 2017 15.7 and later) +* include files from `/showIncludes` (for clang-cl and other MSVC-like compilers + that don't support `/sourceDependencies`) +* the dependency file (`.d`) generated by GCC and similar compilers when using + `-MD` or `-MMD` Advantages: @@ -1724,7 +1722,7 @@ Disadvantages: to some types of changes of compiler options and source code changes. * If `-MD` is used, the manifest entries will include system header files as well, thus slowing down cache hits slightly, just as using `-MD` slows down - make. This is also the case for MSVC with `/showIncludes`. + make. This is also the case for MSVC-like compilers. * If `-MMD` is used, the manifest entries will not include system header files, which means ccache will ignore changes in them. @@ -1732,7 +1730,8 @@ The depend mode will be disabled if any of the following holds: * <> is false. * The compiler is not generating dependencies using `-MD` or `-MMD` (for MSVC, - `/showIncludes` is added automatically if not specified by the user). + `/sourceDependencies` is used if the compiler supports it, otherwise caching + is disabled in depend mode). == Handling of newly created source files diff --git a/src/ccache/argprocessing.cpp b/src/ccache/argprocessing.cpp index 7c3d00b1..de6ece91 100644 --- a/src/ccache/argprocessing.cpp +++ b/src/ccache/argprocessing.cpp @@ -27,6 +27,7 @@ #include #include #include +#include #include #include #include @@ -1758,13 +1759,6 @@ process_args(Context& ctx) state.add_compiler_only_arg_no_hash(*diagnostics_color_arg); } - if (ctx.config.depend_mode() && !args_info.generating_includes - && ctx.config.compiler_type() == CompilerType::msvc) { - ctx.auto_depend_mode = true; - args_info.generating_includes = true; - state.add_compiler_only_arg_no_hash("/showIncludes"); - } - if (state.found_c_opt) { state.add_compiler_only_arg_no_hash(*state.found_c_opt); } diff --git a/src/ccache/ccache.cpp b/src/ccache/ccache.cpp index 1dfc96d5..8638af24 100644 --- a/src/ccache/ccache.cpp +++ b/src/ccache/ccache.cpp @@ -520,6 +520,11 @@ process_preprocessed_file(Context& ctx, Hash& hash, const fs::path& path) return tl::unexpected(Statistic::internal_error); } + // If includes were already extracted from /sourceDependencies or + // /showIncludes (this avoids source encoding issues on Windows), don't + // extract them from the preprocessor output. + const bool includes_already_extracted = !ctx.included_files.empty(); + std::unordered_map relative_inc_path_cache; // Bytes between p and q are pending to be hashed. @@ -626,34 +631,40 @@ process_preprocessed_file(Context& ctx, Hash& hash, const fs::path& path) r++; } - // p and q span the include file path. - std::string inc_path(p, q - p); - while (!inc_path.empty() && inc_path.back() == '/') { - inc_path.pop_back(); - } - fs::path inc_fs_path; - try { - inc_fs_path = inc_path; - } catch (const std::filesystem::filesystem_error&) { - return tl::unexpected(Failure(Statistic::unsupported_source_encoding)); - } - if (!ctx.config.base_dirs().empty()) { - auto it = relative_inc_path_cache.find(inc_path); - if (it == relative_inc_path_cache.end()) { - std::string rel_inc_path = - util::pstr(core::make_relative_path(ctx, inc_fs_path)); - relative_inc_path_cache.emplace(inc_path, rel_inc_path); - inc_path = util::pstr(rel_inc_path); - } else { - inc_path = it->second; + if (includes_already_extracted) { + hash.hash(p, q - p); + } else { + // p and q span the include file path. + std::string inc_path(p, q - p); + while (!inc_path.empty() && inc_path.back() == '/') { + inc_path.pop_back(); + } + fs::path inc_fs_path; + try { + inc_fs_path = inc_path; + } catch (const std::filesystem::filesystem_error&) { + return tl::unexpected( + Failure(Statistic::unsupported_source_encoding)); + } + if (!ctx.config.base_dirs().empty()) { + auto it = relative_inc_path_cache.find(inc_path); + if (it == relative_inc_path_cache.end()) { + std::string rel_inc_path = + util::pstr(core::make_relative_path(ctx, inc_fs_path)); + relative_inc_path_cache.emplace(inc_path, rel_inc_path); + inc_path = util::pstr(rel_inc_path); + } else { + inc_path = it->second; + } } - } - if (inc_fs_path != ctx.apparent_cwd || ctx.config.hash_dir()) { - hash.hash(inc_fs_path); + if (inc_fs_path != ctx.apparent_cwd || ctx.config.hash_dir()) { + hash.hash(inc_fs_path); + } + + TRY(remember_include_file(ctx, inc_fs_path, hash, system, nullptr)); } - TRY(remember_include_file(ctx, inc_fs_path, hash, system, nullptr)); p = q; // Everything of interest between p and q has been hashed now. } else if (strncmp(q, incbin_directive, sizeof(incbin_directive)) == 0 && ((q[7] == ' ' @@ -701,11 +712,6 @@ process_preprocessed_file(Context& ctx, Hash& hash, const fs::path& path) // mention of it in the preprocessed output. TRY(check_included_pch_file(ctx, hash)); - bool debug_included = getenv("CCACHE_DEBUG_INCLUDED"); - if (debug_included) { - print_included_files(ctx, stdout); - } - return {}; } @@ -746,11 +752,6 @@ result_key_from_depfile(Context& ctx, Hash& hash) // dependencies output. TRY(check_included_pch_file(ctx, hash)); - bool debug_included = getenv("CCACHE_DEBUG_INCLUDED"); - if (debug_included) { - print_included_files(ctx, stdout); - } - return hash.digest(); } @@ -784,28 +785,84 @@ struct DoExecuteResult util::Bytes stderr_data; }; -// Extract the used includes from /showIncludes (or clang-cl's -// /showIncludes:user) output in stdout. Note that we cannot distinguish system -// headers from other includes when /showIncludes is used. -static tl::expected -result_key_from_includes(Context& ctx, Hash& hash, std::string_view stdout_data) +static tl::expected +extract_includes_from_msvc_stdout(Context& ctx, + Hash& hash, + std::string_view stdout_data) { + ASSERT(ctx.config.is_compiler_group_msvc()); + ASSERT(ctx.config.compiler_type() != CompilerType::msvc); + for (std::string_view include : compiler::get_includes_from_msvc_show_includes( stdout_data, ctx.config.msvc_dep_prefix())) { - const fs::path path = core::make_relative_path(ctx, include); - TRY(remember_include_file(ctx, path, hash, false, &hash)); + try { + const fs::path path = core::make_relative_path(ctx, include); + TRY(remember_include_file(ctx, path, hash, false, &hash)); + } catch (const std::filesystem::filesystem_error&) { + return tl::unexpected(Failure(Statistic::unsupported_source_encoding)); + } } - // Explicitly check the .pch file as it is not mentioned in the - // includes output. + // Explicitly check the .pch file as it is not mentioned in the output. TRY(check_included_pch_file(ctx, hash)); - const bool debug_included = getenv("CCACHE_DEBUG_INCLUDED"); - if (debug_included) { - print_included_files(ctx, stdout); + return {}; +} + +static tl::expected +extract_includes_from_msvc_source_deps_file( + Context& ctx, Hash& hash, const fs::path& source_dependencies_file) +{ + ASSERT(ctx.config.compiler_type() == CompilerType::msvc); + + auto json_content = util::read_file(source_dependencies_file); + if (!json_content) { + LOG("Failed to read /sourceDependencies file {}: {}", + source_dependencies_file, + json_content.error()); + return tl::unexpected(Statistic::internal_error); } + auto includes = compiler::get_includes_from_msvc_source_deps(*json_content); + if (!includes) { + LOG("Failed to parse /sourceDependencies file: {}", includes.error()); + return tl::unexpected(Failure(Statistic::internal_error)); + } + for (const auto& include : *includes) { + try { + const fs::path path = core::make_relative_path(ctx, include); + TRY(remember_include_file(ctx, path, hash, false, &hash)); + } catch (const std::filesystem::filesystem_error&) { + return tl::unexpected(Failure(Statistic::unsupported_source_encoding)); + } + } + + // Explicitly check the .pch file as it is not mentioned in the output. + TRY(check_included_pch_file(ctx, hash)); + + return {}; +} + +// Extract used includes from /showIncludes (or clang-cl's /showIncludes:user) +// output in stdout. Note that we cannot distinguish system headers from other +// includes when /showIncludes is used. +static tl::expected +result_key_from_show_includes(Context& ctx, + Hash& hash, + std::string_view stdout_data) +{ + TRY(extract_includes_from_msvc_stdout(ctx, hash, stdout_data)); + return hash.digest(); +} + +// Extract used includes from /sourceDependencies file for MSVC in depend mode. +// Note that we cannot distinguish system headers from other includes when +// /sourceDependencies is used. +static tl::expected +result_key_from_source_deps(Context& ctx, Hash& hash, const fs::path& path) +{ + TRY(extract_includes_from_msvc_source_deps_file(ctx, hash, path)); return hash.digest(); } @@ -1074,63 +1131,52 @@ write_result(Context& ctx, } static util::Bytes -rewrite_stdout_from_compiler(const Context& ctx, util::Bytes&& stdout_data) +rewrite_stdout_from_compiler(const Context& ctx, + util::Bytes&& stdout_data, + bool strip_includes_from_stdout) { using util::Tokenizer; using Mode = Tokenizer::Mode; using IncludeDelimiter = Tokenizer::IncludeDelimiter; - if (!stdout_data.empty()) { - util::Bytes new_stdout_data; - for (const auto line : Tokenizer(util::to_string_view(stdout_data), - "\n", - Mode::include_empty, - IncludeDelimiter::yes)) { - if (util::starts_with(line, "__________")) { - // distcc-pump outputs lines like this: - // - // __________Using # distcc servers in pump mode - // - // We don't want to cache those. - core::send_to_console(ctx, line, STDOUT_FILENO); - } else if (ctx.config.compiler_type() == CompilerType::msvc - && !ctx.config.base_dirs().empty() - && util::starts_with(line, ctx.config.msvc_dep_prefix())) { - // Ninja uses the lines with 'Note: including file: ' to determine the - // used headers. Headers within basedir need to be changed into relative - // paths because otherwise Ninja will use the abs path to original - // header to check if a file needs to be recompiled. - std::string orig_line(line.data(), line.length()); - std::string abs_inc_path = - util::replace_first(orig_line, ctx.config.msvc_dep_prefix(), ""); - abs_inc_path = util::strip_whitespace(abs_inc_path); - fs::path rel_inc_path = core::make_relative_path(ctx, abs_inc_path); - std::string line_with_rel_inc = util::replace_first( - orig_line, abs_inc_path, util::pstr(rel_inc_path).str()); - new_stdout_data.insert(new_stdout_data.end(), - line_with_rel_inc.data(), - line_with_rel_inc.size()); - } else if (ctx.config.is_compiler_group_msvc() - && !ctx.config.base_dirs().empty()) { - // The MSVC /FC option causes paths in diagnostics messages to become - // absolute. Those within basedir need to be changed into relative - // paths. - size_t path_end = core::get_diagnostics_path_length(line); - if (path_end != 0) { - std::string_view abs_path = line.substr(0, path_end); - fs::path rel_path = core::make_relative_path(ctx, abs_path); - std::string line_with_rel = - util::replace_all(line, abs_path, util::pstr(rel_path).str()); - new_stdout_data.insert( - new_stdout_data.end(), line_with_rel.data(), line_with_rel.size()); - } - } else { - new_stdout_data.insert(new_stdout_data.end(), line.data(), line.size()); + + if (stdout_data.empty()) { + return std::move(stdout_data); + } + + util::Bytes new_stdout_data; + for (const auto line : Tokenizer(util::to_string_view(stdout_data), + "\n", + Mode::include_empty, + IncludeDelimiter::yes)) { + if (util::starts_with(line, "__________")) { + // distcc-pump outputs lines like this: + // + // __________Using # distcc servers in pump mode + // + // We don't want to cache those. + core::send_to_console(ctx, line, STDOUT_FILENO); + } else if (strip_includes_from_stdout + && util::starts_with(line, ctx.config.msvc_dep_prefix())) { + // Strip line. + } else if (ctx.config.is_compiler_group_msvc() + && !ctx.config.base_dirs().empty()) { + // The MSVC /FC option causes paths in diagnostics messages to become + // absolute. Those within basedir need to be changed into relative + // paths. + size_t path_end = core::get_diagnostics_path_length(line); + if (path_end != 0) { + std::string_view abs_path = line.substr(0, path_end); + fs::path rel_path = core::make_relative_path(ctx, abs_path); + std::string line_with_rel = + util::replace_all(line, abs_path, util::pstr(rel_path).str()); + new_stdout_data.insert( + new_stdout_data.end(), line_with_rel.data(), line_with_rel.size()); } + } else { + new_stdout_data.insert(new_stdout_data.end(), line.data(), line.size()); } - return new_stdout_data; - } else { - return std::move(stdout_data); } + return new_stdout_data; } static std::string @@ -1228,10 +1274,51 @@ to_cache(Context& ctx, } } + bool capture_stdout = true; + + // For MSVC/clang-cl in depend mode, set up includes extraction. + fs::path msvc_deps_file; + bool strip_includes_from_stdout = false; + + if (ctx.config.depend_mode() && ctx.config.is_compiler_group_msvc()) { + if (ctx.config.compiler_type() == CompilerType::msvc) { + // MSVC: set up /sourceDependencies if needed. If user specified + // /sourceDependencies, use their file. + if (!ctx.args_info.output_sd.empty()) { + msvc_deps_file = ctx.args_info.output_sd; + } else { + auto tmp_deps = + util::value_or_throw(util::TemporaryFile::create( + FMT("{}/source_deps", ctx.config.temporary_dir()), ".json")); + msvc_deps_file = tmp_deps.path; + tmp_deps.fd.close(); + ctx.register_pending_tmp_file(msvc_deps_file); + + // Add flag since we're managing the file (not user-specified). + args.push_back("/sourceDependencies"); + args.push_back(msvc_deps_file); + } + // Also add /showIncludes as a fallback to support older MSVC versions + // without /sourceDependencies. Only strip from stdout if we injected + // the flag ourselves (not when the user already specified it). + if (!ctx.args_info.generating_includes) { + args.push_back("/showIncludes"); + strip_includes_from_stdout = true; + } + } else { + if (!ctx.args_info.generating_includes) { + args.push_back("/showIncludes"); + strip_includes_from_stdout = true; + } + capture_stdout = true; // To be able to parse /showIncludes output + } + } + LOG_RAW("Running real compiler"); + auto result = do_execute(ctx, args, capture_stdout); - tl::expected result; - result = do_execute(ctx, args); + // No early abort: if /sourceDependencies is unsupported, fall back to + // parsing /showIncludes output below. if (!result) { return tl::unexpected(result.error()); @@ -1245,9 +1332,6 @@ to_cache(Context& ctx, ctx.cpp_stderr_data.end()); } - result->stdout_data = - rewrite_stdout_from_compiler(ctx, std::move(result->stdout_data)); - if (result->exit_status != 0) { LOG("Compiler gave exit status {}", result->exit_status); @@ -1255,10 +1339,7 @@ to_cache(Context& ctx, core::send_to_console( ctx, util::to_string_view(result->stderr_data), STDERR_FILENO); core::send_to_console( - ctx, - util::to_string_view(compiler::strip_includes_from_msvc_show_includes( - ctx, std::move(result->stdout_data))), - STDOUT_FILENO); + ctx, util::to_string_view(result->stdout_data), STDOUT_FILENO); auto failure = Failure(Statistic::compile_failed); failure.set_exit_code(result->exit_status); @@ -1267,16 +1348,32 @@ to_cache(Context& ctx, if (ctx.config.depend_mode()) { ASSERT(depend_mode_hash); - if (ctx.args_info.generating_dependencies) { - TRY_ASSIGN(result_key, result_key_from_depfile(ctx, *depend_mode_hash)); - } else if (ctx.args_info.generating_includes) { + if (ctx.config.compiler_type() == CompilerType::msvc) { + if (fs::exists(msvc_deps_file)) { + TRY_ASSIGN( + result_key, + result_key_from_source_deps(ctx, *depend_mode_hash, msvc_deps_file)); + } else { + TRY_ASSIGN( + result_key, + result_key_from_show_includes( + ctx, *depend_mode_hash, util::to_string_view(result->stdout_data))); + } + } else if (ctx.config.is_compiler_group_msvc()) { + // Compiler printed /showIncludes output to stdout. TRY_ASSIGN( result_key, - result_key_from_includes( + result_key_from_show_includes( ctx, *depend_mode_hash, util::to_string_view(result->stdout_data))); } else { - ASSERT(false); + ASSERT(ctx.args_info.generating_dependencies); + TRY_ASSIGN(result_key, result_key_from_depfile(ctx, *depend_mode_hash)); + } + + if (getenv("CCACHE_DEBUG_INCLUDED")) { + print_included_files(ctx, stdout); } + LOG_RAW("Got result key from dependency file"); LOG("Result key: {}", util::format_digest(*result_key)); } @@ -1316,8 +1413,12 @@ to_cache(Context& ctx, } } - if (!write_result( - ctx, *result_key, result->stdout_data, result->stderr_data)) { + // Strip output from /showIncludes if we added it ourselves for depend mode + // and rewrite diagnostics paths as needed, then store and display. + auto rewritten_stdout = rewrite_stdout_from_compiler( + ctx, std::move(result->stdout_data), strip_includes_from_stdout); + + if (!write_result(ctx, *result_key, rewritten_stdout, result->stderr_data)) { return tl::unexpected(Statistic::compiler_produced_no_output); } @@ -1326,10 +1427,7 @@ to_cache(Context& ctx, ctx, util::to_string_view(result->stderr_data), STDERR_FILENO); // Send stdout after stderr, it makes the output clearer with MSVC. core::send_to_console( - ctx, - util::to_string_view(compiler::strip_includes_from_msvc_show_includes( - ctx, std::move(result->stdout_data))), - STDOUT_FILENO); + ctx, util::to_string_view(rewritten_stdout), STDOUT_FILENO); return *result_key; } @@ -1416,13 +1514,35 @@ get_result_key_from_cpp(Context& ctx, util::Args& args, Hash& hash) args.push_back("-C"); } + fs::path msvc_deps_file; + if (ctx.config.is_compiler_group_msvc()) { + // For MSVC: use /sourceDependencies (MSVC 2017 15.7+). For others: use + // /showIncludes since only MSVC supports /sourceDependencies. + if (ctx.config.compiler_type() == CompilerType::msvc) { + // Use existing file from command line if present, otherwise create one + // ourselves. + if (!ctx.args_info.output_sd.empty()) { + msvc_deps_file = ctx.args_info.output_sd; + } else { + auto tmp_deps = + util::value_or_throw(util::TemporaryFile::create( + FMT("{}/source_deps", ctx.config.temporary_dir()), ".json")); + msvc_deps_file = tmp_deps.path; + tmp_deps.fd.close(); + ctx.register_pending_tmp_file(msvc_deps_file); + + args.push_back("/sourceDependencies"); + args.push_back(msvc_deps_file); + } + } else { + args.push_back("/showIncludes"); + } + } + // Send preprocessor output to a file instead of stdout to work around // compilers that don't exit with a proper status on write error to stdout. // See also . if (ctx.config.is_compiler_group_msvc()) { - if (ctx.config.msvc_utf8()) { - args.push_back("-utf-8"); // Avoid garbling filenames in output - } args.push_back("-P"); args.push_back(FMT("-Fi{}", preprocessed_path)); } else { @@ -1438,28 +1558,40 @@ get_result_key_from_cpp(Context& ctx, util::Args& args, Hash& hash) add_prefix(ctx, args, ctx.config.prefix_command_cpp()); LOG_RAW("Running preprocessor"); - const auto result = do_execute(ctx, args, capture_stdout); + auto result = do_execute(ctx, args, capture_stdout); args.pop_back(args.size() - orig_args_size); if (!result) { return tl::unexpected(result.error()); - } else if (result->exit_status != 0) { + } + + if (result->exit_status != 0) { LOG("Preprocessor gave exit status {}", result->exit_status); return tl::unexpected(Statistic::preprocessor_error); } - cpp_stderr_data = result->stderr_data; - cpp_stdout_data = result->stdout_data; + if (!msvc_deps_file.empty() && !fs::exists(msvc_deps_file)) { + LOG_RAW( + "Compiler doesn't support /sourceDependencies, extracting includes" + " from preprocessed output instead"); + msvc_deps_file.clear(); + // The compiler likely printed "cl : Command line warning D9002 : ignoring + // unknown option '/sourceDependencies'" to stdout, but no need to scrub + // that since we don't hash stdout and we don't send it to the console + // either. + } + + cpp_stderr_data = std::move(result->stderr_data); + cpp_stdout_data = std::move(result->stdout_data); - if (ctx.config.is_compiler_group_msvc() && ctx.config.msvc_utf8()) { - // Check that usage of -utf-8 didn't garble the preprocessor output. - static constexpr char warning_c4828[] = - "warning C4828: The file contains a character starting at offset"; - if (util::to_string_view(cpp_stderr_data).find(warning_c4828) - != std::string_view::npos) { - LOG_RAW("Non-UTF-8 source code unsupported in preprocessor mode"); - return tl::unexpected(Statistic::unsupported_source_encoding); - } + if (ctx.config.is_compiler_group_msvc()) { + if (ctx.config.compiler_type() != CompilerType::msvc) { + TRY(extract_includes_from_msvc_stdout( + ctx, hash, util::to_string_view(cpp_stdout_data))); + } else if (!msvc_deps_file.empty()) { + TRY(extract_includes_from_msvc_source_deps_file( + ctx, hash, msvc_deps_file)); + } // else: extract includes from preprocessed output } } @@ -1473,13 +1605,15 @@ get_result_key_from_cpp(Context& ctx, util::Args& args, Hash& hash) for (size_t i = 0; i < chunks.size(); ++i) { TRY(process_cuda_chunk(ctx, hash, chunks[i], i)); } - } else { hash.hash_delimiter("cpp"); - TRY(process_preprocessed_file(ctx, hash, preprocessed_path)); } + if (getenv("CCACHE_DEBUG_INCLUDED")) { + print_included_files(ctx, stdout); + } + hash.hash_delimiter("cppstderr"); hash.hash(util::to_string_view(cpp_stderr_data)); @@ -2835,10 +2969,9 @@ do_cache_compilation(Context& ctx) } } - if (ctx.config.depend_mode() - && !(ctx.args_info.generating_dependencies - || ctx.args_info.generating_includes)) { - LOG_RAW("Disabling depend mode"); + if (ctx.config.depend_mode() && !ctx.config.is_compiler_group_msvc() + && !ctx.args_info.generating_dependencies) { + LOG_RAW("Disabling depend mode since dependency file isn't generated"); ctx.config.set_depend_mode(false); } diff --git a/src/ccache/compiler/msvc.cpp b/src/ccache/compiler/msvc.cpp index 05eec4c2..f563e927 100644 --- a/src/ccache/compiler/msvc.cpp +++ b/src/ccache/compiler/msvc.cpp @@ -19,6 +19,7 @@ #include "msvc.hpp" #include +#include #include namespace compiler { @@ -49,29 +50,11 @@ get_includes_from_msvc_show_includes(std::string_view file_content, return result; } -util::Bytes -strip_includes_from_msvc_show_includes(const Context& ctx, - util::Bytes&& stdout_data) +tl::expected, std::string> +get_includes_from_msvc_source_deps(std::string_view json_content) { - using util::Tokenizer; - using Mode = Tokenizer::Mode; - using IncludeDelimiter = Tokenizer::IncludeDelimiter; - - if (stdout_data.empty() || !ctx.auto_depend_mode - || ctx.config.compiler_type() != CompilerType::msvc) { - return std::move(stdout_data); - } - - util::Bytes new_stdout_data; - for (const auto line : Tokenizer(util::to_string_view(stdout_data), - "\n", - Mode::include_empty, - IncludeDelimiter::yes)) { - if (!util::starts_with(line, ctx.config.msvc_dep_prefix())) { - new_stdout_data.insert(new_stdout_data.end(), line.data(), line.size()); - } - } - return new_stdout_data; + util::SimpleJsonParser parser(json_content); + return parser.get_string_array(".Data.Includes"); } } // namespace compiler diff --git a/src/ccache/compiler/msvc.hpp b/src/ccache/compiler/msvc.hpp index 34da6fd8..9324bf7a 100644 --- a/src/ccache/compiler/msvc.hpp +++ b/src/ccache/compiler/msvc.hpp @@ -18,8 +18,11 @@ #pragma once +#include #include +#include + #include #include @@ -31,7 +34,7 @@ std::vector get_includes_from_msvc_show_includes(std::string_view file_content, std::string_view prefix); -util::Bytes strip_includes_from_msvc_show_includes(const Context& ctx, - util::Bytes&& stdout_data); +tl::expected, std::string> +get_includes_from_msvc_source_deps(std::string_view json_content); } // namespace compiler diff --git a/src/ccache/config.cpp b/src/ccache/config.cpp index 95091d2a..dcf7c17d 100644 --- a/src/ccache/config.cpp +++ b/src/ccache/config.cpp @@ -101,7 +101,6 @@ enum class ConfigItem : uint8_t { max_files, max_size, msvc_dep_prefix, - msvc_utf8, namespace_, path, pch_external_checksum, @@ -158,7 +157,6 @@ const std::unordered_map {"max_files", {ConfigItem::max_files} }, {"max_size", {ConfigItem::max_size} }, {"msvc_dep_prefix", {ConfigItem::msvc_dep_prefix} }, - {"msvc_utf8", {ConfigItem::msvc_utf8} }, {"namespace", {ConfigItem::namespace_} }, {"path", {ConfigItem::path} }, {"pch_external_checksum", {ConfigItem::pch_external_checksum} }, @@ -209,7 +207,6 @@ const std::unordered_map {"MAXFILES", "max_files" }, {"MAXSIZE", "max_size" }, {"MSVC_DEP_PREFIX", "msvc_dep_prefix" }, - {"MSVC_UTF8", "msvc_utf8" }, {"NAMESPACE", "namespace" }, {"PATH", "path" }, {"PCH_EXTSUM", "pch_external_checksum" }, @@ -874,9 +871,6 @@ Config::get_string_value(const std::string& key) const case ConfigItem::msvc_dep_prefix: return m_msvc_dep_prefix; - case ConfigItem::msvc_utf8: - return format_bool(m_msvc_utf8); - case ConfigItem::namespace_: return m_namespace; @@ -1155,10 +1149,6 @@ Config::set_item(const std::string_view& key, m_msvc_dep_prefix = value; break; - case ConfigItem::msvc_utf8: - m_msvc_utf8 = parse_bool(value, env_var_key, negate); - break; - case ConfigItem::namespace_: m_namespace = value; break; diff --git a/src/ccache/config.hpp b/src/ccache/config.hpp index 6351d073..83cd77ec 100644 --- a/src/ccache/config.hpp +++ b/src/ccache/config.hpp @@ -85,7 +85,6 @@ public: uint64_t max_files() const; uint64_t max_size() const; const std::string& msvc_dep_prefix() const; - bool msvc_utf8() const; const std::string& path() const; bool pch_external_checksum() const; const std::string& prefix_command() const; @@ -127,7 +126,6 @@ public: void set_inode_cache(bool value); void set_max_files(uint64_t value); void set_msvc_dep_prefix(const std::string& value); - void set_msvc_utf8(bool value); void set_temporary_dir(const std::filesystem::path& value); // Where to write configuration changes. @@ -208,7 +206,6 @@ private: uint64_t m_max_files = 0; uint64_t m_max_size = 5ULL * 1024 * 1024 * 1024; std::string m_msvc_dep_prefix = "Note: including file:"; - bool m_msvc_utf8 = true; std::string m_path; bool m_pch_external_checksum = false; std::string m_prefix_command; @@ -434,12 +431,6 @@ Config::msvc_dep_prefix() const return m_msvc_dep_prefix; } -inline bool -Config::msvc_utf8() const -{ - return m_msvc_utf8; -} - inline const std::string& Config::path() const { @@ -638,12 +629,6 @@ Config::set_msvc_dep_prefix(const std::string& value) m_msvc_dep_prefix = value; } -inline void -Config::set_msvc_utf8(bool value) -{ - m_msvc_utf8 = value; -} - inline void Config::set_temporary_dir(const std::filesystem::path& value) { diff --git a/src/ccache/context.hpp b/src/ccache/context.hpp index be9b8e9d..99fce912 100644 --- a/src/ccache/context.hpp +++ b/src/ccache/context.hpp @@ -111,10 +111,6 @@ public: // `nullopt` if there is no such configuration. std::optional original_umask; - // Whether we have added "/showIncludes" ourselves since it's missing and - // depend mode is enabled. - bool auto_depend_mode = false; - // Register a temporary file to remove at program exit. void register_pending_tmp_file(const std::filesystem::path& path); diff --git a/src/ccache/core/resultretriever.cpp b/src/ccache/core/resultretriever.cpp index 8451d602..85bb8a96 100644 --- a/src/ccache/core/resultretriever.cpp +++ b/src/ccache/core/resultretriever.cpp @@ -67,11 +67,7 @@ ResultRetriever::on_embedded_file(uint8_t file_number, data.size()); if (file_type == FileType::stdout_output) { - core::send_to_console( - m_ctx, - util::to_string_view( - compiler::strip_includes_from_msvc_show_includes(m_ctx, data)), - STDOUT_FILENO); + core::send_to_console(m_ctx, util::to_string_view(data), STDOUT_FILENO); } else if (file_type == FileType::stderr_output) { core::send_to_console(m_ctx, util::to_string_view(data), STDERR_FILENO); } else { diff --git a/src/ccache/core/statistic.hpp b/src/ccache/core/statistic.hpp index 0b08e704..e6a3a46f 100644 --- a/src/ccache/core/statistic.hpp +++ b/src/ccache/core/statistic.hpp @@ -82,8 +82,9 @@ enum class Statistic { bad_input_file = 82, modified_input_file = 83, unsupported_source_encoding = 84, + unsupported_compiler = 85, - END = 85, + END = 86, }; enum class StatisticsFormat { diff --git a/src/ccache/core/statistics.cpp b/src/ccache/core/statistics.cpp index 2ea938d6..d12eda3d 100644 --- a/src/ccache/core/statistics.cpp +++ b/src/ccache/core/statistics.cpp @@ -263,6 +263,9 @@ const StatisticsField k_statistics_fields[] = { "Unsupported source language", FLAG_UNCACHEABLE), + // Compiler type or version was not supported. + FIELD(unsupported_compiler, "Unsupported compiler", FLAG_UNCACHEABLE), + // subdir_files_base and subdir_size_kibibyte_base are intentionally omitted // since they are not interesting to show. }; diff --git a/test/msvc/test_basic_functionality.py b/test/msvc/test_basic_functionality.py index af73bbb7..09ca317b 100644 --- a/test/msvc/test_basic_functionality.py +++ b/test/msvc/test_basic_functionality.py @@ -53,3 +53,126 @@ def test_basedir_normalizes_paths(ccache_test): stats_2 = ccache_test.stats() assert stats_2["miss"] == 1 assert stats_2["total_hit"] == 1 + + +def test_source_dependencies_output(ccache_test): + command = ["/c", "test.c", "/sourceDependencies", "sourceDependencies.json"] + + header = ccache_test.workdir / "myheader.h" + header.write_text( + """ +#define GREETING "Hello from header" +""" + ) + + source = ccache_test.workdir / "test.c" + source.write_text( + """ +#include "myheader.h" +#include +int main(void) { + printf(GREETING "\\n"); + return 0; +} +""" + ) + + ccache_test.compile(command) + stats_1 = ccache_test.stats() + assert stats_1["miss"] == 1 + + source_deps = ccache_test.workdir / "sourceDependencies.json" + test_obj = ccache_test.workdir / "test.obj" + assert source_deps.exists() + test_obj.unlink() + source_deps.unlink() + + ccache_test.compile(command) + stats_2 = ccache_test.stats() + assert stats_2["total_hit"] == 1 + assert test_obj.exists() + assert source_deps.exists() + + +def test_preprocessor_mode_detects_header_changes(ccache_test): + # Force preprocessor mode. + ccache_test.env["CCACHE_NODEPEND"] = "1" + ccache_test.env["CCACHE_NODIRECT"] = "1" + + command = ["/c", "test.c"] + + header = ccache_test.workdir / "test.h" + header.write_text("int x;\n") + + source = ccache_test.workdir / "test.c" + source.write_text('#include "test.h"\n') + + test_obj = ccache_test.workdir / "test.obj" + + ccache_test.compile(command) + stats_1 = ccache_test.stats() + assert stats_1["total_hit"] == 0 + assert stats_1["miss"] == 1 + test_obj.unlink() + + ccache_test.compile(command) + stats_2 = ccache_test.stats() + assert stats_2["total_hit"] == 1 + assert stats_2["miss"] == 1 + test_obj.unlink() + + header.write_text("int y;\n") + + ccache_test.compile(command) + stats_3 = ccache_test.stats() + assert stats_3["miss"] == 2 + assert stats_3["total_hit"] == 1 + + ccache_test.compile(command) + stats_4 = ccache_test.stats() + assert stats_4["miss"] == 2 + assert stats_4["total_hit"] == 2 + + +def test_depend_mode_with_source_dependencies(ccache_test): + ccache_test.env["CCACHE_DEPEND"] = "1" + + command = ["/c", "test.c"] + + header = ccache_test.workdir / "test.h" + header.write_text('#define VERSION "1.0"\n') + + # Create source that includes the header + source = ccache_test.workdir / "test.c" + source.write_text("""\ +#include "test.h" +#include +int main(void) { + printf("Version: " VERSION "\\n"); + return 0; +} +""") + + test_obj = ccache_test.workdir / "test.obj" + + ccache_test.compile(command) + stats_1 = ccache_test.stats() + assert stats_1["miss"] == 1 + test_obj.unlink() + + ccache_test.compile(command) + stats_2 = ccache_test.stats() + assert stats_2["total_hit"] == 1 + test_obj.unlink() + + header.write_text('#define VERSION "2.0"\n') + + ccache_test.compile(command) + stats_3 = ccache_test.stats() + assert stats_3["miss"] == 2 + assert stats_3["total_hit"] == 1 + + ccache_test.compile(command) + stats_4 = ccache_test.stats() + assert stats_4["miss"] == 2 + assert stats_4["total_hit"] == 2 diff --git a/unittest/test_compiler_msvc.cpp b/unittest/test_compiler_msvc.cpp index 74c2241f..b82d8d93 100644 --- a/unittest/test_compiler_msvc.cpp +++ b/unittest/test_compiler_msvc.cpp @@ -24,7 +24,7 @@ #include -static const std::string defaultPrefix = "Note: including file:"; +const std::string_view k_default_prefix = "Note: including file:"; TEST_SUITE_BEGIN("msvc"); @@ -33,8 +33,8 @@ TEST_CASE("get_includes_from_msvc_show_includes") SUBCASE("Parse empty output") { std::string contents; - const auto result = - compiler::get_includes_from_msvc_show_includes(contents, defaultPrefix); + const auto result = compiler::get_includes_from_msvc_show_includes( + contents, k_default_prefix); CHECK(result.size() == 0); } @@ -47,8 +47,8 @@ Note: including file: F:\Projects\ccache\src\Args.hpp Note: including file: F:\Projects\ccache\src\NonCopyable.hpp Note: including file: C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.33.31629\include\deque )"; - const auto result = - compiler::get_includes_from_msvc_show_includes(contents, defaultPrefix); + const auto result = compiler::get_includes_from_msvc_show_includes( + contents, k_default_prefix); REQUIRE(result.size() == 5); CHECK(result[0] == "F:/Projects/ccache/build-msvc/config.h"); CHECK(result[1] == R"(F:\Projects\ccache\unittest\../src/Context.hpp)"); @@ -64,8 +64,8 @@ Note: including file: C:\Program Files\Microsoft Visual Studio\2022\Community\ std::string contents = "Note: including file: foo\r\n" "Note: including file: bar\r\n"; - const auto result = - compiler::get_includes_from_msvc_show_includes(contents, defaultPrefix); + const auto result = compiler::get_includes_from_msvc_show_includes( + contents, k_default_prefix); REQUIRE(result.size() == 2); CHECK(result[0] == "foo"); CHECK(result[1] == "bar"); @@ -77,8 +77,8 @@ Note: including file: C:\Program Files\Microsoft Visual Studio\2022\Community\ "Note: including file: foo\n" "Note: including file: \n" "Note: including file: bar\n"; - const auto result = - compiler::get_includes_from_msvc_show_includes(contents, defaultPrefix); + const auto result = compiler::get_includes_from_msvc_show_includes( + contents, k_default_prefix); REQUIRE(result.size() == 2); CHECK(result[0] == "foo"); CHECK(result[1] == "bar"); @@ -97,79 +97,117 @@ Just a line with custom in the middle)"; } } -TEST_CASE("strip_includes_from_msvc_show_includes") +TEST_CASE("get_includes_from_msvc_source_deps") { - Context ctx; - const util::Bytes input = util::to_span( - "First\n" - "Note: including file: foo\n" - "Second\n"); - - SUBCASE("Empty output") + SUBCASE("Simple case") { - const util::Bytes result = - compiler::strip_includes_from_msvc_show_includes(ctx, {}); - CHECK(result.size() == 0); + std::string json = R"({ + "Version": "1.1", + "Data": { + "Source": "C:\\path\\to\\source.cpp", + "Includes": [ + "C:\\path\\to\\header1.h", + "C:\\path\\to\\header2.h" + ] + } +})"; + + auto includes_res = compiler::get_includes_from_msvc_source_deps(json); + REQUIRE(includes_res); + const auto& includes = *includes_res; + REQUIRE(includes.size() == 2); + CHECK(includes[0] == "C:\\path\\to\\header1.h"); + CHECK(includes[1] == "C:\\path\\to\\header2.h"); } - SUBCASE("Feature disabled") + SUBCASE("Empty includes array") { - const util::Bytes result = - compiler::strip_includes_from_msvc_show_includes(ctx, util::Bytes(input)); - CHECK(result == input); + std::string json = R"({ + "Version": "1.1", + "Data": { + "Source": "C:\\path\\to\\source.cpp", + "Includes": [] } +})"; - ctx.auto_depend_mode = true; + auto includes_res = compiler::get_includes_from_msvc_source_deps(json); + REQUIRE(includes_res); + CHECK(includes_res->empty()); + } - SUBCASE("Wrong compiler") + SUBCASE("Escaped paths") { - const util::Bytes result = - compiler::strip_includes_from_msvc_show_includes(ctx, util::Bytes(input)); - CHECK(result == input); + std::string json = R"({ + "Version": "1.1", + "Data": { + "Source": "C:\\path\\to\\source.cpp", + "Includes": [ + "C:\\path\\to\\header\"with\"quotes.h", + "C:\\path\\to\\header\\with\\backslashes.h" + ] + } +})"; + + auto includes_res = compiler::get_includes_from_msvc_source_deps(json); + REQUIRE(includes_res); + const auto& includes = *includes_res; + REQUIRE(includes.size() == 2); + CHECK(includes[0] == "C:\\path\\to\\header\"with\"quotes.h"); + CHECK(includes[1] == "C:\\path\\to\\header\\with\\backslashes.h"); } - ctx.config.set_compiler_type(CompilerType::msvc); - - SUBCASE("Simple output") + SUBCASE("Minified JSON") { - const util::Bytes result = - compiler::strip_includes_from_msvc_show_includes(ctx, util::Bytes(input)); - CHECK(result == util::to_span("First\nSecond\n")); + std::string json = + R"({"Version":"1.1","Data":{"Source":"C:\\source.cpp","Includes":["C:\\header1.h","C:\\header2.h"]}})"; + + auto includes_res = compiler::get_includes_from_msvc_source_deps(json); + REQUIRE(includes_res); + const auto& includes = *includes_res; + REQUIRE(includes.size() == 2); + CHECK(includes[0] == "C:\\header1.h"); + CHECK(includes[1] == "C:\\header2.h"); } - SUBCASE("Empty lines") + SUBCASE("UTF-8 paths") { - const util::Bytes result = compiler::strip_includes_from_msvc_show_includes( - ctx, - util::to_span("First\n" - "\n" - "Note: including file: foo\n" - "\n" - "Second\n" - "\n")); - CHECK(result == util::to_span("First\n\n\nSecond\n\n")); - } - - SUBCASE("CRLF") + std::string json = R"({ + "Version": "1.1", + "Data": { + "Source": "C:\\日本語\\source.cpp", + "Includes": [ + "C:\\日本語\\header1.h", + "C:\\Ελληνικά\\header2.h" + ] + } +})"; + + auto includes_res = compiler::get_includes_from_msvc_source_deps(json); + REQUIRE(includes_res); + const auto& includes = *includes_res; + REQUIRE(includes.size() == 2); + CHECK(includes[0] == "C:\\日本語\\header1.h"); + CHECK(includes[1] == "C:\\Ελληνικά\\header2.h"); + } + + SUBCASE("Invalid JSON") { - const util::Bytes result = compiler::strip_includes_from_msvc_show_includes( - ctx, - util::to_span("First\r\n" - "Note: including file: foo\r\n" - "Second\r\n")); - CHECK(result == util::to_span("First\r\nSecond\r\n")); + auto includes_res = + compiler::get_includes_from_msvc_source_deps("not json"); + REQUIRE(!includes_res); + CHECK(includes_res.error().find("Expected object") != std::string::npos); } - SUBCASE("Custom prefix") + SUBCASE("Unicode escape sequences are rejected") { - ctx.config.set_msvc_dep_prefix("custom"); - const util::Bytes result = compiler::strip_includes_from_msvc_show_includes( - ctx, - util::to_span("First\n" - "custom: including file: foo\n" - "Second\n" - "Third custom line\n")); - CHECK(result == util::to_span("First\nSecond\nThird custom line\n")); + std::string json = R"({ + "Version": "1.1", + "Data": { + "Includes": ["C:\\path\\to\\\u65E5\u672C\u8A9E.h"] + } +})"; + auto includes_res = compiler::get_includes_from_msvc_source_deps(json); + CHECK_FALSE(includes_res); } } diff --git a/unittest/test_config.cpp b/unittest/test_config.cpp index 6624827a..572966d9 100644 --- a/unittest/test_config.cpp +++ b/unittest/test_config.cpp @@ -72,7 +72,6 @@ TEST_CASE("Config: default values") CHECK(config.max_files() == 0); CHECK(config.max_size() == static_cast(5) * 1024 * 1024 * 1024); CHECK(config.msvc_dep_prefix() == "Note: including file:"); - CHECK(config.msvc_utf8()); CHECK(config.path().empty()); CHECK_FALSE(config.pch_external_checksum()); CHECK(config.prefix_command().empty()); @@ -134,7 +133,6 @@ TEST_CASE("Config::update_from_file") "max_files = 17\n" "max_size = 123M\n" "msvc_dep_prefix = Some other prefix:\n" - "msvc_utf8 = false\n" "path = $USER.x\n" "pch_external_checksum = true\n" "prefix_command = x$USER\n" @@ -180,7 +178,6 @@ TEST_CASE("Config::update_from_file") CHECK(config.max_files() == 17); CHECK(config.max_size() == 123 * 1000 * 1000); CHECK(config.msvc_dep_prefix() == "Some other prefix:"); - CHECK_FALSE(config.msvc_utf8()); CHECK(config.path() == FMT("{}.x", user)); CHECK(config.pch_external_checksum()); CHECK(config.prefix_command() == FMT("x{}", user)); @@ -620,7 +617,6 @@ TEST_CASE("Config::visit_items") "max_files = 4711\n" "max_size = 98.7M\n" "msvc_dep_prefix = mdp\n" - "msvc_utf8 = true\n" "namespace = ns\n" "path = p\n" "pch_external_checksum = true\n" @@ -684,7 +680,6 @@ TEST_CASE("Config::visit_items") "(test.conf) max_files = 4711", "(test.conf) max_size = 98.7 MB", "(test.conf) msvc_dep_prefix = mdp", - "(test.conf) msvc_utf8 = true", "(test.conf) namespace = ns", "(test.conf) path = p", "(test.conf) pch_external_checksum = true", -- 2.47.3