why: Render info about all providers on virtual packages

Refactor code into a doOne helper lambda, and then add loop
over the providers to use it.

This fixes a crash seen by Benjamin in a nice way than just
failing :)

Reported-by: Benjamin Drung

2 files changed, 35 insertions, 13 deletions

diff --git a/apt-pkg/solver3.cc b/apt-pkg/solver3.cc
index 096cbb521..a09c27f2a 100644
--- a/apt-pkg/solver3.cc
+++ b/apt-pkg/solver3.cc
@@ -1372,19 +1372,34 @@ bool DependencySolver::ToDepCache(pkgDepCache &depcache) const
 std::string DependencySolver::InternalCliWhy(pkgDepCache &cache, pkgCache::PkgIterator pkg, bool assignment)
 {
    DependencySolver solver(cache.GetCache(), cache.GetPolicy(), EDSP::Request::Flags(0));
-   // In case nothing has a positive dependency on pkg it may not actually be discovered in a `why-not`
-   // scenario, so make sure we discover it explicitly.
-   solver.Discover(Var(pkg));
-   if (not solver.FromDepCache(cache) || not solver.Solve())
-      return "";
-   std::unordered_set<Var> seen;
    std::string buf;
-   if (solver[Var(pkg)].assignment == LiftedBool::Undefined)
-      return strprintf(buf, "%s is undecided\n", pkg.FullName().c_str()), buf;
-   if (assignment && solver[Var(pkg)].assignment != LiftedBool::True)
-      return strprintf(buf, "%s is not actually marked for install\n", pkg.FullName().c_str()), buf;
-   if (not assignment && solver[Var(pkg)].assignment == LiftedBool::True)
-      return strprintf(buf, "%s is actually marked for install\n", pkg.FullName().c_str()), buf;
-   return solver.LongWhyStr(Var(pkg), assignment, solver[Var(pkg)].reason, "", seen);
+
+   auto doOne = [&](Var var) -> std::string
+   {
+      solver.Discover(var);
+      if (not solver.FromDepCache(cache) || not solver.Solve())
+	 return "";
+      std::unordered_set<Var> seen;
+      if (solver[var].assignment == LiftedBool::Undefined)
+	 return strprintf(buf, "%s is undecided\n", var.toString(cache).c_str()), buf;
+      if (assignment && solver[var].assignment != LiftedBool::True)
+	 return strprintf(buf, "%s is not actually marked for install\n", var.toString(cache).c_str()), buf;
+      if (not assignment && solver[var].assignment == LiftedBool::True)
+	 return strprintf(buf, "%s is actually marked for install\n", var.toString(cache).c_str()), buf;
+      return solver.LongWhyStr(var, assignment, solver[var].reason, "", seen);
+   };
+
+   if (pkg.VersionList().end())
+   {
+      if (pkg.ProvidesList().end())
+	 return strprintf(buf, "%s is a virtual package without a provider\n", pkg.FullName().c_str()), buf;
+
+      std::ostringstream out;
+      ioprintf(out, "%s is a virtual package, printing all providers\n", pkg.FullName().c_str());
+      for (auto prv = pkg.ProvidesList(); not prv.end(); ++prv)
+	 out << doOne(Var(prv.OwnerVer()));
+      return out.str();
+   }
+   return doOne(Var(pkg));
 }
 } // namespace APT::Solver
diff --git a/test/integration/test-apt-cli-why b/test/integration/test-apt-cli-why
index 8a4c94b30..8bcd0c4d9 100755
--- a/test/integration/test-apt-cli-why
+++ b/test/integration/test-apt-cli-why
@@ -13,6 +13,8 @@ insertpackage 'unstable' 'other-level-2' 'all' '1' 'Conflicts: level-2
 Provides: level-2'
 insertpackage 'unstable' 'almost-level-2' 'all' '1' 'Conflicts: level-2'
 insertpackage 'unstable' 'level-3' 'all' '1' 'Conflicts: level-3'
+insertpackage 'unstable' 'provider-for-virtual-package' 'all' '1' 'Provides: virtual-package'
+insertpackage 'unstable' 'depends-on-not-really-virtual-package' 'all' '1' 'Depends: not-really-virtual-package'
 setupaptarchive
 
 testsuccess aptmark auto level-1 level-2
@@ -69,3 +71,8 @@ For context, additional choices that could not be installed:
 
 
 testsuccessequal "level-3:amd64 is undecided" apt why level-3
+
+testsuccessequal "virtual-package:amd64 is a virtual package, printing all providers
+provider-for-virtual-package:amd64=1 is undecided" apt why virtual-package
+testsuccessequal "not-really-virtual-package:amd64 is a virtual package without a provider" apt why not-really-virtual-package
+