[PATCH setup 00/11] Improve handling of specifying an obsolete package to be installed on the command line

[PATCH setup 00/11] Improve handling of specifying an obsolete package to be installed on the command line

[Jon Turney]

e.g. setup -q -g -P python3-lxml (which used to do something useful) 
currently gets you an (empty) python3-lxml package, which will be replaced 
by python36-lxml (which obsoletes it) on the next setup run. After this 
change, python36-lxml is installed instead.

See also the dicusssion at 
https://cygwin.com/ml/cygwin-apps/2017-10/msg00092.html et seq. (where I 
come to the (incorrect) conclusion that since we don't need this for 
interactive use, it's not needed)

Jon Turney (11):
  Remove 'Bin?' column
  Remove unused packagemeta::key
  Make packagemeta::message private
  Rename 'Default' packagemeta action to 'NoChange' for clarity
  Store the requested action in packagemeta::set_action()
  Use packagemeta::set_action() to update action
  Use stored action in setting up solver
  Allow better handling of an obsolete package specified on command line
  Use stored action in packagemeta::list_actions()
  Use stored action in packagemeta::action_caption()
  Ensure we only set user_picked when appropriate

 PickCategoryLine.cc |  2 +-
 PickPackageLine.cc  | 32 +---------------
 PickView.cc         |  3 +-
 PickView.h          | 12 +++---
 choose.cc           |  9 ++---
 libsolv.cc          | 69 ++++++++++++++++++++-------------
 libsolv.h           |  1 +
 package_db.cc       |  4 +-
 package_meta.cc     | 93 ++++++++++++++++++++++-----------------------
 package_meta.h      | 16 ++++----
 10 files changed, 113 insertions(+), 128 deletions(-)

-- 
2.21.0

[PATCH setup 02/11] Remove unused packagemeta::key

[Jon Turney]

Contains an identical value to packagemeta::name
---
 package_meta.cc | 2 +-
 package_meta.h  | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/package_meta.cc b/package_meta.cc
index 3509f05..a4f6c93 100644
--- a/package_meta.cc
+++ b/package_meta.cc
@@ -70,7 +70,7 @@ packagemeta::action_caption (_actions _value)
 }
 
 packagemeta::packagemeta (packagemeta const &rhs) :
-  name (rhs.name), key (rhs.name),
+  name (rhs.name),
   categories (rhs.categories), versions (rhs.versions),
   installed (rhs.installed),
   curr (rhs.curr),
diff --git a/package_meta.h b/package_meta.h
index c9bfe7c..1f0361e 100644
--- a/package_meta.h
+++ b/package_meta.h
@@ -37,7 +37,7 @@ public:
   static void ScanDownloadedFiles (bool);
   packagemeta (packagemeta const &);
   packagemeta (const std::string& pkgname)
-    : name (pkgname), key(pkgname), user_picked (false),
+    : name (pkgname), user_picked (false),
     _picked(false), _srcpicked(false)
   {
   }
@@ -103,7 +103,6 @@ public:
   }
 
   std::string name;			/* package name, like "cygwin" */
-  std::string key;
 
   /* true if package was selected on command-line. */
   bool isManuallyWanted() const;
-- 
2.21.0

[PATCH setup 01/11] Remove 'Bin?' column

[Jon Turney]

The only use this column appears to have now is that unticking it is the
same as selecting 'uninstall'.

Future work: Make 'Src?' column more independent, rather than
interacting with the 'New' (action) column in non-obvious ways. We
should be able to choose to install source irrespective of the state of
the binary package. (That should also take into account the
'--include-source' option)
---
 PickPackageLine.cc | 32 ++------------------------------
 PickView.cc        |  3 +--
 PickView.h         | 10 +++++-----
 choose.cc          |  1 -
 4 files changed, 8 insertions(+), 38 deletions(-)

diff --git a/PickPackageLine.cc b/PickPackageLine.cc
index 685d632..f64c101 100644
--- a/PickPackageLine.cc
+++ b/PickPackageLine.cc
@@ -32,20 +32,6 @@ PickPackageLine::get_text(int col_num) const
     {
       return pkg.action_caption ();
     }
-  else if (col_num == bintick_col)
-    {
-      const char *bintick = "?";
-      if (/* uninstall or skip */ !pkg.desired ||
-          /* current version */ pkg.desired == pkg.installed ||
-          /* no source */ !pkg.desired.accessible())
-        bintick = "n/a";
-      else if (pkg.picked())
-        bintick = "yes";
-      else
-        bintick = "no";
-
-      return bintick;
-    }
   else if (col_num == srctick_col)
     {
       const char *srctick = "?";
@@ -118,25 +104,11 @@ PickPackageLine::do_action(int col_num, int action_id)
       pkg.select_action(action_id, theView.deftrust);
       return 1;
     }
