[PATCH setup] Report dependencies which don't exist

[PATCH setup] Report dependencies which don't exist

[Jon Turney]

At the moment, dependencies which can't be found are silently ignored.
Instead, record and report these dependency problems.

Signed-off-by: Jon Turney <jon.turney@dronecode.org.uk>
---
 prereq.cc | 54 ++++++++++++++++++++++++++++++++++++++++++++----------
 prereq.h  |  1 +
 2 files changed, 45 insertions(+), 10 deletions(-)

diff --git a/prereq.cc b/prereq.cc
index c766055..bdc609e 100644
--- a/prereq.cc
+++ b/prereq.cc
@@ -163,11 +163,16 @@ PrereqPage::OnUnattended ()
 
 // instantiate the static members
 map <packagemeta *, vector <packagemeta *>, packagemeta_ltcomp> PrereqChecker::unmet;
+map <std::string, vector <packagemeta *> > PrereqChecker::notfound;
 trusts PrereqChecker::theTrust = TRUST_CURR;
 
 /* This function builds a list of unmet dependencies to present to the user on
-   the PrereqPage propsheet.  The data is stored as an associative map of
-   unmet[missing-package] = vector of packages that depend on missing-package */
+   the PrereqPage propsheet.
+
+   The data is stored in two associative maps:
+     unmet[package] = vector of packages that depend on package.
+     notfound[package-name] = vector of packages that depend on package.
+*/
 bool
 PrereqChecker::isMet ()
 {
@@ -177,8 +182,9 @@ PrereqChecker::isMet ()
   Progress.SetText2 ("");
   Progress.SetText3 ("");
 
-  // unmet is static - clear it each time this is called
+  // clear static data each time this is called
   unmet.clear ();
+  notfound.clear ();
 
   // packages that need to be checked for dependencies
   queue <packagemeta *> todo;
@@ -223,20 +229,31 @@ PrereqChecker::isMet ()
           PackageSpecification *dep_spec = (*d)->at(0);
           packagemeta *dep = db.findBinary (*dep_spec);
 
-          if (dep && !(dep->desired && dep_spec->satisfies (dep->desired)))
+          if (dep)
             {
-              // we've got an unmet dependency
-              if (unmet.find (dep) == unmet.end ())
+              if (!(dep->desired && dep_spec->satisfies (dep->desired)))
                 {
-                  // newly found dependency: add to worklist
-                  todo.push (dep);
+                  // we've got an unmet dependency
+                  if (unmet.find (dep) == unmet.end ())
+                    {
+                      // newly found dependency: add to worklist
+                      todo.push (dep);
+                    }
+                  unmet[dep].push_back (pack);
                 }
-              unmet[dep].push_back (pack);
+            }
+          else
+            {
+              // dependency on a package which doesn't have any binary versions
+              // (i.e. it is source only or doesn't exist)
+              Log (LOG_PLAIN) << "package " << pack->name << " has dependency "
+                              << dep_spec->packageName() << " we can't find" << endLog;
+              notfound[dep_spec->packageName()].push_back (pack);
             }
         }
     }
 
-  return unmet.empty ();
+  return unmet.empty () && notfound.empty ();
 }
 
 /* Formats 'unmet' as a string for display to the user.  */
@@ -245,6 +262,23 @@ PrereqChecker::getUnmetString (std::string &s)
 {
   s = "";
 
+  {
+    map <std::string, vector <packagemeta *> >::iterator i;
+    for (i = notfound.begin(); i != notfound.end(); i++)
+      {
+        s = s + i->first
+          + "\t(not found)"
+          + "\r\n\tRequired by: ";
+        for (unsigned int j = 0; j < i->second.size(); j++)
+          {
+            s += i->second[j]->name;
+            if (j != i->second.size() - 1)
+              s += ", ";
+          }
+        s += "\r\n\r\n";
+      }
+  }
+
   map <packagemeta *, vector <packagemeta *>, packagemeta_ltcomp>::iterator i;
   for (i = unmet.begin(); i != unmet.end(); i++)
     {
diff --git a/prereq.h b/prereq.h
index 39347aa..2aed63a 100644
--- a/prereq.h
+++ b/prereq.h
@@ -51,6 +51,7 @@ private:
   
   // this is the actual hash_map that does all the work
   static map <packagemeta *, vector <packagemeta *>, packagemeta_ltcomp> unmet;
+  static map <std::string, vector <packagemeta *> > notfound;
   static trusts theTrust;
 };
 
-- 
2.8.3

Re: [PATCH setup] Report dependencies which don't exist

[Corinna Vinschen]

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

Hi Jon,

On Jul  7 14:46, Jon Turney wrote:
> At the moment, dependencies which can't be found are silently ignored.
> Instead, record and report these dependency problems.

Nice idea.

One question, though.  Apart from getting a report which won't be read,
what is a user supposed to do?

In theory, shouldn't calm make sure that the deps exist?


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH setup] Report dependencies which don't exist

[Jon Turney]

On 07/07/2016 15:59, Corinna Vinschen wrote:
> On Jul  7 14:46, Jon Turney wrote:
>> At the moment, dependencies which can't be found are silently ignored.
>> Instead, record and report these dependency problems.
>
> Nice idea.
>
> One question, though.  Apart from getting a report which won't be read,
> what is a user supposed to do?
>
> In theory, shouldn't calm make sure that the deps exist?

Yes, in fact it already does so.

However, that doesn't help in the case of cross-repo dependencies (e.g 
cygwinports contains several packages which depend on cygwin packages 
which seem to have been removed or renamed)

It would be nice to have a tool to detect those problems at setup.ini 
generation time, but that doesn't exist yet.

Additionally, adding this check reveals a problem with the way this 
check is done in calm, as it reports the following as non-existent 
dependencies:

gcc4 (x86 only) -> gcc
logiweb (x86 only) -> gcc
varnish -> gcc, libedit

This is because setup has, by this stage, discarded source-only 
packages, which it doesn't make any sense to depend on.

I'll make calm check for odd dependencies like that, once I've fixed 
those on sourceware.

Re: [PATCH setup] Report dependencies which don't exist

[Corinna Vinschen]

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

On Jul  7 16:36, Jon Turney wrote:
> On 07/07/2016 15:59, Corinna Vinschen wrote:
> > On Jul  7 14:46, Jon Turney wrote:
> > > At the moment, dependencies which can't be found are silently ignored.
> > > Instead, record and report these dependency problems.
> > 
> > Nice idea.
> > 
> > One question, though.  Apart from getting a report which won't be read,
> > what is a user supposed to do?
> > 
> > In theory, shouldn't calm make sure that the deps exist?
> 
> Yes, in fact it already does so.
> 
> However, that doesn't help in the case of cross-repo dependencies (e.g
> cygwinports contains several packages which depend on cygwin packages which
> seem to have been removed or renamed)
> 
> It would be nice to have a tool to detect those problems at setup.ini
> generation time, but that doesn't exist yet.
> 
> Additionally, adding this check reveals a problem with the way this check is
> done in calm, as it reports the following as non-existent dependencies:
> 
> gcc4 (x86 only) -> gcc
> logiweb (x86 only) -> gcc
> varnish -> gcc, libedit
> 
> This is because setup has, by this stage, discarded source-only packages,
> which it doesn't make any sense to depend on.
> 
> I'll make calm check for odd dependencies like that, once I've fixed those
> on sourceware.

Thank you.  Please feel free to apply the patch.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]