On 3/8/2018 4:59 PM, Ken Brown wrote:
On 3/8/2018 10:59 AM, Ken Brown wrote:
On 3/7/2018 4:52 PM, Ken Brown wrote:
On 3/6/2018 1:47 PM, Jon Turney wrote:
On 06/03/2018 15:18, Jon Turney wrote:
So yeah, I guess putting some complexity back in accessible() would work, or perhaps the attached?  (This doesn't do the right thing for a few packages, for reasons I'm still looking into...)

To be specific it was doing the wrong thing for those few packages with no source

  /* scan for local copies of package */
-void
+bool
  packagemeta::scan (const packageversion &pkg, bool mirror_mode)
  {
-  /* Already have something */
+  /* empty version */
    if (!pkg)
-    return;
+    return true;

So, this needs to be 'return false', as the empty version is always inaccessible, to get the same behaviour as before.

I've found another problem with local installs: If a package needs upgrading, then the chooser will offer the upgraded version for install, even if there's no archive available.  As a result, the current version will get uninstalled, and then setup will discover that it doesn't have the archive to install the new version.

The problem occurs because packagedb::defaultTrust() is called after ScanDownloadedFiles() has already done its work.  solution.update() and solution.trans2db() are called, and pkg->desired is set equal to an inaccessible version pv (which has been previously removed from pkg->versions).

I guess trans2db() should check that pv is in pkg->versions before acting on an install transaction for pv.  And then we also have to make sure to ignore the erase transaction for the current version of pkg.

Alternatively, can we just remove an inaccessible packageversion from the libsolv pool, or at at least just tell libsolv that we don't want to install it?

Still another alternative, and maybe the simplest, is to make sure that the chooser never shows a version that is not in pkg->versions. packagemeta::set_action (trusts const trust) almost does this, except for one useless desired.accessible() call.  So that should be fixed, as well as the setting of the initial action.

Sorry for this stream of consciousness series of posts, but I'll stop after this one.  I think it's possible that restoring the previous meaning of 'accessible()', at least for local installs, might solve this problem.

Patches attached (for the topic/fix-local-install branch).

From 1645f2b13616ec71054a5ad35bce864f1f3e0dca Mon Sep 17 00:00:00 2001
From: Ken Brown <kbr...@cornell.edu>
Date: Sun, 11 Mar 2018 10:43:06 -0400
Subject: [PATCH 1/2] Make SolvableVersion::accessible() useful for local
 installs

For a local install, make SolvableVersion::accessible() indicate
whether or not there is an archive available in the local cache.  This
is used in packagemeta::set_action() to prevent the chooser from
offering unavailable versions.
---
 libsolv.cc | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/libsolv.cc b/libsolv.cc
index 0dc7557..604ff7e 100644
--- a/libsolv.cc
+++ b/libsolv.cc
@@ -14,6 +14,7 @@
 #include "libsolv.h"
 #include "package_db.h"
 #include "package_meta.h"
+#include "resource.h"
 
 #include "solv/solver.h"
 #include "solv/solverdebug.h"
@@ -239,21 +240,16 @@ SolvableVersion::source() const
   return (packagesource *)repo_lookup_num(solvable->repo, id, psrc_attr, 
(unsigned long long)&empty_source);
 }
 
+// If we're doing a locall install, is there an archive available?
+// This assumes that ScanDownloadedFiles() has already been called.
 bool
 SolvableVersion::accessible () const
 {
-  // The 'accessible' check used to test if an archive was available locally or
-  // from a mirror.
-  //
-  // This seems utterly pointless. as binary packages which aren't 'accessible'
-  // never get to appear in the package list.
-  //
-  // For source packages, it's equivalent to the bool conversion operator.)
-  //
-  if (id != 0)
-    return TRUE;
-  else
-    return FALSE;
+  if (id == 0)
+    return false;
+  if (::source == IDC_SOURCE_LOCALDIR)
+    return source ()->Cached ();
+  return true;
 }
 
 package_stability_t
-- 
2.16.2

From 365fa9afb8a49c2360bf7cfd2a2c2557522f85e5 Mon Sep 17 00:00:00 2001
From: Ken Brown <kbr...@cornell.edu>
Date: Mon, 12 Mar 2018 09:13:41 -0400
Subject: [PATCH 2/2] Don't put inaccessible packageversions in the package
 database

In a local install, libsolv might choose an inaccessible version for
install.  Change SolverSolution::trans2db() so that such a version is
never used.
---
 libsolv.cc | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/libsolv.cc b/libsolv.cc
index 604ff7e..1342f3b 100644
--- a/libsolv.cc
+++ b/libsolv.cc
@@ -637,6 +637,8 @@ void
 SolverSolution::trans2db() const
 {
   packagedb db;
+  std::vector<packagemeta *> inaccessible;
+
   // First reset all packages to the "no change" state
   for (packagedb::packagecollection::iterator i = db.packages.begin();
        i != db.packages.end(); i++)
@@ -667,6 +669,8 @@ SolverSolution::trans2db() const
             {
               pkg->desired = pkg->default_version = pv;
               pkg->pick(true);
+             if (!pv.accessible())
+               inaccessible.push_back (pkg);
             }
           else // source package
             pkg->srcpick(true);
@@ -680,6 +684,14 @@ SolverSolution::trans2db() const
           break;
         }
     }
+  for (std::vector<packagemeta *>::iterator i = inaccessible.begin();
+       i != inaccessible.end(); i++)
+    {
+      packagemeta *pkg = *i;
+      // Reset pkg to its "no change" state.
+      pkg->desired = pkg->default_version = pkg->installed;
+      pkg->pick(false);
+    }
 }
 
 void
-- 
2.16.2

Reply via email to