From a4ce187b40f3bbd110a7cc35f9a5af5a8b7b0c9e Mon Sep 17 00:00:00 2001 From: Julian Andres Klode Date: Wed, 12 Mar 2025 18:34:48 +0100 Subject: solver3: Fix Recommends/Suggests vs Enhances confusion We accidentally considered an Enhances a reason to keep a package installed, which of course it is not, fix the determination of "existing soft dependency" to only include soft dependencies that should keep a package installed in the autoremover to solve the issue. This also fixes edsp/mantic-upgrade-rel-to-2024-05-29.edsp to not install llvm-13-dev for clang-13 which is an installed package with no upgrade. Also ensure we stop after the first match; the DependsList() is ordered by decreasing priority and we don't want to override Recommends by Suggests in case a package declares both... LP: #2101800 --- apt-pkg/solver3.cc | 10 +++++++++- apt-pkg/solver3.h | 5 +++++ 2 files changed, 14 insertions(+), 1 deletion(-) (limited to 'apt-pkg') diff --git a/apt-pkg/solver3.cc b/apt-pkg/solver3.cc index 739309c59..f992d55b4 100644 --- a/apt-pkg/solver3.cc +++ b/apt-pkg/solver3.cc @@ -833,9 +833,17 @@ APT::Solver::Clause APT::Solver::TranslateOrGroup(pkgCache::DepIterator start, p bool important = policy.IsImportantDep(start); bool newOptional = true; bool wasImportant = false; + auto importantToKeep = [this](pkgCache::DepIterator d) + { + return policy.IsImportantDep(d) || (KeepRecommends && d->Type == pkgCache::Dep::Recommends) || (KeepSuggests && d->Type == pkgCache::Dep::Suggests); + }; + for (auto D = start.ParentPkg().CurrentVer().DependsList(); not D.end(); D++) - if (not D.IsCritical() && not D.IsNegative() && D.TargetPkg() == start.TargetPkg()) + if (not D.IsCritical() && importantToKeep(D) && D.TargetPkg() == start.TargetPkg()) + { newOptional = false, wasImportant = policy.IsImportantDep(D); + break; + } bool satisfied = std::any_of(clause.solutions.begin(), clause.solutions.end(), [this](auto var) { return var.Pkg(cache) ? var.Pkg(cache)->CurrentVer != nullptr : Var(var.CastPkg(cache).CurrentVer()) == var; }); diff --git a/apt-pkg/solver3.h b/apt-pkg/solver3.h index b7000c52e..1bdc13559 100644 --- a/apt-pkg/solver3.h +++ b/apt-pkg/solver3.h @@ -243,6 +243,11 @@ class Solver // \brief If set, we use strict pinning. int Timeout{_config->FindI("APT::Solver::Timeout", 10)}; + // \brief Keep recommends installed + bool KeepRecommends{_config->FindB("APT::AutoRemove::RecommendsImportant", true)}; + // \brief Keep suggests installed + bool KeepSuggests{_config->FindB("APT::AutoRemove::SuggestsImportant", true)}; + // \brief Discover a variable, translating the underlying dependencies to the SAT presentation // // This does a breadth-first search of the entire dependency tree of var, -- cgit v1.2.3-70-g09d2 From 460753f991ca42c3c4a5ed494038db76c55e2a08 Mon Sep 17 00:00:00 2001 From: Julian Andres Klode Date: Wed, 12 Mar 2025 18:47:55 +0100 Subject: solver3: Fix test-resolve-by-keep-new-recommend Reorder the if branches a bit to simplify the understanding of the promotions, and allow us to keep track of which dependency existed in the installed version. Change the rule for promoting new recommends to only promote new recommends to depends in *upgrade* and not *dist-upgrade* per the test-resolve-by-keep-new-recommends test case. This makes some sense: In a dist-upgrade, the solver could otherwise decide to remove an installed package if both the following hold: (1) something needs the upgraded version | something else (2) the upgraded version is not installable due to unsat recommends --- apt-pkg/solver3.cc | 42 +++++++++++----------- test/integration/solver3.broken | 1 - .../test-resolve-by-keep-new-recommends | 18 ++++++++++ 3 files changed, 40 insertions(+), 21 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/solver3.cc b/apt-pkg/solver3.cc index f992d55b4..cebface27 100644 --- a/apt-pkg/solver3.cc +++ b/apt-pkg/solver3.cc @@ -831,52 +831,54 @@ APT::Solver::Clause APT::Solver::TranslateOrGroup(pkgCache::DepIterator start, p if (clause.optional && start.ParentPkg()->CurrentVer) { bool important = policy.IsImportantDep(start); - bool newOptional = true; - bool wasImportant = false; auto importantToKeep = [this](pkgCache::DepIterator d) { return policy.IsImportantDep(d) || (KeepRecommends && d->Type == pkgCache::Dep::Recommends) || (KeepSuggests && d->Type == pkgCache::Dep::Suggests); }; + bool satisfied = std::any_of(clause.solutions.begin(), clause.solutions.end(), [this](auto var) + { return var.Pkg(cache) ? var.Pkg(cache)->CurrentVer != nullptr : Var(var.CastPkg(cache).CurrentVer()) == var; }); + // Find the existing dependency + pkgCache::DepIterator existing; for (auto D = start.ParentPkg().CurrentVer().DependsList(); not D.end(); D++) if (not D.IsCritical() && importantToKeep(D) && D.TargetPkg() == start.TargetPkg()) { - newOptional = false, wasImportant = policy.IsImportantDep(D); + existing = D; break; } - bool satisfied = std::any_of(clause.solutions.begin(), clause.solutions.end(), [this](auto var) - { return var.Pkg(cache) ? var.Pkg(cache)->CurrentVer != nullptr : Var(var.CastPkg(cache).CurrentVer()) == var; }); - - if (important && wasImportant && not newOptional && not satisfied) + if (not existing.end() && not important && importantToKeep(start) && satisfied) { if (unlikely(debug >= 3)) - std::cerr << "Ignoring unsatisfied Recommends " << clause.toString(cache) << std::endl; - clause.solutions.clear(); + std::cerr << "Try to keep satisfied: " << clause.toString(cache, true) << std::endl; } - else if (not important && not wasImportant && not newOptional && satisfied) + else if (not important) { if (unlikely(debug >= 3)) - std::cerr << "Promoting satisfied Suggests to Recommends: " << clause.toString(cache) << std::endl; - important = true; + std::cerr << "Ignore unimportant clause: " << clause.toString(cache, true) << std::endl; + return Clause{reason, Group::Satisfy, true}; } - else if (satisfied && important && wasImportant && clause.solutions.size() > 0) + else if (not existing.end() && policy.IsImportantDep(existing) && not satisfied) { if (unlikely(debug >= 3)) - std::cerr << "Promoting existing Recommends " << clause.toString(cache) << " to depends in upgrade" << std::endl; - clause.optional = false; + std::cerr << "Ignoring unsatisfied clause: " << clause.toString(cache, true) << std::endl; + return Clause{reason, Group::Satisfy, true}; } - else if (newOptional && important && reason.Ver() && clause.solutions.size() > 0 && reason.Ver(cache) != reason.CastPkg(cache).CurrentVer() && IsUpgrade) + else if (IsUpgrade && not existing.end() && satisfied) { if (unlikely(debug >= 3)) - std::cerr << "Promoting new Recommends " << clause.toString(cache) << " to depends in upgrade" << std::endl; + std::cerr << "Promoting previously satisfied clause to hard dependency: " << clause.toString(cache, true) << std::endl; clause.optional = false; } - else if (not important) + else if ( + IsUpgrade && not(AllowRemove && AllowInstall) // promote Recommends to Depends in upgrade, not in dist-upgrade. + && reason.Ver() && reason.Ver(cache) != reason.CastPkg(cache).CurrentVer() // and if this an upgrade to an installed package + && (existing.end() || not policy.IsImportantDep(existing)) // new Recommends, or upgraded from Suggests + ) { if (unlikely(debug >= 3)) - std::cerr << "Ignoring Suggests " << clause.toString(cache) << std::endl; - return Clause{reason, Group::Satisfy, true}; + std::cerr << "Promoting new clause to hard dependency: " << clause.toString(cache) << std::endl; + clause.optional = false; } } diff --git a/test/integration/solver3.broken b/test/integration/solver3.broken index 4a4e1532b..0e2005d60 100644 --- a/test/integration/solver3.broken +++ b/test/integration/solver3.broken @@ -3,6 +3,5 @@ test-apt-get-upgrade-by-source # TBD: Upgrading by test-apt-install-order-matters-a-bit # Wontfix: Cannot fix, the order is not recorded in the depcache test-bug-470115-new-and-tighten-recommends # TBD: Calculation of what is already satisfied Recommends is broken test-prevent-markinstall-multiarch-same-versionscrew # TBD: We consider the skewed ones obsolete and remove them... -test-resolve-by-keep-new-recommends # TBD: Fixing this seems to break test-bug-591882-conkeror, why? test-resolve-by-keep-obsolete-removals # TBD: ResolveByKeep() usage is badly aligned here test-ubuntu-bug-1304403-obsolete-priority-standard # TBD: Solver3 here happily removes 10 deps to upgrade a package diff --git a/test/integration/test-resolve-by-keep-new-recommends b/test/integration/test-resolve-by-keep-new-recommends index 3591ed84d..16acbe0ce 100755 --- a/test/integration/test-resolve-by-keep-new-recommends +++ b/test/integration/test-resolve-by-keep-new-recommends @@ -19,3 +19,21 @@ The following packages have been kept back: 0 upgraded, 0 newly installed, 0 to remove and 1 not upgraded." testsuccessequal "$UPGRADE_KEEP" aptget upgrade -s +UPGRADE_KEEP="Reading package lists... +Building dependency tree... +Calculating upgrade... +The following packages have been kept back: + foo +0 upgraded, 0 newly installed, 0 to remove and 1 not upgraded." +testsuccessequal "$UPGRADE_KEEP" apt upgrade -s + +UPGRADE_KEEP="Reading package lists... +Building dependency tree... +Calculating upgrade... +The following packages will be upgraded: + foo +1 upgraded, 0 newly installed, 0 to remove and 0 not upgraded. +Inst foo [1.0] (2.0 unstable [i386]) +Conf foo (2.0 unstable [i386])" +testsuccessequal "$UPGRADE_KEEP" aptget dist-upgrade -s + -- cgit v1.2.3-70-g09d2