-  if (col_num == bintick_col)
-    {
-      if (pkg.desired.accessible ())
-        pkg.pick (!pkg.picked ());
-    }
-  else if (col_num == srctick_col)
+
+  if (col_num == srctick_col)
     {
       if (pkg.desired.sourcePackage ().accessible ())
         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 ((col_num == bintick_col) || (col_num == srctick_col))
-    {
-      if (!pkg.picked () && !pkg.srcpicked ())
-        pkg.desired = packageversion ();
 
       return 1;
     }
diff --git a/PickView.cc b/PickView.cc
index 8412282..6d7d83d 100644
--- a/PickView.cc
+++ b/PickView.cc
@@ -236,9 +236,8 @@ PickView::init_headers (void)
 {
   listview->noteColumnWidthStart();
 
-  // widths of the 'bin' and 'src' checkbox columns just need to accommodate the
+  // width of the 'src' checkbox column just needs to accommodate the
   // column name
-  listview->noteColumnWidth (bintick_col, "");
   listview->noteColumnWidth (srctick_col, "");
 
   // (In category view) accommodate the width of each category name
diff --git a/PickView.h b/PickView.h
index 3715d93..9443a78 100644
--- a/PickView.h
+++ b/PickView.h
@@ -71,16 +71,16 @@ private:
   void insert_category (CategoryTree *);
 };
 
+// column numbers, must match index into pkg_headers[]
 enum
 {
  pkgname_col = 0, // package/category name
  current_col = 1,
  new_col = 2,     // action
- bintick_col = 3,
- srctick_col = 4,
- cat_col = 5,
- size_col = 6,
- pkg_col = 7,     // desc
+ srctick_col = 3,
+ cat_col = 4,
+ size_col = 5,
+ pkg_col = 6     // desc
 };
 
 bool isObsolete (std::set <std::string, casecompare_lt_op> &categories);
diff --git a/choose.cc b/choose.cc
index 2b5e465..c4b8773 100644
--- a/choose.cc
+++ b/choose.cc
@@ -133,7 +133,6 @@ static ListView::Header pkg_headers[] = {
   {"Package",     LVCFMT_LEFT,  ListView::ControlType::text},
   {"Current",     LVCFMT_LEFT,  ListView::ControlType::text},
   {"New",         LVCFMT_LEFT,  ListView::ControlType::popup},
-  {"Bin?",        LVCFMT_LEFT,  ListView::ControlType::checkbox},
   {"Src?",        LVCFMT_LEFT,  ListView::ControlType::checkbox},
   {"Categories",  LVCFMT_LEFT,  ListView::ControlType::text},
   {"Size",        LVCFMT_RIGHT, ListView::ControlType::text},
-- 
2.21.0

[PATCH setup 03/11] Make packagemeta::message private

[Jon Turney]

---
 package_meta.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/package_meta.h b/package_meta.h
index 1f0361e..2c40102 100644
--- a/package_meta.h
+++ b/package_meta.h
@@ -140,8 +140,6 @@ public:
   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? */
   bool accessible () const;
   bool sourceAccessible() const;
@@ -167,6 +165,7 @@ private:
   bool _picked; /* true if desired version is to be (re)installed */
   bool _srcpicked;
 
+  packagemessage message;
   std::set <std::string> version_blacklist;
 };
 
-- 
2.21.0

[PATCH setup 04/11] Rename 'Default' packagemeta action to 'NoChange' for clarity

[Jon Turney]

---
 PickCategoryLine.cc |  2 +-
 PickView.h          |  2 +-
 choose.cc           |  6 +++---
 package_meta.cc     | 19 +++++++++++--------
 package_meta.h      |  2 +-
 5 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/PickCategoryLine.cc b/PickCategoryLine.cc
index 7aa1f28..7c1d464 100644
--- a/PickCategoryLine.cc
+++ b/PickCategoryLine.cc
@@ -68,7 +68,7 @@ PickCategoryLine::get_actions(int col) const
   ActionList *al = new ActionList();
   packagemeta::_actions current_default = cat_tree->action();
 
-  al->add("Default", (int)packagemeta::Default_action, (current_default == packagemeta::Default_action), TRUE);
+  al->add("Default", (int)packagemeta::NoChange_action, (current_default == packagemeta::NoChange_action), TRUE);
   al->add("Install", (int)packagemeta::Install_action, (current_default == packagemeta::Install_action), TRUE);
   al->add(packagedb::task == PackageDB_Install ? "Reinstall" : "Retrieve",
           (int)packagemeta::Reinstall_action, (current_default == packagemeta::Reinstall_action), TRUE);
diff --git a/PickView.h b/PickView.h
index 9443a78..2db0562 100644
--- a/PickView.h
+++ b/PickView.h
@@ -97,7 +97,7 @@ public:
   CategoryTree(Category & cat, bool collapsed) :
     _cat (cat),
     _collapsed(collapsed),
-    _action (packagemeta::Default_action)
+    _action (packagemeta::NoChange_action)
   {
   }
 
diff --git a/choose.cc b/choose.cc
index c4b8773..277baba 100644
--- a/choose.cc
+++ b/choose.cc
@@ -305,11 +305,11 @@ ChooserPage::applyCommandLinePackageSelection()
       else if (uninstall)
 	pkg.set_action (packagemeta::Uninstall_action, packageversion ());
       else if (PruneInstallOption)
-	pkg.set_action (packagemeta::Default_action, pkg.curr);
+	pkg.set_action (packagemeta::NoChange_action, pkg.curr);
       else if (upgrade)
-	pkg.set_action (packagemeta::Default_action, pkg.trustp(true, TRUST_UNKNOWN));
+	pkg.set_action (packagemeta::NoChange_action, pkg.trustp(true, TRUST_UNKNOWN));
       else
-	pkg.set_action (packagemeta::Default_action, pkg.installed);
+	pkg.set_action (packagemeta::NoChange_action, pkg.installed);
     }
 }
 
