Merge branch 'solver3' into 'main'

solver3: Various bug fixes

See merge request apt-team/apt!459

4 files changed, 90 insertions, 12 deletions

diff --git a/apt-pkg/edsp.cc b/apt-pkg/edsp.cc
index 196f9d307..88569d3c4 100644
--- a/apt-pkg/edsp.cc
+++ b/apt-pkg/edsp.cc
@@ -373,8 +373,12 @@ bool EDSP::WriteRequest(pkgDepCache &Cache, FileFd &output,
    }
    if (flags & Request::FORBID_NEW_INSTALL)
       WriteOkay(Okay, output, "Forbid-New-Install: yes\n");
+   else if (flags & Request::FORBID_REMOVE)
+      WriteOkay(Okay, output, "Forbid-New-Install: no\n");
    if (flags & Request::FORBID_REMOVE)
       WriteOkay(Okay, output, "Forbid-Remove: yes\n");
+   else if (flags & Request::FORBID_NEW_INSTALL)
+      WriteOkay(Okay, output, "Forbid-Remove: no\n");
    auto const solver = _config->Find("APT::Solver", "internal");
    WriteOkay(Okay, output, "Solver: ", solver, "\n");
    if (_config->FindB("APT::Solver::Strict-Pinning", true) == false)
diff --git a/apt-pkg/solver3.cc b/apt-pkg/solver3.cc
index c993c1096..739309c59 100644
--- a/apt-pkg/solver3.cc
+++ b/apt-pkg/solver3.cc
@@ -634,6 +634,23 @@ bool APT::Solver::Propagate()
    return true;
 }
 
+static bool SameOrGroup(pkgCache::DepIterator a, pkgCache::DepIterator b)
+{
+   while (1)
+   {
+      if (a->DependencyData != b->DependencyData)
+	 return false;
+
+      // At least one has reached the end, break
+      if (not(a->CompareOp & pkgCache::Dep::Or) || not(b->CompareOp & pkgCache::Dep::Or))
+	 break;
+
+      ++a, ++b;
+   }
+   // Fully iterated over a and b
+   return not(a->CompareOp & pkgCache::Dep::Or) && not(b->CompareOp & pkgCache::Dep::Or);
+}
+
 const APT::Solver::Clause *APT::Solver::RegisterClause(Clause &&clause)
 {
    auto &clauses = (*this)[clause.reason].clauses;
@@ -701,8 +718,8 @@ void APT::Solver::Discover(Var var)
 
 	    // This dependency is shared across all versions, skip it.
 	    if (auto &pkgClauses = (*this)[Ver.ParentPkg()].clauses;
-		std::any_of(pkgClauses.begin(), pkgClauses.end(), [start](auto &c)
-			    { return c->dep && c->dep->DependencyData == start->DependencyData; }))
+		std::any_of(pkgClauses.begin(), pkgClauses.end(), [this, start](auto &c)
+			    { return c->dep && SameOrGroup(start, pkgCache::DepIterator(cache, c->dep)); }))
 	       continue;
 
 	    auto clause = TranslateOrGroup(start, end, Var(Ver));
