Re: [PATCH setup 08/10] Track if a package was installed by user, or as a dependency

[Jon Turney <jon.turney@dronecode.org.uk>]

[-- Attachment #1: Type: text/plain, Size: 138 bytes --]


Attached is an updated version of the [08/10] patch which doesn't 
gratuitously break compatibility with other readers of installed.db.


[-- Attachment #2: 0008-Track-if-a-package-was-installed-by-user-or-as-a-dep.patch --]
[-- Type: text/plain, Size: 10909 bytes --]

From 280d51aa8c159164637039e7a4de47eb53b4fe39 Mon Sep 17 00:00:00 2001
From: Jon Turney <jon.turney@dronecode.org.uk>
Date: Tue, 2 Aug 2016 12:19:09 +0100
Subject: [PATCH setup 08/10] Track if a package was installed by user, or as a
 dependency

Update the installed.db file format to version 3, containing the user_picked
flag.

This extends the semantics of user_pick somewhat: currently it is only used
for UI purposes, to record if a package was picked in the current session.

Now we also use it to record if an installed package has ever been picked
via the UI (otherwise it is only installed because it is a dependency).

So, we are careful not to set it when a currently installed package has it's
installed version adjusted via the GUI.

We also arrange for user_pick to be set when a package was selected for
installation via CLI.

Add a heuristic to initially populate user_pick when upgrading from older
installed.db formats: All non-base installed packages which aren't
dependenies are assumed to be user_pick-ed.

Note: other tools (e.g. cygcheck) which read the installed.db file will need
updating appropriately

v2:
Don't gratuitously break compatiblity with other existing readers of
installed.db, keep recording version as a notional filename.

Don't treat unknown future installed.db format versions as format version 1,
instead treat setup downgrading as a fatal error.
---
 ini.cc          |   4 ++
 package_db.cc   | 136 ++++++++++++++++++++++++++++++++++++++++++++------------
 package_db.h    |   3 ++
 package_meta.cc |   4 +-
 res.rc          |   1 +
 resource.h      |   1 +
 6 files changed, 120 insertions(+), 29 deletions(-)

diff --git a/ini.cc b/ini.cc
index 456eb6e..f925bf5 100644
--- a/ini.cc
+++ b/ini.cc
@@ -51,6 +51,7 @@
 #include "compress.h"
 #include "Exception.h"
 #include "crypto.h"
+#include "package_db.h"
 
 extern ThreeBarProgressPage Progress;
 
@@ -351,6 +352,9 @@ do_ini_thread (HINSTANCE h, HWND owner)
   else
     ini_count = do_remote_ini (owner);
 
+  packagedb db;
+  db.upgrade();
+
   if (ini_count == 0)
     return false;
 
diff --git a/package_db.cc b/package_db.cc
index f437daf..79f2f93 100644
--- a/package_db.cc
+++ b/package_db.cc
@@ -42,6 +42,8 @@ static const char *cvsid =
 #include "package_meta.h"
 #include "Exception.h"
 #include "Generic.h"
+#include "LogSingleton.h"
+#include "resource.h"
 
 using namespace std;
 
@@ -55,37 +57,44 @@ packagedb::packagedb ()
       installeddbread = 1;
       if (!db)
 	return;
-      /* flush_local_db_package_data */
-      char line[1000], pkgname[1000], inst[1000];
-      int instsz;
+      char line[1000], pkgname[1000];
 
       if (db->gets (line, 1000))
 	{
+	  /* Look for header line (absent in version 1) */
+	  int instsz;
 	  int dbver;
 	  sscanf (line, "%s %d", pkgname, &instsz);
-	  if (!strcasecmp (pkgname, "INSTALLED.DB") && instsz == 2)
-	    dbver = 2;
+	  if (!strcasecmp (pkgname, "INSTALLED.DB"))
+	    dbver = instsz;
 	  else
 	    dbver = 1;
 	  delete db;
 	  db = 0;
-	  /* Later versions may not use installed.db other than to record the version. */
-	  if (dbver == 1 || dbver == 2)
+
+	  Log (LOG_BABBLE) << "INSTALLED.DB version " << dbver << endLog;
+
+	  if (dbver <= 3)
 	    {
+	      char inst[1000];
+
 	      db =
 		io_stream::open ("cygfile:///etc/setup/installed.db", "rt", 0);
-	      if (dbver == 2)
+
+	      // skip over already-parsed header line
+	      if (dbver >= 2)
 		db->gets (line, 1000);
+
 	      while (db->gets (line, 1000))
 		{
 		  int parseable;
-		  int ign;
+		  int user_picked = 0;
 		  pkgname[0] = '\0';
 		  inst[0] = '\0';
 
-		  sscanf (line, "%s %s %d", pkgname, inst, &ign);
+		  int res = sscanf (line, "%s %s %d", pkgname, inst, &user_picked);
 
-		  if (pkgname[0] == '\0' || inst[0] == '\0')
+		  if (res < 3 || pkgname[0] == '\0' || inst[0] == '\0')
 			continue;
 
 		  fileparse f;
@@ -98,27 +107,29 @@ packagedb::packagedb ()
 		    {
 		      pkg = new packagemeta (pkgname);
 		      packages.insert (packagedb::packagecollection::value_type(pkgname, pkg));
-		      /* we should install a new handler then not check this...
-		       */
-		      //if (!pkg)
-		      //die badly
 		    }
 
 		  packageversion binary = 
 		    cygpackage::createInstance (pkgname, f.ver,
-	    					package_installed,
-	    					package_binary);
+						package_installed,
+						package_binary);
 
 		  pkg->add_version (binary);
 		  pkg->set_installed (binary);
 		  pkg->desired = pkg->installed;
+
+		  if (dbver == 3)
+		    pkg->user_picked = (user_picked != 0);
 		}
 	      delete db;
 	      db = 0;
 	    }
 	  else
-	    // unknown dbversion
-	    exit (1);
+	    {
+              fatal(NULL, IDS_INSTALLEDB_VERSION);
+	    }
+
+	  installeddbver = dbver;
 	}
     }
 }