diff --git a/package_meta.cc b/package_meta.cc
index a4f6c93..3224f1c 100644
--- a/package_meta.cc
+++ b/package_meta.cc
@@ -51,12 +51,13 @@ bool hasManualSelections = 0;
 
 /*****************/
 
+/* Return an appropriate category caption given the action */
 char const *
 packagemeta::action_caption (_actions _value)
 {
   switch (_value)
     {
-    case Default_action:
+    case NoChange_action:
       return "Default";
     case Install_action:
       return "Install";
@@ -454,7 +455,7 @@ packagemeta::select_action (int id, trusts const deftrust)
     }
   else
     {
-      if (id == packagemeta::Default_action)
+      if (id == packagemeta::NoChange_action)
         set_action((packagemeta::_actions)id, installed);
       else
         set_action((packagemeta::_actions)id, trustp (true, deftrust));
@@ -472,7 +473,7 @@ packagemeta::toggle_action ()
 {
   if (desired != installed)
     {
-      set_action(Default_action, installed);
+      set_action(NoChange_action, installed);
     }
   else
     {
@@ -495,11 +496,11 @@ packagemeta::list_actions(trusts const trust)
   if (!desired && installed)
     action = Uninstall_action;
   else if (!desired)
-    action = Default_action; // skip
+    action = NoChange_action; // skip
   else if (desired == installed && picked())
     action = Reinstall_action;
   else if (desired == installed)
-    action = Default_action; // keep
+    action = NoChange_action; // keep
   else
     action = Install_action;
 
@@ -507,14 +508,14 @@ packagemeta::list_actions(trusts const trust)
   ActionList *al = new ActionList();
 
   al->add("Uninstall", (int)Uninstall_action, (action == Uninstall_action), bool(installed));
-  al->add("Skip", (int)Default_action, (action == Default_action) && !installed, !installed);
+  al->add("Skip", (int)NoChange_action, (action == NoChange_action) && !installed, !installed);
 
   std::set<packageversion>::iterator i;
   for (i = versions.begin (); i != versions.end (); ++i)
     {
       if (*i == installed)
         {
-          al->add("Keep", (int)Default_action, (action == Default_action), TRUE);
+          al->add("Keep", (int)NoChange_action, (action == NoChange_action), TRUE);
           al->add(packagedb::task == PackageDB_Install ? "Reinstall" : "Retrieve",
                   (int)Reinstall_action, (action == Reinstall_action), TRUE);
         }
@@ -537,8 +538,9 @@ packagemeta::list_actions(trusts const trust)
 void
 packagemeta::set_action (_actions action, packageversion const &default_version)
 {
-  if (action == Default_action)
+  if (action == NoChange_action)
     {
+      // if installed, keep
       if (installed
 	  || categories.find ("Base") != categories.end ()
 	  || categories.find ("Orphaned") != categories.end ())
@@ -551,6 +553,7 @@ packagemeta::set_action (_actions action, packageversion const &default_version)
 	    }
 	}
       else
+        // else, if not installed, skip
 	desired = packageversion ();
       return;
     }
diff --git a/package_meta.h b/package_meta.h
index 2c40102..12f2de2 100644
--- a/package_meta.h
+++ b/package_meta.h
@@ -53,7 +53,7 @@ public:
 
   enum _actions
     {
-     Default_action = 1,
+     NoChange_action = 1,  // keep if installed, skip if not installed
      Install_action,
      Reinstall_action,
      Uninstall_action,
-- 
2.21.0

[PATCH setup 06/11] Use packagemeta::set_action() to update action

[Jon Turney]

Use packagemeta::set_action() to update the action for packagemeta
object in more (hopefully all) the places it gets changed.

(Future work: ideally we'd opaque packagemeta internals more by making
installed/curr/desired/etc. object private, rather than letting users
grovel around in those details)
---
 libsolv.cc      | 16 ++++++++++------
 package_db.cc   |  4 ++--
 package_meta.cc |  7 +++++--
 3 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/libsolv.cc b/libsolv.cc
index 9e3b066..bd8fa4c 100644
--- a/libsolv.cc
+++ b/libsolv.cc
@@ -740,16 +740,19 @@ SolverSolution::trans2db() const
         case SolverTransaction::transInstall:
           if (pv.Type() == package_binary)
             {
-              pkg->desired = pkg->default_version = pv;
-              pkg->pick(true);
+              pkg->set_action(packagemeta::Install_action, pv);
+              pkg->default_version = pv;
             }
           else // source package
             pkg->srcpick(true);
           break;
         case SolverTransaction::transErase:
           // Only relevant if pkg is still in its "no change" state
-          if (pkg->desired == pkg->installed && !pkg->picked())
-            pkg->desired = pkg->default_version = packageversion();
+          if (pkg->get_action() == packagemeta::NoChange_action)
+            {
+              pkg->set_action(packagemeta::Uninstall_action, packageversion());
+              pkg->default_version = packageversion();
+            }
           break;
         default:
           break;
@@ -767,13 +770,14 @@ SolverSolution::db2trans()
        p != db.packages.end (); ++p)
     {
       packagemeta *pkg = p->second;
-      if (pkg->desired && pkg->picked()) // install/upgrade/reinstall
+      if ((pkg->get_action() == packagemeta::Install_action) ||
+          (pkg->get_action() == packagemeta::Reinstall_action))
         {
           trans.push_back(SolverTransaction(pkg->desired, SolverTransaction::transInstall));
           if (pkg->installed)
             trans.push_back(SolverTransaction(pkg->installed, SolverTransaction::transErase));
         }
-      else if (!pkg->desired && pkg->installed) // uninstall
+      else if (pkg->get_action() == packagemeta::Uninstall_action)
         trans.push_back(SolverTransaction(pkg->installed, SolverTransaction::transErase));
 
       if (pkg->srcpicked())
diff --git a/package_db.cc b/package_db.cc
index 59c59ef..847f44e 100644
--- a/package_db.cc
+++ b/package_db.cc
@@ -758,7 +758,7 @@ packagedb::noChanges ()
        i != packages.end(); i++)
     {
       packagemeta *pkg = i->second;
-      pkg->desired = pkg->default_version = pkg->installed;
-      pkg->pick(false);
+      pkg->set_action(packagemeta::NoChange_action, pkg->installed);
+      pkg->default_version = pkg->installed;
     }
 }
diff --git a/package_meta.cc b/package_meta.cc
index b731254..2564f95 100644
--- a/package_meta.cc
+++ b/package_meta.cc
@@ -553,8 +553,11 @@ packagemeta::set_action (_actions action, packageversion const &default_version)
 	    }
 	}
       else
-        // else, if not installed, skip
-	desired = packageversion ();
+        {
+          // else, if not installed, skip
+          desired = packageversion ();
+          pick(false);
+        }
     }
   else if (action == Install_action)
     {
-- 
2.21.0

[PATCH setup 05/11] Store the requested action in packagemeta::set_action()

[Jon Turney]

Store the action requested with set_action() in the packagemeta object,
rather than just encoding it in _picked/installed/desired (see
discussion in commit 4209699d)

Try to avoid meaningless states occuring, i.e. action=Install with
desired=installed is converted to action=Keep.

Future work: There's still some odd stuff left in set_action() which we
need to do some hard thinking about: action=NoChange treats Base
category packages specially, action=Install will switch to installing
the source if binary isn't accessible.
---
 choose.cc       | 2 +-
 package_meta.cc | 5 +++--
 package_meta.h  | 5 ++++-
 3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/choose.cc b/choose.cc
index 277baba..7f6e332 100644
--- a/choose.cc
+++ b/choose.cc
@@ -307,7 +307,7 @@ ChooserPage::applyCommandLinePackageSelection()
       else if (PruneInstallOption)
 	pkg.set_action (packagemeta::NoChange_action, pkg.curr);
       else if (upgrade)
-	pkg.set_action (packagemeta::NoChange_action, pkg.trustp(true, TRUST_UNKNOWN));
+	pkg.set_action (packagemeta::Install_action, pkg.trustp(true, TRUST_UNKNOWN));
       else
 	pkg.set_action (packagemeta::NoChange_action, pkg.installed);
     }
