public inbox for cygwin-apps@cygwin.com
 help / color / mirror / Atom feed
From: Jon Turney <jon.turney@dronecode.org.uk>
To: cygwin-apps@cygwin.com
Cc: Jon Turney <jon.turney@dronecode.org.uk>
Subject: [PATCH setup 4/5] Correctly order preparing packagedb for chooser
Date: Tue, 07 Nov 2017 18:17:00 -0000	[thread overview]
Message-ID: <20171107181703.51016-4-jon.turney@dronecode.org.uk> (raw)
In-Reply-To: <20171107181703.51016-1-jon.turney@dronecode.org.uk>

Do packagedb::prep() in ChooserPage::OnActivate(), as doing it in OnInit()
is wrong, as that only gets called once (but lazily).

Make packagedb::prep() idempotent after packagedb::init(), so it doesn't do
it's work again, if we come back to chooser from a later page.

Re-arrange applying command line package selection, and determining the
initial solution, which should only happen once, but after packagedb::prep()
---
 choose.cc     | 81 +++++++++++++++++++++++++++++++++--------------------------
 choose.h      |  3 +++
 package_db.cc | 30 ++++++++++++++++++++++
 package_db.h  | 23 +++++++++++------
 4 files changed, 93 insertions(+), 44 deletions(-)