@@ -723,18 +740,21 @@ void APT::Solver::RegisterCommonDependencies(pkgCache::PkgIterator Pkg)
 {
    for (auto dep = Pkg.VersionList().DependsList(); not dep.end();)
    {
-      pkgCache::DepIterator start;
-      pkgCache::DepIterator end;
+      pkgCache::DepIterator start, end;
       dep.GlobOr(start, end); // advances dep
 
       bool allHaveDep = true;
-      for (auto ver = Pkg.VersionList()++; not ver.end(); ver++)
+      for (auto ver = Pkg.VersionList()++; allHaveDep && not ver.end(); ver++)
       {
 	 bool haveDep = false;
-	 for (auto otherDep = ver.DependsList(); not haveDep && not otherDep.end(); otherDep++)
-	    haveDep = otherDep->DependencyData == start->DependencyData;
-	 if (!haveDep)
-	    allHaveDep = haveDep;
+	 for (auto otherDep = ver.DependsList(); not haveDep && not otherDep.end();)
+	 {
+	    pkgCache::DepIterator otherStart, otherEnd;
+	    otherDep.GlobOr(otherStart, otherEnd); // advances other dep
+	    haveDep = SameOrGroup(start, otherStart);
+	 }
+	 if (not haveDep)
+	    allHaveDep = false;
       }
       if (not allHaveDep)
 	 continue;
@@ -746,7 +766,6 @@ void APT::Solver::RegisterCommonDependencies(pkgCache::PkgIterator Pkg)
 APT::Solver::Clause APT::Solver::TranslateOrGroup(pkgCache::DepIterator start, pkgCache::DepIterator end, Var reason)
 {
    auto TgtPkg = start.TargetPkg();
-   auto Ver = start.ParentVer();
 
    // Non-important dependencies can only be installed if they are currently satisfied, see the check further
    // below once we have calculated all possible solutions.
@@ -756,7 +775,7 @@ APT::Solver::Clause APT::Solver::TranslateOrGroup(pkgCache::DepIterator start, p
    if (start->Type == pkgCache::Dep::Replaces || start->Type == pkgCache::Dep::Enhances)
       return Clause{reason, Group::Satisfy, true};
    if (unlikely(debug >= 3))
-      std::cerr << "Found dependency critical " << Ver.ParentPkg().FullName() << "=" << Ver.VerStr() << " -> " << start.TargetPkg().FullName() << "\n";
+      std::cerr << "Found dependency critical " << reason.toString(cache) << " -> " << start.TargetPkg().FullName() << "\n";
 
    Clause clause{reason, Group::Satisfy, not start.IsCritical() /* optional */, start.IsNegative()};
 
@@ -819,7 +838,7 @@ APT::Solver::Clause APT::Solver::TranslateOrGroup(pkgCache::DepIterator start, p
 	    newOptional = false, wasImportant = policy.IsImportantDep(D);
 
       bool satisfied = std::any_of(clause.solutions.begin(), clause.solutions.end(), [this](auto var)
-				   { return Var(var.CastPkg(cache).CurrentVer()) == var; });
+				   { return var.Pkg(cache) ? var.Pkg(cache)->CurrentVer != nullptr : Var(var.CastPkg(cache).CurrentVer()) == var; });
 
       if (important && wasImportant && not newOptional && not satisfied)
       {
diff --git a/test/integration/test-external-dependency-solver-protocol b/test/integration/test-external-dependency-solver-protocol
index bc32886c5..cbfd7b739 100755
--- a/test/integration/test-external-dependency-solver-protocol
+++ b/test/integration/test-external-dependency-solver-protocol
@@ -136,6 +136,43 @@ testsuccess test -s "$APT_EDSP_DUMP_FILENAME"
 testequal 'Install: awesomecoolstuff:i386' grep :i386 "$APT_EDSP_DUMP_FILENAME"
 testfailure grep -e ':amd64' -e 'Architecture: any' "$APT_EDSP_DUMP_FILENAME"
 
+rm -f "$APT_EDSP_DUMP_FILENAME"
+testfailure apt upgrade --solver dump
+testsuccess test -s "$APT_EDSP_DUMP_FILENAME"
+testsuccessequal "Request: EDSP 0.5
+Architecture: amd64
+Architectures: amd64 i386
+Machine-ID: 912e43bd1c1d4ba481f9f8ccab25f9ee
+Upgrade-All: yes
+Upgrade: yes
+Forbid-New-Install: no
+Forbid-Remove: yes
+Solver: dump" awk '/^$/ {exit} {print}' "$APT_EDSP_DUMP_FILENAME"
+
+rm -f "$APT_EDSP_DUMP_FILENAME"
+testfailure aptget upgrade --solver dump
+testsuccess test -s "$APT_EDSP_DUMP_FILENAME"
+testsuccessequal "Request: EDSP 0.5
+Architecture: amd64
+Architectures: amd64 i386
+Machine-ID: 912e43bd1c1d4ba481f9f8ccab25f9ee
+Upgrade-All: yes
+Upgrade: yes
+Forbid-New-Install: yes
+Forbid-Remove: yes
+Solver: dump" awk '/^$/ {exit} {print}' "$APT_EDSP_DUMP_FILENAME"
+
+rm -f "$APT_EDSP_DUMP_FILENAME"
+testfailure aptget dist-upgrade --solver dump
+testsuccess test -s "$APT_EDSP_DUMP_FILENAME"
+testsuccessequal "Request: EDSP 0.5
+Architecture: amd64
+Architectures: amd64 i386
+Machine-ID: 912e43bd1c1d4ba481f9f8ccab25f9ee
+Upgrade-All: yes
+Dist-Upgrade: yes
+Solver: dump" awk '/^$/ {exit} {print}' "$APT_EDSP_DUMP_FILENAME"
+
 testsuccess aptget dist-upgrade -s
 testsuccess aptget dist-upgrade -s --solver apt
 
diff --git a/test/integration/test-solver3-similar-depends b/test/integration/test-solver3-similar-depends
new file mode 100755
index 000000000..a301b9ac5
--- /dev/null
+++ b/test/integration/test-solver3-similar-depends
@@ -0,0 +1,18 @@
+#!/bin/sh
+set -e
+
+TESTDIR="$(readlink -f "$(dirname "$0")")"
+. "$TESTDIR/framework"
+setupenvironment
+configarchitecture 'amd64'
+
+insertpackage 'installed' 'a' 'all' '2' 'Depends: a|b'
+insertpackage 'unstable' 'a' 'all' '3' 'Depends: a|c'
+
+setupaptarchive
+testsuccess apt install -o debug::apt::solver=3 a -s --solver 3.0
+cp rootdir/tmp/testsuccess.output log
+msgmsg "Test that æ|b and a|c are different or groups"
+testsuccess grep "Found dependency critical a:amd64=2 -> a:amd64" log
+testsuccess grep "Found dependency critical a:amd64=3 -> a:amd64" log
+