We are always writing packagemeta.desired.pick(bool, packagemeta). This kind of suggests something not quite right.
The pick flag means install/reinstall, so despite being stored per packageversion, is only significant to download/install for the desired version. There's a slight wrinkle in that we want to also set/clear this flag for the source packageversion. We can't change this to point to packagemeta rather than packageversion, as that may not be the same for all versions, so instead just track this flag separately as srcpicked. Note that there is still a complicated mapping between the state of desired and pick and the action represented in the UI: desired == empty, installed == desired : skip desired == empty, installed != desired : uninstall desired == installed, pick == true : reinstall desired == installed, pick == false : keep desired != installed, pick == true : upgrade desired != installed, pick == false : invalid --- PickPackageLine.cc | 13 +++++------ PickView.cc | 8 +++---- download.cc | 12 +++++----- install.cc | 19 ++++++++-------- package_db.cc | 2 +- package_meta.cc | 67 ++++++++++++++++++++++++++++++++++++++---------------- package_meta.h | 11 ++++++++- package_version.cc | 16 ------------- package_version.h | 7 ------ prereq.cc | 11 +++++---- 10 files changed, 90 insertions(+), 76 deletions(-) diff --git a/PickPackageLine.cc b/PickPackageLine.cc index 60ece7f..95c1557 100644 --- a/PickPackageLine.cc +++ b/PickPackageLine.cc @@ -44,7 +44,7 @@ PickPackageLine::paint (HDC hdc, HRGN unused, int x, int y, int col_num, int sho /* current version */ pkg.desired == pkg.installed || /* no source */ !pkg.desired.accessible()) theView.DrawIcon (hdc, x + HMARGIN/2, by, theView.bm_checkna); - else if (pkg.desired.picked()) + else if (pkg.picked()) theView.DrawIcon (hdc, x + HMARGIN/2, by, theView.bm_checkyes); else theView.DrawIcon (hdc, x + HMARGIN/2, by, theView.bm_checkno); @@ -67,7 +67,7 @@ PickPackageLine::paint (HDC hdc, HRGN unused, int x, int y, int col_num, int sho /* when no source mirror available */ !pkg.desired.sourcePackage().accessible()) theView.DrawIcon (hdc, x + HMARGIN/2, by, theView.bm_checkna); - else if (pkg.desired.sourcePackage().picked()) + else if (pkg.srcpicked()) theView.DrawIcon (hdc, x + HMARGIN/2, by, theView.bm_checkyes); else theView.DrawIcon (hdc, x + HMARGIN/2, by, theView.bm_checkno); @@ -100,7 +100,7 @@ PickPackageLine::paint (HDC hdc, HRGN unused, int x, int y, int col_num, int sho /* Include the size of the binary package, and if selected, the source package as well. */ sz += picked.source()->size; - if (picked.sourcePackage().picked()) + if (pkg.srcpicked()) sz += picked.sourcePackage().source()->size; /* If size still 0, size must be unknown. */ @@ -133,20 +133,19 @@ PickPackageLine::click (int const myrow, int const ClickedRow, int const x) && x <= theView.headers[theView.bintick_col + 1].x - HMARGIN / 2) { if (pkg.desired.accessible ()) - pkg.desired.pick (!pkg.desired.picked (), &pkg); + pkg.pick (!pkg.picked ()); } else if (x >= theView.headers[theView.srctick_col].x - HMARGIN / 2 && x <= theView.headers[theView.srctick_col + 1].x - HMARGIN / 2) { if (pkg.desired.sourcePackage ().accessible ()) - pkg.desired.sourcePackage ().pick ( - !pkg.desired.sourcePackage ().picked (), NULL); + pkg.srcpick (!pkg.srcpicked ()); } /* Unchecking binary while source is unchecked or vice versa is equivalent to uninstalling. It's essential to set desired correctly, otherwise the package gets uninstalled without visual feedback to the user. The package will not even show up in the "Pending" view! */ - if (!pkg.desired.picked () && !pkg.desired.sourcePackage ().picked ()) + if (!pkg.picked () && !pkg.srcpicked ()) pkg.desired = packageversion (); return 0; } diff --git a/PickView.cc b/PickView.cc index 222bcb8..4c728f8 100644 --- a/PickView.cc +++ b/PickView.cc @@ -175,13 +175,13 @@ PickView::setViewMode (views mode) || (view_mode == PickView::views::PackagePending && ((!pkg.desired && pkg.installed) || // uninstall (pkg.desired && - (pkg.desired.picked () || // install bin - pkg.desired.sourcePackage ().picked ())))) // src + (pkg.picked () || // install bin + pkg.srcpicked ())))) // src // "Up to date" : installed packages that will not be changed || (view_mode == PickView::views::PackageKeeps && - (pkg.installed && pkg.desired && !pkg.desired.picked () - && !pkg.desired.sourcePackage ().picked ())) + (pkg.installed && pkg.desired && !pkg.picked () + && !pkg.srcpicked ())) // "Not installed" || (view_mode == PickView::views::PackageSkips && diff --git a/download.cc b/download.cc index 80615f3..a2237a7 100644 --- a/download.cc +++ b/download.cc @@ -208,18 +208,18 @@ do_download_thread (HINSTANCE h, HWND owner) i != db.packages.end (); ++i) { packagemeta & pkg = *(i->second); - if (pkg.desired.picked () || pkg.desired.sourcePackage ().picked ()) + if (pkg.picked () || pkg.srcpicked ()) { packageversion version = pkg.desired; packageversion sourceversion = version.sourcePackage(); try { - if (version.picked()) + if (pkg.picked()) { if (!check_for_cached (*version.source())) total_download_bytes += version.source()->size; } - if (sourceversion.picked () || IncludeSource) + if (pkg.srcpicked () || IncludeSource) { if (!check_for_cached (*sourceversion.source())) total_download_bytes += sourceversion.source()->size; @@ -243,16 +243,16 @@ do_download_thread (HINSTANCE h, HWND owner) i != db.packages.end (); ++i) { packagemeta & pkg = *(i->second); - if (pkg.desired.picked () || pkg.desired.sourcePackage ().picked ()) + if (pkg.picked () || pkg.srcpicked ()) { int e = 0; packageversion version = pkg.desired; packageversion sourceversion = version.sourcePackage(); - if (version.picked()) + if (pkg.picked()) { e += download_one (*version.source(), owner); } - if (sourceversion && (sourceversion.picked() || IncludeSource)) + if (sourceversion && (pkg.srcpicked() || IncludeSource)) { e += download_one (*sourceversion.source (), owner); } diff --git a/install.cc b/install.cc index cd3128c..fee2e19 100644 --- a/install.cc +++ b/install.cc @@ -740,12 +740,12 @@ do_install_thread (HINSTANCE h, HWND owner) { packagemeta & pkg = *(i->second); - if (pkg.desired.picked()) + if (pkg.picked()) { md5sum_total_bytes += pkg.desired.source()->size; } - if (pkg.desired.sourcePackage ().picked() || IncludeSource) + if (pkg.srcpicked() || IncludeSource) { md5sum_total_bytes += pkg.desired.sourcePackage ().source()->size; } @@ -759,7 +759,7 @@ do_install_thread (HINSTANCE h, HWND owner) { packagemeta & pkg = *(i->second); - if (pkg.desired.picked()) + if (pkg.picked()) { try { @@ -768,9 +768,9 @@ do_install_thread (HINSTANCE h, HWND owner) catch (Exception *e) { if (yesno (owner, IDS_SKIP_PACKAGE, e->what()) == IDYES) - pkg.desired.pick (false, &pkg); + pkg.pick (false); } - if (pkg.desired.picked()) + if (pkg.picked()) { md5sum_total_bytes_sofar += pkg.desired.source()->size; total_bytes += pkg.desired.source()->size; @@ -778,7 +778,7 @@ do_install_thread (HINSTANCE h, HWND owner) } } - if (pkg.desired.sourcePackage ().picked() || IncludeSource) + if (pkg.srcpicked() || IncludeSource) { bool skiprequested = false ; try @@ -790,10 +790,10 @@ do_install_thread (HINSTANCE h, HWND owner) if (yesno (owner, IDS_SKIP_PACKAGE, e->what()) == IDYES) { skiprequested = true ; //(err occurred,) skip pkg desired - pkg.desired.sourcePackage ().pick (false, &pkg); + pkg.srcpick (false); } } - if (pkg.desired.sourcePackage().picked() || (IncludeSource && !skiprequested)) + if (pkg.srcpicked() || (IncludeSource && !skiprequested)) { md5sum_total_bytes_sofar += pkg.desired.sourcePackage ().source()->size; total_bytes += pkg.desired.sourcePackage ().source()->size; @@ -801,8 +801,9 @@ do_install_thread (HINSTANCE h, HWND owner) } } + /* Upgrade or reinstall */ if ((pkg.installed && pkg.desired != pkg.installed) - || pkg.installed.picked ()) + || pkg.picked ()) { uninstall_q.push_back (&pkg); } diff --git a/package_db.cc b/package_db.cc index 3d6d0de..4e22953 100644 --- a/package_db.cc +++ b/package_db.cc @@ -445,7 +445,7 @@ packagedb::defaultTrust (trusts trust) { pkg.desired = pkg.trustp (true, trust); if (pkg.desired) - pkg.desired.pick (pkg.desired.accessible() && pkg.desired != pkg.installed, &pkg); + pkg.pick (pkg.desired.accessible() && pkg.desired != pkg.installed); } else pkg.desired = packageversion (); diff --git a/package_meta.cc b/package_meta.cc index f37340b..55fe471 100644 --- a/package_meta.cc +++ b/package_meta.cc @@ -385,9 +385,9 @@ packagemeta::action_caption () const return "Uninstall"; else if (!desired) return "Skip"; - else if (desired == installed && desired.picked()) + else if (desired == installed && picked()) return packagedb::task == PackageDB_Install ? "Reinstall" : "Retrieve"; - else if (desired == installed && desired.sourcePackage() && desired.sourcePackage().picked()) + else if (desired == installed && desired.sourcePackage() && srcpicked()) /* FIXME: Redo source should come up if the tarball is already present locally */ return "Source"; else if (desired == installed) /* and neither src nor bin */ @@ -405,15 +405,15 @@ packagemeta::set_action (trusts const trust) /* Keep the picked settings of the former desired version, if any, and make sure at least one of them is picked. If both are unpicked, pick the binary version. */ - bool source_picked = desired && desired.sourcePackage().picked (); - bool binary_picked = !desired || desired.picked () || !source_picked; + bool source_picked = desired && srcpicked (); + bool binary_picked = !desired || picked () || !source_picked; /* If we're on "Keep" on the installed version, and the version is available, switch to "Reinstall". */ - if (desired && desired == installed && !desired.picked () + if (desired && desired == installed && !picked () && desired.accessible ()) { - desired.pick (true, this); + pick (true); return; } @@ -445,9 +445,8 @@ packagemeta::set_action (trusts const trust) /* If the next version is the installed version, unpick it. This will have the desired effect to show the package in "Keep" mode. See also above for the code switching to "Reinstall". */ - desired.pick (desired != installed && binary_picked, this); - desired.sourcePackage ().pick (desired.sourcePackage().accessible () - && source_picked, NULL); + pick (desired != installed && binary_picked); + srcpick (desired.sourcePackage().accessible () && source_picked); } else desired = packageversion (); @@ -469,8 +468,8 @@ packagemeta::set_action (_actions action, packageversion const &default_version) desired = default_version; if (desired) { - desired.pick (desired != installed, this); - desired.sourcePackage ().pick (false, NULL); + pick (desired != installed); + srcpick (false); } } else @@ -486,18 +485,18 @@ packagemeta::set_action (_actions action, packageversion const &default_version) if (desired.accessible ()) { user_picked = true; - desired.pick (true, this); - desired.sourcePackage ().pick (false, NULL); + pick (true); + srcpick (false); } else { - desired.pick (false, NULL); - desired.sourcePackage ().pick (true, NULL); + pick (false); + srcpick (true); } else { - desired.pick (false, NULL); - desired.sourcePackage ().pick (false, NULL); + pick (false); + srcpick (false); } } return; @@ -507,8 +506,8 @@ packagemeta::set_action (_actions action, packageversion const &default_version) desired = installed; if (desired) { - desired.pick (true, this); - desired.sourcePackage ().pick (false, NULL); + pick (true); + srcpick (false); } } else if (action == Uninstall_action) @@ -518,6 +517,34 @@ packagemeta::set_action (_actions action, packageversion const &default_version) } bool +packagemeta::picked () const +{ + return _picked; +} + +void +packagemeta::pick (bool picked) +{ + _picked = picked; + + // side effect: display message when picked (if not already seen) + if (picked) + this->message.display (); +} + +bool +packagemeta::srcpicked () const +{ + return _srcpicked; +} + +void +packagemeta::srcpick (bool picked) +{ + _srcpicked = picked; +} + +bool packagemeta::accessible () const { for (set<packageversion>::iterator i=versions.begin(); @@ -607,7 +634,7 @@ packagemeta::logSelectionStatus() const const std::string installed = pkg.installed ? pkg.installed.Canonical_version () : "none"; - Log (LOG_BABBLE) << "[" << pkg.name << "] action=" << action << " trust=" << trust << " installed=" << installed << " src?=" << (pkg.desired && pkg.desired.sourcePackage().picked() ? "yes" : "no") << endLog; + Log (LOG_BABBLE) << "[" << pkg.name << "] action=" << action << " trust=" << trust << " installed=" << installed << " src?=" << (pkg.desired && srcpicked() ? "yes" : "no") << endLog; if (pkg.categories.size ()) Log (LOG_BABBLE) << " categories=" << for_each(pkg.categories.begin(), pkg.categories.end(), StringConcatenator(", ")).result << endLog; #if 0 diff --git a/package_meta.h b/package_meta.h index 640c253..dbd8eb9 100644 --- a/package_meta.h +++ b/package_meta.h @@ -35,7 +35,8 @@ public: static void ScanDownloadedFiles (bool); packagemeta (packagemeta const &); packagemeta (const std::string& pkgname) - : name (pkgname), key(pkgname), user_picked (false) + : name (pkgname), key(pkgname), user_picked (false), + _picked(false), _srcpicked(false) { } @@ -134,6 +135,12 @@ public: /* What version does the user want ? */ packageversion desired; + bool picked() const; /* true if desired version is to be (re-)installed */ + void pick(bool); /* trigger an install/reinstall */ + + bool srcpicked() const; /* true if source for desired version is to be installed */ + void srcpick(bool); + packagemessage message; /* can one or more versions be installed? */ @@ -153,6 +160,8 @@ protected: private: std::string trustLabel(packageversion const &) const; std::vector <Script> scripts_; + bool _picked; /* true if desired version is to be (re)installed */ + bool _srcpicked; }; #endif /* SETUP_PACKAGE_META_H */ diff --git a/package_version.cc b/package_version.cc index 60aae06..bb68229 100644 --- a/package_version.cc +++ b/package_version.cc @@ -53,8 +53,6 @@ public: const std::string LDesc () {return std::string();} void set_ldesc (const std::string& ) {} void uninstall (){} - void pick(bool const &newValue){/* Ignore attempts to pick this!. Throw an exception here if you want to detect such attemtps instead */} - virtual bool accessible () const {return false;} }; static _defaultversion defaultversion; @@ -237,20 +235,6 @@ packageversion::depends() const return data->depends; } -bool -packageversion::picked () const -{ - return data->picked; -} - -void -packageversion::pick (bool aBool, packagemeta *pkg) -{ - data->pick(aBool); - if (pkg && aBool) - pkg->message.display (); -} - void packageversion::uninstall () { diff --git a/package_version.h b/package_version.h index 0f83fdc..9351f26 100644 --- a/package_version.h +++ b/package_version.h @@ -111,9 +111,6 @@ public: void setDepends(const PackageDepends); const PackageDepends depends() const; - bool picked() const; /* true if this version is to be installed */ - void pick(bool, packagemeta *); /* trigger an install/reinsall */ - void uninstall (); /* invariant: never null */ packagesource *source() const; /* where can we source the file from */ @@ -168,10 +165,6 @@ public: PackageDepends depends; - virtual void pick(bool const &newValue) { picked = newValue;} - bool picked; /* non zero if this version is to be installed */ - /* This will also trigger reinstalled if it is set */ - virtual void uninstall () = 0; packagesource source; /* where can we source the file from */ diff --git a/prereq.cc b/prereq.cc index 0e7fdf5..16af7fa 100644 --- a/prereq.cc +++ b/prereq.cc @@ -302,20 +302,21 @@ PrereqChecker::selectMissing () map <packagemeta *, vector <packagemeta *>, packagemeta_ltcomp>::iterator i; for (i = unmet.begin(); i != unmet.end(); i++) { - packageversion vers = i->first->trustp (false, theTrust); - i->first->desired = vers; - vers.sourcePackage ().pick (false, NULL); + packagemeta *pkg = i->first; + packageversion vers = pkg->trustp (false, theTrust); + pkg->desired = vers; + pkg->srcpick (false); if (vers == i->first->installed) { - vers.pick (false, NULL); + pkg->pick (false); Log (LOG_PLAIN) << "Adding required dependency " << i->first->name << ": Selecting already-installed version " << i->first->installed.Canonical_version () << "." << endLog; } else { - vers.pick (vers.accessible (), i->first); + pkg->pick (vers.accessible ()); Log (LOG_PLAIN) << "Adding required dependency " << i->first->name << ": Selecting version " << vers.Canonical_version () << " for installation." << endLog; -- 2.12.3