diff --git a/package_meta.cc b/package_meta.cc
index 3224f1c..b731254 100644
--- a/package_meta.cc
+++ b/package_meta.cc
@@ -555,7 +555,6 @@ packagemeta::set_action (_actions action, packageversion const &default_version)
       else
         // else, if not installed, skip
 	desired = packageversion ();
-      return;
     }
   else if (action == Install_action)
     {
@@ -576,11 +575,11 @@ packagemeta::set_action (_actions action, packageversion const &default_version)
 	      }
 	  else
 	    {
+	      action = NoChange_action;
 	      pick (false);
 	      srcpick (false);
 	    }
 	}
-      return;
     }
   else if (action == Reinstall_action)
     {
@@ -595,6 +594,8 @@ packagemeta::set_action (_actions action, packageversion const &default_version)
     {
       desired = packageversion ();
     }
+
+  _action = action;
 }
 
 bool
diff --git a/package_meta.h b/package_meta.h
index 12f2de2..e2144ad 100644
--- a/package_meta.h
+++ b/package_meta.h
@@ -38,7 +38,7 @@ public:
   packagemeta (packagemeta const &);
   packagemeta (const std::string& pkgname)
     : name (pkgname), user_picked (false),
-    _picked(false), _srcpicked(false)
+      _action(NoChange_action), _picked(false), _srcpicked(false)
   {
   }
 
