diff options
| author | Daniel Burrows <dburrows@debian.org> | 2008-10-29 09:16:17 -0700 |
|---|---|---|
| committer | Daniel Burrows <dburrows@debian.org> | 2008-10-29 09:16:17 -0700 |
| commit | cf760f50bf8963da7c35a061ba3b2a162c0fe2cd (patch) | |
| tree | 27961cebaa3f14caf7356bd168f6ecc6fbe18909 | |
| parent | 0a17acce0bf192c2b7ece3b5f74b474aaa187b3e (diff) | |
| download | aptitude-cf760f50bf8963da7c35a061ba3b2a162c0fe2cd.tar.gz | |
Massive refactoring and redesign to properly support background downloads in the GUI and running dpkg in a terminal.
This should probably have been split into a lot of smaller commits, but
it's too late now.
The GUI now runs downloads in a background thread, just like the curses
interface. In fact, it is *just* like the curses interface: the code of
ui_download_manager and download_thread has been parameterized so that it
can be used from either the curses frontend or the GUI frontend. These
files should eventually migrate to the generic directory. Some of the
parameterization left ugly stuff behind, like refcounted_progress and the
wacky redesign of the curses progress class (which took place solely so
that it could be unified with the GTK+ one). It may be worth taking some
time to clean this code up.
In addition, the code for download_install_manager has been refactored
again. Instead of exposing the different parts of "finish", which would
eliminate any hope of having a unified ui_download_manager harness, the
"finish" method (for all download managers) is now explicitly continuation
based, and the install manager uses this feature to break up the download.
The function that runs dpkg in a terminal now takes a continuation to be
invoked in the main thread, so it can fork dpkg in a subprocess, and then
call the continuation when the subprocess terminates. This means we don't
need to mess around with threads at all in the dpkg-terminal-creation
process, a huge relief. It also fits in with the generally continuation-y
feel of a lot of the aptitude code, and everyone loves continuations in
their GUI programs anyway!
Note: in this commit, I've adopted a policy of always returning right
after I invoke the continuation, to make it clear in the code what's
going on and to minimize the chances of a future edit screwing this up.
(if I really wanted to get serious about that, of course, I would write
an INVOKE_CONTINUATION macro...)
A few other minor things were touched up here: for instance, the program
no longer enters an infinite loop after running dpkg, and more download
events are logged. There's still more work to do around the edges:
changelog downloading from the GUI is broken, and the download status
tab is all messed up. Also, I'd like to support the status pipe from
dpkg (but I think that's a post-0.5 thing).
35 files changed, 1081 insertions, 683 deletions
diff --git a/src/cmdline/cmdline_do_action.cc b/src/cmdline/cmdline_do_action.cc index 0bf23869..e0ab6eb1 100644 --- a/src/cmdline/cmdline_do_action.cc +++ b/src/cmdline/cmdline_do_action.cc @@ -27,9 +27,10 @@ using namespace std; namespace { - pkgPackageManager::OrderResult run_dpkg_directly(sigc::slot1<pkgPackageManager::OrderResult, int> f) + void run_dpkg_directly(sigc::slot1<pkgPackageManager::OrderResult, int> f, + sigc::slot1<void, pkgPackageManager::OrderResult> k) { - return f(aptcfg->FindI("APT::Status-Fd", -1)); + k(f(aptcfg->FindI("APT::Status-Fd", -1))); } } diff --git a/src/cmdline/cmdline_upgrade.cc b/src/cmdline/cmdline_upgrade.cc index f523840a..9bd23dfd 100644 --- a/src/cmdline/cmdline_upgrade.cc +++ b/src/cmdline/cmdline_upgrade.cc @@ -29,9 +29,10 @@ namespace { - pkgPackageManager::OrderResult run_dpkg_directly(sigc::slot1<pkgPackageManager::OrderResult, int> f) + void run_dpkg_directly(sigc::slot1<pkgPackageManager::OrderResult, int> f, + sigc::slot1<void, pkgPackageManager::OrderResult> k) { - return f(aptcfg->FindI("APT::Status-Fd", -1)); + k(f(aptcfg->FindI("APT::Status-Fd", -1))); } } diff --git a/src/cmdline/cmdline_util.cc b/src/cmdline/cmdline_util.cc index 30a78875..d4e8cd7d 100644 --- a/src/cmdline/cmdline_util.cc +++ b/src/cmdline/cmdline_util.cc @@ -75,7 +75,7 @@ void ui_solution_screen() file_quit.connect(sigc::ptr_fun(cwidget::toplevel::exitmain)); progress_ref p = gen_progress_bar(); - do_new_package_view(*p.unsafe_get_ref()); + do_new_package_view(*p->get_progress().unsafe_get_ref()); do_examine_solution(); ui_main(); @@ -371,6 +371,15 @@ namespace } } +namespace +{ + template<typename T> + void assign(const T &val, T *target) + { + *target = val; + } +} + download_manager::result cmdline_do_download(download_manager *m, int verbose) { @@ -403,7 +412,9 @@ download_manager::result cmdline_do_download(download_manager *m, do { pkgAcquire::RunResult download_res = m->do_download(); - finish_res = m->finish(download_res, progress); + m->finish(download_res, &progress, + sigc::bind(sigc::ptr_fun(&assign<download_manager::result>), + &finish_res)); } while(finish_res == download_manager::do_again); diff --git a/src/download_list.cc b/src/download_list.cc index 3a73b781..876aed87 100644 --- a/src/download_list.cc +++ b/src/download_list.cc @@ -72,7 +72,7 @@ download_list::workerinf::workerinf(const string &_msg, unsigned long _current, static cw::widget_ref download_summary(const download_list_ref &l, bool failed, bool already_cancelled, - cw::util::slot0arg cancel_slot, + const sigc::slot0<void> &cancel_slot, double FetchedBytes, unsigned long ElapsedTime, double CurrentCPS) @@ -131,10 +131,9 @@ static cw::widget_ref download_summary(const download_list_ref &l, } -download_list::download_list(cw::util::slot0arg _abortslot, - bool _display_messages, bool _display_cumulative_progress) - :start(0), sticky_end(true), cancelled(false), failed(false), - abortslot(_abortslot), display_messages(_display_messages), +download_list::download_list(bool _display_messages, bool _display_cumulative_progress) + :start(0), sticky_end(true), was_cancelled(false), failed(false), + display_messages(_display_messages), display_cumulative_progress(_display_cumulative_progress), startx(0), @@ -147,10 +146,9 @@ download_list::download_list(cw::util::slot0arg _abortslot, void download_list::cancel() { - cancelled=true; + was_cancelled=true; - if(abortslot) - (*abortslot)(); + cancelled(); } void download_list::destroy() @@ -449,8 +447,8 @@ void download_list::Stop(download_signal_log &manager, const sigc::slot0<void> & { cw::widget_ref summary = download_summary(this, failed, - cancelled, - abortslot, + was_cancelled, + cancelled.make_slot(), manager.get_fetched_bytes(), manager.get_elapsed_time(), manager.get_currentCPS()); @@ -485,7 +483,7 @@ void download_list::Pulse(pkgAcquire *Owner, download_signal_log &manager, if(get_visible()) cw::toplevel::queuelayout(); // Force an update - if(cancelled) + if(was_cancelled) k(false); else k(true); diff --git a/src/download_list.h b/src/download_list.h index 0735d6b6..e8520dc6 100644 --- a/src/download_list.h +++ b/src/download_list.h @@ -76,13 +76,11 @@ class download_list:public cwidget::widgets::widget bool sticky_end; // Set to true if the user asks to cancel the download - bool cancelled; + bool was_cancelled; // Set to true if an item failed. bool failed; - cwidget::util::slot0arg abortslot; - // Will we display things other than the current workers? bool display_messages; @@ -125,15 +123,13 @@ protected: void paint(const cwidget::style &st); bool handle_key(const cwidget::config::key &k); - download_list(cwidget::util::slot0arg _abortslot = NULL, - bool _display_messages = true, + download_list(bool _display_messages = true, bool _display_cumulative_progress = true); public: - static cwidget::util::ref_ptr<download_list> create(cwidget::util::slot0arg abortslot = NULL, - bool display_messages = true, + static cwidget::util::ref_ptr<download_list> create(bool display_messages = true, bool display_cumulative_progress = true) { - cwidget::util::ref_ptr<download_list> rval(new download_list(abortslot, display_messages, display_cumulative_progress)); + cwidget::util::ref_ptr<download_list> rval(new download_list(display_messages, display_cumulative_progress)); rval->decref(); return rval; } @@ -158,6 +154,8 @@ public: cwidget::widgets::point get_cursorloc() { return cwidget::widgets::point(0,0); } bool focus_me() {return true;} + sigc::signal<void> cancelled; + // FIXME: overriding this is a terrible hack. A cancel() hook should be // used instead. void destroy(); diff --git a/src/download_thread.cc b/src/download_thread.cc index e79b90bb..591bbb02 100644 --- a/src/download_thread.cc +++ b/src/download_thread.cc @@ -1,6 +1,6 @@ // download_thread.cc // -// Copyright (C) 2005 Daniel Burrows +// Copyright (C) 2005, 2008 Daniel Burrows // // This program is free software; you can redistribute it and/or // modify it under the terms of the GNU General Public License as @@ -31,53 +31,32 @@ namespace cw = cwidget; template<typename RVal> -class background_execute : public cw::toplevel::event +void background_execute(sigc::slot0<RVal> slot, + cw::threads::box<RVal> *return_box) { - sigc::slot0<RVal> slot; - - cw::threads::box<RVal> &return_box; -public: - background_execute(const sigc::slot0<RVal> &_slot, - cw::threads::box<RVal> &_return_box) - :slot(_slot), return_box(_return_box) - { - } - - void dispatch() - { - return_box.put(slot()); - } -}; + return_box->put(slot()); +} template<> -class background_execute<void> : public cw::toplevel::event -{ - sigc::slot0<void> slot; - cw::threads::box<void> &return_box; -public: - background_execute(const sigc::slot0<void> &_slot, - cw::threads::box<void> &_return_box) - :slot(_slot), return_box(_return_box) - { - } - - void dispatch() - { - slot(); - return_box.put(); - } -}; +void background_execute<void>(sigc::slot0<void> slot, + cw::threads::box<void> *return_box) +{ + slot(); + return_box->put(); +} /** Run the given method call in the foreground and return its value. */ template<typename C, typename RVal> static RVal do_foreground_execute(C *inst, - RVal (C::* fun) ()) + RVal (C::* fun) (), + const sigc::slot1<void, sigc::slot0<void> > &post_thunk) { cw::threads::box<RVal> return_box; - cw::toplevel::post_event(new background_execute<RVal>(sigc::mem_fun(inst, fun), - return_box)); + post_thunk(sigc::bind(sigc::ptr_fun(&background_execute<RVal>), + sigc::mem_fun(inst, fun), + &return_box)); return return_box.take(); } @@ -87,12 +66,14 @@ template<typename C, typename RVal, typename Arg0> static RVal do_foreground_execute(C *inst, Arg0 arg0, - RVal (C::* fun) (Arg0)) + RVal (C::* fun) (Arg0), + const sigc::slot1<void, sigc::slot0<void> > &post_thunk) { cw::threads::box<RVal> return_box; - cw::toplevel::post_event(new background_execute<RVal>(bind(sigc::mem_fun(inst, fun), arg0), - return_box)); + post_thunk(sigc::bind(sigc::ptr_fun(&background_execute<RVal>), + sigc::bind(sigc::mem_fun(inst, fun), arg0), + &return_box)); return return_box.take(); } @@ -103,12 +84,14 @@ static RVal do_foreground_execute(C *inst, Arg0 arg0, Arg1 arg1, - RVal (C::* fun) (Arg0, Arg1)) + RVal (C::* fun) (Arg0, Arg1), + const sigc::slot1<void, sigc::slot0<void> > &post_thunk) { cw::threads::box<RVal> return_box; - cw::toplevel::post_event(new background_execute<RVal>(bind(sigc::mem_fun(inst, fun), arg0, arg1), - return_box)); + post_thunk(sigc::bind(sigc::ptr_fun(&background_execute<RVal>), + sigc::bind(sigc::mem_fun(inst, fun), arg0, arg1), + &return_box)); return return_box.take(); } @@ -120,12 +103,14 @@ RVal do_foreground_execute(C *inst, Arg0 arg0, Arg1 arg1, Arg2 arg2, - RVal (C::* fun) (Arg0, Arg1, Arg2)) + RVal (C::* fun) (Arg0, Arg1, Arg2), + const sigc::slot1<void, sigc::slot0<void> > &post_thunk) { cw::threads::box<RVal> return_box; - cw::toplevel::post_event(new background_execute<RVal>(bind(sigc::mem_fun(inst, fun), arg0, arg1, arg2), - return_box)); + post_thunk(sigc::bind(sigc::ptr_fun(&background_execute<RVal>), + sigc::bind(sigc::mem_fun(inst, fun), arg0, arg1, arg2), + &return_box)); return return_box.take(); } @@ -134,7 +119,8 @@ void background_status::Fetched(unsigned long Size, unsigned long ResumePoint) { do_foreground_execute(real_status, Size, ResumePoint, - &download_signal_log::Fetched); + &download_signal_log::Fetched, + post_thunk); } bool background_status::MediaChange(std::string Media, std::string Drive) @@ -146,29 +132,30 @@ bool background_status::MediaChange(std::string Media, std::string Drive) const sigc::slot1<void, bool> &> (real_status, Media, Drive, sigc::mem_fun(return_box, &cw::threads::box<bool>::put), - &download_signal_log::MediaChange); + &download_signal_log::MediaChange, + post_thunk); return return_box.take(); } void background_status::IMSHit(pkgAcquire::ItemDesc &item) { - do_foreground_execute<download_signal_log, void, pkgAcquire::ItemDesc &>(real_status, item, &download_signal_log::IMSHit); + do_foreground_execute<download_signal_log, void, pkgAcquire::ItemDesc &>(real_status, item, &download_signal_log::IMSHit, post_thunk); } void background_status::Fetch(pkgAcquire::ItemDesc &item) { - do_foreground_execute<download_signal_log, void, pkgAcquire::ItemDesc &>(real_status, item, &download_signal_log::Fetch); + do_foreground_execute<download_signal_log, void, pkgAcquire::ItemDesc &>(real_status, item, &download_signal_log::Fetch, post_thunk); } void background_status::Done(pkgAcquire::ItemDesc &item) { - do_foreground_execute<download_signal_log, void, pkgAcquire::ItemDesc &>(real_status, item, &download_signal_log::Done); + do_foreground_execute<download_signal_log, void, pkgAcquire::ItemDesc &>(real_status, item, &download_signal_log::Done, post_thunk); } void background_status::Fail(pkgAcquire::ItemDesc &item) { - do_foreground_execute<download_signal_log, void, pkgAcquire::ItemDesc &>(real_status, item, &download_signal_log::Fail); + do_foreground_execute<download_signal_log, void, pkgAcquire::ItemDesc &>(real_status, item, &download_signal_log::Fail, post_thunk); } bool background_status::Pulse(pkgAcquire *Owner) @@ -180,14 +167,15 @@ bool background_status::Pulse(pkgAcquire *Owner) const sigc::slot1<void, bool> &>(real_status, Owner, sigc::mem_fun(&return_box, &cw::threads::box<bool>::put), - &download_signal_log::Pulse); + &download_signal_log::Pulse, + post_thunk); return return_box.take(); } void background_status::Start() { - do_foreground_execute(real_status, &download_signal_log::Start); + do_foreground_execute(real_status, &download_signal_log::Start, post_thunk); } void background_status::Stop() @@ -199,34 +187,28 @@ void background_status::Stop() const sigc::slot0<void> &>(real_status, sigc::mem_fun(&return_box, &cw::threads::box<void>::put), - &download_signal_log::Stop); + &download_signal_log::Stop, + post_thunk); return_box.take(); } -class download_thread_complete_event : public cw::toplevel::event +namespace { - download_thread *t; - pkgAcquire::RunResult res; - - sigc::slot2<void, download_thread *, pkgAcquire::RunResult> continuation; -public: - download_thread_complete_event(download_thread *_t, - pkgAcquire::RunResult _res, - sigc::slot2<void, download_thread *, pkgAcquire::RunResult> &_continuation) - :t(_t), res(_res), continuation(_continuation) - { - } - - void dispatch() + void do_download_complete(download_thread *t, + pkgAcquire::RunResult res, + sigc::slot2<void, download_thread *, pkgAcquire::RunResult> &continuation) { t->join(); continuation(t, res); } -}; +} void download_thread::operator()() { - cw::toplevel::post_event(new download_thread_complete_event(this, m->do_download(), - continuation)); + pkgAcquire::RunResult res = m->do_download(); + + post_thunk(sigc::bind(sigc::ptr_fun(&do_download_complete), + this, res, + continuation)); } diff --git a/src/download_thread.h b/src/download_thread.h index c539c03a..44311505 100644 --- a/src/download_thread.h +++ b/src/download_thread.h @@ -1,6 +1,6 @@ // download_thread -*-c++-*- // -// Copyright (C) 2005 Daniel Burrows +// Copyright (C) 2005, 2008 Daniel Burrows // // This program is free software; you can redistribute it and/or // modify it under the terms of the GNU General Public License as @@ -44,6 +44,8 @@ class download_signal_log; class background_status : public pkgAcquireStatus { download_signal_log *real_status; + sigc::slot1<void, sigc::slot0<void> > post_thunk; + public: void Fetched(unsigned long Size, unsigned long ResumePoint); bool MediaChange(std::string Media, std::string Drive); @@ -56,8 +58,10 @@ public: void Stop(); void Complete(); - background_status(download_signal_log *_real_status) - :real_status(_real_status) + background_status(download_signal_log *_real_status, + const sigc::slot1<void, sigc::slot0<void> > &_post_thunk) + : real_status(_real_status), + post_thunk(_post_thunk) { } }; @@ -67,6 +71,9 @@ class download_thread { cwidget::threads::box<bool> cancelled; + /** A callback that posts thunks to be run in the main thread. */ + sigc::slot1<void, sigc::slot0<void> > post_thunk; + /** The bundled download_manager object. It should have been * initialized using a background_status wrapper as above, and you * should join() this thread before deleting it. (it is also OK @@ -87,8 +94,10 @@ class download_thread download_thread &operator=(const download_thread &other); public: download_thread(download_manager *manager, + const sigc::slot1<void, sigc::slot0<void> > &_post_thunk, const sigc::slot2<void, download_thread *, pkgAcquire::RunResult> &_continuation) - : cancelled(false), m(manager), continuation(_continuation), t(NULL) + : cancelled(false), post_thunk(_post_thunk), + m(manager), continuation(_continuation), t(NULL) { } diff --git a/src/generic/apt/download_install_manager.cc b/src/generic/apt/download_install_manager.cc index c5fb4fd5..d03271c2 100644 --- a/src/generic/apt/download_install_manager.cc +++ b/src/generic/apt/download_install_manager.cc @@ -30,6 +30,8 @@ #include <apt-pkg/error.h> #include <apt-pkg/sourcelist.h> +#include <sigc++/bind.h> + #include <pthread.h> #include <signal.h> @@ -179,13 +181,9 @@ pkgPackageManager::OrderResult download_install_manager::run_dpkg(int status_fd) return pmres; } -pkgPackageManager::OrderResult download_install_manager::finish_run_dpkg() -{ - return run_dpkg_in_terminal(sigc::mem_fun(*this, &download_install_manager::run_dpkg)); -} - -download_manager::result download_install_manager::finish_post_dpkg(pkgPackageManager::OrderResult dpkg_result, - OpProgress &progress) +void download_install_manager::finish_post_dpkg(pkgPackageManager::OrderResult dpkg_result, + OpProgress *progress, + const sigc::slot1<void, result> &k) { result rval = success; @@ -228,32 +226,41 @@ download_manager::result download_install_manager::finish_post_dpkg(pkgPackageMa // world. // // This implicitly updates the package state file on disk. - apt_load_cache(&progress, true); + apt_load_cache(progress, true); if(aptcfg->FindB(PACKAGE "::Forget-New-On-Install", false)) { if(apt_cache_file != NULL) { (*apt_cache_file)->forget_new(NULL); - (*apt_cache_file)->save_selection_list(progress); + (*apt_cache_file)->save_selection_list(*progress); post_forget_new_hook(); } } } - return rval; + k(rval); } -download_manager::result download_install_manager::finish(pkgAcquire::RunResult result, - OpProgress &progress) +void download_install_manager::finish(pkgAcquire::RunResult result, + OpProgress *progress, + const sigc::slot1<void, result> &k) { const download_manager::result pre_res = finish_pre_dpkg(result); - pkgPackageManager::OrderResult dpkg_res; if(pre_res == success) - dpkg_res = finish_run_dpkg(); + { + run_dpkg_in_terminal(sigc::mem_fun(*this, &download_install_manager::run_dpkg), + sigc::bind(sigc::mem_fun(*this, &download_install_manager::finish_post_dpkg), + progress, + k)); + return; + } else - dpkg_res = pkgPackageManager::Failed; - - return finish_post_dpkg(dpkg_res, progress); + { + finish_post_dpkg(pkgPackageManager::Failed, + progress, + k); + return; + } } diff --git a/src/generic/apt/download_install_manager.h b/src/generic/apt/download_install_manager.h index 49a7760c..cfa096c7 100644 --- a/src/generic/apt/download_install_manager.h +++ b/src/generic/apt/download_install_manager.h @@ -39,12 +39,16 @@ // This complex slot-based mechanism is used so that we can still have // a generic downloading framework (e.g., ui_download_manager). -// The type of a function that runs dpkg in a terminal. It's passed a -// function that invokes dpkg as currently appropriate, given the -// status file descriptor on which to invoke it (or -1 to discard -// status messages). The slot that's passed in can be invoked in a -// background thread or a sub-process. -typedef sigc::slot1<pkgPackageManager::OrderResult, sigc::slot1<pkgPackageManager::OrderResult, int> > run_dpkg_in_terminal_func; +// The type of a function that runs dpkg in a terminal. Its first +// argument is a function that invokes dpkg as currently appropriate, +// given the status file descriptor on which to invoke it (or -1 to +// discard status messages); it can be invoked in a background thread +// or a sub-process. Its second argument is a continuation to invoke +// after we finish running dpkg, and it should be invoked in a +// foreground thread. +typedef sigc::slot2<void, + sigc::slot1<pkgPackageManager::OrderResult, int>, + sigc::slot1<void, pkgPackageManager::OrderResult> > run_dpkg_in_terminal_func; /** Manages downloading and installing packages. */ class download_install_manager : public download_manager @@ -74,6 +78,38 @@ class download_install_manager : public download_manager */ result execute_install_run(pkgAcquire::RunResult res, OpProgress &load_progress); + + /** \brief Invoke the part of finish() that runs after dpkg. + * + * This function must be invoked in the foreground thread. + * + * \param dpkg_result The return value of run_dpkg(). + * \param progress An object to use in displaying the + * progress when reloading. + * + * \param k A continuation to invoke with the result of + * the install process; if the result is do_again, + * then the receiver should repeat the process + * beginning with do_download(). + */ + void finish_post_dpkg(pkgPackageManager::OrderResult dpkg_result, + OpProgress *progress, + const sigc::slot1<void, result> &k); + + + /** \brief Invoke the part of finish() that runs before dpkg. + * + * This function must be invoked in the foreground thread. + * + * \param result The return value of do_download(). + * + * \return success or failure; if success is returned, you may + * proceed to invoke finish_run_dpkg(). Otherwise, + * finish_post_dpkg() should be invoked with + * pkgPackageManager::Failed. + */ + result finish_pre_dpkg(pkgAcquire::RunResult result); + public: /** \param _download_only if \b true, this download process will * stop after downloading files (i.e., it won't run the package @@ -108,46 +144,11 @@ public: /** If download_only is false, call the package manager to install * or remove packages. * - * The status file descriptor will be set - */ - result finish(pkgAcquire::RunResult result, - OpProgress &progress); - - /** \brief Invoke the part of finish() that runs before dpkg. - * - * This function must be invoked in the foreground thread. - * - * \param result The return value of do_download(). - * - * \return success or failure; if success is returned, you may - * proceed to invoke finish_run_dpkg(). Otherwise, - * finish_post_dpkg() should be invoked with - * pkgPackageManager::Failed. - */ - result finish_pre_dpkg(pkgAcquire::RunResult result); - - /** \brief Actually run dpkg. - * - * This function may be invoked in a background thread or even in a - * sub-process; it will use the callback passed to the constructor - * to wrap the actual invocation of dpkg. Its return value should - * be passed to finish_post_dpkg(). - */ - pkgPackageManager::OrderResult finish_run_dpkg(); - - /** \brief Invoke the part of finish() that runs after dpkg. - * - * This function must be invoked in the foreground thread. - * - * \param dpkg_result The return value of finish_run_dpkg(). - * \param progress An object to use in displaying the - * progress when reloading. - * - * \return the result of the install process; if do_again, then the - * caller should repeat the process beginning with do_download(). + * The package manager will be invoked via run_dpkg_in_terminal. */ - result finish_post_dpkg(pkgPackageManager::OrderResult dpkg_result, - OpProgress &progress); + void finish(pkgAcquire::RunResult result, + OpProgress *progress, + const sigc::slot1<void, result> &k); /** Invoked after an automatic 'forget new' operation. */ sigc::signal0<void> post_forget_new_hook; diff --git a/src/generic/apt/download_manager.h b/src/generic/apt/download_manager.h index 647ec0b3..b077d9c0 100644 --- a/src/generic/apt/download_manager.h +++ b/src/generic/apt/download_manager.h @@ -1,6 +1,6 @@ // download_manager.h -*-c++-*- // -// Copyright (C) 2005 Daniel Burrows +// Copyright (C) 2005, 2008 Daniel Burrows // // This program is free software; you can redistribute it and/or // modify it under the terms of the GNU General Public License as @@ -24,6 +24,7 @@ // For RunResult #include <apt-pkg/acquire.h> +#include <sigc++/slot.h> #include <sigc++/trackable.h> /** \brief An abstract interface for download processes, and two implementations. @@ -88,15 +89,31 @@ public: * * \param result the result of do_download(). * - * \param progress a progress bar that may be used by the - * finishing process. + * \param progress a progress bar that may be used by the finishing + * process. This should not be destroyed until the + * continuation is invoked. * - * \return the result of the post-download operations; if this is - * do_again, then the frontend code is expected to repeat the - * download and post-download tasks (as many times as needed). + * \param k a continuation to invoke when the post-download + * operations are complete; it is passed the result of finish(). + * If the argument is do_again, the frontend should repeat the + * download and post-download tasks. The download manager may + * arrange for finish() to complete early while a long-running task + * executes in a background thread, as long as k() is later invoked + * in a foreground thread. + * + * The reason for passing a continuation is that some download + * managers need to run even more stuff in a background thread; + * this gives them room to do so. The main example of this is the + * download_install_manager. By building this support into the + * download_manager interface, we support generic install harnesses + * like ui_download_manager. + * + * \note Why is prepare() not built using continuations? Simple: + * none of the managers seem to need that. */ - virtual result finish(pkgAcquire::RunResult result, - OpProgress &progress) = 0; + virtual void finish(pkgAcquire::RunResult result, + OpProgress *progress, + const sigc::slot1<void, result> &k) = 0; }; #endif diff --git a/src/generic/apt/download_update_manager.cc b/src/generic/apt/download_update_manager.cc index 99b4a870..e9f6e3e7 100644 --- a/src/generic/apt/download_update_manager.cc +++ b/src/generic/apt/download_update_manager.cc @@ -267,9 +267,9 @@ namespace } } -download_manager::result -download_update_manager::finish(pkgAcquire::RunResult res, - OpProgress &progress) +void download_update_manager::finish(pkgAcquire::RunResult res, + OpProgress *progress, + const sigc::slot1<void, result> &k) { if(log != NULL) log->Complete(); @@ -277,7 +277,10 @@ download_update_manager::finish(pkgAcquire::RunResult res, apt_close_cache(); if(res != pkgAcquire::Continue) - return failure; + { + k(failure); + return; + } bool transientNetworkFailure = true; result rval = success; @@ -312,7 +315,8 @@ download_update_manager::finish(pkgAcquire::RunResult res, fetcher->Clean(aptcfg->FindDir("Dir::State::lists")+"partial/") == false)) { _error->Error(_("Couldn't clean out list directories")); - return failure; + k(failure); + return; } // Rebuild the apt caches as done in apt-get. cachefile is scoped @@ -321,10 +325,11 @@ download_update_manager::finish(pkgAcquire::RunResult res, // redundant work at the command-line. { pkgCacheFile cachefile; - if(!cachefile.BuildCaches(progress, true)) + if(!cachefile.BuildCaches(*progress, true)) { _error->Error(_("Couldn't rebuild package cache")); - return failure; + k(failure); + return; } } @@ -360,7 +365,7 @@ download_update_manager::finish(pkgAcquire::RunResult res, } else { - progress.OverallProgress(0, 0, 1, _("Updating debtags database...")); + progress->OverallProgress(0, 0, 1, _("Updating debtags database...")); std::string debtags_options = aptcfg->Find(PACKAGE "::Debtags-Update-Options", "--local"); @@ -407,13 +412,13 @@ download_update_manager::finish(pkgAcquire::RunResult res, debtags.c_str(), debtags_options.c_str(), e.errmsg().c_str()); } - progress.Progress(1); - progress.Done(); + progress->Progress(1); + progress->Done(); } #endif if(need_forget_new || need_autoclean) - apt_load_cache(&progress, true); + apt_load_cache(progress, true); if(apt_cache_file != NULL && need_forget_new) { @@ -433,6 +438,7 @@ download_update_manager::finish(pkgAcquire::RunResult res, post_autoclean_hook(); } - return rval; + k(rval); + return; } diff --git a/src/generic/apt/download_update_manager.h b/src/generic/apt/download_update_manager.h index 7a3f4d1d..9705ee4b 100644 --- a/src/generic/apt/download_update_manager.h +++ b/src/generic/apt/download_update_manager.h @@ -1,6 +1,6 @@ // download_update_manager.h -*-c++-*- // -// Copyright (C) 2005 Daniel Burrows +// Copyright (C) 2005, 2008 Daniel Burrows // // This program is free software; you can redistribute it and/or // modify it under the terms of the GNU General Public License as @@ -71,8 +71,9 @@ public: pkgAcquireStatus &acqlog, download_signal_log *signallog); - result finish(pkgAcquire::RunResult result, - OpProgress &progress); + void finish(pkgAcquire::RunResult result, + OpProgress *progress, + const sigc::slot1<void, result> &k); /** A signal that is invoked after an automatic 'forget new' * operation. diff --git a/src/generic/apt/pkg_changelog.cc b/src/generic/apt/pkg_changelog.cc index 23eb621a..56e23fa5 100644 --- a/src/generic/apt/pkg_changelog.cc +++ b/src/generic/apt/pkg_changelog.cc @@ -175,8 +175,9 @@ public: return true; } - result finish(pkgAcquire::RunResult res, - OpProgress &progress) + void finish(pkgAcquire::RunResult res, + OpProgress *progress, + const sigc::slot1<void, result> &k) { if(fetcher != NULL) { @@ -206,10 +207,14 @@ public: } } - return rval; + k(rval); + return; } else - return failure; + { + k(failure); + return; + } } }; diff --git a/src/generic/util/refcounted_wrapper.h b/src/generic/util/refcounted_wrapper.h new file mode 100644 index 00000000..6a608159 --- /dev/null +++ b/src/generic/util/refcounted_wrapper.h @@ -0,0 +1,52 @@ +// refcounted_wrapper.h -*-c++-*- +// +// Copyright (C) 2008 Daniel Burrows +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License as +// published by the Free Software Foundation; either version 2 of +// the License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +// General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program; see the file COPYING. If not, write to +// the Free Software Foundation, Inc., 59 Temple Place - Suite 330, +// Boston, MA 02111-1307, USA. + +#ifndef REFCOUNTED_WRAPPER_H +#define REFCOUNTED_WRAPPER_H + +#include "refcounted_base.h" + +// \todo Lift this to a more generic location. +namespace aptitude +{ + namespace util + { + /** \brief Wrap an arbitrary value in a class suitable for use + * with ref_ptr. + */ + template<typename T> + class refcounted_wrapper : public refcounted_base + { + T *p; + + public: + refcounted_wrapper(T *_p) + : p(_p) + { + } + + ~refcounted_wrapper() + { + delete p; + } + }; + } +} + +#endif // REFCOUNTED_WRAPPER_H diff --git a/src/gtk/changelog.cc b/src/gtk/changelog.cc index b4a64dc5..5579971e 100644 --- a/src/gtk/changelog.cc +++ b/src/gtk/changelog.cc @@ -34,6 +34,7 @@ #include <generic/apt/download_manager.h> #include <generic/apt/pkg_changelog.h> +#include <gtk/gui.h> #include <gtk/progress.h> namespace gui @@ -225,20 +226,11 @@ namespace gui return; } - download_manager *manager = get_changelog(ver, - sigc::bind(sigc::mem_fun(*this, &ChangeLogView::do_view_changelog), pkgname, curverstr)); - - guiOpProgress progress; - dummyPkgAcquireStatus acqlog; - if (!manager->prepare(progress, acqlog, NULL)) - return; - download_manager::result result = download_manager::do_again; - while (result == download_manager::do_again) - { - manager->do_download(100); - result = manager->finish(pkgAcquire::Continue, progress); - } + std::auto_ptr<download_manager> manager(get_changelog(ver, sigc::bind(sigc::mem_fun(*this, &ChangeLogView::do_view_changelog), pkgname, curverstr))); + start_download(manager.release(), + _("Downloading changelogs"), + false, + pMainWindow->get_notifyview()); } - } diff --git a/src/gtk/download.cc b/src/gtk/download.cc index 0a997c78..d6155a70 100644 --- a/src/gtk/download.cc +++ b/src/gtk/download.cc @@ -39,7 +39,7 @@ namespace cw = cwidget; namespace gui { - void guiPkgAcquireStatus::update_workers(pkgAcquire *Owner) + void download_list_model::update_workers(pkgAcquire *Owner) { pkgAcquire::Worker *serf = Owner->WorkersBegin(); while (serf) @@ -58,7 +58,7 @@ namespace gui } } - void guiPkgAcquireStatus::update_item(string URI, int progress, string status) + void download_list_model::update_item(string URI, int progress, string status) { std::map<string, Gtk::TreeModel::iterator>::iterator item_iter = item_map.find(URI); if (item_iter == item_map.end()) @@ -68,7 +68,7 @@ namespace gui row[download_columns.Status] = status; } - void guiPkgAcquireStatus::maybe_new_item(pkgAcquire::ItemDesc &Itm) + void download_list_model::maybe_new_item(pkgAcquire::ItemDesc &Itm) { if (item_map.find(Itm.URI) == item_map.end()) { @@ -81,66 +81,41 @@ namespace gui } } - guiPkgAcquireStatus::guiPkgAcquireStatus() + download_list_model::download_list_model() : download_columns(), download_store(Gtk::ListStore::create(download_columns)), - cancelled(false), failed(false) { } - bool guiPkgAcquireStatus::Pulse(pkgAcquire *Owner) + void download_list_model::connect(download_signal_log *log) { - if(cancelled) - return false; - - pkgAcquireStatus::Pulse(Owner); - double fraction = 0; - if (TotalItems != 0) - fraction = ((double)(CurrentBytes+CurrentItems))/((double)(TotalBytes+TotalItems)); - - std::string rate_string; - if(CurrentBytes > 0 || CurrentItems > 0) - rate_string = ssprintf(_("%sB/s"), - SizeToStr(CurrentCPS).c_str()); - - std::string progress_string; - if(CurrentCPS > 0) - progress_string = ssprintf(_("%sB of %sB at %sB/s, %s remaining"), - SizeToStr(CurrentBytes).c_str(), - SizeToStr(TotalBytes).c_str(), - SizeToStr(CurrentCPS).c_str(), - TimeToStr(((TotalBytes - CurrentBytes)/CurrentCPS)).c_str()); - else if(CurrentBytes > 0 || CurrentItems > 0) - progress_string = ssprintf(_("%sB of %sB, stalled"), - SizeToStr(CurrentBytes).c_str(), - SizeToStr(TotalBytes).c_str()); - - progress(fraction); - progress_rate(rate_string); - progress_rate_and_estimate(progress_string); + log->IMSHit_sig.connect(sigc::mem_fun(*this, &download_list_model::IMSHit)); + log->Fetch_sig.connect(sigc::mem_fun(*this, &download_list_model::Fetch)); + log->Done_sig.connect(sigc::mem_fun(*this, &download_list_model::Done)); + log->Fail_sig.connect(sigc::mem_fun(*this, &download_list_model::Fail)); + log->Pulse_sig.connect(sigc::mem_fun(*this, &download_list_model::Pulse)); + log->Start_sig.connect(sigc::mem_fun(*this, &download_list_model::Start)); + } - update_workers(Owner); - gtk_update(); - return !want_to_quit; + void download_list_model::Start(download_signal_log &manager) + { + Gtk::TreeModel::iterator store_iter = download_store->append(); + Gtk::TreeModel::Row row = *store_iter; + row[download_columns.URI] = ""; + row[download_columns.ShortDesc] = ""; + row[download_columns.Description] = "Download started"; } - bool guiPkgAcquireStatus::MediaChange(string media, string drive) + void download_list_model::Pulse(pkgAcquire *Owner, + download_signal_log &manager, + const sigc::slot1<void, bool> &k) { - if(cancelled) - return false; - - const Glib::ustring msg = _("Change media"); - Gtk::Dialog dialog(msg, *pMainWindow, true, true); - Gtk::Label label(ssprintf(_("Please insert the disc labeled \"%s\" into the drive \"%s\""), - media.c_str(), drive.c_str())); - dialog.get_vbox()->pack_end(label, true, true, 0); - dialog.add_button(_("Continue"), Gtk::RESPONSE_OK); - dialog.add_button(_("Abort"), Gtk::RESPONSE_CANCEL); - return dialog.run() == Gtk::RESPONSE_OK; + update_workers(Owner); } - void guiPkgAcquireStatus::Fail(pkgAcquire::ItemDesc &Itm) + void download_list_model::Fail(pkgAcquire::ItemDesc &Itm, + download_signal_log &log) { switch(Itm.Owner->Status) { @@ -161,26 +136,24 @@ namespace gui } } - void guiPkgAcquireStatus::Fetch(pkgAcquire::ItemDesc &Itm) + void download_list_model::IMSHit(pkgAcquire::ItemDesc &Itm, + download_signal_log &manager) { maybe_new_item(Itm); - update_item(Itm.URI, 100, _("Fetch")); + update_item(Itm.URI, 100, _("Already downloaded")); } - void guiPkgAcquireStatus::Done(pkgAcquire::ItemDesc &Itm) + void download_list_model::Fetch(pkgAcquire::ItemDesc &Itm, + download_signal_log &manager) { - update_item(Itm.URI, 100, _("Done")); - } - - void guiPkgAcquireStatus::Stop() - { - done(!failed); - //pMainWindow->get_progress_bar()->set_text("Download done"); + maybe_new_item(Itm); + update_item(Itm.URI, 100, _("Fetch")); } - void guiPkgAcquireStatus::cancel() + void download_list_model::Done(pkgAcquire::ItemDesc &Itm, + download_signal_log &manager) { - cancelled = true; + update_item(Itm.URI, 100, _("Done")); } DownloadColumns::DownloadColumns() @@ -208,7 +181,7 @@ namespace gui } DownloadTab::DownloadTab(const Glib::ustring &label, - const cw::util::ref_ptr<guiPkgAcquireStatus> &status) + const cw::util::ref_ptr<download_list_model> &status) : Tab(Download, label, Gnome::Glade::Xml::create(glade_main_file, "main_download_scrolledwindow"), "main_download_scrolledwindow") @@ -244,18 +217,30 @@ namespace gui namespace { + // This is the main class that's responsible for managing a GUI + // download. class DownloadNotification : public Notification { - cw::util::ref_ptr<guiPkgAcquireStatus> status; + cw::util::ref_ptr<download_list_model> status; DownloadTab *tab; Gtk::ProgressBar *progress; const std::string title; + // True if we should use a pulse-style progress bar instead of a + // percent-based one. + bool pulse; + bool cancelled; + bool success; void tab_destroyed() { tab = NULL; } + void cancel() + { + cancelled = true; + } + void view_details() { if(tab != NULL) @@ -281,64 +266,33 @@ namespace gui progress->set_tooltip_text(progress_text); } - void handle_done(bool success) + static void finishMediaChange(int response_id, + sigc::slot1<void, bool> k) { - string s = aptcfg->Find(PACKAGE "::UI::Pause-After-Download", "OnlyIfError"); - - bool keep_summary = false; - - if(s == "OnlyIfError") - keep_summary = !success; - else if(StringToBool(s, 0)) - keep_summary = true; - - if(keep_summary) - { - if(!success) - // \todo The "error" color is copied around; it should - // be a constant somewhere. - set_color(Gdk::Color("#FFE0E0")); - - progress->hide(); - Glib::RefPtr<Gtk::TextBuffer> buffer(Gtk::TextBuffer::create()); + k(response_id == Gtk::RESPONSE_OK); + } - if(success) - buffer->set_text(title + ": " + _("Completed")); - else - buffer->set_text(title + ": " + _("Completed with errors")); - set_buffer(buffer); - } - else - close_clicked(); + void do_pulse() + { + progress->pulse(); } public: DownloadNotification(const std::string &_title, - bool pulse, - const cw::util::ref_ptr<guiPkgAcquireStatus> &_status) + bool _pulse, + const cw::util::ref_ptr<download_list_model> &_status) : Notification(true), status(_status), tab(NULL), progress(new Gtk::ProgressBar), - title(_title) + title(_title), + pulse(_pulse), + cancelled(false), + success(false) { progress->set_text(title); - // It seems like we pulse about 100 times a second; this is - // probably too fast! - progress->set_pulse_step(0.02); + progress->set_pulse_step(0.1); progress->set_ellipsize(Pango::ELLIPSIZE_END); - if(pulse) - { - status->progress.connect(sigc::hide(sigc::mem_fun(*progress, - &Gtk::ProgressBar::pulse))); - status->progress_rate.connect(sigc::mem_fun(*this, &DownloadNotification::set_progress_text)); - } - else - { - status->progress.connect(sigc::mem_fun(*progress, - &Gtk::ProgressBar::set_fraction)); - status->progress_rate_and_estimate.connect(sigc::mem_fun(*this, &DownloadNotification::set_progress_text)); - } progress->show(); prepend_widget(progress); @@ -347,20 +301,121 @@ namespace gui view_details_button->show(); add_button(view_details_button); - close_clicked.connect(sigc::mem_fun(status.unsafe_get_ref(), &guiPkgAcquireStatus::cancel)); + close_clicked.connect(sigc::mem_fun(*this, &DownloadNotification::cancel)); - status->done.connect(sigc::mem_fun(*this, &DownloadNotification::handle_done)); + if(pulse) + Glib::signal_timeout().connect(sigc::bind_return(sigc::mem_fun(*this, &DownloadNotification::do_pulse), + true), + 100); finalize(); show(); } + + void connect(download_signal_log *log) + { + log->MediaChange_sig.connect(sigc::mem_fun(*this, + &DownloadNotification::MediaChange)); + + log->Pulse_sig.connect(sigc::mem_fun(*this, + &DownloadNotification::Pulse)); + + log->Stop_sig.connect(sigc::mem_fun(*this, + &DownloadNotification::Stop)); + } + + void Fail(pkgAcquire::ItemDesc &Itm, + download_signal_log &log) + { + success = false; + } + + void MediaChange(string media, string drive, + download_signal_log &manager, + const sigc::slot1<void, bool> &k) + { + const Glib::ustring msg = _("Change media"); + Gtk::Dialog dialog(msg, *pMainWindow, true, true); + Gtk::Label label(ssprintf(_("Please insert the disc labeled \"%s\" into the drive \"%s\""), + media.c_str(), drive.c_str())); + dialog.get_vbox()->pack_end(label, true, true, 0); + dialog.add_button(_("Continue"), Gtk::RESPONSE_OK); + dialog.add_button(_("Abort"), Gtk::RESPONSE_CANCEL); + dialog.signal_response().connect(sigc::bind(sigc::ptr_fun(&finishMediaChange), k)); + dialog.show(); + } + + void Pulse(pkgAcquire *Owner, + download_signal_log &manager, + const sigc::slot1<void, bool> &k) + { + if(pulse) + { + std::string rate_string; + if(manager.get_current_bytes() > 0 || manager.get_current_items() > 0) + rate_string = ssprintf(_("%sB/s"), + SizeToStr(manager.get_currentCPS()).c_str()); + + progress->set_text(rate_string); + } + else + { + double fraction = 0; + if (manager.get_total_items() != 0) + fraction = ((double)(manager.get_current_bytes() + manager.get_current_items())) / ((double)(manager.get_total_bytes() + manager.get_total_items())); + + progress->set_fraction(fraction); + + std::string progress_string; + if(manager.get_currentCPS() > 0) + progress_string = ssprintf(_("%sB of %sB at %sB/s, %s remaining"), + SizeToStr(manager.get_current_bytes()).c_str(), + SizeToStr(manager.get_total_bytes()).c_str(), + SizeToStr(manager.get_currentCPS()).c_str(), + TimeToStr(((manager.get_total_bytes() - manager.get_current_bytes()) / manager.get_currentCPS())).c_str()); + else if(manager.get_current_bytes() > 0 || manager.get_current_items() > 0) + progress_string = ssprintf(_("%sB of %sB, stalled"), + SizeToStr(manager.get_current_bytes()).c_str(), + SizeToStr(manager.get_total_bytes()).c_str()); + + progress->set_text(progress_string); + } + + k(!cancelled); + } + + void Stop(download_signal_log &manager, const sigc::slot0<void> &k) + { + // \todo Maybe use some other condition here? + if(!success) + { + if(!success) + // \todo The "error" color is copied around; it should + // be a constant somewhere. + set_color(Gdk::Color("#FFE0E0")); + + progress->hide(); + Glib::RefPtr<Gtk::TextBuffer> buffer(Gtk::TextBuffer::create()); + + if(success) + buffer->set_text(title + ": " + _("Completed")); + else + buffer->set_text(title + ": " + _("Completed with errors")); + set_buffer(buffer); + } + + k(); + } }; } Notification *make_download_notification(const std::string &title, bool pulse, - const cw::util::ref_ptr<guiPkgAcquireStatus> &status) + const cw::util::ref_ptr<download_list_model> &status, + download_signal_log *log) { - return new DownloadNotification(title, pulse, status); + DownloadNotification *rval = new DownloadNotification(title, pulse, status); + rval->connect(log); + return rval; } } diff --git a/src/gtk/download.h b/src/gtk/download.h index 3cfec6f8..6b9ec7fd 100644 --- a/src/gtk/download.h +++ b/src/gtk/download.h @@ -59,67 +59,44 @@ namespace gui * situations where the status object doesn't belong to any * particular scope can be handled cleanly and safely. */ - class guiPkgAcquireStatus : public pkgAcquireStatus, - public aptitude::util::refcounted_base + class download_list_model : public aptitude::util::refcounted_base { - private: - DownloadColumns download_columns; - Glib::RefPtr<Gtk::ListStore> download_store; - bool cancelled; - bool failed; // True if an item failed to download. - - std::map<string, Gtk::TreeModel::iterator> item_map; - void update_workers(pkgAcquire *Owner); - void update_item(string URI, int progress, string status); - void maybe_new_item(pkgAcquire::ItemDesc &Itm); - - // Noncopyable. - guiPkgAcquireStatus(const guiPkgAcquireStatus &); - - guiPkgAcquireStatus(); - - public: - static cwidget::util::ref_ptr<guiPkgAcquireStatus> create() - { - return new guiPkgAcquireStatus; - } - - bool Pulse(pkgAcquire *Owner); - bool MediaChange(string media, string drive); - void Fail(pkgAcquire::ItemDesc &Itm); - void Fetch(pkgAcquire::ItemDesc &Itm); - void Done(pkgAcquire::ItemDesc &Itm); - void Stop(); - - const DownloadColumns &get_columns() const { return download_columns; } - Glib::RefPtr<Gtk::TreeModel> get_model() const { return download_store; } - /** \brief Cancel the download by returning "false" from the - * next Pulse or MediaChange. - */ - void cancel(); - - /** \brief Emitted when the download completes. - * - * The parameter is "true" if the download succeeded and - * "false" if something failed to download. - */ - sigc::signal1<void, bool> done; - - /** \brief Signal emitted when any overall progress bar should - * be updated. - */ - sigc::signal1<void, double> progress; - - /** \brief Signal emitted when the progress bar text should - * change to indicate the current download rate. - */ - sigc::signal1<void, std::string> progress_rate; - - /** \brief Signal emitted when the progress bar text should change - * to indicate the current download rate and an estimate of the - * time remaining. - */ - sigc::signal1<void, std::string> progress_rate_and_estimate; + private: + DownloadColumns download_columns; + Glib::RefPtr<Gtk::ListStore> download_store; + bool failed; // True if an item failed to download. + + std::map<string, Gtk::TreeModel::iterator> item_map; + void update_workers(pkgAcquire *Owner); + void update_item(string URI, int progress, string status); + void maybe_new_item(pkgAcquire::ItemDesc &Itm); + + // Noncopyable. + download_list_model(const download_list_model &); + + download_list_model(); + + public: + static cwidget::util::ref_ptr<download_list_model> create() + { + return new download_list_model; + } + + /** \brief Convenience routine to connect the signals + * from log to appropriate slots in this object. + */ + void connect(download_signal_log *log); + + void IMSHit(pkgAcquire::ItemDesc &itemdesc, download_signal_log &manager); + void Fetch(pkgAcquire::ItemDesc &Itm, download_signal_log &manager); + void Done(pkgAcquire::ItemDesc &Itm, download_signal_log &manager); + void Fail(pkgAcquire::ItemDesc &Itm, download_signal_log &manager); + void Pulse(pkgAcquire *Owner, download_signal_log &manager, + const sigc::slot1<void, bool> &k); + void Start(download_signal_log &manager); + + const DownloadColumns &get_columns() const { return download_columns; } + Glib::RefPtr<Gtk::TreeModel> get_model() const { return download_store; } }; class DownloadTab : public Tab @@ -141,7 +118,7 @@ namespace gui public: DownloadTab(const Glib::ustring &label, - const cwidget::util::ref_ptr<guiPkgAcquireStatus> &status); + const cwidget::util::ref_ptr<download_list_model> &status); Gtk::TreeView * get_treeview() { return treeview; }; }; @@ -166,12 +143,18 @@ namespace gui * meaningful, so we don't try to * use it. * - * \param guiPkgAcquireStatus The GUI status object that + * \param download_list_model The GUI status object that * this notification will track. + * + * \param log The go-between to deliver download + * status notifications from the + * background thread. This pointer + * is owned by the returned notification. */ Notification *make_download_notification(const std::string &title, bool pulse, - const cwidget::util::ref_ptr<guiPkgAcquireStatus> &status); + const cwidget::util::ref_ptr<download_list_model> &status, + download_signal_log *log); } #endif /* DOWNLOAD_H_ */ diff --git a/src/gtk/dpkg_terminal.cc b/src/gtk/dpkg_terminal.cc index 5a6e5ac8..5145cd87 100644 --- a/src/gtk/dpkg_terminal.cc +++ b/src/gtk/dpkg_terminal.cc @@ -53,23 +53,24 @@ namespace gui bool process_data_from_dpkg_socket(Glib::IOCondition condition, int fd) { - switch(condition) + if(condition & Glib::IO_IN) { - case Glib::IO_IN: - { - char c; - while(recv(fd, &c, 1, MSG_DONTWAIT) > 0) - ; // Just snarf all the data in a lame way for now. - return true; - } - case Glib::IO_NVAL: - case Glib::IO_ERR: - case Glib::IO_HUP: - return false; - - default: - return true; // ?? + char c; + // If we can't read anything, assume we got EOF and stop + // listening to this FD. + bool read_anything = false; + while(recv(fd, &c, 1, MSG_DONTWAIT) > 0) + read_anything = true; // Just snarf all the data in a lame way for now. + return read_anything; } + + else if( (condition & Glib::IO_NVAL) || + (condition & Glib::IO_ERR) || + (condition & Glib::IO_HUP) ) + return false; + + _error->Warning("Unexpected IO condition %d", condition); + return false; } int open_unix_socket() @@ -94,15 +95,15 @@ namespace gui // The PID of the child to monitor. pid_t pid; - // The box to place the child's result in. - cw::threads::box<pkgPackageManager::OrderResult> *result_box; + // The continuation of the child. + sigc::slot1<void, pkgPackageManager::OrderResult> k; gulong handler_id; child_exited_info(pid_t _pid, - cw::threads::box<pkgPackageManager::OrderResult> *_result_box) + const sigc::slot1<void, pkgPackageManager::OrderResult> &_k) : pid(_pid), - result_box(_result_box) + k(_k) { } }; @@ -127,21 +128,19 @@ namespace gui if(WIFEXITED(status)) result = (pkgPackageManager::OrderResult) WEXITSTATUS(status); - // TODO: we'll leave the background thread running forever if an - // exception gets thrown somehow. But I don't think we can - // avoid this. - info->result_box->put(pkgPackageManager::Failed); + info->k(pkgPackageManager::Failed); g_signal_handler_disconnect(vte_reaper_get(), info->handler_id); } void connect_dpkg_result(pid_t pid, - cw::threads::box<pkgPackageManager::OrderResult> *result_box) + sigc::slot1<void, pkgPackageManager::OrderResult> k) { - child_exited_info *info = new child_exited_info(pid, result_box); - // We use implicit locking here to know that the signal won't be - // triggered before handler_id is set. + child_exited_info *info = new child_exited_info(pid, k); + // We use implicit locking here (plus the fact that we are + // running in the foreground thread) to know that the signal + // won't be triggered before handler_id is set. info->handler_id = g_signal_connect_data(vte_reaper_get(), "child-exited", @@ -151,13 +150,10 @@ namespace gui (GConnectFlags)0); } - // The dpkg stuff is actually run from the main loop. Why? - // Because we want to be able to connect signals safely, and to - // avoid racing with the VTE reaper (which seems to be somewhat - // hard to eliminate) or threads accessing _error. + // This should always run from the foreground thread. void do_run_dpkg(const sigc::slot1<pkgPackageManager::OrderResult, int> f, const sigc::slot1<void, Gtk::Widget *> register_terminal, - cw::threads::box<pkgPackageManager::OrderResult> *result_box) + const sigc::slot1<void, pkgPackageManager::OrderResult> k) { GtkWidget *vte = vte_terminal_new(); @@ -175,7 +171,8 @@ namespace gui int listen_sock = open_unix_socket(); if(listen_sock == -1) { - result_box->put(pkgPackageManager::Failed); + k(pkgPackageManager::Failed); + return; } // Ensure that the socket is always closed when this routine @@ -189,7 +186,8 @@ namespace gui if(socketname.get_name().size() > max_socket_name) { _error->Error("Internal error: the temporary socket name is too long!"); - result_box->put(pkgPackageManager::Failed); + k(pkgPackageManager::Failed); + return; } sa.sun_family = AF_UNIX; @@ -201,7 +199,8 @@ namespace gui _error->Error("%s: Unable to bind to the temporary socket: %s", __PRETTY_FUNCTION__, err.c_str()); - result_box->put(pkgPackageManager::Failed); + k(pkgPackageManager::Failed); + return; } if(listen(listen_sock, 1) != 0) @@ -211,7 +210,8 @@ namespace gui _error->Error("%s: Unable to listen on the temporary socket: %s", __PRETTY_FUNCTION__, err.c_str()); - result_box->put(pkgPackageManager::Failed); + k(pkgPackageManager::Failed); + return; } pid_t pid = vte_terminal_forkpty(VTE_TERMINAL(vte), @@ -278,28 +278,22 @@ namespace gui // can connect to it because its signal executions go through // the main loop, and this function call is blocking the main // loop. - connect_dpkg_result(pid, result_box); + connect_dpkg_result(pid, k); vte_reaper_add_child(pid); } } } - pkgPackageManager::OrderResult run_dpkg_in_terminal(const sigc::slot1<pkgPackageManager::OrderResult, int> &f, - const sigc::slot1<void, Gtk::Widget *> ®ister_terminal) + void run_dpkg_in_terminal(const sigc::slot1<pkgPackageManager::OrderResult, int> &f, + const sigc::slot1<void, Gtk::Widget *> ®ister_terminal, + const sigc::slot1<void, pkgPackageManager::OrderResult> &k) { - cw::threads::box<pkgPackageManager::OrderResult> result_box; - - // Ask the main thread to start the dpkg run and store the result - // in result_box. + // Ask the main thread to start the dpkg run and to invoke the + // continuation. post_event(sigc::bind(sigc::ptr_fun(&do_run_dpkg), f, register_terminal, - &result_box)); - - // Once the result-box is filled in, we're done. Correctness here - // relies on the fact that no-one accesses the box after put()-ing - // into it. - return result_box.take(); + k)); } } diff --git a/src/gtk/dpkg_terminal.h b/src/gtk/dpkg_terminal.h index 7a642703..59f37d7b 100644 --- a/src/gtk/dpkg_terminal.h +++ b/src/gtk/dpkg_terminal.h @@ -37,20 +37,18 @@ namespace gui /** \brief Run dpkg in a terminal, blocking this thread until it * completes. * - * Note that this *must* be invoked in a background thread, or it - * will deadlock itself! + * This may be invoked in either a foreground thread or a + * background thread. * * \param f The function to invoke when we want to run dpkg. * * \param register_terminal A function to invoke to register the * terminal (e.g., to store it in a variable or to add it to a new * tab). This function assumes ownership of the terminal widget. - * - * register_terminal is invoked in the same thread that called - * run_dpkg_in_terminal(). */ - pkgPackageManager::OrderResult run_dpkg_in_terminal(const sigc::slot1<pkgPackageManager::OrderResult, int> &f, - const sigc::slot1<void, Gtk::Widget *> ®ister_terminal); + void run_dpkg_in_terminal(const sigc::slot1<pkgPackageManager::OrderResult, int> &f, + const sigc::slot1<void, Gtk::Widget *> ®ister_terminal, + const sigc::slot1<void, pkgPackageManager::OrderResult> &k); } #endif diff --git a/src/gtk/gui.cc b/src/gtk/gui.cc index 0e790abc..a7e5dec2 100644 --- a/src/gtk/gui.cc +++ b/src/gtk/gui.cc @@ -41,6 +41,8 @@ #include <generic/apt/download_update_manager.h> #include <generic/apt/tags.h> +#include <generic/util/refcounted_wrapper.h> + #include <sigc++/signal.h> #include <cwidget/generic/threads/event_queue.h> @@ -56,6 +58,9 @@ #include <gtk/resolver.h> #include <gtk/tab.h> +// \todo ui_download_manager should live in generic/. +#include "../ui_download_manager.h" + namespace cw = cwidget; namespace gui @@ -202,48 +207,70 @@ namespace gui } }; - struct download_scope + progress_with_destructor make_gui_progress() { - download_scope() - { - active_download = true; - } + cw::util::ref_ptr<guiOpProgress> rval = + guiOpProgress::create(); + return std::make_pair(rval, + sigc::mem_fun(*rval.unsafe_get_ref(), + &guiOpProgress::destroy)); + } - ~download_scope() - { - active_download = false; - } - }; + void start_download(download_manager *manager, + const std::string &title, + bool pulse, + NotifyView *view, + const sigc::slot0<void> &download_starts_slot, + const sigc::slot0<void> &download_stops_slot) + { + cw::util::ref_ptr<download_list_model> model(download_list_model::create()); + download_signal_log *log = new download_signal_log; + model->connect(log); + + Notification *n = make_download_notification(title, + pulse, + model, + log); + + view->add_notification(n); + + using aptitude::util::refcounted_wrapper; + cwidget::util::ref_ptr<refcounted_wrapper<Notification> > + n_wrapper(new refcounted_wrapper<Notification>(n)); + + ui_download_manager *uim = + new ui_download_manager(manager, + log, + n_wrapper, + sigc::ptr_fun(&make_gui_progress), + sigc::ptr_fun(&post_event)); + + uim->download_starts.connect(download_starts_slot); + uim->download_stops.connect(download_stops_slot); + + uim->start(); + } + + void gui_finish_download() + { + active_download = false; + // Update indicators that expect to know something about arbitrary + // package states (e.g., the count of broken packages). + (*apt_cache_file)->package_state_changed(); + } // \todo make this use the threaded download system. void really_do_update_lists() { std::auto_ptr<download_update_manager> m(new download_update_manager); - download_scope scope; - - guiOpProgress progress; - cw::util::ref_ptr<guiPkgAcquireStatus> acqlog(guiPkgAcquireStatus::create()); - pMainWindow->get_notifyview()->add_notification(make_download_notification(_("Checking for updates"), true, acqlog)); - // \todo Why do we set Update and MorePulses? - acqlog->Update = true; - acqlog->MorePulses = true; - if (!m->prepare(progress, *acqlog.unsafe_get_ref(), NULL)) - return; - // \todo Why do we set Update and MorePulses? - acqlog->Update = true; - acqlog->MorePulses = true; - download_manager::result result = download_manager::do_again; - while (result == download_manager::do_again) - { - m->do_download(1000); - result = m->finish(pkgAcquire::Continue, progress); - } - guiOpProgress * p = gen_progress_bar(); - apt_load_cache(p, true, NULL); - delete p; + active_download = true; - active_download = false; + start_download(m.release(), + _("Checking for updates"), + true, + pMainWindow->get_notifyview(), + sigc::ptr_fun(&gui_finish_download)); } void do_update_lists() @@ -307,44 +334,25 @@ namespace gui tab_add(new DpkgTerminalTab(w)); } - // Callback for running dpkg from a background thread. - pkgPackageManager::OrderResult - gui_run_dpkg(sigc::slot1<pkgPackageManager::OrderResult, int> f, - pkgPackageManager::OrderResult *set_loc) + // Asynchronously post the outcome of a dpkg run to the main + // thread. + void finish_gui_run_dpkg(pkgPackageManager::OrderResult res, + sigc::slot1<void, pkgPackageManager::OrderResult> k) { - // \todo We should change the download notification to tell the - // user that they can click to obtain a terminal; this is just a - // proof-of-concept. - pkgPackageManager::OrderResult result = run_dpkg_in_terminal(f, sigc::ptr_fun(®ister_dpkg_terminal)); - *set_loc = result; - return result; + post_event(sigc::bind(k, res)); } - void handle_install(download_install_manager *m, - bool *in_dpkg, bool *finished) + // Callback that kicks off a dpkg run. + void gui_run_dpkg(sigc::slot1<pkgPackageManager::OrderResult, int> f, + sigc::slot1<void, pkgPackageManager::OrderResult> k) { - OpProgress progress; - - download_manager::result result = download_manager::do_again; - while (result == download_manager::do_again) - { - pkgAcquire::RunResult download_result = m->do_download(100); - - *in_dpkg = true; - // \todo Calling finish() from a background thread will - // crash because it invokes callbacks that ultimately end up - // calling GTK+ functions. finish() should be split up into - // a part that runs before invoking dpkg and a part that - // runs afterwards, the same way that it's split around the - // download process. In fact, the whole business of - // downloading could be viewed as being a series of - // background actions whose results are passed to a - // suspended continuation. There may be something to chew - // on there... - result = m->finish(download_result, progress); - *in_dpkg = false; - } - *finished = true; + // \todo We should change the download notification to tell the + // user that they can click to obtain a terminal; this is just a + // proof-of-concept. + run_dpkg_in_terminal(f, + sigc::ptr_fun(®ister_dpkg_terminal), + sigc::bind(sigc::ptr_fun(&finish_gui_run_dpkg), + k)); } void install_or_remove_packages() @@ -361,62 +369,18 @@ namespace gui return; } - download_scope scope; - pkgPackageManager::OrderResult result = pkgPackageManager::Incomplete; + active_download = true; + download_install_manager *m = new download_install_manager(false, - sigc::bind(sigc::ptr_fun(&gui_run_dpkg), - &result)); - - guiOpProgress progress; - cw::util::ref_ptr<guiPkgAcquireStatus> acqlog(guiPkgAcquireStatus::create()); - pMainWindow->get_notifyview()->add_notification(Gtk::manage(make_download_notification(_("Downloading packages"), false, acqlog))); - // \todo Why do we set Update and MorePulses here? - acqlog->Update = true; - acqlog->MorePulses = true; - if (!m->prepare(progress, *acqlog.unsafe_get_ref(), NULL)) - return; - // \todo Why do we set Update and MorePulses here? - acqlog->Update = true; - acqlog->MorePulses = true; - // FIXME: Hack while finding a nonblocking thread join or something else. - - bool in_dpkg = false; - bool finished = false; - - pMainWindow->get_progress_bar()->set_text(_("Installing / removing packages...")); - Glib::Thread * install_thread = - Glib::Thread::create(sigc::bind(sigc::ptr_fun(&handle_install), m, &in_dpkg, &finished), true); - while(!finished) - { - if (in_dpkg) - pMainWindow->get_progress_bar()->pulse(); - gtk_update(); - Glib::usleep(100000); - } - install_thread->join(); - - pMainWindow->get_progress_bar()->set_text(""); - - Gtk::MessageDialog dialog(*pMainWindow, "Install run finished", - false, Gtk::MESSAGE_INFO, Gtk::BUTTONS_OK, true); - switch(result) - { - case pkgPackageManager::Completed: - dialog.set_secondary_text("Successfully completed!"); - break; - case pkgPackageManager::Incomplete: - dialog.set_secondary_text("Partially completed!"); - break; - case pkgPackageManager::Failed: - dialog.set_secondary_text("Failed!"); - break; - } - dialog.run(); - - (*apt_cache_file)->package_state_changed(); - - //m->finish(pkgAcquire::Continue, progress); + sigc::ptr_fun(&gui_run_dpkg)); + + start_download(m, + _("Downloading packages"), + false, + pMainWindow->get_notifyview(), + sigc::slot0<void>(), + sigc::ptr_fun(&gui_finish_download)); } } @@ -1008,8 +972,8 @@ namespace gui void do_apt_init() { { - std::auto_ptr<guiOpProgress> p(gen_progress_bar()); - apt_init(p.get(), true, NULL); + cwidget::util::ref_ptr<guiOpProgress> p(guiOpProgress::create()); + apt_init(p.unsafe_get_ref(), true, NULL); } if(getuid() == 0 && aptcfg->FindB(PACKAGE "::Update-On-Startup", true)) diff --git a/src/gtk/gui.h b/src/gtk/gui.h index b85d9fdb..22ba6348 100644 --- a/src/gtk/gui.h +++ b/src/gtk/gui.h @@ -36,6 +36,8 @@ #include <gtk/errortab.h> #include <gtk/notify.h> +class download_manager; + namespace gui { // Local forward declarations: @@ -181,6 +183,37 @@ namespace gui void add_packages_tab(const std::string &pattern); }; + + /** \brief Start a new download, creating the appropriate GUI + * elements. + * + * \param manager The download manager defining the download + * process to run. + * + * \param title A string describing the download. + * + * \param pulse If \b true, a pulsing progress bar will be + * displayed instead of a percentage-based one. + * + * \param view The notification view in which to place the + * notification corresponding to this download. + * + * \param download_starts_slot A slot to be invoked in the + * foreground thread when the + * download is about to start. + * + * \param download_stops_slot A slot to be invoked in the + * background thread after the + * download and post-download + * actions complete. + */ + void start_download(download_manager *manager, + const std::string &title, + bool pulse, + NotifyView *view, + const sigc::slot0<void> &download_starts_slot = sigc::slot0<void>(), + const sigc::slot0<void> &download_stops_slot = sigc::slot0<void>()); + void main(int argc, char *argv[]); /** \brief Start an install/remove run, as if it had been triggered diff --git a/src/gtk/pkgview.cc b/src/gtk/pkgview.cc index 3e055fe8..197efb55 100644 --- a/src/gtk/pkgview.cc +++ b/src/gtk/pkgview.cc @@ -347,7 +347,8 @@ namespace gui store_reloading(); std::auto_ptr<PkgTreeModelGenerator> generator(generatorK(get_columns())); - guiOpProgress * p = gen_progress_bar(); + cwidget::util::ref_ptr<guiOpProgress> p = + guiOpProgress::create(); bool limited = false; cwidget::util::ref_ptr<pattern> filter; @@ -403,7 +404,9 @@ namespace gui } sort_thread->join(); - delete p; + // Erase our reference to the progress bar (it should be deleted + // when we do). + p = NULL; set_model(generator->get_model()); store_reloaded(); diff --git a/src/gtk/progress.cc b/src/gtk/progress.cc index fe55ae06..0e054c6a 100644 --- a/src/gtk/progress.cc +++ b/src/gtk/progress.cc @@ -35,10 +35,14 @@ namespace gui return rval; } + guiOpProgress::guiOpProgress() + : destroyed(false) + { + } + guiOpProgress::~guiOpProgress() { - pMainWindow->get_progress_bar()->set_text(""); - pMainWindow->get_progress_bar()->set_fraction(0); + destroy(); } void guiOpProgress::Update() @@ -51,10 +55,13 @@ namespace gui } } - // TODO: The use of this function should be deprecated. - guiOpProgress * gen_progress_bar() + void guiOpProgress::destroy() { - return new guiOpProgress; + if(!destroyed) + { + destroyed = true; + pMainWindow->get_progress_bar()->set_text(""); + pMainWindow->get_progress_bar()->set_fraction(0); + } } - } diff --git a/src/gtk/progress.h b/src/gtk/progress.h index 5450b603..af3a0f96 100644 --- a/src/gtk/progress.h +++ b/src/gtk/progress.h @@ -25,20 +25,32 @@ #include <apt-pkg/progress.h> +#include "../ui_download_manager.h" // For refcounted_progress. + namespace gui { - class guiOpProgress : public OpProgress + class guiOpProgress : public refcounted_progress { // must derive to read protected member.. - private: - double sanitizePercentFraction(float percent); - public: - ~guiOpProgress(); - void Update(); - }; + private: + double sanitizePercentFraction(float percent); + + bool destroyed; - guiOpProgress * gen_progress_bar(); + guiOpProgress(); + public: + ~guiOpProgress(); + static cwidget::util::ref_ptr<guiOpProgress> create() + { + return new guiOpProgress; + } + + void Update(); + + // Clear out the progress bar. + void destroy(); + }; } #endif /* PROGRESS_H_ */ diff --git a/src/main.cc b/src/main.cc index 0e58e70a..222607d1 100644 --- a/src/main.cc +++ b/src/main.cc @@ -787,7 +787,7 @@ int main(int argc, char *argv[]) // since we have to get information about what to install from // somewhere... if(!update_only) - apt_init(p.unsafe_get_ref(), true, status_fname); + apt_init(p->get_progress().unsafe_get_ref(), true, status_fname); if(status_fname) free(status_fname); check_apt_errors(); @@ -801,9 +801,9 @@ int main(int argc, char *argv[]) } if(!aptcfg->FindB(PACKAGE "::UI::Flat-View-As-First-View", false)) - do_new_package_view(*p.unsafe_get_ref()); + do_new_package_view(*p->get_progress().unsafe_get_ref()); else - do_new_flat_view(*p.unsafe_get_ref()); + do_new_flat_view(*p->get_progress().unsafe_get_ref()); p->destroy(); p = NULL; diff --git a/src/pkg_item.cc b/src/pkg_item.cc index f74cb32a..b6fa76c4 100644 --- a/src/pkg_item.cc +++ b/src/pkg_item.cc @@ -453,7 +453,7 @@ bool pkg_item::dispatch_key(const cw::config::key &k, cw::tree *owner) } progress_ref p = gen_progress_bar(); - apt_reload_cache(p.unsafe_get_ref(), true); + apt_reload_cache(p->get_progress().unsafe_get_ref(), true); p->destroy(); } } diff --git a/src/pkg_tree.cc b/src/pkg_tree.cc index 8c020e00..cd7b82ff 100644 --- a/src/pkg_tree.cc +++ b/src/pkg_tree.cc @@ -258,7 +258,7 @@ bool pkg_tree::build_tree(OpProgress &progress) bool pkg_tree::build_tree() { progress_ref p=gen_progress_bar(); - bool rval=build_tree(*p.unsafe_get_ref()); + bool rval=build_tree(*p->get_progress().unsafe_get_ref()); p->destroy(); return rval; diff --git a/src/pkg_ver_item.cc b/src/pkg_ver_item.cc index 96e30245..a9ae7e17 100644 --- a/src/pkg_ver_item.cc +++ b/src/pkg_ver_item.cc @@ -786,7 +786,7 @@ bool pkg_ver_item::dispatch_key(const cw::config::key &k, cw::tree *owner) cw::toplevel::resume(); progress_ref p = gen_progress_bar(); - apt_reload_cache(p.unsafe_get_ref(), true); + apt_reload_cache(p->get_progress().unsafe_get_ref(), true); p->destroy(); return true; diff --git a/src/progress.cc b/src/progress.cc index 3fb7b04b..289c5b2c 100644 --- a/src/progress.cc +++ b/src/progress.cc @@ -1,6 +1,6 @@ // cw::progress.cc // -// Copyright 2000, 2004-2007 Daniel Burrows +// Copyright 2000, 2004-2008 Daniel Burrows // // This program is free software; you can redistribute it and/or // modify it under the terms of the GNU General Public License as @@ -32,10 +32,6 @@ namespace cwidget using namespace widgets; } -progress::progress() -{ -} - namespace { // Converts a percentage between 0 and 100 to an integer for @@ -57,15 +53,15 @@ void progress::paint(const cw::style &st) { int width=getmaxx(); - if(!Op.empty()) + if(!p->Op.empty()) { - int truncPercent = convertPercent(Percent); + int truncPercent = convertPercent(p->Percent); std::ostringstream percentstream; percentstream << " " << truncPercent << "%"; std::string percentstr = percentstream.str(); - mvaddstr(0, 0, cw::util::transcode(Op)); + mvaddstr(0, 0, cw::util::transcode(p->Op)); mvaddstr(0, width - percentstr.size(), cw::util::transcode(percentstr)); } else @@ -79,7 +75,7 @@ bool progress::get_cursorvisible() cw::point progress::get_cursorloc() { - double xd = (Percent * getmaxx()) / 100.0; + double xd = (p->Percent * getmaxx()) / 100.0; int x = static_cast<int>(xd); int maxx = getmaxx(); @@ -92,7 +88,7 @@ void progress::Update() { show(); - if(CheckChange(0.25)) + if(p->CheckChange(0.25)) { cw::toplevel::update(); cw::toplevel::updatecursor(); @@ -117,3 +113,20 @@ int progress::height_request(int w) { return 1; } + +progress::progress() + : p(progress_progress::create()) +{ + p->Update_sig.connect(sigc::mem_fun(*this, &progress::Update)); + p->Done_sig.connect(sigc::mem_fun(*this, &progress::Done)); +} + +void progress::progress_progress::Update() +{ + Update_sig(); +} + +void progress::progress_progress::Done() +{ + Done_sig(); +} diff --git a/src/progress.h b/src/progress.h index b3ce7ec5..16f6f9d8 100644 --- a/src/progress.h +++ b/src/progress.h @@ -1,6 +1,6 @@ // progress.h -*-c++-*- // -// Copyright 2000, 2004-2005, 2007 Daniel Burrows +// Copyright 2000, 2004-2005, 2007-2008 Daniel Burrows // // This program is free software; you can redistribute it and/or modify // it under the terms of the GNU General Public License as published by @@ -26,15 +26,52 @@ #include <apt-pkg/progress.h> +// \todo This should move to src/generic. +#include "ui_download_manager.h" // for refcounted_progress + /** \brief A cwidget::widgets::widget that also acts as a progress bar. * * \file progress.h */ -class progress : public cwidget::widgets::widget, public OpProgress +// The two-level structure here is so we can derive from +// refcounted_progress (whose refcounting is incompatible with the +// widget class's refcounting). +// +// Probably at some point progress_widget should become a generic +// progress bar that lives in cwidget, at which point this would just +// be a small shim to hook it up. +class progress : public cwidget::widgets::widget { + class progress_progress : public +refcounted_progress + { + progress_progress() + { + } + + public: + static cwidget::util::ref_ptr<progress_progress> create() + { + return new progress_progress; + }; + + // I use signals to interconnect the two parts so that if the + // widget dies we don't have bad things happen. + sigc::signal<void> Update_sig; + sigc::signal<void> Done_sig; + + void Update(); + void Done(); + + friend class progress; + }; + + cwidget::util::ref_ptr<progress_progress> p; + protected: progress(); + public: static cwidget::util::ref_ptr<progress> create() { @@ -42,14 +79,17 @@ public: } virtual void paint(const cwidget::style &st); - virtual void Update(); - virtual void Done(); + + void Update(); + void Done(); int width_request(); int height_request(int w); bool get_cursorvisible(); cwidget::widgets::point get_cursorloc(); + + cwidget::util::ref_ptr<refcounted_progress> get_progress() const { return p; } }; typedef cwidget::util::ref_ptr<progress> progress_ref; @@ -564,7 +564,7 @@ static void do_su_to_root(string args) // We have to clear these out or the cache won't reload properly (?) progress_ref p = gen_progress_bar(); - apt_reload_cache(p.unsafe_get_ref(), true, statusname.get_name().c_str()); + apt_reload_cache(p->get_progress().unsafe_get_ref(), true, statusname.get_name().c_str()); p->destroy(); } else @@ -844,7 +844,7 @@ void do_new_package_view(OpProgress &progress) static void do_new_package_view_with_new_bar() { progress_ref p = gen_progress_bar(); - do_new_package_view(*p.unsafe_get_ref()); + do_new_package_view(*p->get_progress().unsafe_get_ref()); p->destroy(); } @@ -867,7 +867,7 @@ static void do_new_recommendations_view() _("View packages that it is recommended that you install"), _("Recommendations")); - tree->build_tree(*p.unsafe_get_ref()); + tree->build_tree(*p->get_progress().unsafe_get_ref()); p->destroy(); } @@ -891,7 +891,7 @@ void do_new_flat_view(OpProgress &progress) static void do_new_flat_view_with_new_bar() { progress_ref p = gen_progress_bar(); - do_new_flat_view(*p.unsafe_get_ref()); + do_new_flat_view(*p->get_progress().unsafe_get_ref()); p->destroy(); } @@ -912,7 +912,7 @@ static void do_new_tag_view_with_new_bar() _("View available packages and choose actions to perform"), _("Packages")); - tree->build_tree(*p.unsafe_get_ref()); + tree->build_tree(*p->get_progress().unsafe_get_ref()); p->destroy(); } @@ -941,7 +941,7 @@ void do_new_hier_view(OpProgress &progress) static void do_new_hier_view_with_new_bar() { progress_ref p=gen_progress_bar(); - do_new_hier_view(*p.unsafe_get_ref()); + do_new_hier_view(*p->get_progress().unsafe_get_ref()); p->destroy(); } @@ -1148,7 +1148,8 @@ static void maybe_show_old_tmpdir_message() namespace { - pkgPackageManager::OrderResult run_dpkg_with_cwidget_suspended(sigc::slot1<pkgPackageManager::OrderResult, int> f) + void run_dpkg_with_cwidget_suspended(sigc::slot1<pkgPackageManager::OrderResult, int> f, + sigc::slot1<void, pkgPackageManager::OrderResult> k) { cw::toplevel::suspend(); pkgPackageManager::OrderResult rval = f(-1); @@ -1168,7 +1169,28 @@ namespace cw::toplevel::resume(); - return rval; + k(rval); + return; + } +} + +namespace +{ + // Note that this is only safe if it's OK to copy the thunk in a + // background thread (i.e., it won't be invalidated by an object being + // destroyed in another thread). In the special cases where we use + // this it should be all right. + void do_post_thunk(const sigc::slot0<void> &thunk) + { + cw::toplevel::post_event(new cw::toplevel::slot_event(thunk)); + } + + progress_with_destructor make_progress_bar() + { + progress_ref rval = gen_progress_bar(); + return std::make_pair(rval->get_progress(), + sigc::mem_fun(*rval.unsafe_get_ref(), + &progress::destroy)); } } @@ -1178,11 +1200,22 @@ void install_or_remove_packages() m->post_forget_new_hook.connect(package_states_changed.make_slot()); - ui_download_manager *uim = - new ui_download_manager(m, false, false, true, - _("Downloading packages"), - _("View the progress of the package download"), - _("Package Download")); + std::pair<download_signal_log *, download_list_ref> + download_log_pair = gen_download_progress(false, false, + _("Downloading packages"), + _("View the progress of the package download"), + _("Package Download")); + + ui_download_manager *uim = new ui_download_manager(m, + download_log_pair.first, + download_log_pair.second, + sigc::ptr_fun(&make_progress_bar), + sigc::ptr_fun(&do_post_thunk)); + + download_log_pair.second->cancelled.connect(sigc::mem_fun(*uim, &ui_download_manager::aborted)); + + uim->download_starts.connect(sigc::bind(sigc::ptr_fun(&ui_start_download), true)); + uim->download_stops.connect(sigc::ptr_fun(&ui_stop_download)); uim->download_complete.connect(install_finished.make_slot()); uim->start(); @@ -1302,7 +1335,7 @@ static void do_show_preview() _("Preview")); progress_ref p=gen_progress_bar(); - active_preview_tree->build_tree(*p.unsafe_get_ref()); + active_preview_tree->build_tree(*p->get_progress().unsafe_get_ref()); p->destroy(); } else @@ -1618,11 +1651,23 @@ void really_do_update_lists() m->pre_autoclean_hook.connect(sigc::bind(sigc::ptr_fun(lists_autoclean_msg), m)); m->post_forget_new_hook.connect(package_states_changed.make_slot()); - ui_download_manager *uim = - new ui_download_manager(m, false, true, false, - _("Updating package lists"), - _("View the progress of the package list update"), - _("List Update")); + + std::pair<download_signal_log *, download_list_ref> + download_log_pair = gen_download_progress(false, true, + _("Updating package lists"), + _("View the progress of the package list update"), + _("List Update")); + + ui_download_manager *uim = new ui_download_manager(m, + download_log_pair.first, + download_log_pair.second, + sigc::ptr_fun(&make_progress_bar), + sigc::ptr_fun(&do_post_thunk)); + + download_log_pair.second->cancelled.connect(sigc::mem_fun(*uim, &ui_download_manager::aborted)); + + uim->download_starts.connect(sigc::bind(sigc::ptr_fun(&ui_start_download), false)); + uim->download_stops.connect(sigc::ptr_fun(&ui_stop_download)); uim->download_complete.connect(update_finished.make_slot()); uim->start(); @@ -1793,7 +1838,7 @@ void do_forget_new() static void do_reload_cache() { progress_ref p = gen_progress_bar(); - apt_reload_cache(p.unsafe_get_ref(), true); + apt_reload_cache(p->get_progress().unsafe_get_ref(), true); p->destroy(); } #endif @@ -2746,7 +2791,7 @@ void ui_main() apt_cache_file->is_locked()) { progress_ref p=gen_progress_bar(); - (*apt_cache_file)->save_selection_list(*p.unsafe_get_ref()); + (*apt_cache_file)->save_selection_list(*p->get_progress().unsafe_get_ref()); p->destroy(); } @@ -2854,13 +2899,12 @@ static void reset_status_download() } std::pair<download_signal_log *, - cw::widget_ref> + download_list_ref> gen_download_progress(bool force_noninvasive, bool list_update, const wstring &title, const wstring &longtitle, - const wstring &tablabel, - cw::util::slot0arg abortslot) + const wstring &tablabel) { download_signal_log *m=new download_signal_log; download_list_ref w=NULL; @@ -2868,14 +2912,14 @@ gen_download_progress(bool force_noninvasive, if(force_noninvasive || aptcfg->FindB(PACKAGE "::UI::Minibuf-Download-Bar", false)) { - w=download_list::create(abortslot, false, !list_update); + w = download_list::create(false, !list_update); main_status_multiplex->add_visible_widget(w, true); active_status_download=w; w->destroyed.connect(sigc::ptr_fun(&reset_status_download)); } else { - w=download_list::create(abortslot, true, !list_update); + w = download_list::create(true, !list_update); add_main_widget(w, title, longtitle, tablabel); } @@ -2905,7 +2949,7 @@ gen_download_progress(bool force_noninvasive, m->Complete_sig.connect(sigc::mem_fun(w.unsafe_get_ref(), &download_list::Complete)); - return std::pair<download_signal_log *, cw::widget_ref>(m, w); + return std::make_pair(m, w); } static void do_prompt_string(const wstring &s, @@ -2916,20 +2960,18 @@ static void do_prompt_string(const wstring &s, realslot(); } -std::pair<download_signal_log *, cw::widget_ref> +std::pair<download_signal_log *, download_list_ref> gen_download_progress(bool force_noninvasive, bool list_update, const string &title, const string &longtitle, - const string &tablabel, - cw::util::slot0arg abortslot) + const string &tablabel) { return gen_download_progress(force_noninvasive, list_update, cw::util::transcode(title), cw::util::transcode(longtitle), - cw::util::transcode(tablabel), - abortslot); + cw::util::transcode(tablabel)); } void prompt_string(const std::wstring &prompt, @@ -49,6 +49,9 @@ namespace cwidget } } +class download_list; +typedef cwidget::util::ref_ptr<download_list> download_list_ref; + class progress; typedef cwidget::util::ref_ptr<progress> progress_ref; /****************************************************************************** @@ -250,18 +253,16 @@ cwidget::fragment *wrapbox(cwidget::fragment *contents); * used as its title; it will be cwidget::util::transcoded. * \param longtitle if a new view is generated, this string is * used as its long title; it will be cwidget::util::transcoded. - * \param abortslot the slot to trigger if the download is aborted. * * \return the new download manager and the download status widget. */ std::pair<download_signal_log *, - cwidget::widgets::widget_ref> + download_list_ref> gen_download_progress(bool force_noninvasive, bool list_update, const std::string &title, const std::string &longtitle, - const std::string &tablabel, - cwidget::util::slot0arg abortslot); + const std::string &tablabel); // Asks the user for simple input (the question will appear in a "minibuffer" // or in a dialog according to preferences) diff --git a/src/ui_download_manager.cc b/src/ui_download_manager.cc index 1db73c41..7e7921b3 100644 --- a/src/ui_download_manager.cc +++ b/src/ui_download_manager.cc @@ -1,6 +1,6 @@ // ui_download_manager.cc // -// Copyright (C) 2005, 2007 Daniel Burrows +// Copyright (C) 2005, 2007-2008 Daniel Burrows // // This program is free software; you can redistribute it and/or // modify it under the terms of the GNU General Public License as @@ -37,62 +37,49 @@ namespace cwidget using namespace widgets; } -ui_download_manager::ui_download_manager(download_manager *_manager, - bool force_noninvasive, - bool list_update, - bool hide_preview, - const std::string &title, - const std::string &longtitle, - const std::string &tablabel) - : manager(_manager), - t(NULL) -{ - std::pair<download_signal_log *, cw::widget_ref> progpair = - gen_download_progress(force_noninvasive, list_update, - title, longtitle, tablabel, - cw::util::arg(sigc::mem_fun(abort_state, - &aborter::abort))); - - log = progpair.first; - download_status = progpair.second; - st = new background_status(log); - - ui_start_download(hide_preview); -} - ui_download_manager::~ui_download_manager() { log->Complete(); delete manager; - ui_stop_download(); + download_stops(); abort_state.abort(); - delete t; delete log; delete st; } void ui_download_manager::done(download_thread *, pkgAcquire::RunResult res) { - progress_ref p = gen_progress_bar(); - - download_manager::result run_res = download_manager::failure; + progress_with_destructor pair = make_progress_bar(); + done_progress = pair.first; + done_progress_destructor = pair.second; if(!abort_state.get_aborted()) - run_res = manager->finish(res, *p.unsafe_get_ref()); - - apt_load_cache(p.unsafe_get_ref(), true); + { + manager->finish(res, done_progress.unsafe_get_ref(), + sigc::mem_fun(*this, &ui_download_manager::finish_done)); + return; + } + else + { + finish_done(download_manager::failure); + return; + } +} - p->destroy(); +void ui_download_manager::finish_done(download_manager::result run_res) +{ + apt_load_cache(done_progress.unsafe_get_ref(), true); - delete t; - t = NULL; + done_progress_destructor(); + done_progress = NULL; if(run_res == download_manager::do_again && !abort_state.get_aborted()) (new download_thread(manager, + post_thunk, sigc::mem_fun(this, &ui_download_manager::done)))->start(); else @@ -104,14 +91,19 @@ void ui_download_manager::done(download_thread *, pkgAcquire::RunResult res) void ui_download_manager::start() { - progress_ref p = gen_progress_bar(); + download_starts(); + + progress_with_destructor pair = make_progress_bar(); + refcounted_progress_ref p = pair.first; + sigc::slot0<void> p_destructor = pair.second; bool ok = manager->prepare(*p.unsafe_get_ref(), *st, log); - p->destroy(); + p_destructor(); if(ok) (new download_thread(manager, + post_thunk, sigc::mem_fun(this, &ui_download_manager::done)))->start(); else diff --git a/src/ui_download_manager.h b/src/ui_download_manager.h index 9bcadf3b..34e7a1b1 100644 --- a/src/ui_download_manager.h +++ b/src/ui_download_manager.h @@ -1,6 +1,7 @@ + // ui_download_manager.h -*-c++-*- // -// Copyright (C) 2005, 2007 Daniel Burrows +// Copyright (C) 2005, 2007-2008 Daniel Burrows // // This program is free software; you can redistribute it and/or // modify it under the terms of the GNU General Public License as @@ -24,9 +25,14 @@ #include "download_thread.h" #include <apt-pkg/acquire.h> +#include <apt-pkg/progress.h> #include <cwidget/generic/util/ref_ptr.h> +#include <generic/apt/download_manager.h> + +#include <generic/util/refcounted_base.h> + #include <sigc++/signal.h> #include <sigc++/trackable.h> @@ -35,7 +41,6 @@ * \file ui_download_manager.h */ -class download_manager; class download_signal_log; class download_thread; namespace cwidget @@ -46,6 +51,52 @@ namespace cwidget } } +/** A base class for objects that provide the OpProgress interface and + * that can be refcounted. + * + * Note that this class is not abstract: you can instantiate it to + * get a NOP progress object. + */ +class refcounted_progress : public OpProgress, + virtual public aptitude::util::refcounted_base +{ +protected: + refcounted_progress() + { + } + +public: + static cwidget::util::ref_ptr<refcounted_progress> create() + { + return new refcounted_progress; + } + + static std::pair<cwidget::util::ref_ptr<refcounted_progress>, + sigc::slot0<void> > + make() + { + return std::make_pair(create(), sigc::slot0<void>()); + } +}; +typedef cwidget::util::ref_ptr<refcounted_progress> refcounted_progress_ref; + +/** \brief A progress bar paired with a slot that tells + * us how to destroy it. + */ +typedef std::pair<refcounted_progress_ref, + sigc::slot0<void> > progress_with_destructor; + +/** \brief A slot that creates a progress bar and + * tells us how to hide it. + * + * The first returned value is a new progress reporting + * object; the second value is a slot to be invoked when + * any widget associated with the progress object + * should be cleaned up. + */ +typedef sigc::slot0<progress_with_destructor> +make_refcounted_progress_slot; + /** Represents the UI end of a download process. This object * completely handles its own memory management -- you don't have to * delete it and you shouldn't try. @@ -74,32 +125,133 @@ class ui_download_manager : public sigc::trackable download_manager *manager; - download_thread *t; - aborter abort_state; download_signal_log *log; background_status *st; + // Used to hide the details of how we make a progress bar. Invoked + // every time the ui_download_manager needs a progress bar to do + // something. + make_refcounted_progress_slot make_progress_bar; + + // How to post a thunk to the main thread. + sigc::slot1<void, sigc::slot0<void> > post_thunk; + + // Wrapper so we can use generic refcounted stuff. + // + // All we want is a pointer that will drop a reference on the target + // when we destroy it. + // + // This seems a little icky; it seems like there should be a better + // way of handling these lifetime issues. + template<typename T> + class generic_refcounted : public aptitude::util::refcounted_base + { + cwidget::util::ref_ptr<T> ptr; + + public: + generic_refcounted(const cwidget::util::ref_ptr<T> &_ptr) + : ptr(_ptr) + { + } + }; + /** Used to keep the download status widget alive until the download * completes. */ - cwidget::util::ref_ptr<cwidget::widgets::widget> download_status; + cwidget::util::ref_ptr<aptitude::util::refcounted_base> download_status; + + /** \brief A progress object, used to display progrss in done(). + * + * This is stored here to handle lifetime issues sensibly: we + * create it in done() and free it in finish_done(), and storing + * refcounted values in slot bindings is dangerous. + * + * \todo Would it be reasonable to somehow just use two progress + * bars instead of passing one around? The current scheme is the + * result of blindly refactoring the old done() and seems likely to + * become unmaintainable. + */ + refcounted_progress_ref done_progress; + + /** A thunk that makes done_progress invisible and prepares + * to free its memory. + */ + sigc::slot0<void> done_progress_destructor; void done(download_thread *, pkgAcquire::RunResult res); + + /** \brief Finishes the work of done() after the post-download + * actions of the download manager have run. + * + * \param run_res The result of download_manager::finish(). + */ + void finish_done(download_manager::result run_res); + public: + /** \brief Create a ui_download_manager. + * + * \param _manager The download_manager object to wrap. + * + * \param _log How to report the download status (the + * signals will be invoked in a foreground + * thread). + * + * \param _download_status An arbitrary object that + * should be kept around until + * the download completes (its + * references will be adjusted + * in the foreground thread). + * + * \param _make_progress_bar A callback that will be invoked + * whenever the download process + * needs a fresh progress bar. + * + * param _post_thunk A callback that schedules a + * thunk for execution in the + * foreground thread. + */ + template<typename T> ui_download_manager(download_manager *_manager, - bool force_noninvasive, - bool list_update, - bool hide_preview, - const std::string &title, - const std::string &longtitle, - const std::string &tablabel); + download_signal_log *_log, + const cwidget::util::ref_ptr<T> &_download_status, + const make_refcounted_progress_slot &_make_progress_bar, + const sigc::slot1<void, sigc::slot0<void> > &_post_thunk) + : manager(_manager), + abort_state(), + log(_log), + st(new background_status(_log, _post_thunk)), + make_progress_bar(_make_progress_bar), + post_thunk(_post_thunk), + download_status(new generic_refcounted<T>(_download_status)) + { + } + ~ui_download_manager(); + /** \brief Invoke this method if the download is cancelled. + * + * This doesn't cancel the download itself, but it changes + * the flow-of-control appropriately: for instance, dpkg + * won't be invoked for an aborted install run. + */ + void aborted() + { + abort_state.abort(); + } + void start(); + /** \brief A signal emitted when the download is about to start. */ + sigc::signal<void> download_starts; + + /** \brief A signal emitted when the download finishes, + * after Complete() is invoked on the log object. + */ + sigc::signal<void> download_stops; + /** \brief A signal emitted when the download and any post-download * actions are finished. * diff --git a/src/view_changelog.cc b/src/view_changelog.cc index 878f1bbd..f8dcc5c9 100644 --- a/src/view_changelog.cc +++ b/src/view_changelog.cc @@ -21,11 +21,13 @@ #include <cwidget/config/keybindings.h> #include <cwidget/fragment.h> #include <cwidget/generic/util/transcode.h> +#include <cwidget/toplevel.h> #include <cwidget/widgets/pager.h> #include <cwidget/widgets/scrollbar.h> #include <cwidget/widgets/table.h> #include <cwidget/widgets/text_layout.h> +#include "download_list.h" #include "menu_redirect.h" #include "menu_text_layout.h" #include "ui.h" @@ -311,6 +313,20 @@ static void do_view_changelog(temp::name n, insert_main_widget(t, menulabel, desclabel, tablabel); } +namespace +{ + // \todo This should only be defined in one place. + + // Note that this is only safe if it's OK to copy the thunk in a + // background thread (i.e., it won't be invalidated by an object being + // destroyed in another thread). In the special cases where we use + // this it should be all right. + void do_post_thunk(const sigc::slot0<void> &thunk) + { + cw::toplevel::post_event(new cw::toplevel::slot_event(thunk)); + } +} + void view_changelog(pkgCache::VerIterator ver) { bool in_debian=false; @@ -340,8 +356,20 @@ void view_changelog(pkgCache::VerIterator ver) sigc::bind(sigc::ptr_fun(&do_view_changelog), pkgname, curverstr)); if(manager != NULL) - (new ui_download_manager(manager, true, false, false, - _("Downloading Changelog"), - "", - _("Download Changelog")))->start(); + { + std::pair<download_signal_log *, download_list_ref> + download_log_pair = gen_download_progress(true, false, + _("Downloading Changelog"), + "", + _("Download Changelog")); + + ui_download_manager *uim = + new ui_download_manager(manager, + download_log_pair.first, + download_log_pair.second, + sigc::ptr_fun(&refcounted_progress::make), + sigc::ptr_fun(&do_post_thunk)); + + uim->start(); + } } |