From 9be2c15d10e92c87d5419d5d8a34644d792dae32 Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Fri, 26 Sep 2014 21:59:24 +0100 Subject: [PATCH] Implemented loot_update_masterlist. Also removed dummy Masterlist member functions from API code, and unused language parameter from Masterlist::Update(). Game condition cache lookup during condition parsing tests was removed for simplicity, though it should hopefully not have too much effect on the test speed. --- CMakeLists.txt | 18 +++++++--------- src/api/api.cpp | 17 +-------------- src/backend/game.cpp | 2 +- src/backend/game.h | 5 ++++- src/backend/git.cpp | 45 +++++++++++++++++++++------------------- src/backend/metadata.cpp | 23 ++++++++------------ src/backend/metadata.h | 4 ++-- src/backend/parsers.h | 33 +++++++++++++++-------------- 8 files changed, 67 insertions(+), 80 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index dc5b2302..499e9de9 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -50,7 +50,8 @@ set (LOOT_SRC "${CMAKE_SOURCE_DIR}/src/backend/metadata.cpp" "${CMAKE_SOURCE_DIR}/src/backend/helpers.cpp" "${CMAKE_SOURCE_DIR}/src/backend/globals.cpp" "${CMAKE_SOURCE_DIR}/src/backend/generators.cpp" - "${CMAKE_SOURCE_DIR}/src/backend/graph.cpp") + "${CMAKE_SOURCE_DIR}/src/backend/graph.cpp" + "${CMAKE_SOURCE_DIR}/src/backend/git.cpp") set (LOOT_HEADERS "${CMAKE_SOURCE_DIR}/src/backend/metadata.h" "${CMAKE_SOURCE_DIR}/src/backend/game.h" @@ -63,9 +64,6 @@ set (LOOT_HEADERS "${CMAKE_SOURCE_DIR}/src/backend/metadata.h" "${CMAKE_SOURCE_DIR}/src/backend/streams.h") set (LOOT_GUI_SRC ${LOOT_SRC} - # Code the API doesn't need. - "${CMAKE_SOURCE_DIR}/src/backend/git.cpp" - # Actual GUI code. "${CMAKE_SOURCE_DIR}/src/gui/main_win.cpp" "${CMAKE_SOURCE_DIR}/src/gui/handler.cpp" "${CMAKE_SOURCE_DIR}/src/gui/app.cpp" @@ -168,13 +166,13 @@ ELSEIF (MSVC) ENDIF () set (CMAKE_EXE_LINKER_FLAGS "/SUBSYSTEM:WINDOWS /LARGEADDRESSAWARE") - set (LOOT_LIBS libyaml-cppmt - version - loadorder${PROJECT_ARCH} - ws2_32 - shlwapi) + set (LOOT_LIBS git2 + libyaml-cppmt + version + loadorder${PROJECT_ARCH} + ws2_32 + shlwapi) set (LOOT_GUI_LIBS ${LOOT_LIBS} - git2 cef_sandbox libcef libcef_dll_wrapper diff --git a/src/api/api.cpp b/src/api/api.cpp index ca7c9d24..c19a57ef 100644 --- a/src/api/api.cpp +++ b/src/api/api.cpp @@ -172,22 +172,6 @@ unsigned int c_error(const unsigned int code, const std::string& what) { return c_error(loot::error(code, what.c_str())); } -//////////////////////////////////// -// Dummy Masterlist member functions -//////////////////////////////////// - -// The API doesn't depend on libgit2, by not compiling ".git.cpp", so the member functions are defined -// below as dummies. - -namespace loot { - void Masterlist::GetGitInfo(const boost::filesystem::path& path) {} - - bool Masterlist::Update(Game& game, const unsigned int language) { - this->MetadataList::Load(game.MasterlistPath()); - return false; - } -} - ////////////////////////////// // Error Handling Functions ////////////////////////////// @@ -458,6 +442,7 @@ LOOT_API unsigned int loot_update_masterlist(loot_db db, try { loot::Masterlist masterlist; + *updated = masterlist.Update(masterlistPath, remoteURL, remoteBranch); } catch (loot::error &e) { return c_error(e); diff --git a/src/backend/game.cpp b/src/backend/game.cpp index 52f8c6fa..066fd16d 100644 --- a/src/backend/game.cpp +++ b/src/backend/game.cpp @@ -252,7 +252,7 @@ namespace loot { bool Masterlist::Load(Game& game, const unsigned int language) { try { - return Update(game, language); + return Update(game); } catch (error& e) { if (e.code() != error::ok) { diff --git a/src/backend/game.h b/src/backend/game.h index 591707dc..0394f1b3 100644 --- a/src/backend/game.h +++ b/src/backend/game.h @@ -84,7 +84,10 @@ namespace loot { public: bool Load(Game& game, const unsigned int language); //Handles update with load fallback. - bool Update(Game& game, const unsigned int language); + bool Update(const Game& game); + bool Update(const boost::filesystem::path& path, + const std::string& repoURL, + const std::string& repoBranch); std::string GetRevision(const boost::filesystem::path& path, bool shortID); std::string GetDate(const boost::filesystem::path& path); diff --git a/src/backend/git.cpp b/src/backend/git.cpp index 77f49531..7f92b4b1 100644 --- a/src/backend/git.cpp +++ b/src/backend/git.cpp @@ -207,10 +207,13 @@ namespace loot { } } - bool Masterlist::Update(Game& game, const unsigned int language) { + bool Masterlist::Update(const Game& game) { + return Update(game.MasterlistPath(), game.RepoURL(), game.RepoBranch()); + } + + bool Masterlist::Update(const boost::filesystem::path& path, const std::string& repoURL, const std::string& repoBranch) { git_handler git; - fs::path repo_path = game.MasterlistPath().parent_path(); - string repo_branch = game.RepoBranch(); + fs::path repo_path = path.parent_path(); // First initialise some stuff that isn't specific to a repository. BOOST_LOG_TRIVIAL(debug) << "Creating a reflog signature to use."; @@ -257,7 +260,7 @@ namespace loot { git_clone_options clone_options = GIT_CLONE_OPTIONS_INIT; clone_options.checkout_opts = checkout_opts; clone_options.bare = 0; - clone_options.checkout_branch = repo_branch.c_str(); + clone_options.checkout_branch = repoBranch.c_str(); clone_options.signature = git.sig; #ifndef _WIN32 //OpenSSL doesn't seem to like GitHub's certificate. @@ -265,7 +268,7 @@ namespace loot { #endif //Now perform the clone. - git.call(git_clone(&git.repo, game.RepoURL().c_str(), repo_path.string().c_str(), &clone_options)); + git.call(git_clone(&git.repo, repoURL.c_str(), repo_path.string().c_str(), &clone_options)); if (fs::exists(temp_path)) { //Move contents back in. @@ -294,12 +297,12 @@ namespace loot { git.call(git_remote_load(&git.remote, git.repo, "origin")); const char * url = git_remote_url(git.remote); - BOOST_LOG_TRIVIAL(info) << "Remote URL given: " << game.RepoURL(); + BOOST_LOG_TRIVIAL(info) << "Remote URL given: " << repoURL; BOOST_LOG_TRIVIAL(info) << "Remote URL in repository settings: " << url; - if (url != game.RepoURL()) { + if (url != repoURL) { BOOST_LOG_TRIVIAL(info) << "URLs do not match, setting repository URL to URL in settings."; // The URLs don't match. Change the remote URL to match the one LOOT has. - git.call(git_remote_set_url(git.remote, game.RepoURL().c_str())); + git.call(git_remote_set_url(git.remote, repoURL.c_str())); // Now save change. git.call(git_remote_save(git.remote)); @@ -317,21 +320,21 @@ namespace loot { // Check that a branch with the correct name exists. git.ui_message = "An error occurred while trying to access the local masterlist repository. If this error happens again, try deleting the \".git\" folder in " + repo_path.string() + "\"."; - int ret = git_branch_lookup(&git.ref, git.repo, repo_branch.c_str(), GIT_BRANCH_LOCAL); + int ret = git_branch_lookup(&git.ref, git.repo, repoBranch.c_str(), GIT_BRANCH_LOCAL); if (ret == GIT_ENOTFOUND) { // Branch doesn't exist. Create a new branch using the remote branch's latest commit. - BOOST_LOG_TRIVIAL(trace) << "Looking up commit referred to by the remote branch \"" << repo_branch << "\"."; - git.call(git_revparse_single(&git.obj, git.repo, (string("origin/") + repo_branch).c_str())); + BOOST_LOG_TRIVIAL(trace) << "Looking up commit referred to by the remote branch \"" << repoBranch << "\"."; + git.call(git_revparse_single(&git.obj, git.repo, (string("origin/") + repoBranch).c_str())); const git_oid * commit_id = git_object_id(git.obj); BOOST_LOG_TRIVIAL(trace) << "Creating the new branch."; // Create a branch. git.call(git_commit_lookup(&git.commit, git.repo, commit_id)); - git.call(git_branch_create(&git.ref, git.repo, repo_branch.c_str(), git.commit, 0, git.sig, NULL)); + git.call(git_branch_create(&git.ref, git.repo, repoBranch.c_str(), git.commit, 0, git.sig, NULL)); // Set upstream. Don't really know if this is necessary or not. - git.call(git_branch_set_upstream(git.ref, (string("origin/") + repo_branch).c_str())); + git.call(git_branch_set_upstream(git.ref, (string("origin/") + repoBranch).c_str())); BOOST_LOG_TRIVIAL(trace) << "Setting the upstream for the new branch."; // Free tree and commit pointers. Reference pointer is still used below. @@ -347,8 +350,8 @@ namespace loot { // Check if HEAD points to the desired branch and set it to if not. if (!git_branch_is_head(git.ref)) { - BOOST_LOG_TRIVIAL(trace) << "Setting HEAD to follow branch: " << repo_branch; - git.call(git_repository_set_head(git.repo, (string("refs/heads/") + repo_branch).c_str(), git.sig, (string("Updated HEAD to ") + repo_branch).c_str())); + BOOST_LOG_TRIVIAL(trace) << "Setting HEAD to follow branch: " << repoBranch; + git.call(git_repository_set_head(git.repo, (string("refs/heads/") + repoBranch).c_str(), git.sig, (string("Updated HEAD to ") + repoBranch).c_str())); } if (ret == 0) { @@ -361,7 +364,7 @@ namespace loot { BOOST_LOG_TRIVIAL(trace) << "Checking that local and remote branches can be merged by fast-forward."; git_merge_analysis_t analysis; git_merge_preference_t pref; - git.call(git_reference_lookup(&git.ref2, git.repo, (string("refs/remotes/origin/") + repo_branch).c_str())); + git.call(git_reference_lookup(&git.ref2, git.repo, (string("refs/remotes/origin/") + repoBranch).c_str())); git.call(git_merge_head_from_ref(&git.merge_head, git.repo, git.ref2)); git.call(git_merge_analysis(&analysis, &pref, git.repo, (const git_merge_head **)&git.merge_head, 1)); @@ -429,7 +432,7 @@ namespace loot { FixRepoPermissions(repo_path / ".git"); git.free(); fs::remove_all(repo_path / ".git"); - return this->Update(game, language); + return this->Update(path, repoURL, repoBranch); //throw error(error::git_error, "Local repository has been edited, an automatic fast-forward merge update is not possible."); } } @@ -484,16 +487,16 @@ namespace loot { //Now try parsing the masterlist. BOOST_LOG_TRIVIAL(debug) << "Testing masterlist parsing."; try { - this->MetadataList::Load(game.MasterlistPath()); + this->MetadataList::Load(path); for (auto &plugin : plugins) { - plugin.ParseAllConditions(game); + plugin.ParseAllConditions(); } for (auto &plugin : regexPlugins) { - plugin.ParseAllConditions(game); + plugin.ParseAllConditions(); } for (auto &message : messages) { - message.ParseCondition(game); + message.ParseCondition(); } parsingFailed = false; diff --git a/src/backend/metadata.cpp b/src/backend/metadata.cpp index d7c22558..a570801c 100644 --- a/src/backend/metadata.cpp +++ b/src/backend/metadata.cpp @@ -126,7 +126,7 @@ namespace loot { if (it != game.conditionCache.end()) return it->second; - condition_grammar grammar(game, false); + condition_grammar grammar(&game, false); boost::spirit::qi::space_type skipper; std::string::const_iterator begin, end; bool eval; @@ -153,18 +153,13 @@ namespace loot { return eval; } - void ConditionStruct::ParseCondition(Game& game) const { + void ConditionStruct::ParseCondition() const { if (_condition.empty()) return; BOOST_LOG_TRIVIAL(trace) << "Testing condition syntax: " << _condition; - // If the same condition string has already been evaluated, it must be written correctly. - unordered_map::const_iterator it = game.conditionCache.find(boost::locale::to_lower(_condition)); - if (it != game.conditionCache.end()) - return; - - condition_grammar grammar(game, true); + condition_grammar grammar(nullptr, true); boost::spirit::qi::space_type skipper; std::string::const_iterator begin, end; @@ -732,21 +727,21 @@ namespace loot { return *this; } - void Plugin::ParseAllConditions(Game& game) const { + void Plugin::ParseAllConditions() const { for (const File& file : loadAfter) { - file.ParseCondition(game); + file.ParseCondition(); } for (const File& file : requirements) { - file.ParseCondition(game); + file.ParseCondition(); } for (const File& file : incompatibilities) { - file.ParseCondition(game); + file.ParseCondition(); } for (const Message& message : messages) { - message.ParseCondition(game); + message.ParseCondition(); } for (const Tag& tag : tags) { - tag.ParseCondition(game); + tag.ParseCondition(); } } diff --git a/src/backend/metadata.h b/src/backend/metadata.h index 614a1700..8598bfc8 100644 --- a/src/backend/metadata.h +++ b/src/backend/metadata.h @@ -83,7 +83,7 @@ namespace loot { bool IsConditional() const; bool EvalCondition(Game& game) const; - void ParseCondition(Game& game) const; // Throws error on parsing failure. + void ParseCondition() const; // Throws error on parsing failure. std::string Condition() const; private: @@ -226,7 +226,7 @@ namespace loot { void Locations(const std::set& locations); Plugin& EvalAllConditions(Game& game, const unsigned int language); - void ParseAllConditions(Game& game) const; + void ParseAllConditions() const; bool HasNameOnly() const; bool IsRegexPlugin() const; bool LoadsBSA(const Game& game) const; diff --git a/src/backend/parsers.h b/src/backend/parsers.h index 7b9b20ef..ce3ff117 100644 --- a/src/backend/parsers.h +++ b/src/backend/parsers.h @@ -431,7 +431,10 @@ namespace loot { template class condition_grammar : public qi::grammar < Iterator, bool(), Skipper > { public: - condition_grammar(Game& game, bool parseOnly) : condition_grammar::base_type(expression, "condition grammar"), _game(game), _parseOnly(parseOnly) { + condition_grammar(Game * game, bool parseOnly) : condition_grammar::base_type(expression, "condition grammar"), _game(game), _parseOnly(parseOnly) { + if (!_parseOnly && _game == nullptr) + throw error(error::invalid_args, "A valid game pointer was not passed during a condition evaluation."); + expression = compound[qi::labels::_val = qi::labels::_1] >> *((qi::lit("or") >> compound)[qi::labels::_val = qi::labels::_val || qi::labels::_1]) @@ -503,7 +506,7 @@ namespace loot { qi::rule quotedStr, filePath, comparator; qi::rule invalidPathChars; - Game& _game; + Game * _game; bool _parseOnly; //Eval's exact paths. Check for files and ghosted plugins. @@ -524,9 +527,9 @@ namespace loot { } if (IsPlugin(file)) - result = boost::filesystem::exists(_game.DataPath() / file) || boost::filesystem::exists(_game.DataPath() / (file + ".ghost")); + result = boost::filesystem::exists(_game->DataPath() / file) || boost::filesystem::exists(_game->DataPath() / (file + ".ghost")); else - result = boost::filesystem::exists(_game.DataPath() / file); + result = boost::filesystem::exists(_game->DataPath() / file); if (result) BOOST_LOG_TRIVIAL(trace) << "The file does exist."; @@ -580,7 +583,7 @@ namespace loot { //Now we have a valid parent path and a regex filename. Check that //the parent path exists and is a directory. - boost::filesystem::path parent_path = _game.DataPath() / parent; + boost::filesystem::path parent_path = _game->DataPath() / parent; if (!boost::filesystem::exists(parent_path) || !boost::filesystem::is_directory(parent_path)) { BOOST_LOG_TRIVIAL(trace) << "The path \"" << parent_path << "\" does not exist or is not a directory."; return; @@ -616,23 +619,23 @@ namespace loot { } uint32_t crc; - unordered_map::iterator it = _game.crcCache.find(boost::locale::to_lower(file)); + unordered_map::iterator it = _game->crcCache.find(boost::locale::to_lower(file)); - if (it != _game.crcCache.end()) + if (it != _game->crcCache.end()) crc = it->second; else { if (file == "LOOT") crc = GetCrc32(boost::filesystem::absolute("LOOT.exe")); - if (boost::filesystem::exists(_game.DataPath() / file)) - crc = GetCrc32(_game.DataPath() / file); - else if (IsPlugin(file) && boost::filesystem::exists(_game.DataPath() / (file + ".ghost"))) - crc = GetCrc32(_game.DataPath() / (file + ".ghost")); + if (boost::filesystem::exists(_game->DataPath() / file)) + crc = GetCrc32(_game->DataPath() / file); + else if (IsPlugin(file) && boost::filesystem::exists(_game->DataPath() / (file + ".ghost"))) + crc = GetCrc32(_game->DataPath() / (file + ".ghost")); else { result = false; return; } - _game.crcCache.emplace(boost::locale::to_lower(file), crc); + _game->crcCache.emplace(boost::locale::to_lower(file), crc); } result = checksum == crc; @@ -657,11 +660,11 @@ namespace loot { if (file == "LOOT") trueVersion = Version(boost::filesystem::absolute("LOOT.exe")); else if (IsPlugin(file)) { - Plugin plugin(_game, file, true); + Plugin plugin(*_game, file, true); trueVersion = Version(plugin.Version()); } else - trueVersion = Version(_game.DataPath() / file); + trueVersion = Version(_game->DataPath() / file); BOOST_LOG_TRIVIAL(trace) << "Version extracted: " << trueVersion.AsString(); @@ -683,7 +686,7 @@ namespace loot { if (file == "LOOT") result = false; else - result = _game.IsActive(file); + result = _game->IsActive(file); BOOST_LOG_TRIVIAL(trace) << "Active check result: " << result; }