diff --git a/choose.cc b/choose.cc
index 3a990d5..00e5977 100644
--- a/choose.cc
+++ b/choose.cc
@@ -61,7 +61,6 @@ static BoolOption UpgradeAlsoOption (false, 'g', "upgrade-also", "also upgrade i
 static BoolOption CleanOrphansOption (false, 'o', "delete-orphans", "remove orphaned packages");
 static BoolOption ForceCurrentOption (false, 'f', "force-current", "select the current version for all packages");
 static BoolOption PruneInstallOption (false, 'Y', "prune-install", "prune the installation to only the requested packages");
-static BoolOption MirrorOption (false, 'm', "mirror-mode", "Skip availability check when installing from local directory (requires local directory to be clean mirror!)");
 
 using namespace std;
 
@@ -89,7 +88,7 @@ static ControlAdjuster::ControlInfo ChooserControlsInfo[] = {
 
 ChooserPage::ChooserPage () :
   cmd_show_set (false), saved_geom (false), saw_geom_change (false),
-  timer_id (DEFAULT_TIMER_ID)
+  timer_id (DEFAULT_TIMER_ID), activated (false)
 {
   sizeProcessor.AddControlInfo (ChooserControlsInfo);
 
@@ -150,7 +149,12 @@ ChooserPage::createListview ()
   chooser->setViewMode (!is_new_install || UpgradeAlsoOption || hasManualSelections ?
 			PickView::views::PackagePending : PickView::views::Category);
   SendMessage (GetDlgItem (IDC_CHOOSE_VIEW), CB_SETCURSEL, (WPARAM)chooser->getViewMode(), 0);
+  ClearBusy ();
+}
 
+void
+ChooserPage::initialUpdateState()
+{
   // set the initial update state
   if (ForceCurrentOption)
     {
@@ -171,8 +175,6 @@ ChooserPage::createListview ()
 
   static int ta[] = { IDC_CHOOSE_KEEP, IDC_CHOOSE_BEST, IDC_CHOOSE_SYNC, 0 };
   rbset (GetHWND (), ta, update_mode_id);
-
-  ClearBusy ();
 }
 
 /* TODO: review ::overrides for possible consolidation */
@@ -263,20 +265,30 @@ ChooserPage::OnInit ()
       SendMessage(viewlist, CB_ADDSTRING, 0, (LPARAM)PickView::mode_caption((PickView::views)view));
     }
 
-  SetBusy ();
-  packagedb db;
-  db.makeBase();
-  db.read();
-  db.upgrade();
-  db.fixup_source_package_ids();
-  db.removeEmptyCategories();
+  if (source == IDC_SOURCE_DOWNLOAD)
+    setPrompt("Select packages to download ");
+  else
+    setPrompt("Select packages to install ");
 
-  if (source == IDC_SOURCE_DOWNLOAD || source == IDC_SOURCE_LOCALDIR)
-    packagemeta::ScanDownloadedFiles (MirrorOption);
+  createListview ();
 
-  db.setExistence ();
-  db.fillMissingCategory ();
+  AddTooltip (IDC_CHOOSE_KEEP, IDS_TRUSTKEEP_TOOLTIP);
+  AddTooltip (IDC_CHOOSE_BEST, IDS_TRUSTCURR_TOOLTIP);
+  AddTooltip (IDC_CHOOSE_SYNC, IDS_TRUSTSYNC_TOOLTIP);
+  AddTooltip (IDC_CHOOSE_EXP, IDS_TRUSTEXP_TOOLTIP);
+  AddTooltip (IDC_CHOOSE_VIEW, IDS_VIEWBUTTON_TOOLTIP);
+  AddTooltip (IDC_CHOOSE_HIDE, IDS_HIDEOBS_TOOLTIP);
+  AddTooltip (IDC_CHOOSE_SEARCH_EDIT, IDS_SEARCH_TOOLTIP);
+
+  /* Set focus to search edittext control. */
+  PostMessage (GetHWND (), WM_NEXTDLGCTL,
+	       (WPARAM) GetDlgItem (IDC_CHOOSE_SEARCH_EDIT), TRUE);
+}
 
+void
+ChooserPage::applyCommandLinePackageSelection()
+{
+  packagedb db;
   for (packagedb::packagecollection::iterator i = db.packages.begin ();
        i != db.packages.end (); ++i)
     {
@@ -303,32 +315,29 @@ ChooserPage::OnInit ()
       else
 	pkg.set_action (packagemeta::Default_action, pkg.installed);
     }
-
-  ClearBusy ();
-
-  if (source == IDC_SOURCE_DOWNLOAD)
-    setPrompt("Select packages to download ");
-  else
-    setPrompt("Select packages to install ");
-  createListview ();
-
-  AddTooltip (IDC_CHOOSE_KEEP, IDS_TRUSTKEEP_TOOLTIP);
-  AddTooltip (IDC_CHOOSE_BEST, IDS_TRUSTCURR_TOOLTIP);
-  AddTooltip (IDC_CHOOSE_SYNC, IDS_TRUSTSYNC_TOOLTIP);
-  AddTooltip (IDC_CHOOSE_EXP, IDS_TRUSTEXP_TOOLTIP);
-  AddTooltip (IDC_CHOOSE_VIEW, IDS_VIEWBUTTON_TOOLTIP);
-  AddTooltip (IDC_CHOOSE_HIDE, IDS_HIDEOBS_TOOLTIP);
-  AddTooltip (IDC_CHOOSE_SEARCH_EDIT, IDS_SEARCH_TOOLTIP);
-
-  /* Set focus to search edittext control. */
-  PostMessage (GetHWND (), WM_NEXTDLGCTL,
-	       (WPARAM) GetDlgItem (IDC_CHOOSE_SEARCH_EDIT), TRUE);
 }
 
 void
 ChooserPage::OnActivate()
 {
-  chooser->refresh();;
+  SetBusy();
+
+  packagedb db;
+  db.prep();
+
+  if (!activated)
+    {
+      // Do things which should only happen once, but rely on packagedb being
+      // ready to use, so OnInit() is too early
+      applyCommandLinePackageSelection();
+      initialUpdateState();
+
+      activated = true;
+    }
+
+  ClearBusy();
+
+  chooser->refresh();
   PlaceDialog (true);
 }
 
diff --git a/choose.h b/choose.h
index 6839b0b..32a1650 100644
--- a/choose.h
+++ b/choose.h
@@ -59,6 +59,8 @@ private:
   void logResults();
   void setPrompt(char const *aPrompt);
   void PlaceDialog (bool);
+  void applyCommandLinePackageSelection();
+  void initialUpdateState();
 
   PickView *chooser;
   static HWND ins_dialog;
@@ -74,6 +76,7 @@ private:
     UINT wpi[sizeof (WINDOWPLACEMENT) / sizeof (UINT)];
   };
   int update_mode_id;
+  bool activated;
 };
 
 #endif /* SETUP_CHOOSE_H */
diff --git a/package_db.cc b/package_db.cc
index 8fbec44..92fe4f9 100644
--- a/package_db.cc
+++ b/package_db.cc
@@ -40,6 +40,9 @@
 #include "resource.h"
 #include "libsolv.h"
 #include "csu_util/version_compare.h"
+#include "getopt++/BoolOption.h"
+
+static BoolOption MirrorOption (false, 'm', "mirror-mode", "Skip availability check when installing from local directory (requires local directory to be clean mirror!)");
 
 using namespace std;
 
