From ecc2166ec623c27b6db8815650502bca1c778153 Mon Sep 17 00:00:00 2001 From: WrinklyNinja Date: Fri, 22 Aug 2014 23:58:17 +0100 Subject: [PATCH] Improved masterlist update handling. If the local repo was already at the latest masterlist, parsing gets skipped (nb. update needs UI feedback). If a fast-forward update isn't possible, the repo is deleted and re-cloned. Closes #240, #241. --- resources/report/js/script.js | 3 + src/backend/game.cpp | 4 +- src/backend/game.h | 4 +- src/backend/git.cpp | 31 +++++++--- src/gui/handler.cpp | 109 ++++++++++++++++++---------------- 5 files changed, 88 insertions(+), 63 deletions(-) diff --git a/resources/report/js/script.js b/resources/report/js/script.js index 60921875..b3323af8 100644 --- a/resources/report/js/script.js +++ b/resources/report/js/script.js @@ -322,6 +322,9 @@ function openReadme(evt) { } function updateMasterlist(evt) { loot.query('updateMasterlist').then(JSON.parse).then(function(result){ + if (result == null) { + return; + } /* Update JS variables. */ loot.game.masterlist.revision = result.masterlist.revision; diff --git a/src/backend/game.cpp b/src/backend/game.cpp index c0dd78fd..d1108019 100644 --- a/src/backend/game.cpp +++ b/src/backend/game.cpp @@ -169,9 +169,9 @@ namespace loot { // Masterlist member functions //---------------------------- - void Masterlist::Load(Game& game, const unsigned int language) { + bool Masterlist::Load(Game& game, const unsigned int language) { try { - Update(game, language); + return Update(game, language); } catch (error& e) { if (e.code() != error::ok) { diff --git a/src/backend/game.h b/src/backend/game.h index 5a69c713..b03ddd89 100644 --- a/src/backend/game.h +++ b/src/backend/game.h @@ -70,8 +70,8 @@ namespace loot { class Masterlist : public MetadataList { public: - void Load(Game& game, const unsigned int language); //Handles update with load fallback. - void Update(Game& game, const unsigned int language); + bool Load(Game& game, const unsigned int language); //Handles update with load fallback. + bool Update(Game& game, const unsigned int language); std::string GetRevision(const boost::filesystem::path& path); std::string GetDate(const boost::filesystem::path& path); diff --git a/src/backend/git.cpp b/src/backend/git.cpp index 99a4b7aa..89211a6b 100644 --- a/src/backend/git.cpp +++ b/src/backend/git.cpp @@ -100,6 +100,15 @@ namespace loot { return git_repository_open_ext(NULL, path.string().c_str(), GIT_REPOSITORY_OPEN_NO_SEARCH, NULL) == 0; } + // Removes the read-only flag from some files in git repositories created by libgit2. + void FixRepoPermissions(const fs::path& path) { + BOOST_LOG_TRIVIAL(trace) << "Recursively setting write permission on directory: " << path; + for (fs::recursive_directory_iterator it(path); it != fs::recursive_directory_iterator(); ++it) { + BOOST_LOG_TRIVIAL(trace) << "Setting write permission for: " << it->path(); + fs::permissions(it->path(), fs::add_perms | fs::owner_write); + } + } + int diffFileCallback(const git_diff_delta *delta, float progress, void * payload) { BOOST_LOG_TRIVIAL(trace) << "Checking diff for: " << delta->old_file.path; if (strcmp(delta->old_file.path, "masterlist.yaml") == 0) { @@ -166,7 +175,7 @@ namespace loot { } } - void Masterlist::Update(Game& game, const unsigned int language) { + bool Masterlist::Update(Game& game, const unsigned int language) { git_handler git; fs::path repo_path = game.MasterlistPath().parent_path(); string repo_branch = game.RepoBranch(); @@ -200,11 +209,7 @@ namespace loot { // seem to be made that way with no detrimental effect on libgit2's operation, but it // stuffs these filesystem commands up. if (fs::exists(temp_path)) { - BOOST_LOG_TRIVIAL(trace) << "Recursively setting write permission on directory: " << temp_path; - for (fs::recursive_directory_iterator it(temp_path); it != fs::recursive_directory_iterator(); ++it) { - BOOST_LOG_TRIVIAL(trace) << "Setting write permission for: " << it->path(); - fs::permissions(it->path(), fs::add_perms | fs::owner_write); - } + FixRepoPermissions(temp_path); fs::remove_all(temp_path); } fs::rename(repo_path, temp_path); @@ -347,10 +352,20 @@ namespace loot { git.ref2 = nullptr; } else if ((analysis & GIT_MERGE_ANALYSIS_UP_TO_DATE) != 0) { + // No update necessary, so exit early to skip unnecessary masterlist parsing. BOOST_LOG_TRIVIAL(trace) << "Local branch is up-to-date with remote branch."; + + BOOST_LOG_TRIVIAL(trace) << "Performing a Git checkout of HEAD."; + git.call(git_checkout_head(git.repo, &checkout_opts)); + + return false; } else { - throw error(error::git_error, "Local repository has been edited, an automatic fast-forward merge update is not possible."); + // The local repository can't be easily merged. It's best just to delete and re-clone it. + FixRepoPermissions(repo_path / ".git"); + fs::remove_all(repo_path / ".git"); + return this->Update(game, language); + //throw error(error::git_error, "Local repository has been edited, an automatic fast-forward merge update is not possible."); } } @@ -448,5 +463,7 @@ namespace loot { if (!parsingError.empty()) throw error(error::ok, parsingError); //Throw an OK because the process still completed in a successful state. + + return true; } } diff --git a/src/gui/handler.cpp b/src/gui/handler.cpp index 19f767cc..b2f24b4b 100644 --- a/src/gui/handler.cpp +++ b/src/gui/handler.cpp @@ -697,81 +697,86 @@ namespace loot { BOOST_LOG_TRIVIAL(info) << "Using message language: " << Language(language).Name(); // Update / parse masterlist. + bool wasChanged = true; try { - g_app_state.CurrentGame().masterlist.Load(g_app_state.CurrentGame(), language); + wasChanged = g_app_state.CurrentGame().masterlist.Load(g_app_state.CurrentGame(), language); } catch (loot::error &e) { if (e.code() == loot::error::ok) { // There was a parsing error, but roll-back was successful, so the process // should still complete. parsingError = e.what(); + wasChanged = true; } else throw e; } - // Now regenerate the JS-side masterlist data. + // Now regenerate the JS-side masterlist data if the masterlist was changed. + if (wasChanged) { + // The data structure is to be set as 'loot.game'. + YAML::Node gameNode; - // The data structure is to be set as 'loot.game'. - YAML::Node gameNode; + // Store the masterlist revision and date. + gameNode["masterlist"]["revision"] = g_app_state.CurrentGame().masterlist.GetRevision(g_app_state.CurrentGame().MasterlistPath()); + gameNode["masterlist"]["date"] = g_app_state.CurrentGame().masterlist.GetDate(g_app_state.CurrentGame().MasterlistPath()); - // Store the masterlist revision and date. - gameNode["masterlist"]["revision"] = g_app_state.CurrentGame().masterlist.GetRevision(g_app_state.CurrentGame().MasterlistPath()); - gameNode["masterlist"]["date"] = g_app_state.CurrentGame().masterlist.GetDate(g_app_state.CurrentGame().MasterlistPath()); + for (const auto& pluginPair : g_app_state.CurrentGame().plugins) { + Plugin mlistPlugin(pluginPair.second); + mlistPlugin.MergeMetadata(g_app_state.CurrentGame().masterlist.FindPlugin(pluginPair.second.Name())); - for (const auto& pluginPair : g_app_state.CurrentGame().plugins) { - Plugin mlistPlugin(pluginPair.second); - mlistPlugin.MergeMetadata(g_app_state.CurrentGame().masterlist.FindPlugin(pluginPair.second.Name())); + YAML::Node pluginNode; + if (!mlistPlugin.HasNameOnly()) { + // Now add the masterlist metadata to the pluginNode. + pluginNode["masterlist"]["after"] = mlistPlugin.LoadAfter(); + pluginNode["masterlist"]["req"] = mlistPlugin.Reqs(); + pluginNode["masterlist"]["inc"] = mlistPlugin.Incs(); + pluginNode["masterlist"]["msg"] = mlistPlugin.Messages(); + pluginNode["masterlist"]["tag"] = mlistPlugin.Tags(); + pluginNode["masterlist"]["dirty"] = mlistPlugin.DirtyInfo(); + } - YAML::Node pluginNode; - if (!mlistPlugin.HasNameOnly()) { - // Now add the masterlist metadata to the pluginNode. - pluginNode["masterlist"]["after"] = mlistPlugin.LoadAfter(); - pluginNode["masterlist"]["req"] = mlistPlugin.Reqs(); - pluginNode["masterlist"]["inc"] = mlistPlugin.Incs(); - pluginNode["masterlist"]["msg"] = mlistPlugin.Messages(); - pluginNode["masterlist"]["tag"] = mlistPlugin.Tags(); - pluginNode["masterlist"]["dirty"] = mlistPlugin.DirtyInfo(); + // Now merge masterlist and userlist metadata and evaluate, + // putting any resulting metadata into the base of the pluginNode. + YAML::Node derivedNode = GenerateDerivedMetadata(pluginPair.second.Name()); + + for (auto it = derivedNode.begin(); it != derivedNode.end(); ++it) { + const string key = it->first.as(); + pluginNode[key] = it->second; + } + + gameNode["plugins"].push_back(pluginNode); } - // Now merge masterlist and userlist metadata and evaluate, - // putting any resulting metadata into the base of the pluginNode. - YAML::Node derivedNode = GenerateDerivedMetadata(pluginPair.second.Name()); - - for (auto it = derivedNode.begin(); it != derivedNode.end(); ++it) { - const string key = it->first.as(); - pluginNode[key] = it->second; + //Evaluate any conditions in the global messages. + BOOST_LOG_TRIVIAL(debug) << "Evaluating global message conditions."; + list messages = g_app_state.CurrentGame().masterlist.messages; + try { + list::iterator it = messages.begin(); + while (it != messages.end()) { + if (!it->EvalCondition(g_app_state.CurrentGame(), language)) + it = messages.erase(it); + else + ++it; + } + } + catch (std::exception& e) { + BOOST_LOG_TRIVIAL(error) << "A global message contains a condition that could not be evaluated. Details: " << e.what(); + messages.push_back(Message(Message::error, (format(loc::translate("A global message contains a condition that could not be evaluated. Details: %1%")) % e.what()).str())); } - gameNode["plugins"].push_back(pluginNode); - } - - //Evaluate any conditions in the global messages. - BOOST_LOG_TRIVIAL(debug) << "Evaluating global message conditions."; - list messages = g_app_state.CurrentGame().masterlist.messages; - try { - list::iterator it = messages.begin(); - while (it != messages.end()) { - if (!it->EvalCondition(g_app_state.CurrentGame(), language)) - it = messages.erase(it); - else - ++it; + // Add the parsing error to the global messages, if it exists. + if (!parsingError.empty()) { + messages.push_back(Message(Message::error, parsingError)); } - } - catch (std::exception& e) { - BOOST_LOG_TRIVIAL(error) << "A global message contains a condition that could not be evaluated. Details: " << e.what(); - messages.push_back(Message(Message::error, (format(loc::translate("A global message contains a condition that could not be evaluated. Details: %1%")) % e.what()).str())); - } - // Add the parsing error to the global messages, if it exists. - if (!parsingError.empty()) { - messages.push_back(Message(Message::error, parsingError)); + // Now store global messages from masterlist. + gameNode["globalMessages"] = messages; + + return JSON::stringify(gameNode); } - - // Now store global messages from masterlist. - gameNode["globalMessages"] = messages; - - return JSON::stringify(gameNode); + else + return "null"; } std::string Handler::ClearAllMetadata() {