From 27baafab0dd8c06b50e6c5024663ab0fc277ee6c Mon Sep 17 00:00:00 2001 From: James Turner Date: Fri, 25 Mar 2016 23:04:45 +0000 Subject: [PATCH] Packages: handle catalog versions better MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a catalog version is stale, disable it but don’t remove it, and still try to refresh it. This should give much better behaviour when the FG version changes, should behave as users expect. --- simgear/package/Catalog.cxx | 61 ++++++++++++++--------- simgear/package/Root.cxx | 96 +++++++++++++++++++++++++++---------- 2 files changed, 109 insertions(+), 48 deletions(-) diff --git a/simgear/package/Catalog.cxx b/simgear/package/Catalog.cxx index a867a81e..c9c65658 100644 --- a/simgear/package/Catalog.cxx +++ b/simgear/package/Catalog.cxx @@ -31,6 +31,7 @@ #include #include #include +#include namespace simgear { @@ -40,18 +41,32 @@ bool checkVersion(const std::string& aVersion, SGPropertyNode_ptr props) { BOOST_FOREACH(SGPropertyNode* v, props->getChildren("version")) { std::string s(v->getStringValue()); - if (s== aVersion) { + if (s == aVersion) { return true; } - // allow 3.5.* to match any of 3.5.0, 3.5.1rc1, 3.5.11 or so on - if (strutils::ends_with(s, ".*")) { - size_t lastDot = aVersion.rfind('.'); - std::string ver = aVersion.substr(0, lastDot); - if (ver == s.substr(0, s.length() - 2)) { - return true; + // examine each dot-seperated component in turn, supporting a wildcard + // in the versions from the catalog. + string_list appVersionParts = simgear::strutils::split(aVersion, "."); + string_list catVersionParts = simgear::strutils::split(s, "."); + + size_t partCount = appVersionParts.size(); + if (partCount != catVersionParts.size()) { + continue; + } + + bool ok = true; + for (unsigned int p=0; p < partCount; ++p) { + if (catVersionParts[p] == "*") { + // always passes + } else if (appVersionParts[p] != catVersionParts[p]) { + ok = false; } } + + if (ok) { + return true; + } } return false; } @@ -196,22 +211,13 @@ CatalogRef Catalog::createFromPath(Root* aRoot, const SGPath& aPath) return NULL; } - if (!checkVersion(aRoot->applicationVersion(), props)) { - std::string redirect = redirectUrlForVersion(aRoot->applicationVersion(), props); - if (!redirect.empty()) { - SG_LOG(SG_GENERAL, SG_WARN, "catalog at " << aPath << ", version mismatch:\n\t" - << "redirecting to alternate URL:" << redirect); - CatalogRef c = Catalog::createFromUrl(aRoot, redirect); - c->m_installRoot = aPath; - return c; - } else { - SG_LOG(SG_GENERAL, SG_WARN, "skipping catalog at " << aPath << ", version mismatch:\n\t" - << props->getStringValue("version") << " vs required " << aRoot->catalogVersion()); - return NULL; - } + bool versionCheckOk = checkVersion(aRoot->applicationVersion(), props); + if (!versionCheckOk) { + SG_LOG(SG_GENERAL, SG_INFO, "catalog at:" << aPath << " failed version check: need" << aRoot->applicationVersion()); + // keep the catalog but mark it as needing an update } else { - SG_LOG(SG_GENERAL, SG_INFO, "creating catalog from:" << aPath); + SG_LOG(SG_GENERAL, SG_DEBUG, "creating catalog from:" << aPath); } CatalogRef c = new Catalog(aRoot); @@ -219,8 +225,12 @@ CatalogRef Catalog::createFromPath(Root* aRoot, const SGPath& aPath) c->parseProps(props); c->parseTimestamp(); - // parsed XML ok, mark status as valid - c->changeStatus(Delegate::STATUS_SUCCESS); + if (versionCheckOk) { + // parsed XML ok, mark status as valid + c->changeStatus(Delegate::STATUS_SUCCESS); + } else { + c->changeStatus(Delegate::FAIL_VERSION); + } return c; } @@ -469,6 +479,11 @@ unsigned int Catalog::ageInSeconds() const bool Catalog::needsRefresh() const { + // always refresh in these cases + if ((m_status == Delegate::FAIL_VERSION) || (m_status == Delegate::FAIL_DOWNLOAD)) { + return true; + } + unsigned int maxAge = m_props->getIntValue("max-age-sec", m_root->maxAgeSeconds()); return (ageInSeconds() > maxAge); } diff --git a/simgear/package/Root.cxx b/simgear/package/Root.cxx index 13f2b91f..7f2e8ea8 100644 --- a/simgear/package/Root.cxx +++ b/simgear/package/Root.cxx @@ -159,6 +159,7 @@ public: std::string locale; HTTP::Client* http; CatalogDict catalogs; + CatalogList disabledCatalogs; unsigned int maxAgeSeconds; std::string version; @@ -240,10 +241,16 @@ Root::Root(const SGPath& aPath, const std::string& aVersion) : return; } - BOOST_FOREACH(SGPath c, dir.children(Dir::TYPE_DIR)) { + BOOST_FOREACH(SGPath c, dir.children(Dir::TYPE_DIR | Dir::NO_DOT_OR_DOTDOT)) { CatalogRef cat = Catalog::createFromPath(this, c); if (cat) { - d->catalogs[cat->id()] = cat; + if (cat->status() == Delegate::STATUS_SUCCESS) { + d->catalogs[cat->id()] = cat; + } else { + // catalog has problems, such as needing an update + // keep it out of the main collection for now + d->disabledCatalogs.push_back(cat); + } } } // of child directories iteration } @@ -357,12 +364,23 @@ Root::packagesNeedingUpdate() const void Root::refresh(bool aForce) { bool didStartAny = false; + + // copy all candidate ctalogs to a seperate list, since refreshing + // can modify both the main collection and/or the disabled list + CatalogList toRefresh; CatalogDict::iterator it = d->catalogs.begin(); for (; it != d->catalogs.end(); ++it) { - if (aForce || it->second->needsRefresh()) { - it->second->refresh(); - didStartAny = true; - } + toRefresh.push_back(it->second); + } + + toRefresh.insert(toRefresh.end(), d->disabledCatalogs.begin(), + d->disabledCatalogs.end()); + + + CatalogList::iterator j = toRefresh.begin(); + for (; j != toRefresh.end(); ++j) { + (*j)->refresh(); + didStartAny = true; } if (!didStartAny) { @@ -486,21 +504,35 @@ void Root::catalogRefreshStatus(CatalogRef aCat, Delegate::StatusCode aReason) d->refreshing.erase(aCat); } - if ((aReason != Delegate::STATUS_REFRESHED) && (aReason != Delegate::STATUS_IN_PROGRESS)) { - // if the failure is permanent, delete the catalog from our - // list (don't touch it on disk) - bool isPermanentFailure = (aReason == Delegate::FAIL_VERSION); - if (isPermanentFailure) { - SG_LOG(SG_GENERAL, SG_WARN, "permanent failure for catalog:" << aCat->id()); - if (catIt != d->catalogs.end()) { - d->catalogs.erase(catIt); - } - } - } - if ((aReason == Delegate::STATUS_REFRESHED) && (catIt == d->catalogs.end())) { assert(!aCat->id().empty()); d->catalogs.insert(catIt, CatalogDict::value_type(aCat->id(), aCat)); + + // catalog might have been previously disabled, let's remove in that case + CatalogList::iterator j = std::find(d->disabledCatalogs.begin(), + d->disabledCatalogs.end(), + aCat); + if (j != d->disabledCatalogs.end()) { + SG_LOG(SG_GENERAL, SG_INFO, "re-enabling disabled catalog:" << aCat->id()); + d->disabledCatalogs.erase(j); + } + } + + if ((aReason != Delegate::STATUS_REFRESHED) && + (aReason != Delegate::STATUS_IN_PROGRESS) && + (aReason != Delegate::STATUS_SUCCESS)) + { + // catalog has errors, disable it + CatalogList::iterator j = std::find(d->disabledCatalogs.begin(), + d->disabledCatalogs.end(), + aCat); + if (j == d->disabledCatalogs.end()) { + SG_LOG(SG_GENERAL, SG_INFO, "disabling catalog:" << aCat->id()); + d->disabledCatalogs.push_back(aCat); + } + + // and remove it from the active collection + d->catalogs.erase(catIt); } if (d->refreshing.empty()) { @@ -511,17 +543,31 @@ void Root::catalogRefreshStatus(CatalogRef aCat, Delegate::StatusCode aReason) bool Root::removeCatalogById(const std::string& aId) { + CatalogRef cat; + CatalogDict::iterator catIt = d->catalogs.find(aId); if (catIt == d->catalogs.end()) { - SG_LOG(SG_GENERAL, SG_WARN, "removeCatalogById: unknown ID:" << aId); - return false; + // check the disabled list + CatalogList::iterator j = d->disabledCatalogs.begin(); + for (; j != d->disabledCatalogs.end(); ++j) { + if ((*j)->id() == aId) { + break; + } + } + + if (j == d->disabledCatalogs.end()) { + SG_LOG(SG_GENERAL, SG_WARN, "removeCatalogById: no catalog with id:" << aId); + return false; + } + + cat = *j; + d->disabledCatalogs.erase(j); + } else { + cat = catIt->second; + // drop the reference + d->catalogs.erase(catIt); } - CatalogRef cat = catIt->second; - - // drop the reference - d->catalogs.erase(catIt); - bool ok = cat->uninstall(); if (!ok) { SG_LOG(SG_GENERAL, SG_WARN, "removeCatalogById: catalog :" << aId