Merge ~slyon/ubuntu-release-upgrader:slyon/focal-python2-fix into ubuntu-release-upgrader:ubuntu/focal

Proposed by Lukas Märdian on 2020-07-08
Status: Merged
Approved by: Brian Murray on 2020-07-16
Approved revision: fec707f21a0420e5638a9c1ea6c67099da180b17
Merged at revision: 8768467e87a8ef12525fa59137602095a2230a4e
Proposed branch: ~slyon/ubuntu-release-upgrader:slyon/focal-python2-fix
Merge into: ubuntu-release-upgrader:ubuntu/focal
Diff against target: 91 lines (+32/-3)
2 files modified
DistUpgrade/DistUpgradeCache.py (+9/-0)
DistUpgrade/DistUpgradeQuirks.py (+23/-3)
Reviewer Review Type Date Requested Status
Brian Murray 2020-07-08 Approve on 2020-07-16
Dimitri John Ledkov lgtm Approve on 2020-07-13
Review via email: mp+387057@code.launchpad.net

Description of the change

Split the python_is_python2 quirk into a "prepare stage" (before calculating the dist-upgrade), where python-minimal is removed to avoid it producing any conflicts with packages depending on 'python' or 'python-minimal' and a "install stage" (after calculating the dist-upgrade), where the python-is-python2 package is installed to provide a clean upgrade path.

Fixes LP: #1875523

To post a comment you must log in.
Brian Murray (brian-murray) wrote :

While this looks fine, I've tested the diff on a bionic virtual machine on which I've applied the apt-clone file in bug 1875523 and I wasn't able to upgrade. Is there something else wrong with the packages in that clone file? Is there another way I can test this?

review: Needs Information
Lukas Märdian (slyon) wrote :

Hmmm... I used the apt-clone file and traced it down to this minimal reproducer:

host:~$ multipass launch -n repr bionic && multipass shell repr

guest:~$ sudo apt update && sudo apt -y upgrade && sudo apt -y install python-pyside python-gi && sudo reboot

host:~$ multipass shell repr

guest:~$ sudo do-release-upgrade -d

The patch works with this minimal reproducer.

But let my try again with the full apt-clone state...

Lukas Märdian (slyon) wrote :

I pushed a commit, which fixes the upgrade for the full apt-clone state of LP #1875523 as well.

Dimitri John Ledkov (xnox) :
review: Approve (lgtm)
Lukas Märdian (slyon) wrote :

The release notes for Focal have been updated accordingly:

https://wiki.ubuntu.com/FocalFossa/ReleaseNotes#Python3_by_default

Brian Murray (brian-murray) wrote :

This looks good to me, thanks for working on it. I think this should also be merged into the master branch for ubuntu-release-upgrader so that we have the "PreDistUpgradeCache" quirks available in the future.

review: Approve
Lukas Märdian (slyon) wrote :

Thanks for the review, Brian!

I applied the same patches to the master branch and prepared a MP for it here:

https://code.launchpad.net/~slyon/ubuntu-release-upgrader/+git/ubuntu-release-upgrader/+merge/387577

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/DistUpgrade/DistUpgradeCache.py b/DistUpgrade/DistUpgradeCache.py
2index 8d9ba57..b2dd086 100644
3--- a/DistUpgrade/DistUpgradeCache.py
4+++ b/DistUpgrade/DistUpgradeCache.py
5@@ -488,6 +488,12 @@ class MyCache(apt.Cache):
6 self._keep_installed(pkg.name, "%s KeepInstalledSection rule: %s" % (key, section))
7
8
9+ def pre_upgrade_rule(self):
10+ " run before the upgrade was done in the cache "
11+ # run the quirks handlers
12+ if not self.partialUpgrade:
13+ self.quirks.run("PreDistUpgradeCache")
14+
15 def post_upgrade_rule(self):
16 " run after the upgrade was done in the cache "
17 for (rule, action) in [("Install", self.mark_install),
18@@ -614,6 +620,9 @@ class MyCache(apt.Cache):
19 # mvo: disabled as it casues to many errornous installs
20 #self._apply_dselect_upgrade()
21
22+ # run PreDistUpgradeCache quirks
23+ self.pre_upgrade_rule()
24+
25 # upgrade (and make sure this way that the cache is ok)
26 self.upgrade(True)
27
28diff --git a/DistUpgrade/DistUpgradeQuirks.py b/DistUpgrade/DistUpgradeQuirks.py
29index 24bd563..4c4b545 100644
30--- a/DistUpgrade/DistUpgradeQuirks.py
31+++ b/DistUpgrade/DistUpgradeQuirks.py
32@@ -67,6 +67,8 @@ class DistUpgradeQuirks(object):
33 to set options that affect the cache
34 - PostInitialUpdate: run *before* the sources.list is rewritten but
35 after an initial apt-get update
36+ - PreDistUpgradeCache: run *right before* the dist-upgrade is
37+ calculated in the cache
38 - PostDistUpgradeCache: run *after* the dist-upgrade was calculated
39 in the cache
40 - StartUpgrade: before the first package gets installed (but the
41@@ -157,13 +159,19 @@ class DistUpgradeQuirks(object):
42 self._pokeScreensaver()
43 self._stopDocvertConverter()
44
45+ # individual quirks handler that run *right before* the dist-upgrade
46+ # is calculated in the cache
47+ def PreDistUpgradeCache(self):
48+ """ run right before calculating the dist-upgrade """
49+ logging.debug("running Quirks.PreDistUpgradeCache")
50+ self._install_python_is_python2()
51+
52 # individual quirks handler that run *after* the dist-upgrade was
53 # calculated in the cache
54 def PostDistUpgradeCache(self):
55 """ run after calculating the dist-upgrade """
56 logging.debug("running Quirks.PostDistUpgradeCache")
57 self._install_linux_metapackage()
58- self._install_python_is_python2()
59
60 # helpers
61 def _get_pci_ids(self):
62@@ -836,8 +844,9 @@ class DistUpgradeQuirks(object):
63
64 def _install_python_is_python2(self):
65 """
66- Ensure the python-is-python2 is installed if python-minimal
67- was installed.
68+ Ensure python is removed, before it can produce a conflict with any
69+ other package and the python-is-python2 package is installed instead,
70+ if python-minimal was installed.
71 """
72 old = 'python-minimal'
73 new = 'python-is-python2'
74@@ -847,6 +856,17 @@ class DistUpgradeQuirks(object):
75 reason = "%s was installed on the system" % old
76 if not cache.mark_install(new, reason):
77 logging.info("failed to install %s" % new)
78+ logging.info("removing %s because %s is being installed" % (old, new))
79+ reason = "%s is being installed on the system" % new
80+ if not cache.mark_remove(old, reason):
81+ logging.info("failed to remove %s", old)
82+
83+ # protect our decision to remove legacy 'python' (as a dependency
84+ # of python-minimal, removed above)
85+ py = 'python'
86+ if py in cache and cache[py].marked_delete:
87+ resolver = apt.cache.ProblemResolver(cache)
88+ resolver.protect(cache[py])
89
90 def ensure_recommends_are_installed_on_desktops(self):
91 """ ensure that on a desktop install recommends are installed

Subscribers

People subscribed via source and target branches