Re: setup: problems with local install

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

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

On 05/03/2018 18:34, Ken Brown wrote:
> This is a followup to the thread started here:
> 
>    https://cygwin.com/ml/cygwin/2018-03/msg00027.html
> 
> There are two problems with installing from a local directory.

Thanks very much for looking into these.

> 2. In several of the views, all packages from setup.ini are listed, even 
> if there is no corresponding archive in the local directory.  What 
> happens is that packagemeta::scan() calls pkg.source()->sites.clear() 
> for such packages, but this information is never used to prevent the 
> package from appearing in the list.
> 
> It used to be that such packages would be declared inaccessible, but 
> SolvableVersion::accessible() no longer does this.
> 
> Jon, you wrote the following comment in the definition of 
> SolvableVersion::accessible():
> 
> "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."
> 
> Do we need to reinstate the old function of the accessibility check?

I guess I looked at packagemeta::ScanDownloadedFiles() and saw that it 
was removing versions, and thought everything was good.

I didn't notice accessible() was indirectly how the result of scan() was 
returned :S

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...)

(I also note we have also have another 'erase an element from a vector 
while we are iterating over it' here, so that needs fixing, as well)

I'm kind of uncertain what the side-effects of this code are when source 
!= IDC_SOURCE_LOCALDIR, or if they are desired?  Perhaps it's removing 
packages which have corrupt local copies or something?  It would be 
clearer to omit the whole thing in that case.


[-- Attachment #2: 0001-Fix-packagemeta-ScanDownloadedFiles.patch --]
[-- Type: text/plain, Size: 2759 bytes --]

From d8aac08cf3d1ddf98ab9a6a8706b2c7b8bdfd7ad Mon Sep 17 00:00:00 2001
From: Jon Turney <jon.turney@dronecode.org.uk>
Date: Tue, 6 Mar 2018 14:56:40 +0000
Subject: [PATCH setup] Fix packagemeta::ScanDownloadedFiles

packagemeta::scan clears the site list if the package was not found, and
packagemeta::ScanDownloadedFiles uses packageversion::accessible() to check
that.

Instead communicate via a return value
---
 package_meta.cc | 27 ++++++++++++---------------
 package_meta.h  |  2 +-
 2 files changed, 13 insertions(+), 16 deletions(-)

diff --git a/package_meta.cc b/package_meta.cc
index c488e35..f0a0c20 100644
--- a/package_meta.cc
+++ b/package_meta.cc
@@ -664,33 +664,30 @@ packagemeta::logSelectionStatus() const
 }
 
 /* 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;
 
-  /* Remove mirror sites.
-   * FIXME: This is a bit of a hack.
-   */
   try
     {
       if (!check_for_cached (*(pkg.source ()), NULL, mirror_mode, false)
-	  && ::source == IDC_SOURCE_LOCALDIR)
-	pkg.source ()->sites.clear ();
+          && ::source == IDC_SOURCE_LOCALDIR)
+        return false;
     }
   catch (Exception * e)
     {
       // We can ignore these, since we're clearing the source list anyway
       if (e->errNo () == APPERR_CORRUPT_PACKAGE)
-	{
-	  pkg.source ()->sites.clear ();
-	  return;
-	}
+        return false;
+
       // Unexpected exception.
       throw e;
     }
+
+  return true;
 }
 
 void
@@ -712,15 +709,15 @@ packagemeta::ScanDownloadedFiles (bool mirror_mode)
 			   && (*i != pkg.installed
 			       || pkg.installed == pkg.curr
 			       || pkg.installed == pkg.exp);
-	  scan (*i, lazy_scan);
+	  bool accessible = scan (*i, lazy_scan);
 	  packageversion foo = *i;
 	  packageversion pkgsrcver = foo.sourcePackage ();
-	  scan (pkgsrcver, lazy_scan);
+	  bool src_accessible = scan (pkgsrcver, lazy_scan);
 
 	  /* For local installs, if there is no src and no bin, the version
 	   * is unavailable
 	   */
-	  if (!i->accessible () && !pkgsrcver.accessible ()
+	  if (!accessible && !src_accessible
 	      && *i != pkg.installed)
 	    {
 	      if (pkg.curr == *i)
diff --git a/package_meta.h b/package_meta.h
index 32372e2..600a163 100644
--- a/package_meta.h
+++ b/package_meta.h
@@ -170,7 +170,7 @@ protected:
 private:
   std::string trustLabel(packageversion const &) const;
   std::vector <Script> scripts_;
-  static void scan (const packageversion &pkg, bool mirror_mode);
+  static bool scan (const packageversion &pkg, bool mirror_mode);
 
   bool _picked; /* true if desired version is to be (re)installed */
   bool _srcpicked;
-- 
2.16.2