commit 9de3174e1660c8774902d31bad7109b001066434
parent 6c220f3410493ff3f721e5f6dec9d91c88e841e1
Author: cfillion <cfillion@users.noreply.github.com>
Date: Mon, 27 Mar 2017 19:35:27 -0400
merge the two index download code paths into a single one
Diffstat:
10 files changed, 117 insertions(+), 212 deletions(-)
diff --git a/src/browser.cpp b/src/browser.cpp
@@ -53,7 +53,7 @@ enum Timers { TIMER_FILTER = 1, TIMER_ABOUT };
Browser::Browser(ReaPack *reapack)
: Dialog(IDD_BROWSER_DIALOG), m_reapack(reapack),
- m_loading(false), m_loaded(false), m_currentIndex(-1)
+ m_loading(false), m_currentIndex(-1)
{
}
@@ -525,41 +525,32 @@ void Browser::updateAbout()
void Browser::refresh(const bool stale)
{
+ // Do nothing when called again when (or while) the index downloading
+ // transaction finishes. populate() handles the next step of the loading process.
if(m_loading)
return;
const vector<Remote> &remotes = m_reapack->config()->remotes.getEnabled();
- if(!m_loaded && remotes.empty()) {
- m_loaded = true;
- show();
+ if(remotes.empty()) {
+ if(!isVisible() || stale) {
+ show();
- MessageBox(handle(), AUTO_STR("No repository enabled!\r\n")
- AUTO_STR("Enable or import repositories from ")
- AUTO_STR("Extensions > ReaPack > Manage repositories."),
- AUTO_STR("Browse packages"), MB_OK);
+ MessageBox(handle(), AUTO_STR("No repository enabled!\r\n")
+ AUTO_STR("Enable or import repositories from ")
+ AUTO_STR("Extensions > ReaPack > Manage repositories."),
+ AUTO_STR("Browse packages"), MB_OK);
+ }
return;
}
- m_loading = true;
-
- m_reapack->fetchIndexes(remotes, [=] (const vector<IndexPtr> &indexes) {
- // if the user wanted to close the window while we were downloading stuff
- if(m_loaded && !isVisible()) {
- close();
- return;
- }
-
- m_loading = false;
-
- populate(indexes);
-
- if(!m_loaded) {
- m_loaded = true;
- show();
- }
- }, handle(), stale);
+ if(Transaction *tx = m_reapack->setupTransaction()) {
+ m_loading = true;
+ tx->fetchIndexes(remotes, stale);
+ tx->onFinish([=] { populate(tx->getIndexes(remotes)); });
+ tx->runTasks();
+ }
}
void Browser::setFilter(const string &newFilter)
@@ -571,6 +562,8 @@ void Browser::setFilter(const string &newFilter)
void Browser::populate(const vector<IndexPtr> &indexes)
{
+ m_loading = false;
+
try {
Registry reg(Path::prefixRoot(Path::REGISTRY));
@@ -609,6 +602,9 @@ void Browser::populate(const vector<IndexPtr> &indexes)
desc.c_str());
MessageBox(handle(), msg, AUTO_STR("ReaPack"), MB_OK);
}
+
+ if(!isVisible())
+ show();
}
void Browser::transferActions()
diff --git a/src/browser.hpp b/src/browser.hpp
@@ -142,7 +142,6 @@ private:
ReaPack *m_reapack;
bool m_loading;
- bool m_loaded;
int m_currentIndex;
Filter m_filter;
diff --git a/src/index.cpp b/src/index.cpp
@@ -17,7 +17,6 @@
#include "index.hpp"
-#include "download.hpp"
#include "encoding.hpp"
#include "errors.hpp"
#include "filesystem.hpp"
@@ -82,24 +81,6 @@ IndexPtr Index::load(const string &name, const char *data)
return IndexPtr(ri);
}
-FileDownload *Index::fetch(const Remote &remote,
- const bool stale, const NetworkOpts &opts)
-{
- time_t mtime = 0, now = time(nullptr);
-
- if(FS::mtime(pathFor(remote.name()), &mtime)) {
- const time_t threshold = stale ? 0 : (7 * 24 * 3600);
-
- if(mtime > now - threshold)
- return nullptr;
- }
-
- const Path &path = pathFor(remote.name());
- auto fd = new FileDownload(path, remote.url(), opts, Download::NoCacheFlag);
- fd->setName(remote.name());
- return fd;
-}
-
Index::Index(const string &name)
: m_name(name)
{
diff --git a/src/index.hpp b/src/index.hpp
@@ -29,7 +29,6 @@
#include "source.hpp"
class Index;
-class FileDownload;
class Path;
class Remote;
class TiXmlElement;
@@ -41,7 +40,6 @@ class Index : public std::enable_shared_from_this<const Index> {
public:
static Path pathFor(const std::string &name);
static IndexPtr load(const std::string &name, const char *data = nullptr);
- static FileDownload *fetch(const Remote &, bool stale, const NetworkOpts &);
Index(const std::string &name);
~Index();
diff --git a/src/manager.cpp b/src/manager.cpp
@@ -66,7 +66,7 @@ void Manager::onInit()
{AUTO_STR("State"), 60},
});
- m_list->onActivate([=] { about(m_list->currentIndex()); });
+ m_list->onActivate([=] { m_reapack->about(getRemote(m_list->currentIndex())); });
m_list->onContextMenu(bind(&Manager::fillContextMenu,
this, placeholders::_1, placeholders::_2));
@@ -185,7 +185,7 @@ void Manager::onCommand(const int id, int)
default:
const int action = id >> 8;
if(action == ACTION_ABOUT)
- about(id & 0xff);
+ m_reapack->about(getRemote(id & 0xff));
break;
}
}
@@ -404,8 +404,8 @@ void Manager::refreshIndex()
for(size_t i = 0; i < selection.size(); i++)
remotes[i] = getRemote(selection[i]);
- m_reapack->fetchIndexes(remotes,
- bind(&ReaPack::refreshBrowser, m_reapack), handle(), true);
+ if(Transaction *tx = m_reapack->setupTransaction())
+ tx->fetchIndexes(remotes, true);
}
void Manager::uninstall()
@@ -447,15 +447,6 @@ void Manager::setChange(const int increment)
disable(m_apply);
}
-void Manager::about(const int index)
-{
- const Remote &remote = getRemote(index);
- m_reapack->fetchIndex(remote, [=] (const IndexPtr &index) {
- if(index)
- m_reapack->about()->setDelegate(make_shared<AboutIndexDelegate>(index));
- }, handle());
-}
-
void Manager::copyUrl()
{
vector<string> values;
diff --git a/src/manager.hpp b/src/manager.hpp
@@ -68,7 +68,6 @@ private:
void uninstall();
void toggle(boost::optional<bool> &, bool current);
void refreshIndex();
- void about(int index);
void copyUrl();
void launchBrowser();
void importExport();
diff --git a/src/reapack.cpp b/src/reapack.cpp
@@ -225,12 +225,26 @@ Remote ReaPack::remote(const string &name) const
return m_config->remotes.get(name);
}
+void ReaPack::about(const Remote &repo)
+{
+ Transaction *tx = setupTransaction();
+ if(!tx)
+ return;
+
+ const vector<Remote> repos = {repo};
+
+ tx->fetchIndexes(repos);
+ tx->onFinish([=] {
+ const auto &indexes = tx->getIndexes(repos);
+ if(!indexes.empty())
+ about()->setDelegate(make_shared<AboutIndexDelegate>(indexes.front()));
+ });
+ tx->runTasks();
+}
+
void ReaPack::aboutSelf()
{
- fetchIndex(remote("ReaPack"), [=] (const IndexPtr &index) {
- if(index)
- about()->setDelegate(make_shared<AboutIndexDelegate>(index));
- }, m_mainWindow);
+ about(remote("ReaPack"));
}
About *ReaPack::about(const bool instantiate)
@@ -273,111 +287,6 @@ Browser *ReaPack::browsePackages()
return m_browser;
}
-void ReaPack::fetchIndex(const Remote &remote,
- const IndexCallback &callback, HWND parent, const bool stale)
-{
- fetchIndexes({remote}, [=] (const vector<IndexPtr> &indexes) {
- callback(indexes.empty() ? nullptr : indexes.front());
- }, parent, stale);
-}
-
-void ReaPack::fetchIndexes(const vector<Remote> &remotes,
- const IndexesCallback &callback, HWND parent, const bool stale)
-{
- // I don't know why, but at least on OSX giving the manager window handle
- // (in `parent`) to the progress dialog prevents it from being shown at all
- // while still taking the focus away from the manager dialog.
-
- ThreadPool *pool = new ThreadPool;
- Dialog *progress = Dialog::Create<Progress>(m_instance, m_mainWindow, pool);
-
- auto load = [=] {
- Dialog::Destroy(progress);
- delete pool;
-
- vector<IndexPtr> indexes;
-
- for(const Remote &remote : remotes) {
- if(!FS::exists(Index::pathFor(remote.name())))
- continue;
-
- IndexPtr index = loadIndex(remote, parent);
-
- if(index)
- indexes.push_back(index);
- }
-
- callback(indexes);
- };
-
- pool->onDone(load);
-
- for(const Remote &remote : remotes)
- doFetchIndex(remote, pool, parent, stale);
-
- if(pool->idle())
- load();
-}
-
-void ReaPack::doFetchIndex(const Remote &remote, ThreadPool *pool,
- HWND parent, const bool stale)
-{
- FileDownload *dl = Index::fetch(remote, stale, m_config->network);
-
- if(!dl)
- return;
-
- const auto warn = [=] (const string &desc, const auto_char *title) {
- auto_char msg[512];
- auto_snprintf(msg, auto_size(msg),
- AUTO_STR("ReaPack could not download %s's index.\n\n")
-
- AUTO_STR("Try again later. ")
- AUTO_STR("If the problem persist, contact the maintainer of this repository.\n\n")
-
- AUTO_STR("[Error description: %s]"),
- make_autostring(remote.name()).c_str(), make_autostring(desc).c_str()
- );
-
- MessageBox(parent, msg, title, MB_OK);
- };
-
- dl->onFinish([=] {
- if(!dl->save())
- warn(FS::lastError(), AUTO_STR("Write Failed"));
- else if(dl->state() == ThreadTask::Failure &&
- (stale || !FS::exists(dl->path().target())))
- warn(dl->error().message, AUTO_STR("Download Failed"));
- });
-
- pool->push(dl);
-}
-
-IndexPtr ReaPack::loadIndex(const Remote &remote, HWND parent)
-{
- try {
- return Index::load(remote.name());
- }
- catch(const reapack_error &e) {
- const auto_string &desc = make_autostring(e.what());
-
- auto_char msg[512];
- auto_snprintf(msg, auto_size(msg),
- AUTO_STR("ReaPack could not read %s's index.\n\n")
-
- AUTO_STR("Synchronize your packages and try again later.\n")
- AUTO_STR("If the problem persist, contact the maintainer of this repository.\n\n")
-
- AUTO_STR("[Error description: %s]"),
- make_autostring(remote.name()).c_str(), desc.c_str()
- );
-
- MessageBox(parent, msg, AUTO_STR("ReaPack"), MB_OK);
- }
-
- return nullptr;
-}
-
Transaction *ReaPack::setupTransaction()
{
if(m_progress && m_progress->isVisible())
@@ -409,13 +318,14 @@ Transaction *ReaPack::setupTransaction()
Dialog::Destroy(m_progress);
m_progress = nullptr;
- if(m_tx->isCancelled() || m_tx->receipt()->empty())
- return;
+ if(!m_tx->isCancelled() && !m_tx->receipt()->empty()) {
+ LockDialog managerLock(m_manager);
+ LockDialog browserLock(m_browser);
- LockDialog managerLock(m_manager);
- LockDialog browserLock(m_browser);
+ Dialog::Show<Report>(m_instance, m_mainWindow, *m_tx->receipt());
+ }
- Dialog::Show<Report>(m_instance, m_mainWindow, *m_tx->receipt());
+ refreshBrowser();
});
m_tx->setObsoleteHandler([=] (vector<Registry::Entry> &entries) {
@@ -436,9 +346,6 @@ void ReaPack::teardownTransaction()
{
delete m_tx;
m_tx = nullptr;
-
- // refresh only once the registry is close
- refreshBrowser();
}
void ReaPack::refreshManager()
diff --git a/src/reapack.hpp b/src/reapack.hpp
@@ -31,20 +31,14 @@
class About;
class Browser;
class Config;
-class Index;
class Manager;
class Progress;
class Remote;
-class ThreadPool;
class Transaction;
-typedef std::shared_ptr<const Index> IndexPtr;
-
class ReaPack {
public:
typedef std::function<void ()> ActionCallback;
- typedef std::function<void (const IndexPtr &)> IndexCallback;
- typedef std::function<void (const std::vector<IndexPtr> &)> IndexesCallback;
static const char *VERSION;
static const char *BUILDTIME;
@@ -72,6 +66,7 @@ public:
void importRemote();
void manageRemotes();
void aboutSelf();
+ void about(const Remote &);
About *about(bool instantiate = true);
Browser *browsePackages();
void refreshManager();
@@ -79,18 +74,11 @@ public:
Remote remote(const std::string &name) const;
- void fetchIndex(const Remote &remote, const IndexCallback &,
- HWND parent, bool stale = false);
- void fetchIndexes(const std::vector<Remote> &,
- const IndexesCallback &, HWND parent, bool stale = false);
-
Transaction *setupTransaction();
Config *config() const { return m_config; }
private:
void registerSelf();
- void doFetchIndex(const Remote &remote, ThreadPool *, HWND, bool stale);
- IndexPtr loadIndex(const Remote &remote, HWND);
void teardownTransaction();
std::map<int, ActionCallback> m_actions;
diff --git a/src/transaction.cpp b/src/transaction.cpp
@@ -30,6 +30,8 @@
using namespace std;
+static const time_t STALE_THRESHOLD = 7 * 24 * 3600;
+
Transaction::Transaction(Config *config)
: m_isCancelled(false), m_config(config),
m_registry(Path::prefixRoot(Path::REGISTRY))
@@ -68,18 +70,7 @@ void Transaction::synchronize(const Remote &remote,
else if(!boost::logic::indeterminate(remote.autoInstall()))
opts.autoInstall = remote.autoInstall();
- fetchIndex(remote, [=] {
- IndexPtr ri;
-
- try {
- ri = Index::load(remote.name());
- }
- catch(const reapack_error &e) {
- // index file is invalid (load error)
- m_receipt.addError({e.what(), remote.name()});
- return;
- }
-
+ fetchIndex(remote, true, [=] (const IndexPtr &ri) {
for(const Package *pkg : ri->packages())
synchronize(pkg, opts);
@@ -116,26 +107,76 @@ void Transaction::synchronize(const Package *pkg, const InstallOpts &opts)
m_nextQueue.push(make_shared<InstallTask>(latest, false, regEntry, nullptr, this));
}
-void Transaction::fetchIndex(const Remote &remote, const function<void()> &cb)
+void Transaction::fetchIndexes(const vector<Remote> &remotes, const bool stale)
+{
+ for(const Remote &remote : remotes)
+ fetchIndex(remote, stale);
+}
+
+vector<IndexPtr> Transaction::getIndexes(const vector<Remote> &remotes) const
+{
+ vector<IndexPtr> indexes;
+
+ for(const Remote &remote : remotes) {
+ const auto it = m_indexes.find(remote.name());
+ if(it != m_indexes.end())
+ indexes.push_back(it->second);
+ }
+
+ return indexes;
+}
+
+void Transaction::fetchIndex(const Remote &remote, const bool stale,
+ const function<void (const IndexPtr &)> &cb)
{
- FileDownload *dl = Index::fetch(remote, true, m_config->network);
+ const auto load = [=] {
+ const IndexPtr ri = loadIndex(remote);
+ if(cb && ri)
+ cb(ri);
+ };
+
+ const Path &path = Index::pathFor(remote.name());
+ time_t mtime = 0, now = time(nullptr);
+ FS::mtime(path, &mtime);
- if(!dl) {
- // the index was last downloaded less than a few seconds ago
- cb();
+ if(!stale && mtime > now - STALE_THRESHOLD) {
+ load();
return;
}
+ auto dl = new FileDownload(path, remote.url(),
+ m_config->network, Download::NoCacheFlag);
+ dl->setName(remote.name());
+
dl->onFinish([=] {
if(!dl->save())
m_receipt.addError({FS::lastError(), dl->path().target().join()});
- else if(dl->state() == ThreadTask::Success)
- cb();
+
+ if(FS::exists(path))
+ load(); // try to load anyway, even on failure
});
m_threadPool.push(dl);
}
+IndexPtr Transaction::loadIndex(const Remote &remote)
+{
+ const auto it = m_indexes.find(remote.name());
+ if(it != m_indexes.end())
+ return it->second;
+
+ try {
+ const IndexPtr &ri = Index::load(remote.name());
+ m_indexes[remote.name()] = ri;
+ return ri;
+ }
+ catch(const reapack_error &e) {
+ m_receipt.addError({
+ "Couldn't load repository index: " + string(e.what()), remote.name()});
+ return nullptr;
+ }
+}
+
void Transaction::install(const Version *ver,
const bool pin, const ArchiveReaderPtr &reader)
{
diff --git a/src/transaction.hpp b/src/transaction.hpp
@@ -52,6 +52,8 @@ public:
void setCleanupHandler(const CleanupHandler &cb) { m_cleanupHandler = cb; }
void setObsoleteHandler(const ObsoleteHandler &cb) { m_promptObsolete = cb; }
+ void fetchIndexes(const std::vector<Remote> &, bool stale = false);
+ std::vector<IndexPtr> getIndexes(const std::vector<Remote> &) const;
void synchronize(const Remote &,
boost::optional<bool> forceAutoInstall = boost::none);
void install(const Version *, bool pin = false, const ArchiveReaderPtr & = nullptr);
@@ -83,7 +85,9 @@ private:
typedef std::priority_queue<TaskPtr,
std::vector<TaskPtr>, CompareTask> TaskQueue;
- void fetchIndex(const Remote &, const std::function<void ()> &);
+ void fetchIndex(const Remote &, bool stale,
+ const std::function<void (const IndexPtr &)> & = {});
+ IndexPtr loadIndex(const Remote &);
void synchronize(const Package *, const InstallOpts &);
bool allFilesExists(const std::set<Path> &) const;
void registerQueued();
@@ -99,6 +103,7 @@ private:
Receipt m_receipt;
std::unordered_set<std::string> m_syncedRemotes;
+ std::map<std::string, IndexPtr> m_indexes;
std::unordered_set<std::string> m_inhibited;
std::unordered_set<Registry::Entry> m_obsolete;
