Do not mark updates for install that are still phasing

julian-klode · julian-klode · commit 68ef41ea912f · 2023-07-07T14:28:59.000+02:00
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.
diff --git 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,22 +133,31 @@ 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);
 
    /* 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,22 +185,32 @@ 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);
 
    /* 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
@@ -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
