diff options
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(); + } } |