From 89dd48bdea93849246fc33b447d6d7ad52bb1c4b Mon Sep 17 00:00:00 2001 From: Julian Andres Klode Date: Mon, 3 Jul 2023 11:13:00 +0200 Subject: dist-upgrade: Revert phased updates using keeps only In the bug, mutter was kept back due to phasing and the new gnome-shell depended on that, and was therefore kept back as well, however, gnome-shell-common was not broken, and apt decided to continue upgrading it by removing gnome-shell and the ubuntu desktop meta packages. This is potentially a regression of LP#1990586 where we added keep back calls to the start of the dist-upgrade to ensure that we do not mark stuff for upgrade in the first place that depends on phasing updates, however it was generally allowed by the resolver to also do those removals. To fix this, we need to resolve the update normally and then use ResolveByKeepInternal to keep back any changes broken by held back packages. However, doing so breaks test-bug-591882-conkeror because ResolveByKeep keeps back packages for broken Recommends as well, which is not something we generally want to do in a dist-upgrade after we already decided to upgrade it. To circumvent that issue, extend the pkgProblemResolver to allow a package to be policy broken, and mark all packages that already were already going to be policy broken to be allowed to be that, such that we don't try to undo their installs. LP: #2025462 --- .../test-ubuntu-bug-2025462-phased-dist-upgrade | 50 ++++++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100755 test/integration/test-ubuntu-bug-2025462-phased-dist-upgrade (limited to 'test') diff --git a/test/integration/test-ubuntu-bug-2025462-phased-dist-upgrade b/test/integration/test-ubuntu-bug-2025462-phased-dist-upgrade new file mode 100755 index 000000000..eecadcda3 --- /dev/null +++ b/test/integration/test-ubuntu-bug-2025462-phased-dist-upgrade @@ -0,0 +1,50 @@ +#!/bin/sh +set -e + +TESTDIR="$(readlink -f "$(dirname "$0")")" +. "$TESTDIR/framework" + +setupenvironment +echo 'Debug::Phasing "1";' > rootdir/etc/apt/apt.conf.d/debug-phasing +configarchitecture 'i386' + +insertinstalledpackage 'gnome-shell' 'all' '1' 'Depends: mutter (>= 1), gnome-shell-common (= 1)' +insertinstalledpackage 'gnome-shell-common' 'all' '1' +insertinstalledpackage 'mutter' 'all' '1' + +insertpackage 'unstable-updates' 'gnome-shell-common' 'all' '2' 'Phased-Update-Percentage: 100' +insertpackage 'unstable-updates' 'gnome-shell' 'all' '2' 'Phased-Update-Percentage: 100 +Depends: mutter (>= 2), gnome-shell-common (= 2)' +insertpackage 'unstable-updates' 'mutter' 'all' '2' 'Phased-Update-Percentage: 0' + +setupaptarchive + +# This is the broken test case: +# +# The following packages will be REMOVED: +# gnome-shell +# The following packages have been kept back: +# mutter +# The following packages will be upgraded: +# gnome-shell-common +# 1 upgraded, 0 newly installed, 1 to remove and 1 not upgraded. + +testsuccessequal "Reading package lists... +Building dependency tree... +Calculating upgrade... +The following packages have been kept back: + gnome-shell gnome-shell-common mutter +0 upgraded, 0 newly installed, 0 to remove and 3 not upgraded." aptget dist-upgrade -s -q + +testsuccessequal "Reading package lists... +Building dependency tree... +Calculating upgrade... +The following packages will be upgraded: + gnome-shell gnome-shell-common mutter +3 upgraded, 0 newly installed, 0 to remove and 0 not upgraded. +Inst mutter [1] (2 unstable-updates [all]) +Inst gnome-shell [1] (2 unstable-updates [all]) [] +Inst gnome-shell-common [1] (2 unstable-updates [all]) +Conf mutter (2 unstable-updates [all]) +Conf gnome-shell (2 unstable-updates [all]) +Conf gnome-shell-common (2 unstable-updates [all])" aptget dist-upgrade -s -q -o APT::Get::Always-Include-Phased-Updates=1 -- cgit v1.2.3-70-g09d2 From 68ef41ea912f4879b0ee43419c13a3a8c9bfcd22 Mon Sep 17 00:00:00 2001 From: Julian Andres Klode Date: Fri, 7 Jul 2023 14:24:52 +0200 Subject: Do not mark updates for install that are still phasing This fixes an issue where phased updates gain new dependencies and cause them to be installed despite themselves not being installed. In the cause of investigation, it turned out that we also need to evaluate the candidate version at those early stage rather than the install version (which is only valid *after* MarkInstall). This does not fully resolve the problem: If an update pulls in a phased update, depends are still being installed. Resolving this while ensuring that phased updates cannot uninstall packages requires us to do a minimization of changes by trying to keep back each new install removal and then seeing if any dependency is being broken by it. This is more complex and will happen later. --- apt-pkg/upgrade.cc | 27 ++++++- test/integration/test-phased-updates-new-depends | 96 ++++++++++++++++++++++++ 2 files changed, 119 insertions(+), 4 deletions(-) create mode 100755 test/integration/test-phased-updates-new-depends (limited to 'test') diff --git a/apt-pkg/upgrade.cc b/apt-pkg/upgrade.cc index 3e1bb292b..994a05859 100644 --- a/apt-pkg/upgrade.cc +++ b/apt-pkg/upgrade.cc @@ -85,13 +85,13 @@ struct PhasedUpgrader { if (Pkg->CurrentVer == 0) return false; - if (Cache[Pkg].InstallVer == 0) + if (Cache[Pkg].CandidateVer == 0) return false; - if (Cache[Pkg].InstVerIter(Cache).PhasedUpdatePercentage() == 100) + if (Cache[Pkg].CandidateVerIter(Cache).PhasedUpdatePercentage() == 100) return false; - if (IsSecurityUpdate(Cache[Pkg].InstVerIter(Cache))) + if (IsSecurityUpdate(Cache[Pkg].CandidateVerIter(Cache))) return false; - if (!IsIgnoredPhasedUpdate(Cache[Pkg].InstVerIter(Cache))) + if (!IsIgnoredPhasedUpdate(Cache[Pkg].CandidateVerIter(Cache))) return false; return true; @@ -133,13 +133,18 @@ static bool pkgDistUpgrade(pkgDepCache &Cache, OpProgress * const Progress) Progress->OverallProgress(0, 100, 1, _("Calculating upgrade")); pkgDepCache::ActionGroup group(Cache); + PhasedUpgrader phasedUpgrader; /* Upgrade all installed packages first without autoinst to help the resolver in versioned or-groups to upgrade the old solver instead of installing a new one (if the old solver is not the first one [anymore]) */ for (pkgCache::PkgIterator I = Cache.PkgBegin(); I.end() == false; ++I) + { + if (phasedUpgrader.ShouldKeep(Cache, I)) + continue; if (I->CurrentVer != 0) Cache.MarkInstall(I, false, 0, false); + } if (Progress != NULL) Progress->Progress(10); @@ -147,8 +152,12 @@ static bool pkgDistUpgrade(pkgDepCache &Cache, OpProgress * const Progress) /* Auto upgrade all installed packages, this provides the basis for the installation */ for (pkgCache::PkgIterator I = Cache.PkgBegin(); I.end() == false; ++I) + { + if (phasedUpgrader.ShouldKeep(Cache, I)) + continue; if (I->CurrentVer != 0) Cache.MarkInstall(I, true, 0, false); + } if (Progress != NULL) Progress->Progress(50); @@ -176,13 +185,19 @@ static bool pkgDistUpgrade(pkgDepCache &Cache, OpProgress * const Progress) if (isEssential == false || instEssential == true) continue; pkgCache::PkgIterator P = G.FindPreferredPkg(); + if (phasedUpgrader.ShouldKeep(Cache, P)) + continue; Cache.MarkInstall(P, true, 0, false); } } else if (essential != "none") for (pkgCache::PkgIterator I = Cache.PkgBegin(); I.end() == false; ++I) + { + if (phasedUpgrader.ShouldKeep(Cache, I)) + continue; if ((I->Flags & pkgCache::Flag::Essential) == pkgCache::Flag::Essential) Cache.MarkInstall(I, true, 0, false); + } if (Progress != NULL) Progress->Progress(55); @@ -190,8 +205,12 @@ static bool pkgDistUpgrade(pkgDepCache &Cache, OpProgress * const Progress) /* We do it again over all previously installed packages to force conflict resolution on them all. */ for (pkgCache::PkgIterator I = Cache.PkgBegin(); I.end() == false; ++I) + { + if (phasedUpgrader.ShouldKeep(Cache, I)) + continue; if (I->CurrentVer != 0) Cache.MarkInstall(I, false, 0, false); + } if (Progress != NULL) Progress->Progress(65); diff --git a/test/integration/test-phased-updates-new-depends b/test/integration/test-phased-updates-new-depends new file mode 100755 index 000000000..8cc9314ad --- /dev/null +++ b/test/integration/test-phased-updates-new-depends @@ -0,0 +1,96 @@ +#!/bin/sh +set -e + +TESTDIR="$(readlink -f "$(dirname "$0")")" +. "$TESTDIR/framework" + +setupenvironment +echo 'Debug::Phasing "1";' > rootdir/etc/apt/apt.conf.d/debug-phasing +configarchitecture 'i386' + +insertinstalledpackage 'has-new-depends' 'all' '1' '' +insertinstalledpackage 'has-new-recommends' 'all' '1' '' +insertinstalledpackage 'has-new-conflicts' 'all' '1' '' +insertinstalledpackage 'new-conflicts' 'all' '1' '' + +insertpackage 'unstable-updates' 'new-depends' 'all' '2' +insertpackage 'unstable-updates' 'new-recommends' 'all' '2' +insertpackage 'unstable-updates' 'has-new-depends' 'all' '2' 'Phased-Update-Percentage: 0 +Depends: new-depends' +insertpackage 'unstable-updates' 'has-new-recommends' 'all' '2' 'Phased-Update-Percentage: 0 +Recommends: new-recommends' +insertpackage 'unstable-updates' 'has-new-conflicts' 'all' '2' 'Phased-Update-Percentage: 0 +Conflicts: new-conflicts' + +setupaptarchive + + +testsuccessequal "Reading package lists... +Building dependency tree... +Calculating upgrade... +The following packages have been kept back: + has-new-conflicts has-new-depends has-new-recommends +0 upgraded, 0 newly installed, 0 to remove and 3 not upgraded." aptget upgrade -s -q -o APT::Get::Always-Include-Phased-Updates=1 + +testsuccessequal "Reading package lists... +Building dependency tree... +Calculating upgrade... +The following packages have been kept back: + has-new-conflicts has-new-depends has-new-recommends +0 upgraded, 0 newly installed, 0 to remove and 3 not upgraded." aptget upgrade -s -q + + +testsuccessequal "Reading package lists... +Building dependency tree... +Calculating upgrade... +The following NEW packages will be installed: + new-depends new-recommends +The following packages have been kept back: + has-new-conflicts +The following packages will be upgraded: + has-new-depends has-new-recommends +2 upgraded, 2 newly installed, 0 to remove and 1 not upgraded. +Inst new-depends (2 unstable-updates [all]) +Inst has-new-depends [1] (2 unstable-updates [all]) +Inst has-new-recommends [1] (2 unstable-updates [all]) +Inst new-recommends (2 unstable-updates [all]) +Conf new-depends (2 unstable-updates [all]) +Conf has-new-depends (2 unstable-updates [all]) +Conf has-new-recommends (2 unstable-updates [all]) +Conf new-recommends (2 unstable-updates [all])" aptget upgrade -s -q --with-new-pkgs -o APT::Get::Always-Include-Phased-Updates=1 + +testsuccessequal "Reading package lists... +Building dependency tree... +Calculating upgrade... +The following packages have been kept back: + has-new-conflicts has-new-depends has-new-recommends +0 upgraded, 0 newly installed, 0 to remove and 3 not upgraded." aptget upgrade -s -q --with-new-pkgs + +: testsuccessequal "Reading package lists... +Building dependency tree... +Calculating upgrade... +The following packages will be REMOVED: + new-conflicts +The following NEW packages will be installed: + new-depends new-recommends +The following packages will be upgraded: + has-new-conflicts has-new-depends has-new-recommends +3 upgraded, 2 newly installed, 1 to remove and 0 not upgraded. +Remv new-conflicts [1] +Inst has-new-conflicts [1] (2 unstable-updates [all]) +Inst new-depends (2 unstable-updates [all]) +Inst has-new-depends [1] (2 unstable-updates [all]) +Inst has-new-recommends [1] (2 unstable-updates [all]) +Inst new-recommends (2 unstable-updates [all]) +Conf has-new-conflicts (2 unstable-updates [all]) +Conf new-depends (2 unstable-updates [all]) +Conf has-new-depends (2 unstable-updates [all]) +Conf has-new-recommends (2 unstable-updates [all]) +Conf new-recommends (2 unstable-updates [all])" aptget dist-upgrade -s -q -o APT::Get::Always-Include-Phased-Updates=1 + +testsuccessequal "Reading package lists... +Building dependency tree... +Calculating upgrade... +The following packages have been kept back: + has-new-conflicts has-new-depends has-new-recommends +0 upgraded, 0 newly installed, 0 to remove and 3 not upgraded." aptget dist-upgrade -s -q -- cgit v1.2.3-70-g09d2