@@ -138,21 +149,22 @@ packagedb::flush ()
   if (!ndb)
     return errno ? errno : 1;
 
-  ndb->write ("INSTALLED.DB 2\n", strlen ("INSTALLED.DB 2\n"));
+  ndb->write ("INSTALLED.DB 3\n", strlen ("INSTALLED.DB 3\n"));
   for (packagedb::packagecollection::iterator i = packages.begin ();
        i != packages.end (); ++i)
     {
       packagemeta & pkgm = *(i->second);
       if (pkgm.installed)
 	{
-	  /* size here is irrelevant - as we can assume that this install source
-	   * no longer exists, and it does not correlate to used disk space
-	   * also note that we are writing a fictional install source 
-	   * to keep cygcheck happy.               
-	   */
+          /*
+            In INSTALLED.DB 3, lines are: 'packagename version user-picked',
+            where version is encoded in a notional filename for backwards
+            compatibility.
+          */
 	  std::string line;
-	  line = pkgm.name + " " + pkgm.name + "-" + 
-	    std::string(pkgm.installed.Canonical_version()) + ".tar.bz2 0\n";
+	  line = pkgm.name + " " +
+	    pkgm.name + "-" + std::string(pkgm.installed.Canonical_version()) + ".tar.bz2 " +
+	    (pkgm.user_picked ? "1" : "0") + "\n";
 	  ndb->write (line.c_str(), line.size());
 	}
     }
@@ -166,6 +178,18 @@ packagedb::flush ()
   return 0;
 }
 
+void
+packagedb::upgrade()
+{
+  if (installeddbver < 3)
+    {
+      /* Guess which packages were user_picked.  This has to take place after
+         setup.ini has been parsed as it needs dependency information. */
+      guessUserPicked();
+      installeddbver = 3;
+    }
+}
+
 packagemeta *
 packagedb::findBinary (PackageSpecification const &spec) const
 {
@@ -199,13 +223,13 @@ packagedb::findSource (PackageSpecification const &spec) const
 /* static members */
 
 int packagedb::installeddbread = 0;
+int packagedb::installeddbver = 0;
 packagedb::packagecollection packagedb::packages;
 packagedb::categoriesType packagedb::categories;
 packagedb::packagecollection packagedb::sourcePackages;
 PackageDBActions packagedb::task = PackageDB_Install;
 std::vector <packagemeta *> packagedb::dependencyOrderedPackages;
 
-#include "LogSingleton.h"
 #include <stack>
 
 class
@@ -449,3 +473,59 @@ packagedb::defaultTrust (trusts trust)
         packagedb::categories.erase (n++);
       }
 }