@@ -64,6 +64,7 @@ public:
   ActionList *list_actions(trusts const trust);
   void select_action (int id, trusts const deftrust);
   void toggle_action ();
+  _actions get_action () { return _action; }
 
   void set_message (const std::string& message_id, const std::string& message_string)
   {
@@ -162,6 +163,8 @@ private:
   std::vector <Script> scripts_;
   static bool scan (const packageversion &pkg, bool mirror_mode);
 
+  _actions _action;
+
   bool _picked; /* true if desired version is to be (re)installed */
   bool _srcpicked;
 
-- 
2.21.0

[PATCH setup 07/11] Use stored action in setting up solver

[Jon Turney]

Use stored action in setting up the solver, rather than working out what
the action was from _picked/installed/desired.
---
 libsolv.cc | 47 ++++++++++++++++++++++++++---------------------
 1 file changed, 26 insertions(+), 21 deletions(-)

diff --git a/libsolv.cc b/libsolv.cc
index bd8fa4c..cbc651b 100644
--- a/libsolv.cc
+++ b/libsolv.cc
@@ -631,34 +631,39 @@ SolverTasks::setTasks()
     {
       packagemeta *pkg = p->second;
 
-      // decode UI state to action
-      // keep and skip need attention only when they differ from the
-      // solver's solution
-      if (pkg->installed != pkg->desired)
+      switch (pkg->get_action())
         {
-          if (pkg->desired)
-            add(pkg->desired, taskInstall); // install/upgrade
-          else
-            add(pkg->installed, taskUninstall); // uninstall
-        }
-      else if (pkg->installed)
-        {
-          if (pkg->picked())
-            add(pkg->installed, taskReinstall); // reinstall
-          else if (pkg->installed != pkg->default_version)
-            add(pkg->installed, taskKeep); // keep
-          else
+        case packagemeta::NoChange_action: // skip/keep
+          // keep and skip need attention only when they differ from the
+          // solver's solution
+          if (pkg->installed)
             {
-              // if installed (with no action selected), but blacklisted, force
-              // a distupgrade of this package
-              if (pkg->isBlacklisted(pkg->installed))
+              if (!pkg->picked() && pkg->installed != pkg->default_version)
+                add(pkg->installed, taskKeep); // keep
+              else if (pkg->isBlacklisted(pkg->installed))
                 {
+                  // if installed (with no action selected), but blacklisted,
+                  // force a distupgrade of this package
                   add(pkg->installed, taskForceDistUpgrade);
                 }
             }
+          else if (pkg->default_version)
+             add(pkg->default_version, taskSkip); // skip
+
+          break;
+
+        case packagemeta::Install_action:
+          add(pkg->desired, taskInstall); // install/upgrade
+          break;
+
+        case packagemeta::Uninstall_action:
+          add(pkg->installed, taskUninstall); // uninstall
+          break;
+
+        case packagemeta::Reinstall_action:
+          add(pkg->installed, taskReinstall); // reinstall
+          break;
         }
-      else if (pkg->default_version)
-        add(pkg->default_version, taskSkip); // skip
 
       // only install action makes sense for source packages
       if (pkg->srcpicked())
-- 
2.21.0

[PATCH setup 09/11] Use stored action in packagemeta::list_actions()

[Jon Turney]

Use the stored action in packagemeta::list_actions(), rather than
working backwards from desired/installed/picked.
---
 package_meta.cc | 26 ++++++--------------------
 1 file changed, 6 insertions(+), 20 deletions(-)

diff --git a/package_meta.cc b/package_meta.cc
index 2564f95..8fad50d 100644
--- a/package_meta.cc
+++ b/package_meta.cc
@@ -490,34 +490,20 @@ packagemeta::toggle_action ()
 ActionList *
 packagemeta::list_actions(trusts const trust)
 {
-  // first work out the current action, so we can indicate that
-  _actions action;
-
-  if (!desired && installed)
-    action = Uninstall_action;
-  else if (!desired)
-    action = NoChange_action; // skip
-  else if (desired == installed && picked())
-    action = Reinstall_action;
-  else if (desired == installed)
-    action = NoChange_action; // keep
-  else
-    action = Install_action;
-
-  // now build the list of possible actions
+  // build the list of possible actions
   ActionList *al = new ActionList();
 
-  al->add("Uninstall", (int)Uninstall_action, (action == Uninstall_action), bool(installed));
-  al->add("Skip", (int)NoChange_action, (action == NoChange_action) && !installed, !installed);
+  al->add("Uninstall", (int)Uninstall_action, (_action == Uninstall_action), bool(installed));
+  al->add("Skip", (int)NoChange_action, (_action == NoChange_action) && !installed, !installed);
 
   std::set<packageversion>::iterator i;
   for (i = versions.begin (); i != versions.end (); ++i)
     {
       if (*i == installed)
         {
-          al->add("Keep", (int)NoChange_action, (action == NoChange_action), TRUE);
+          al->add("Keep", (int)NoChange_action, (_action == NoChange_action), TRUE);
           al->add(packagedb::task == PackageDB_Install ? "Reinstall" : "Retrieve",
-                  (int)Reinstall_action, (action == Reinstall_action), TRUE);
+                  (int)Reinstall_action, (_action == Reinstall_action), TRUE);
         }
       else
         {
@@ -526,7 +512,7 @@ packagemeta::list_actions(trusts const trust)
             label += " (Test)";
           al->add(label,
                   -std::distance(versions.begin (), i),
-                  (action == Install_action) && (*i == desired),
+                  (_action == Install_action) && (*i == desired),
                   TRUE);
         }
     }
-- 
2.21.0

[PATCH setup 08/11] Allow better handling of an obsolete package specified on command line

[Jon Turney]

Now we have the flexibilty to record a package as explicitly required to
be installed, rather than merely installed because the desired version
is different to the installed version, we can record packages selected
for installation on the command line as distinct from choosing a
specific version of that package via the picker.

Use this to allow the solver to make a better choice of what provides a
package name specified on the command line.

(Note that we need to use SOLVER_SOLVABLE_PROVIDES rather than
SOLVER_SOLVABLE_NAME to allow the solver to take packages which provide
and obsolete the named package into account.)

Only turn this behaviour on when --upgrade-also is specified.

e.g. setup -q -g -P python3-lxml currently gets you an (empty)
python3-lxml package, which is replaced by python36-lxml (which
obsoletes it) on the next setup run.  After this change, python36-lxml
is installed instead.

See also the dicusssion at
https://cygwin.com/ml/cygwin-apps/2017-10/msg00092.html et seq.
---
 choose.cc  | 2 +-
 libsolv.cc | 8 +++++++-
 libsolv.h  | 1 +
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/choose.cc b/choose.cc
index 7f6e332..be08627 100644
--- a/choose.cc
+++ b/choose.cc
@@ -299,7 +299,7 @@ ChooserPage::applyCommandLinePackageSelection()
       bool uninstall = (!(wanted  || base) && (deleted || PruneInstallOption))
 		     || (orphaned && CleanOrphansOption);
       if (install)
-	pkg.set_action (packagemeta::Install_action, pkg.curr);
+	pkg.set_action (packagemeta::Install_action, UpgradeAlsoOption ? packageversion () : pkg.curr);
       else if (reinstall)
 	pkg.set_action (packagemeta::Reinstall_action, pkg.curr);
       else if (uninstall)
diff --git a/libsolv.cc b/libsolv.cc
index cbc651b..d7a9d01 100644
--- a/libsolv.cc
+++ b/libsolv.cc
@@ -653,7 +653,10 @@ SolverTasks::setTasks()
           break;
 
         case packagemeta::Install_action:
-          add(pkg->desired, taskInstall); // install/upgrade
+          if (pkg->desired)
+            add(pkg->desired, taskInstall); // install/upgrade
+          else
+            add(pkg->curr, taskInstallAny); // install
           break;
 
         case packagemeta::Uninstall_action:
@@ -829,6 +832,9 @@ SolverSolution::tasksToJobs(SolverTasks &tasks, updateMode update, Queue &job)
         case SolverTasks::taskInstall:
           queue_push2(&job, SOLVER_INSTALL | SOLVER_SOLVABLE, sv.id);
           break;
+        case SolverTasks::taskInstallAny:
+          queue_push2(&job, SOLVER_INSTALL | SOLVER_SOLVABLE_PROVIDES, sv.name_id());
+          break;
         case SolverTasks::taskUninstall:
           queue_push2(&job, SOLVER_ERASE | SOLVER_SOLVABLE, sv.id);
           break;
diff --git a/libsolv.h b/libsolv.h
index 4fd6d61..43f7269 100644
--- a/libsolv.h
+++ b/libsolv.h
@@ -197,6 +197,7 @@ class SolverTasks
     taskKeep,
     taskSkip,
     taskForceDistUpgrade,
+    taskInstallAny,
   };
   void add(const SolvableVersion &v, task t)
   {
-- 
2.21.0

[PATCH setup 10/11] Use stored action in packagemeta::action_caption()

[Jon Turney]

Use the stored action in packagemeta::action_caption(), rather than
working backwards from desired/installed/picked.
---
 package_meta.cc | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/package_meta.cc b/package_meta.cc
index 8fad50d..b417fb0 100644
--- a/package_meta.cc
+++ b/package_meta.cc
@@ -426,19 +426,23 @@ packagemeta::LDesc () const
 std::string
 packagemeta::action_caption () const
 {
-  if (!desired && installed)
-    return "Uninstall";
-  else if (!desired)
-    return "Skip";
-  else if (desired == installed && picked())
-    return packagedb::task == PackageDB_Install ? "Reinstall" : "Retrieve";
-  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 */
-    return "Keep";
-  else
-    return desired.Canonical_version ();
+  switch (_action)
+    {
+    case Uninstall_action:
+      return "Uninstall";
+    case NoChange_action:
+      if (!desired)
+        return "Skip";
+      if (desired.sourcePackage() && srcpicked())
+        /* FIXME: Redo source should come up if the tarball is already present locally */
+        return "Source";
+      return "Keep";
+    case Reinstall_action:
+      return packagedb::task == PackageDB_Install ? "Reinstall" : "Retrieve";
+    case Install_action:
+      return desired.Canonical_version ();
+    }
+  return "Unknown";
 }
 
 void
-- 
2.21.0

[PATCH setup 11/11] Ensure we only set user_picked when appropriate

[Jon Turney]

Add an optional parameter to set_action(), to indicate the action is the
result of user action, and only set user_picked then.

Set that parameter when installing from the command line. All other user
actions come through select_action(), so also set that parameter there.
---
 choose.cc       |  2 +-
 package_meta.cc | 14 +++++++-------
 package_meta.h  |  3 ++-
 3 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/choose.cc b/choose.cc
index be08627..4fa0c74 100644
--- a/choose.cc
+++ b/choose.cc
@@ -299,7 +299,7 @@ ChooserPage::applyCommandLinePackageSelection()
       bool uninstall = (!(wanted  || base) && (deleted || PruneInstallOption))
 		     || (orphaned && CleanOrphansOption);
       if (install)
-	pkg.set_action (packagemeta::Install_action, UpgradeAlsoOption ? packageversion () : pkg.curr);
+        pkg.set_action (packagemeta::Install_action, UpgradeAlsoOption ? packageversion () : pkg.curr, true);
       else if (reinstall)
 	pkg.set_action (packagemeta::Reinstall_action, pkg.curr);
       else if (uninstall)
diff --git a/package_meta.cc b/package_meta.cc
index b417fb0..940f69a 100644
--- a/package_meta.cc
+++ b/package_meta.cc
@@ -462,12 +462,8 @@ packagemeta::select_action (int id, trusts const deftrust)
       if (id == packagemeta::NoChange_action)
         set_action((packagemeta::_actions)id, installed);
       else
-        set_action((packagemeta::_actions)id, trustp (true, deftrust));
+        set_action((packagemeta::_actions)id, trustp (true, deftrust), true);
     }
-
-  /* Memorize the fact that the user picked at least once. */
-  if (!installed)
-    user_picked = true;
 }
 
 // toggle between the currently installed version (or uninstalled, if not
