From c2ebbbfec549ae0247710e80d593c96859e22be9 Mon Sep 17 00:00:00 2001 From: Mu-Le Lee Date: Wed, 3 Aug 2022 07:12:50 +0000 Subject: [PATCH] Change the format of header-abi-diff config from .ini to .json Since Cross-Version ABI Check is introduced, the flags could be different when diffing with previous or current dumps. The structure and format of the newly proposed config are changed so users can configure flags for different diff targets. This CL changes the config format from .ini to .json with the same features the format of the json is described in go/cross-version-abi-diff-configuration. Test: preform abi diff with config.json Bug: 239792343 Change-Id: I731bafbfdacd2a780c5f7a27997893cecb2eb1f3 --- vndk/tools/header-checker/Android.bp | 5 + .../src/diff/header_abi_diff.cpp | 12 +-- .../header-checker/src/utils/config_file.cpp | 73 ++++----------- .../header-checker/src/utils/config_file.h | 72 +++------------ .../src/utils/config_file_test.cpp | 91 +++++++------------ 5 files changed, 78 insertions(+), 175 deletions(-) diff --git a/vndk/tools/header-checker/Android.bp b/vndk/tools/header-checker/Android.bp index e626b7f24..e5b7e1d8b 100644 --- a/vndk/tools/header-checker/Android.bp +++ b/vndk/tools/header-checker/Android.bp @@ -127,6 +127,10 @@ cc_binary_host { "src/diff/abi_diff_wrappers.cpp", "src/diff/header_abi_diff.cpp", ], + + static_libs: [ + "libjsoncpp", + ], } cc_library_host_static { @@ -197,6 +201,7 @@ cc_test_host { "libgtest", "libgtest_main", "libheader-checker", + "libjsoncpp", ], shared_libs: [ diff --git a/vndk/tools/header-checker/src/diff/header_abi_diff.cpp b/vndk/tools/header-checker/src/diff/header_abi_diff.cpp index e947d2f43..c14f69b35 100644 --- a/vndk/tools/header-checker/src/diff/header_abi_diff.cpp +++ b/vndk/tools/header-checker/src/diff/header_abi_diff.cpp @@ -15,7 +15,6 @@ #include "diff/abi_diff.h" #include "utils/config_file.h" -#include "utils/string_utils.h" #include #include @@ -31,8 +30,6 @@ using header_checker::repr::CompatibilityStatusIR; using header_checker::repr::DiffPolicyOptions; using header_checker::repr::TextFormatIR; using header_checker::utils::ConfigFile; -using header_checker::utils::ConfigParser; -using header_checker::utils::ParseBool; static llvm::cl::OptionCategory header_checker_category( @@ -149,16 +146,19 @@ static std::set LoadIgnoredSymbols(std::string &symbol_list_path) { static std::string GetConfigFilePath(const std::string &dump_file_path) { llvm::SmallString<128> config_file_path(dump_file_path); llvm::sys::path::remove_filename(config_file_path); - llvm::sys::path::append(config_file_path, "config.ini"); + llvm::sys::path::append(config_file_path, "config.json"); return std::string(config_file_path); } static void ReadConfigFile(const std::string &config_file_path) { - ConfigFile cfg = ConfigParser::ParseFile(config_file_path); + ConfigFile cfg; + if (!cfg.Load(config_file_path)) { + ::exit(1); + } if (cfg.HasSection("global")) { for (auto &&p : cfg.GetSection("global")) { auto &&key = p.first; - bool value_bool = ParseBool(p.second); + bool value_bool = p.second; if (key == "allow_adding_removing_weak_symbols") { allow_adding_removing_weak_symbols = value_bool; } else if (key == "advice_only") { diff --git a/vndk/tools/header-checker/src/utils/config_file.cpp b/vndk/tools/header-checker/src/utils/config_file.cpp index e4861c62a..9a49b08ad 100644 --- a/vndk/tools/header-checker/src/utils/config_file.cpp +++ b/vndk/tools/header-checker/src/utils/config_file.cpp @@ -14,10 +14,10 @@ #include "utils/config_file.h" -#include "utils/string_utils.h" +#include +#include #include -#include #include @@ -25,61 +25,28 @@ namespace header_checker { namespace utils { -ConfigFile ConfigParser::ParseFile(std::istream &istream) { - ConfigParser parser(istream); - return parser.ParseFile(); -} - - -ConfigFile ConfigParser::ParseFile(const std::string &path) { - std::ifstream stream(path, std::ios_base::in); - return ParseFile(stream); -} - - -ConfigFile ConfigParser::ParseFile() { - size_t line_no = 0; - std::string line; - while (std::getline(stream_, line)) { - ParseLine(++line_no, line); +bool ConfigFile::Load(std::istream &istream) { + Json::Value root; + Json::CharReaderBuilder builder; + std::string errorMessage; + if (!Json::parseFromStream(builder, istream, &root, &errorMessage)) { + llvm::errs() << "Failed to parse JSON: " << errorMessage << "\n"; + return false; } - return std::move(cfg_); -} - - -void ConfigParser::ParseLine(size_t line_no, std::string_view line) { - if (line.empty() || line[0] == ';' || line[0] == '#') { - // Skip empty or comment line. - return; - } - - // Parse section name line. - if (line[0] == '[') { - std::string::size_type pos = line.rfind(']'); - if (pos == std::string::npos) { - ReportError(line_no, "bad section name line"); - return; + for (auto &key : root.getMemberNames()) { + map_[key] = ConfigSection(); + if (root[key].isMember("flags")) { + for (auto &flag_keys : root[key]["flags"].getMemberNames()) { + map_[key].map_[flag_keys] = root[key]["flags"][flag_keys].asBool(); + } } - std::string_view section_name = line.substr(1, pos - 1); - section_ = &cfg_.map_[std::string(section_name)]; - return; } + return true; +} - // Parse key-value line. - std::string::size_type pos = line.find('='); - if (pos == std::string::npos) { - ReportError(line_no, "bad key-value line"); - return; - } - - // Add key-value entry to current section. - std::string_view key = Trim(line.substr(0, pos)); - std::string_view value = Trim(line.substr(pos + 1)); - - if (!section_) { - section_ = &cfg_.map_[""]; - } - section_->map_[std::string(key)] = std::string(value); +bool ConfigFile::Load(const std::string &path) { + std::ifstream stream(path); + return Load(stream); } diff --git a/vndk/tools/header-checker/src/utils/config_file.h b/vndk/tools/header-checker/src/utils/config_file.h index cf8c6d905..2dfe7eab0 100644 --- a/vndk/tools/header-checker/src/utils/config_file.h +++ b/vndk/tools/header-checker/src/utils/config_file.h @@ -25,12 +25,9 @@ namespace header_checker { namespace utils { -class ConfigParser; - - class ConfigSection { public: - using MapType = std::map; + using MapType = std::map; using const_iterator = MapType::const_iterator; @@ -43,15 +40,15 @@ class ConfigSection { return map_.find(name) != map_.end(); } - std::string GetProperty(const std::string &name) const { + bool GetProperty(const std::string &name) const { auto &&it = map_.find(name); if (it == map_.end()) { - return ""; + return false; } return it->second; } - std::string operator[](const std::string &name) const { + bool operator[](const std::string &name) const { return GetProperty(name); } @@ -70,9 +67,9 @@ class ConfigSection { private: - std::map map_; + MapType map_; - friend class ConfigParser; + friend class ConfigFile; }; @@ -87,6 +84,9 @@ class ConfigFile { ConfigFile(ConfigFile &&) = default; ConfigFile &operator=(ConfigFile &&) = default; + bool Load(const std::string &path); + bool Load(std::istream &istream); + bool HasSection(const std::string §ion_name) const { return map_.find(section_name) != map_.end(); } @@ -110,11 +110,11 @@ class ConfigFile { return it->second.HasProperty(property_name); } - std::string GetProperty(const std::string §ion_name, + bool GetProperty(const std::string §ion_name, const std::string &property_name) const { auto &&it = map_.find(section_name); if (it == map_.end()) { - return ""; + return false; } return it->second.GetProperty(property_name); } @@ -134,55 +134,7 @@ class ConfigFile { private: - std::map map_; - - friend class ConfigParser; -}; - - -class ConfigParser { - public: - using ErrorListener = std::function; - - - public: - ConfigParser(std::istream &stream) - : stream_(stream), section_(nullptr) { } - - ConfigFile ParseFile(); - - static ConfigFile ParseFile(std::istream &istream); - - static ConfigFile ParseFile(const std::string &path); - - void SetErrorListener(ErrorListener listener) { - error_listener_ = std::move(listener); - } - - - private: - void ParseLine(size_t line_no, std::string_view line); - - void ReportError(size_t line_no, const char *cause) { - if (error_listener_) { - error_listener_(line_no, cause); - } - } - - - private: - ConfigParser(const ConfigParser &) = delete; - ConfigParser &operator=(const ConfigParser &) = delete; - - - private: - std::istream &stream_; - - ErrorListener error_listener_; - - ConfigSection *section_; - - ConfigFile cfg_; + MapType map_; }; diff --git a/vndk/tools/header-checker/src/utils/config_file_test.cpp b/vndk/tools/header-checker/src/utils/config_file_test.cpp index 28ead1040..2660de4ba 100644 --- a/vndk/tools/header-checker/src/utils/config_file_test.cpp +++ b/vndk/tools/header-checker/src/utils/config_file_test.cpp @@ -23,87 +23,66 @@ namespace header_checker { namespace utils { -TEST(ConfigParserTest, Parse) { +TEST(ConfigFileTest, Parse) { std::stringstream stream(R"( -# Comment line starts with hash symbol -; Comment line starts with semicolon - -[section1] -key1 = value1 -key2 = value2 - -[section2] -key1 = true -key2 = false +// Comment line starts with slash +/* embedded comment */ +{ + "section1": { + "flags": { + "key1": true, + "key2": false, + }, + }, + "section2": { + "flags": { + "key1": true, + "key2": false, + }, + }, +} )"); - auto &&cfg = ConfigParser::ParseFile(stream); + ConfigFile cfg; + cfg.Load(stream); EXPECT_TRUE(cfg.HasSection("section1")); EXPECT_TRUE(cfg.HasSection("section2")); EXPECT_FALSE(cfg.HasSection("section3")); auto &§ion1 = cfg.GetSection("section1"); EXPECT_TRUE(section1.HasProperty("key1")); - EXPECT_EQ("value1", section1.GetProperty("key1")); + EXPECT_TRUE(section1.GetProperty("key1")); EXPECT_TRUE(section1.HasProperty("key2")); - EXPECT_EQ("value2", section1.GetProperty("key2")); + EXPECT_FALSE(section1.GetProperty("key2")); EXPECT_FALSE(section1.HasProperty("key3")); - EXPECT_EQ("", section1.GetProperty("key3")); + EXPECT_FALSE(section1.GetProperty("key3")); auto &§ion2 = cfg.GetSection("section2"); EXPECT_TRUE(section2.HasProperty("key1")); - EXPECT_EQ("true", section2.GetProperty("key1")); + EXPECT_TRUE(section2.GetProperty("key1")); EXPECT_TRUE(section2.HasProperty("key2")); - EXPECT_EQ("false", section2.GetProperty("key2")); + EXPECT_FALSE(section2.GetProperty("key2")); - EXPECT_EQ("value1", cfg.GetProperty("section1", "key1")); - EXPECT_EQ("value2", cfg.GetProperty("section1", "key2")); + EXPECT_TRUE(cfg.GetProperty("section1", "key1")); + EXPECT_FALSE(cfg.GetProperty("section1", "key2")); - EXPECT_EQ("value1", cfg["section1"]["key1"]); - EXPECT_EQ("value2", cfg["section1"]["key2"]); + EXPECT_TRUE(cfg["section1"]["key1"]); + EXPECT_FALSE(cfg["section1"]["key2"]); } - -TEST(ConfigParserTest, BadSectionNameLine) { +TEST(ConfigFileTest, BadJsonFormat) { std::stringstream stream(R"( -[section1 -key1 = value1 -)"); - - size_t num_errors = 0; - - ConfigParser parser(stream); - parser.SetErrorListener( - [&num_errors](size_t line_no, const char *cause) { - ++num_errors; - EXPECT_EQ(2, line_no); - EXPECT_STREQ("bad section name line", cause); - }); - parser.ParseFile(); - - EXPECT_EQ(1, num_errors); +{ + "section1: { + } } - - -TEST(ConfigParserTest, BadKeyValueLine) { - std::stringstream stream(R"( -[section1] -key1 )"); - size_t num_errors = 0; + ConfigFile cfg; + bool result = cfg.Load(stream); - ConfigParser parser(stream); - parser.SetErrorListener( - [&num_errors](size_t line_no, const char *cause) { - ++num_errors; - EXPECT_EQ(3, line_no); - EXPECT_STREQ("bad key-value line", cause); - }); - parser.ParseFile(); - - EXPECT_EQ(1, num_errors); + EXPECT_FALSE(result); }