+
+void
+packagedb::guessUserPicked()
+{
+  /*
+    Assume that any non-base installed package which is a dependency of an
+    installed package wasn't user_picked
+
+    i.e. only installed packages which aren't in the base category, and aren't
+    a dependency of any installed package are user_picked
+  */
+
+  /* First mark all installed non-base packages */
+  for (packagedb::packagecollection::iterator i = packages.begin ();
+       i != packages.end (); ++i)
+    {
+      packagemeta & pkgm = *(i->second);
+
+      if (pkgm.categories.find ("Base") != pkgm.categories.end ())
+	continue;
+
+      if (pkgm.installed)
+	pkgm.user_picked = TRUE;
+    }
+
+  /* Then clear the mark for all dependencies of all installed packages */
+  for (packagedb::packagecollection::iterator i = packages.begin ();
+       i != packages.end (); ++i)
+    {
+      packagemeta & pkgm = *(i->second);
+
+      if (!pkgm.installed)
+	continue;
+
+      /* walk through each and clause */
+      vector <vector <PackageSpecification *> *>::const_iterator dp = pkgm.installed.depends()->begin();
+      while (dp != pkgm.installed.depends()->end())
+	{
+	  /* check each or clause for an installed match */
+	  vector <PackageSpecification *>::const_iterator i = find_if ((*dp)->begin(), (*dp)->end(), checkForInstalled);
+	  if (i != (*dp)->end())
+	    {
+	      const packagedb::packagecollection::iterator n = packages.find((*i)->packageName());
+	      if (n != packages.end())
+		{
+		  packagemeta *pkgm2 = n->second;
+		  pkgm2->user_picked = FALSE;
+		}
+	      /* skip to next and clause */
+	      ++dp;
+	      continue;
+	    }
+	  ++dp;
+	}
+    }
+}
diff --git a/package_db.h b/package_db.h
index bc828a1..6a99398 100644
--- a/package_db.h
+++ b/package_db.h
@@ -65,6 +65,7 @@ public:
   packagedb ();
   /* 0 on success */
   int flush ();
+  void upgrade ();
   packagemeta * findBinary (PackageSpecification const &) const;
   packagemeta * findSource (PackageSpecification const &) const;
   PackageDBConnectedIterator connectedBegin();
@@ -84,8 +85,10 @@ public:
   static PackageDBActions task;
 private:
   static int installeddbread;	/* do we have to reread this */
+  static int installeddbver;
   friend class ConnectedLoopFinder;
   static std::vector <packagemeta *> dependencyOrderedPackages;
+  void guessUserPicked(void);
 };
 
 #endif /* SETUP_PACKAGE_DB_H */
diff --git a/package_meta.cc b/package_meta.cc
index 21b21ef..3923b13 100644
--- a/package_meta.cc
+++ b/package_meta.cc
@@ -458,7 +458,8 @@ packagemeta::set_action (trusts const trust)
   else
     desired = packageversion ();
   /* Memorize the fact that the user picked at least once. */
-  user_picked = true;
+  if (!installed)
+    user_picked = true;
 }
 
 int
@@ -510,6 +511,7 @@ packagemeta::set_action (_actions action, packageversion const &default_version)
 	  if (desired != installed)
 	    if (desired.accessible ())
 	      {
+		user_picked = true;
 		desired.pick (true, this);
 		desired.sourcePackage ().pick (false, NULL);
 	      }
diff --git a/res.rc b/res.rc
index f1cf406..8292750 100644
--- a/res.rc
+++ b/res.rc
@@ -569,4 +569,5 @@ BEGIN
     IDS_NO_LOCALDIR    "Local package directory %s not found.\nYou can still use setup-%s.exe to remove installed\npackages, but there "
       "will be nothing to install.\n\nPress OK if that's what you wanted\nor Cancel to choose a different directory."
     IDS_ELEVATED       "Hand installation over to elevated child process."
+    IDS_INSTALLEDB_VERSION "Unknown INSTALLED.DB version"
 END
diff --git a/resource.h b/resource.h
index 48f03ab..68e8023 100644
--- a/resource.h
+++ b/resource.h
@@ -39,6 +39,7 @@
 #define IDS_CANT_MKDIR                    137
 #define IDS_NO_LOCALDIR			  138
 #define IDS_ELEVATED			  139
+#define IDS_INSTALLEDB_VERSION            140
 
 // Dialogs
 
-- 
2.8.3