This is the mail archive of the
cygwin-apps
mailing list for the Cygwin project.
[PATCH] Incidental setup.exe patches #3: Simplify packagedb task handling
- From: Dave Korn <dave dot korn dot cygwin at googlemail dot com>
- To: cygwin-apps <cygwin-apps at cygwin dot com>
- Date: Wed, 14 Apr 2010 05:54:21 +0100
- Subject: [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;
}