@@ -52,6 +55,8 @@ packagedb::init ()
 {
   installeddbread = 0;
   installeddbver = 0;
+  prepped = false;
+
   packages.clear();
   sourcePackages.clear();
   categories.clear();
@@ -382,6 +387,7 @@ packagedb::findSourceVersion (PackageSpecification const &spec) const
 
 int packagedb::installeddbread = 0;
 int packagedb::installeddbver = 0;
+bool packagedb::prepped = false;
 packagedb::packagecollection packagedb::packages;
 packagedb::categoriesType packagedb::categories;
 packagedb::packagecollection packagedb::sourcePackages;
@@ -717,3 +723,27 @@ packagedb::fixup_source_package_ids()
         }
     }
 }
+
+void
+packagedb::prep()
+{
+  /* make packagedb ready for use for chooser */
+  if (prepped)
+    return;
+
+  makeBase();
+  read();
+  upgrade();
+  fixup_source_package_ids();
+  removeEmptyCategories();
+
+  /* XXX: this needs to be broken out somewhere where it can do progress
+     reporting, as it can take a long time... */
+  if (source == IDC_SOURCE_DOWNLOAD || source ==IDC_SOURCE_LOCALDIR)
+    packagemeta::ScanDownloadedFiles (MirrorOption);
+
+  setExistence ();
+  fillMissingCategory ();
+
+  prepped = true;
+}
diff --git a/package_db.h b/package_db.h
index a26c387..e500e4b 100644
--- a/package_db.h
+++ b/package_db.h
@@ -65,24 +65,21 @@ class packagedb
 public:
   packagedb ();
   void init();
-  void read();
-  void makeBase();
   /* 0 on success */
   int flush ();
-  void upgrade ();
+  void prep();
+
   packagemeta * findBinary (PackageSpecification const &) const;
   packageversion findBinaryVersion (PackageSpecification const &) const;
   packagemeta * findSource (PackageSpecification const &) const;
   packageversion findSourceVersion (PackageSpecification const &spec) const;
   packagemeta * addBinary (const std::string &pkgname, const SolverPool::addPackageData &pkgdata);
   packageversion addSource (const std::string &pkgname, const SolverPool::addPackageData &pkgdata);
-  void fixup_source_package_ids();
+
   PackageDBConnectedIterator connectedBegin();
   PackageDBConnectedIterator connectedEnd();
-  void fillMissingCategory();
+
   void defaultTrust (SolverTasks &q, SolverSolution::updateMode mode, bool test);
-  void setExistence();
-  void removeEmptyCategories();
 
   typedef std::map <std::string, packagemeta *> packagecollection;
   /* all seen binary packages */
@@ -100,11 +97,21 @@ public:
   static SolverSolution solution;
 
 private:
+  void makeBase();
+  void read();
+  void upgrade ();
+  void fixup_source_package_ids();
+  void removeEmptyCategories();
+  void fillMissingCategory();
+  void setExistence();
+  void guessUserPicked(void);
+
   static int installeddbread;	/* do we have to reread this */
   static int installeddbver;
+  static bool prepped;
+
   friend class ConnectedLoopFinder;
   static std::vector <packagemeta *> dependencyOrderedPackages;
-  void guessUserPicked(void);
 };
 
 #endif /* SETUP_PACKAGE_DB_H */
-- 
2.15.0

  parent reply	other threads:[~2017-11-07 18:17 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-01 20:38 [setup topic/libsolv] Crash after incomplete download Ken Brown
2017-11-02 17:22 ` Jon Turney
2017-11-02 19:25   ` Ken Brown
2017-11-07 18:15     ` Jon Turney
2017-11-07 18:17       ` [PATCH setup 1/5] Make do_ini() succeed if found_ini_list is empty Jon Turney
2017-11-07 18:17         ` [PATCH setup 2/5] Simplify LocalDirPage::OnNext() Jon Turney
2017-11-07 18:17         ` Jon Turney [this message]
2017-11-07 18:17         ` [PATCH setup 3/5] Ensure packagedb and underlying solver pool is empty before we read setup.ini Jon Turney
2017-11-07 18:17         ` [PATCH setup 5/5] More crash avoidance in SolvableVersion member functions Jon Turney
2017-11-07 23:00       ` [setup topic/libsolv] Crash after incomplete download Ken Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171107181703.51016-4-jon.turney@dronecode.org.uk \
    --to=jon.turney@dronecode.org.uk \
    --cc=cygwin-apps@cygwin.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).