@@ -526,7 +522,8 @@ packagemeta::list_actions(trusts const trust)
 
 // Set a particular type of action.
 void
-packagemeta::set_action (_actions action, packageversion const &default_version)
+packagemeta::set_action (_actions action, packageversion const &default_version,
+                         bool useraction)
 {
   if (action == NoChange_action)
     {
@@ -557,7 +554,10 @@ packagemeta::set_action (_actions action, packageversion const &default_version)
 	  if (desired != installed)
 	    if (desired.accessible ())
 	      {
-		user_picked = true;
+		/* Memorize the fact that the user picked to install this package at least once. */
+		if (useraction)
+		  user_picked = true;
+
 		pick (true);
 		srcpick (false);
 	      }
diff --git a/package_meta.h b/package_meta.h
index e2144ad..1f4f3be 100644
--- a/package_meta.h
+++ b/package_meta.h
@@ -60,7 +60,8 @@ public:
     };
   static const char *action_caption (_actions value);
 
-  void set_action (_actions, packageversion const & default_version);
+  void set_action (_actions, packageversion const & default_version,
+                   bool useraction = false);
   ActionList *list_actions(trusts const trust);
   void select_action (int id, trusts const deftrust);
   void toggle_action ();
-- 
2.21.0

Re: [PATCH setup 00/11] Improve handling of specifying an obsolete package to be installed on the command line

[Ken Brown]

On 8/1/2019 12:05 PM, Jon Turney wrote:
> e.g. setup -q -g -P python3-lxml (which used to do something useful)
> currently gets you an (empty) python3-lxml package, which will be replaced
> by python36-lxml (which obsoletes it) on the next setup run. After this
> change, python36-lxml is installed instead.
> 
> See also the dicusssion at
> https://cygwin.com/ml/cygwin-apps/2017-10/msg00092.html et seq. (where I
> come to the (incorrect) conclusion that since we don't need this for
> interactive use, it's not needed)
> 
> Jon Turney (11):
>    Remove 'Bin?' column
>    Remove unused packagemeta::key
>    Make packagemeta::message private
>    Rename 'Default' packagemeta action to 'NoChange' for clarity
>    Store the requested action in packagemeta::set_action()
>    Use packagemeta::set_action() to update action
>    Use stored action in setting up solver
>    Allow better handling of an obsolete package specified on command line
>    Use stored action in packagemeta::list_actions()
>    Use stored action in packagemeta::action_caption()
>    Ensure we only set user_picked when appropriate
> 
>   PickCategoryLine.cc |  2 +-
>   PickPackageLine.cc  | 32 +---------------
>   PickView.cc         |  3 +-
>   PickView.h          | 12 +++---
>   choose.cc           |  9 ++---
>   libsolv.cc          | 69 ++++++++++++++++++++-------------
>   libsolv.h           |  1 +
>   package_db.cc       |  4 +-
>   package_meta.cc     | 93 ++++++++++++++++++++++-----------------------
>   package_meta.h      | 16 ++++----
>   10 files changed, 113 insertions(+), 128 deletions(-)

Jon,

Since you don't seem to have gotten any feedback on this patch series, I just 
want to let you know that I've been running setup.exe with these patches for a 
couple months, and I haven't seen any regressions.  (But I haven't done any 
systematic testing or review.)

