From 14762f84e30f9da36419225e6688f9b88e50a80c Mon Sep 17 00:00:00 2001 From: Max Winkler Date: Tue, 22 Oct 2024 09:42:32 -0700 Subject: [PATCH] feat: Add response_file_format config option for rsp file format (#1522) --- doc/MANUAL.adoc | 17 +++++++++++++ src/ccache/argprocessing.cpp | 7 ++---- src/ccache/args.cpp | 7 ++++++ src/ccache/args.hpp | 12 +++------ src/ccache/config.cpp | 37 +++++++++++++++++++++++++++ src/ccache/config.hpp | 22 +++++++++++++++++ unittest/test_args.cpp | 25 ++++++++++--------- unittest/test_config.cpp | 48 ++++++++++++++++++++++++++++++++++++ 8 files changed, 150 insertions(+), 25 deletions(-) diff --git a/doc/MANUAL.adoc b/doc/MANUAL.adoc index b11ada7f..7e8b9ed6 100644 --- a/doc/MANUAL.adoc +++ b/doc/MANUAL.adoc @@ -991,6 +991,23 @@ NOTE: In previous ccache versions this option was called *secondary_storage* If true, ccache will write results to remote storage even for local storage cache hits. The default is false. +[#config_response_file_format] +*response_file_format* (*CCACHE_RESPONSE_FILE_FORMAT*):: + + Ccache normally guesses the response file format based on the compiler type. The + *response_file_format* option lets you force the response file quoting behavior. + This can be useful if the compiler supports both posix and windows response file + quoting. Possible values are: ++ +-- +*auto*:: + Guess one of the formats below based on the compiler type. This is the default. +*posix*:: + Posix quoting behavior. +*windows*:: + Windows quoting behavior. +-- + [#config_run_second_cpp] *run_second_cpp* (*CCACHE_CPP2* or *CCACHE_NOCPP2*, see _<>_ above):: diff --git a/src/ccache/argprocessing.cpp b/src/ccache/argprocessing.cpp index 012ee43b..bb918847 100644 --- a/src/ccache/argprocessing.cpp +++ b/src/ccache/argprocessing.cpp @@ -399,10 +399,7 @@ process_option_arg(const Context& ctx, if (argpath[-1] == '-') { ++argpath; } - auto file_args = Args::from_atfile(argpath, - config.is_compiler_group_msvc() - ? Args::AtFileFormat::msvc - : Args::AtFileFormat::gcc); + auto file_args = Args::from_atfile(argpath, config.atfile_format()); if (!file_args) { LOG("Couldn't read arg file {}", argpath); return Statistic::bad_compiler_arguments; @@ -425,7 +422,7 @@ process_option_arg(const Context& ctx, // Argument is a comma-separated list of files. auto paths = util::split_into_strings(args[i], ","); for (auto it = paths.rbegin(); it != paths.rend(); ++it) { - auto file_args = Args::from_atfile(*it); + auto file_args = Args::from_atfile(*it, AtFileFormat::gcc); if (!file_args) { LOG("Couldn't read CUDA options file {}", *it); return Statistic::bad_compiler_arguments; diff --git a/src/ccache/args.cpp b/src/ccache/args.cpp index ff770489..8e7a42f0 100644 --- a/src/ccache/args.cpp +++ b/src/ccache/args.cpp @@ -18,7 +18,9 @@ #include "args.hpp" +#include #include +#include #include #include #include @@ -48,6 +50,8 @@ Args::from_string(std::string_view command) std::optional Args::from_atfile(const std::string& filename, AtFileFormat format) { + ASSERT(format != AtFileFormat::auto_guess); + const auto argtext = util::read_file(filename); if (!argtext) { LOG("Failed to read atfile {}: {}", filename, argtext.error()); @@ -68,6 +72,9 @@ Args::from_atfile(const std::string& filename, AtFileFormat format) switch (*pos) { case '\\': switch (format) { + case AtFileFormat::auto_guess: + ASSERT(false); // Can't happen + break; case AtFileFormat::gcc: pos++; if (*pos == '\0') { diff --git a/src/ccache/args.hpp b/src/ccache/args.hpp index 0f0158c4..325337e5 100644 --- a/src/ccache/args.hpp +++ b/src/ccache/args.hpp @@ -25,14 +25,11 @@ #include #include +enum class AtFileFormat; + class Args { public: - enum class AtFileFormat { - gcc, // '\'' and '"' quote, '\\' escapes any character - msvc, // '"' quotes, '\\' escapes only '"' and '\\' - }; - Args() = default; Args(const Args& other) = default; Args(Args&& other) noexcept; @@ -40,9 +37,8 @@ public: static Args from_argv(int argc, const char* const* argv); static Args from_string(std::string_view command); - static std::optional - from_atfile(const std::string& filename, - AtFileFormat format = AtFileFormat::gcc); + static std::optional from_atfile(const std::string& filename, + AtFileFormat format); Args& operator=(const Args& other) = default; Args& operator=(Args&& other) noexcept; diff --git a/src/ccache/config.cpp b/src/ccache/config.cpp index 193b85a5..ce10d237 100644 --- a/src/ccache/config.cpp +++ b/src/ccache/config.cpp @@ -111,6 +111,7 @@ enum class ConfigItem { remote_only, remote_storage, reshare, + response_file_format, run_second_cpp, sloppiness, stats, @@ -167,6 +168,7 @@ const std::unordered_map k_config_key_table = {"remote_only", {ConfigItem::remote_only}}, {"remote_storage", {ConfigItem::remote_storage}}, {"reshare", {ConfigItem::reshare}}, + {"response_file_format", {ConfigItem::response_file_format}}, {"run_second_cpp", {ConfigItem::run_second_cpp}}, {"secondary_storage", {ConfigItem::remote_storage, "remote_storage"}}, {"sloppiness", {ConfigItem::sloppiness}}, @@ -217,6 +219,7 @@ const std::unordered_map k_env_variable_table = { {"REMOTE_ONLY", "remote_only"}, {"REMOTE_STORAGE", "remote_storage"}, {"RESHARE", "reshare"}, + {"RESPONSE_FILE_FORMAT", "response_file_format"}, {"SECONDARY_STORAGE", "remote_storage"}, // Alias for CCACHE_REMOTE_STORAGE {"SLOPPINESS", "sloppiness"}, {"STATS", "stats"}, @@ -225,6 +228,18 @@ const std::unordered_map k_env_variable_table = { {"UMASK", "umask"}, }; +AtFileFormat +parse_response_file_format(const std::string& value) +{ + if (value == "windows") { + return AtFileFormat::msvc; + } else if (value == "posix") { + return AtFileFormat::gcc; + } else { + return AtFileFormat::auto_guess; + } +} + bool parse_bool(const std::string& value, const std::optional env_var_key, @@ -525,6 +540,21 @@ home_directory() #endif } +std::string +atfile_format_to_string(AtFileFormat atfile_format) +{ + switch (atfile_format) { + case AtFileFormat::auto_guess: + return "auto"; + case AtFileFormat::gcc: + return "posix"; + case AtFileFormat::msvc: + return "windows"; + } + + ASSERT(false); +} + std::string compiler_type_to_string(CompilerType compiler_type) { @@ -876,6 +906,9 @@ Config::get_string_value(const std::string& key) const case ConfigItem::reshare: return format_bool(m_reshare); + case ConfigItem::response_file_format: + return atfile_format_to_string(m_atfile_format); + case ConfigItem::run_second_cpp: return format_bool(m_run_second_cpp); @@ -1143,6 +1176,10 @@ Config::set_item(const std::string& key, m_reshare = parse_bool(value, env_var_key, negate); break; + case ConfigItem::response_file_format: + m_atfile_format = parse_response_file_format(value); + break; + case ConfigItem::run_second_cpp: m_run_second_cpp = parse_bool(value, env_var_key, negate); break; diff --git a/src/ccache/config.hpp b/src/ccache/config.hpp index 74d2b3ec..4e8a60f3 100644 --- a/src/ccache/config.hpp +++ b/src/ccache/config.hpp @@ -43,6 +43,12 @@ enum class CompilerType { other }; +enum class AtFileFormat { + gcc, // '\'' and '"' quote, '\\' escapes any character + msvc, // '"' quotes, '\\' escapes only '"' and '\\' + auto_guess, +}; + std::string compiler_type_to_string(CompilerType compiler_type); class Config : util::NonCopyable @@ -53,6 +59,7 @@ public: void read(const std::vector& cmdline_config_settings = {}); bool absolute_paths_in_stderr() const; + AtFileFormat atfile_format() const; const std::filesystem::path& base_dir() const; const std::filesystem::path& cache_dir() const; const std::string& compiler() const; @@ -168,6 +175,7 @@ private: std::filesystem::path m_system_config_path; bool m_absolute_paths_in_stderr = false; + AtFileFormat m_atfile_format = AtFileFormat::auto_guess; std::filesystem::path m_base_dir; std::filesystem::path m_cache_dir; std::string m_compiler; @@ -236,6 +244,20 @@ Config::absolute_paths_in_stderr() const return m_absolute_paths_in_stderr; } +inline AtFileFormat +Config::atfile_format() const +{ + if (m_atfile_format != AtFileFormat::auto_guess) { + return m_atfile_format; + } + + if (is_compiler_group_msvc()) { + return AtFileFormat::msvc; + } + + return AtFileFormat::gcc; +} + inline const std::filesystem::path& Config::base_dir() const { diff --git a/unittest/test_args.cpp b/unittest/test_args.cpp index b1bb6f63..e9dc3bf2 100644 --- a/unittest/test_args.cpp +++ b/unittest/test_args.cpp @@ -19,6 +19,7 @@ #include "testutil.hpp" #include +#include #include #include @@ -86,20 +87,20 @@ TEST_CASE("Args::from_atfile") SUBCASE("Nonexistent file") { - CHECK(Args::from_atfile("at_file") == std::nullopt); + CHECK(Args::from_atfile("at_file", AtFileFormat::gcc) == std::nullopt); } SUBCASE("Empty") { util::write_file("at_file", ""); - args = *Args::from_atfile("at_file"); + args = *Args::from_atfile("at_file", AtFileFormat::gcc); CHECK(args.size() == 0); } SUBCASE("One argument without newline") { util::write_file("at_file", "foo"); - args = *Args::from_atfile("at_file"); + args = *Args::from_atfile("at_file", AtFileFormat::gcc); CHECK(args.size() == 1); CHECK(args[0] == "foo"); } @@ -107,7 +108,7 @@ TEST_CASE("Args::from_atfile") SUBCASE("One argument with newline") { util::write_file("at_file", "foo\n"); - args = *Args::from_atfile("at_file"); + args = *Args::from_atfile("at_file", AtFileFormat::gcc); CHECK(args.size() == 1); CHECK(args[0] == "foo"); } @@ -115,7 +116,7 @@ TEST_CASE("Args::from_atfile") SUBCASE("Multiple simple arguments") { util::write_file("at_file", "x y z\n"); - args = *Args::from_atfile("at_file"); + args = *Args::from_atfile("at_file", AtFileFormat::gcc); CHECK(args.size() == 3); CHECK(args[0] == "x"); CHECK(args[1] == "y"); @@ -128,7 +129,7 @@ TEST_CASE("Args::from_atfile") "at_file", "first\rsec\\\tond\tthi\\\\rd\nfourth \tfif\\ th \"si'x\\\" th\"" " 'seve\nth'\\"); - args = *Args::from_atfile("at_file"); + args = *Args::from_atfile("at_file", AtFileFormat::gcc); CHECK(args.size() == 7); CHECK(args[0] == "first"); CHECK(args[1] == "sec\tond"); @@ -142,7 +143,7 @@ TEST_CASE("Args::from_atfile") SUBCASE("Ignore single quote in MSVC format") { util::write_file("at_file", "'a b'"); - args = *Args::from_atfile("at_file", Args::AtFileFormat::msvc); + args = *Args::from_atfile("at_file", AtFileFormat::msvc); CHECK(args.size() == 2); CHECK(args[0] == "'a"); CHECK(args[1] == "b'"); @@ -151,7 +152,7 @@ TEST_CASE("Args::from_atfile") SUBCASE("Backslash as directory separator in MSVC format") { util::write_file("at_file", R"("-DDIRSEP='A\B\C'")"); - args = *Args::from_atfile("at_file", Args::AtFileFormat::msvc); + args = *Args::from_atfile("at_file", AtFileFormat::msvc); CHECK(args.size() == 1); CHECK(args[0] == R"(-DDIRSEP='A\B\C')"); } @@ -159,7 +160,7 @@ TEST_CASE("Args::from_atfile") SUBCASE("Backslash before quote in MSVC format") { util::write_file("at_file", R"(/Fo"N.dir\Release\\")"); - args = *Args::from_atfile("at_file", Args::AtFileFormat::msvc); + args = *Args::from_atfile("at_file", AtFileFormat::msvc); CHECK(args.size() == 1); CHECK(args[0] == R"(/FoN.dir\Release\)"); } @@ -167,7 +168,7 @@ TEST_CASE("Args::from_atfile") SUBCASE("Arguments on multiple lines in MSVC format") { util::write_file("at_file", "a\nb"); - args = *Args::from_atfile("at_file", Args::AtFileFormat::msvc); + args = *Args::from_atfile("at_file", AtFileFormat::msvc); CHECK(args.size() == 2); CHECK(args[0] == "a"); CHECK(args[1] == "b"); @@ -180,7 +181,7 @@ TEST_CASE("Args::from_atfile") R"(\ \\ '\\' "\\" '"\\"' "'\\'" '''\\''' ''"\\"'' '"'\\'"' '""\\""' "''\\''" "'"\\"'" ""'\\'"" """\\""" )" R"(\'\' '\'\'' "\'\'" ''\'\''' '"\'\'"' "'\'\''" ""\'\'"" '''\'\'''' ''"\'\'"'' '"'\'\''"' '""\'\'""' "''\'\'''" "'"\'\'"'" ""'\'\''"" """\'\'""" )" R"(\"\" '\"\"' "\"\"" ''\"\"'' '"\"\""' "'\"\"'" ""\"\""" '''\"\"''' ''"\"\""'' '"'\"\"'"' '""\"\"""' "''\"\"''" "'"\"\""'" ""'\"\"'"" """\"\"""")"); - args = *Args::from_atfile("at_file", Args::AtFileFormat::msvc); + args = *Args::from_atfile("at_file", AtFileFormat::msvc); CHECK(args.size() == 44); CHECK(args[0] == R"(\)"); CHECK(args[1] == R"(\\)"); @@ -237,7 +238,7 @@ TEST_CASE("Args::from_atfile") R"(a\\\b d"e f"g h )" R"(a\\\"b c d )" R"(a\\\\"b c" d e)"); - args = *Args::from_atfile("at_file", Args::AtFileFormat::msvc); + args = *Args::from_atfile("at_file", AtFileFormat::msvc); CHECK(args.size() == 12); CHECK(args[0] == R"(abc)"); CHECK(args[1] == R"(d)"); diff --git a/unittest/test_config.cpp b/unittest/test_config.cpp index 9ccd4b41..5f79bb8e 100644 --- a/unittest/test_config.cpp +++ b/unittest/test_config.cpp @@ -308,6 +308,52 @@ TEST_CASE("Config::update_from_environment") CHECK(!config.compression()); } +TEST_CASE("Config::response_file_format") +{ + Config config; + + SUBCASE("from config gcc") + { + util::write_file("ccache.conf", "response_file_format = posix"); + CHECK(config.update_from_file("ccache.conf")); + + CHECK(config.atfile_format() == AtFileFormat::gcc); + } + + SUBCASE("from config msvc") + { + util::write_file("ccache.conf", "response_file_format = windows"); + CHECK(config.update_from_file("ccache.conf")); + + CHECK(config.atfile_format() == AtFileFormat::msvc); + } + + SUBCASE("from config msvc with clang compiler") + { + util::write_file("ccache.conf", + "response_file_format = windows\ncompiler_type = clang"); + CHECK(config.update_from_file("ccache.conf")); + + CHECK(config.atfile_format() == AtFileFormat::msvc); + } + + SUBCASE("guess from compiler gcc") + { + util::write_file("ccache.conf", "compiler_type = clang"); + CHECK(config.update_from_file("ccache.conf")); + + CHECK(config.atfile_format() == AtFileFormat::gcc); + } + + SUBCASE("guess from compiler msvc") + { + util::write_file("ccache.conf", "compiler_type = msvc"); + CHECK(config.update_from_file("ccache.conf")); + + CHECK(config.atfile_format() == AtFileFormat::msvc); + } +} + TEST_CASE("Config::set_value_in_file") { TestContext test_context; @@ -430,6 +476,7 @@ TEST_CASE("Config::visit_items") "remote_only = true\n" "remote_storage = rs\n" "reshare = true\n" + "response_file_format = posix\n" "run_second_cpp = false\n" "sloppiness = include_file_mtime, include_file_ctime, time_macros," " file_stat_matches, file_stat_matches_ctime, pch_defines, system_headers," @@ -492,6 +539,7 @@ TEST_CASE("Config::visit_items") "(test.conf) remote_only = true", "(test.conf) remote_storage = rs", "(test.conf) reshare = true", + "(test.conf) response_file_format = posix", "(test.conf) run_second_cpp = false", "(test.conf) sloppiness = clang_index_store, file_stat_matches," " file_stat_matches_ctime, gcno_cwd, include_file_ctime," -- 2.47.2