From 83bad834c63604035c251f24ee9ea99dac1a1369 Mon Sep 17 00:00:00 2001 From: Julian Andres Klode Date: Thu, 20 Mar 2025 20:47:28 +0100 Subject: solver3: Handle previously satisfied Suggests last In #2103556 we can see that avahi-daemon Suggests avahi-autoipd, which is previously satisfied by dhcpcd-base due to a Provides; that Provides has now been dropped; and solver3 keeps the update back. As in LP: #2102720 this is a Suggests and it seems wrong for the Suggests to actually influence dependency choices and be able to hold back updates. Instead, satisfy previously satisfied Suggests at the end. For the particular case in this bug report, this results in avahi-autoipd, the real package, to be installed to keep the Suggests working which needs further consideration whether that is the right choice (we can argue either way), but in any case this is a better solution. This in turn breaks the conkeror test which is a somewhat awkward unrealistic test these days as it has no automatically installed packages; because previously something must have had a Suggests on say libdatrie1 or something that was processed first. This would be fine before 5daf6dbfd272be2f8e3c59d4bab4be8c119b7aa1 but as of that commit we no longer rewrite conkeror Depends xulrunner-1.9 | xulrunner-1.9.1 into conkeror Depends xulrunner-1.9.1 | xulrunner-1.9 Because xulrunner-1.9 is manually installed. Mark xulrunner-1.9 as automatically installed to fix the test case. The resulting behavior seems correct: If you manually want xulrunner-1.9 specifically we should not replace it with xulrunner-1.9.1; but if it's automatically installed we should. LP: #2103556 --- apt-pkg/solver3.cc | 1 + apt-pkg/solver3.h | 5 ++- test/integration/test-bug-591882-conkeror | 5 +++ test/integration/test-solver3-suggests-last | 52 +++++++++++++++++++++++++++++ 4 files changed, 62 insertions(+), 1 deletion(-) create mode 100755 test/integration/test-solver3-suggests-last diff --git a/apt-pkg/solver3.cc b/apt-pkg/solver3.cc index 4d988b523..66e307133 100644 --- a/apt-pkg/solver3.cc +++ b/apt-pkg/solver3.cc @@ -861,6 +861,7 @@ APT::Solver::Clause APT::Solver::TranslateOrGroup(pkgCache::DepIterator start, p { if (unlikely(debug >= 3)) std::cerr << "Try to keep satisfied: " << clause.toString(cache, true) << std::endl; + clause.group = Group::SatisfySuggests; } else if (not important) { diff --git a/apt-pkg/solver3.h b/apt-pkg/solver3.h index c2d6e035e..6810df426 100644 --- a/apt-pkg/solver3.h +++ b/apt-pkg/solver3.h @@ -122,7 +122,10 @@ class Solver // allowing a simple implementation of an autoremoval code. UpgradeAuto, KeepAuto, - ObsoleteAuto + ObsoleteAuto, + + // Satisfy optional dependencies that were previously satisfied but won't otherwise be installed + SatisfySuggests, }; // \brief Type to record depth at. This may very well be a 16-bit diff --git a/test/integration/test-bug-591882-conkeror b/test/integration/test-bug-591882-conkeror index 21c569756..6d4712a25 100755 --- a/test/integration/test-bug-591882-conkeror +++ b/test/integration/test-bug-591882-conkeror @@ -10,6 +10,7 @@ setupaptarchive UPGRADEFAIL="Reading package lists... Building dependency tree... +Reading state information... Calculating upgrade... The following packages will be REMOVED: conkeror libdatrie0 libkrb53 libxcb-xlib0 xulrunner-1.9 @@ -42,6 +43,7 @@ E: Trivial Only specified but this is not a trivial operation." UPGRADESUCCESS="Reading package lists... Building dependency tree... +Reading state information... Calculating upgrade... The following packages will be REMOVED: libdatrie0 libkrb53 libxcb-xlib0 xulrunner-1.9 @@ -73,6 +75,9 @@ Need to get 0 B/62.9 MB of archives. After this operation, 36.0 MB of additional disk space will be used. E: Trivial Only specified but this is not a trivial operation." +# solver3 otherwise chooses to keep back xulrunner-1.9 rather than replace it with xulrunner-1.9.1,, +# and keep back reverse dependencies of xulrunner-1.9 which seems correct. +testsuccess aptmark auto xulrunner-1.9 # Test that the old behavior can be restored with the option testfailureequal "$UPGRADEFAIL" aptget dist-upgrade --trivial-only -o pkgProblemResolver::FixByInstall=0 --solver internal testfailureequal "$UPGRADESUCCESS" aptget dist-upgrade --trivial-only #-o pkgProblemResolver::FixByInstall=0 diff --git a/test/integration/test-solver3-suggests-last b/test/integration/test-solver3-suggests-last new file mode 100755 index 000000000..1533007b7 --- /dev/null +++ b/test/integration/test-solver3-suggests-last @@ -0,0 +1,52 @@ +#!/bin/sh +set -e + +TESTDIR="$(readlink -f "$(dirname "$0")")" +. "$TESTDIR/framework" +setupenvironment +configarchitecture 'amd64' + +insertpackage 'installed' 'avahi-daemon' 'all' '1' 'Suggests: avahi-autoipd' +insertpackage 'installed' 'dhcpd-base' 'all' '1' 'Provides: avahi-autoipd' +insertpackage 'unstable' 'avahi-autoipd' 'all' 2 +insertpackage 'unstable' 'dhcpd-base' 'all' '2' + +setupaptarchive + +# It seems surprising this does not install avahi-autoipd +testsuccessequal "Reading package lists... +Building dependency tree... +Calculating upgrade... +The following packages will be upgraded: + dhcpd-base +1 upgraded, 0 newly installed, 0 to remove and 0 not upgraded. +Inst dhcpd-base [1] (2 unstable [all]) +Conf dhcpd-base (2 unstable [all])" aptget upgrade -s -q --solver 3.0 + +# Or is it surprising that this does? +for command in "aptget dist-upgrade" "apt upgrade" "apt dist-upgrade"; do +testsuccessequal "Reading package lists... +Building dependency tree... +Calculating upgrade... +The following NEW packages will be installed: + avahi-autoipd +The following packages will be upgraded: + dhcpd-base +1 upgraded, 1 newly installed, 0 to remove and 0 not upgraded. +Inst avahi-autoipd (2 unstable [all]) +Inst dhcpd-base [1] (2 unstable [all]) +Conf avahi-autoipd (2 unstable [all]) +Conf dhcpd-base (2 unstable [all])" $command -s -q --solver 3.0 +done + +for command in "aptget upgrade" "aptget dist-upgrade" "apt upgrade" "apt dist-upgrade"; do +testsuccessequal "Reading package lists... +Building dependency tree... +Calculating upgrade... +The following packages will be upgraded: + dhcpd-base +1 upgraded, 0 newly installed, 0 to remove and 0 not upgraded. +Inst dhcpd-base [1] (2 unstable [all]) +Conf dhcpd-base (2 unstable [all])" $command -s --solver internal -q +done + -- cgit v1.2.3-70-g09d2 From 81804cefd8c24c98e7a0216a9d4277516e19b6e2 Mon Sep 17 00:00:00 2001 From: Julian Andres Klode Date: Thu, 20 Mar 2025 21:15:14 +0100 Subject: solver3: Do not install new packages for Suggests As a followup to the previous commit, restore the exact behavior of the old solver to break the Suggests on upgrade here by erasing any choices not previously installed. This ensures that the Suggests never installs any new package, but it will still retain previously installed packages when solver3 is doing autoremove. --- apt-pkg/solver3.cc | 5 +++++ test/integration/test-solver3-suggests-last | 28 +--------------------------- 2 files changed, 6 insertions(+), 27 deletions(-) diff --git a/apt-pkg/solver3.cc b/apt-pkg/solver3.cc index 66e307133..620156ac3 100644 --- a/apt-pkg/solver3.cc +++ b/apt-pkg/solver3.cc @@ -862,6 +862,11 @@ APT::Solver::Clause APT::Solver::TranslateOrGroup(pkgCache::DepIterator start, p if (unlikely(debug >= 3)) std::cerr << "Try to keep satisfied: " << clause.toString(cache, true) << std::endl; clause.group = Group::SatisfySuggests; + // Erase the non-installed solutions. We will process this last and try to keep the previously installed + // "best" solution installed. + clause.solutions.erase(std::remove_if(clause.solutions.begin(), clause.solutions.end(), [this](auto var) + { return var.CastPkg(cache)->CurrentVer == nullptr; }), + clause.solutions.end()); } else if (not important) { diff --git a/test/integration/test-solver3-suggests-last b/test/integration/test-solver3-suggests-last index 1533007b7..9a280eed8 100755 --- a/test/integration/test-solver3-suggests-last +++ b/test/integration/test-solver3-suggests-last @@ -13,32 +13,6 @@ insertpackage 'unstable' 'dhcpd-base' 'all' '2' setupaptarchive -# It seems surprising this does not install avahi-autoipd -testsuccessequal "Reading package lists... -Building dependency tree... -Calculating upgrade... -The following packages will be upgraded: - dhcpd-base -1 upgraded, 0 newly installed, 0 to remove and 0 not upgraded. -Inst dhcpd-base [1] (2 unstable [all]) -Conf dhcpd-base (2 unstable [all])" aptget upgrade -s -q --solver 3.0 - -# Or is it surprising that this does? -for command in "aptget dist-upgrade" "apt upgrade" "apt dist-upgrade"; do -testsuccessequal "Reading package lists... -Building dependency tree... -Calculating upgrade... -The following NEW packages will be installed: - avahi-autoipd -The following packages will be upgraded: - dhcpd-base -1 upgraded, 1 newly installed, 0 to remove and 0 not upgraded. -Inst avahi-autoipd (2 unstable [all]) -Inst dhcpd-base [1] (2 unstable [all]) -Conf avahi-autoipd (2 unstable [all]) -Conf dhcpd-base (2 unstable [all])" $command -s -q --solver 3.0 -done - for command in "aptget upgrade" "aptget dist-upgrade" "apt upgrade" "apt dist-upgrade"; do testsuccessequal "Reading package lists... Building dependency tree... @@ -47,6 +21,6 @@ The following packages will be upgraded: dhcpd-base 1 upgraded, 0 newly installed, 0 to remove and 0 not upgraded. Inst dhcpd-base [1] (2 unstable [all]) -Conf dhcpd-base (2 unstable [all])" $command -s --solver internal -q +Conf dhcpd-base (2 unstable [all])" $command -s -q done -- cgit v1.2.3-70-g09d2