This is the mail archive of the cygwin-apps mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Incidental setup.exe patches #3: Simplify packagedb task handling



  I found out why we never see packages being set to "Retrieve" in download
mode any more.  There was a bit of a refactoring accident here:

> 2008-08-12  
> 
> 	Revamp for Cygwin 1.7.

> 	* package_db.cc (chosen_db_task): New global variable.
> 	* package_db.h (chosen_db_task): Declare.

> 	(RootPage::OnNext): Initialize packagedb here the first time, to
> 	avoid fetching wrong data from different previous installation.
> 	* source.cc (save_dialog): Don't initialize packagedb here, rather
> 	just memorize setting in chosen_db_task for the deferred initialization
> 	in RootPage::OnNext.

  The problem with this design is that the root page isn't run in download
mode, so the deferred assignment never happens.  As it happens, the default
setting is PackageDB_Install, which means that download mode is the only mode
in which the deferred assignment would actually change anything - but it's
also the one mode in which the deferred assignment never takes place!

  Fortunately, as far as I can tell, it no longer matters.  I couldn't work
out what the original problem referred to might have been, because subsequent
changes have apparently rendered it moot.  The one and only thing for which
the packagedb's task setting is used any more is in order to decide whether to
return the string "Reinstall" or "Retrieve" when generating the action caption
for the PickPackageLine related to a given packagemeta.

  So, I think we can now safely remove chosen_db_task and the whole deferred
init construct, giving the attached patch.

	* package_db.cc (chosen_db_task): Delete variable.  Relocate
	trailing comment.
	* package_db.h (chosen_db_task): Don't declare extern.
	* package_meta.cc (packagemeta::action_caption): Simplify static
	member access to packagedb task.
	* root.cc (RootPage::OnNext): Don't deferred-init packagedb task.
	* source.cc (save_dialog): Set packagedb task directly instead of
	caching value in chosen_db_task for deferred init.

  In download mode, the strings that until now used to say "Reinstall", now
say "Retrieve" again.  OK?

  Also, a more general question: what does it mean that setup.exe offers an
"Uninstall" option in download mode?  (I'm going to have to fire up a VM and
see whether it actually does anything or not, but what could it be *supposed*
to do?)  My first thought is that it's an accident and shouldn't be there.

    cheers,
      DaveK

Index: package_db.cc
===================================================================
RCS file: /cvs/cygwin-apps/setup/package_db.cc,v
retrieving revision 2.43
diff -p -u -r2.43 package_db.cc
--- package_db.cc	18 Dec 2009 11:59:54 -0000	2.43
+++ package_db.cc	14 Apr 2010 03:50:54 -0000
@@ -45,10 +45,6 @@ static const char *cvsid =
 
 using namespace std;
 
-PackageDBActions chosen_db_task = PackageDB_Install;
-
-/* static members */
-
 packagedb::packagedb ()
 {
   io_stream *db = 0;
@@ -199,6 +195,8 @@ packagedb::findSource (PackageSpecificat
   return NULL;
 }
 
+/* static members */
+
 int
   packagedb::installeddbread =
   0;
Index: package_db.h
===================================================================
RCS file: /cvs/cygwin-apps/setup/package_db.h,v
retrieving revision 2.23
diff -p -u -r2.23 package_db.h
--- package_db.h	12 Aug 2008 20:32:08 -0000	2.23
+++ package_db.h	14 Apr 2010 03:50:54 -0000
@@ -29,8 +29,6 @@ typedef enum {
   PackageDB_Download
 } PackageDBActions;
 
-extern PackageDBActions chosen_db_task;
-
 class packagedb;
 typedef std::vector <packagemeta *>::iterator PackageDBConnectedIterator;
 
Index: package_meta.cc
===================================================================
RCS file: /cvs/cygwin-apps/setup/package_meta.cc,v
retrieving revision 2.56
diff -p -u -r2.56 package_meta.cc
--- package_meta.cc	13 Dec 2009 19:23:43 -0000	2.56
+++ package_meta.cc	14 Apr 2010 03:50:54 -0000
@@ -362,10 +362,7 @@ packagemeta::action_caption () const
   else if (!desired)
     return "Skip";
   else if (desired == installed && desired.picked())
-    {
-      packagedb db;
-      return db.task == PackageDB_Install ? "Reinstall" : "Retrieve";
-    }
+    return packagedb::task == PackageDB_Install ? "Reinstall" : "Retrieve";
   else if (desired == installed && desired.sourcePackage() && desired.sourcePackage().picked())
     /* FIXME: Redo source should come up if the tarball is already present locally */
     return "Source";
Index: root.cc
===================================================================
RCS file: /cvs/cygwin-apps/setup/root.cc,v
retrieving revision 2.25
diff -p -u -r2.25 root.cc
--- root.cc	28 Jan 2010 22:59:09 -0000	2.25
+++ root.cc	14 Apr 2010 03:50:54 -0000
@@ -287,11 +287,6 @@ RootPage::OnNext ()
     << (root_text == IDC_ROOT_TEXT ? " text" : " binary")
     << (root_scope == IDC_ROOT_USER ? " user" : " system") << endLog;
 
-  /* Deferred initialization of packagedb *after* the root dir has been
-     chosen. */
-  packagedb db;
-  db.task = chosen_db_task;
-
   return 0;
 }
 
Index: source.cc
===================================================================
RCS file: /cvs/cygwin-apps/setup/source.cc,v
retrieving revision 2.21
diff -p -u -r2.21 source.cc
--- source.cc	28 Jun 2009 03:50:43 -0000	2.21
+++ source.cc	14 Apr 2010 03:50:54 -0000
@@ -54,7 +54,13 @@ static void
 save_dialog (HWND h)
 {
   source = rbget (h, rb);
-  chosen_db_task =
+  /* Deferred initialization of packagedb *after* the root dir has been
+     chosen doesn't seem necessary, as the one and only use for the 'task'
+     static member is to generate strings for the PickPackageLines in the
+     chooser page.  Also, it was a bit of a problem that we skip the root
+     page when we're in download-only mode, since that means it never
+     got set to that task and so was always PackageDB_Install.  */
+  packagedb::task = 
     source == IDC_SOURCE_DOWNLOAD ? PackageDB_Download : PackageDB_Install;
 }
 

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]