Ken

Re: [PATCH setup 00/11] Improve handling of specifying an obsolete package to be installed on the command line

[Jon Turney]

On 01/12/2019 19:31, Ken Brown wrote:
> On 8/1/2019 12:05 PM, Jon Turney wrote:
>> e.g. setup -q -g -P python3-lxml (which used to do something useful)
>> currently gets you an (empty) python3-lxml package, which will be replaced
>> by python36-lxml (which obsoletes it) on the next setup run. After this
>> change, python36-lxml is installed instead.
>>
>> See also the dicusssion at
>> https://cygwin.com/ml/cygwin-apps/2017-10/msg00092.html et seq. (where I
>> come to the (incorrect) conclusion that since we don't need this for
>> interactive use, it's not needed)
>>
[...]
> 
> Since you don't seem to have gotten any feedback on this patch series, I just
> want to let you know that I've been running setup.exe with these patches for a
> couple months, and I haven't seen any regressions.  (But I haven't done any
> systematic testing or review.)

Thanks very much for giving these a try.

I was recently reminded of these patches and thinking of doing a setup 
release to include them.  Are there any other outstanding patches you 
are aware of?

Re: [PATCH setup 00/11] Improve handling of specifying an obsolete package to be installed on the command line

[Ken Brown]

On 12/3/2019 3:44 PM, Jon Turney wrote:
> On 01/12/2019 19:31, Ken Brown wrote:
>> On 8/1/2019 12:05 PM, Jon Turney wrote:
>>> e.g. setup -q -g -P python3-lxml (which used to do something useful)
>>> currently gets you an (empty) python3-lxml package, which will be replaced
>>> by python36-lxml (which obsoletes it) on the next setup run. After this
>>> change, python36-lxml is installed instead.
>>>
>>> See also the dicusssion at
>>> https://cygwin.com/ml/cygwin-apps/2017-10/msg00092.html et seq. (where I
>>> come to the (incorrect) conclusion that since we don't need this for
>>> interactive use, it's not needed)
>>>
> [...]
>>
>> Since you don't seem to have gotten any feedback on this patch series, I just
>> want to let you know that I've been running setup.exe with these patches for a
>> couple months, and I haven't seen any regressions.  (But I haven't done any
>> systematic testing or review.)
> 
> Thanks very much for giving these a try.
> 
> I was recently reminded of these patches and thinking of doing a setup release 
> to include them.  Are there any other outstanding patches you are aware of?

None that I know of.

Ken