Merge lp:~brian-murray/ubuntu-release-upgrader/improve-upgrade-failure-messages into lp:ubuntu-release-upgrader

[Brian Murray]

[lp:~brian-murray/ubuntu-release-upgrader/improve-upgrade-failure-messages]

3201. By Brian Murray on 2018-12-11

* DistUpgrade/DistUpgradeCache.py: in the event there is a failure to calculate the upgrade provide information about the log files in /var/log/dist-upgrade. (LP: #1771387)

3202. By Brian Murray on 2018-12-11

ppa-purge only works with Launchpad PPAs

=== modified symlink 'DistUpgrade/DistUpgrade'
=== target changed u'.' => u'DistUpgrade'
=== modified file 'DistUpgrade/DistUpgradeCache.py'
--- DistUpgrade/DistUpgradeCache.py	2018-11-28 19:12:50 +0000
+++ DistUpgrade/DistUpgradeCache.py	2018-12-11 23:25:23 +0000
@@ -666,14 +666,23 @@
             # rationale why it doesn't
             lock.release()
             t.join()
-            # FIXME: change the text to something more useful
+            # the most likely problem is the 3rd party pkgs so don't address
+            # foreignPkgs and devRelease being True
             details =  _("An unresolvable problem occurred while "
-                         "calculating the upgrade.\n\n "
-                         "This can be caused by:\n"
-                         " * Upgrading to a pre-release version of Ubuntu\n"
-                         " * Running the current pre-release version of Ubuntu\n"
-                         " * Unofficial software packages not provided by Ubuntu\n"
-                         "\n")
+                         "calculating the upgrade.\n\n ")
+            if self.config.get("Options", "foreignPkgs") == "True":
+                details += _("This was likely caused by:\n"
+                             " * Unofficial software packages not provided by Ubuntu\n"
+                             "Please use the tool 'ppa-purge' from the ppa-purge \n"
+                             "package to remove software from a Launchpad PPA and \n"
+                             "try the upgrade again.\n"
+                             "\n")
+            elif self.config.get("Options", "foreignPkgs") == "False" and \
+                self.config.get("Options", "devRelease") == "True":
+                details +=  _("This was caused by:\n"
+                              " * Upgrading to a pre-release version of Ubuntu\n"
+                              "This is most likely a transient problem, \n"
+                              "please try again later.\n")
             # we never have partialUpgrades (including removes) on a stable system
             # with only ubuntu sources so we do not recommend reporting a bug
             if partialUpgrade:
@@ -682,6 +691,9 @@
             else:
                 details += _("If none of this applies, then please report this bug using "
                              "the command 'ubuntu-bug ubuntu-release-upgrader-core' in a terminal.")
+                details += _("If you want to investigate this yourself the log files in "
+                             "'/var/log/dist-upgrade' will contain details about the upgrade. "
+                             "Specifically, look at 'main.log' and 'apt.log'.")
             # make the error text available again on stdout for the
             # text frontend
             self._stopAptResolverLog()

=== modified file 'DistUpgrade/DistUpgradeController.py'
--- DistUpgrade/DistUpgradeController.py	2018-10-31 22:06:06 +0000
+++ DistUpgrade/DistUpgradeController.py	2018-12-11 23:25:23 +0000
@@ -131,6 +131,11 @@
         # ConfigParser deals only with strings it seems *sigh*
         self.config.add_section("Options")
         self.config.set("Options","withNetwork", str(self.useNetwork))
+        if self.options:
+            if self.options.devel_release:
+                self.config.set("Options","devRelease", "True")
+            else:
+                self.config.set("Options","devRelease", "False")
 
         # some constants here
         self.fromDist = self.config.get("Sources","From")
@@ -535,6 +540,7 @@
                         distro.get_sources(self.sources)
                         distro.enable_component("main")
                         main_was_missing = True
+                        logging.debug('get_distro().enable_component("main") succeeded')
                     except NoDistroTemplateException as e:
                         logging.exception('NoDistroTemplateException raised: %s' % e)
                         # fallback if everything else does not work,
@@ -680,8 +686,16 @@
                     foundToDist |= validTo
                 elif entry.dist in fromDists:
                     foundToDist |= validTo
-                    entry.dist = toDists[fromDists.index(entry.dist)]
-                    logging.debug("entry '%s' updated to new dist" % get_string_with_no_auth_from_source_entry(entry))
+                    # check to see whether the archive provides the new dist
+                    test_entry = copy.copy(entry)
+                    test_entry.dist = self.toDist
+                    if not self._sourcesListEntryDownloadable(test_entry):
+                        entry.disabled = True
+                        self.sources_disabled = True
+                        logging.debug("entry '%s' was disabled (no Release file)" % get_string_with_no_auth_from_source_entry(entry))
+                    else:
+                        entry.dist = toDists[fromDists.index(entry.dist)]
+                        logging.debug("entry '%s' updated to new dist" % get_string_with_no_auth_from_source_entry(entry))
                 elif entry.type == 'deb-src':
                     continue
                 elif validMirror:
@@ -933,6 +947,12 @@
         # compare the list after the update again
         self.obsolete_pkgs = self.cache._getObsoletesPkgs()
         self.foreign_pkgs = self.cache._getForeignPkgs(self.origin, self.fromDist, self.toDist)
+        # If a PPA has already been disabled the pkgs won't be considered
+        # foreign
+        if len(self.foreign_pkgs) > 0:
+            self.config.set("Options","foreignPkgs", "True")
+        else:
+            self.config.set("Options","foreignPkgs", "False")
         if self.serverMode:
             self.tasks = self.cache.installedTasks
         logging.debug("Foreign: %s" % " ".join(sorted(self.foreign_pkgs)))
@@ -1486,7 +1506,7 @@
                 self._view.getTerminal().call([script], hidden=True)
             except Exception as e:
                 logging.error("got error from PostInstallScript %s (%s)" % (script, e))
-        
+
     def abort(self):
         """ abort the upgrade, cleanup (as much as possible) """
         logging.debug("abort called")
@@ -1518,7 +1538,7 @@
                     return True
         logging.error("depends '%s' is not satisfied" % depstr)
         return False
-                
+
     def checkViewDepends(self):
         " check if depends are satisfied "
         logging.debug("checkViewDepends()")
@@ -1537,7 +1557,7 @@
                                  _("The required dependency '%s' is not "
                                    "installed. " % dep))
                 sys.exit(1)
-        return res 
+        return res
 
     def _verifyBackports(self):
         # run update (but ignore errors in case the countrymirror

=== modified file 'DistUpgrade/DistUpgradeFetcherCore.py'
--- DistUpgrade/DistUpgradeFetcherCore.py	2016-11-29 16:36:37 +0000
+++ DistUpgrade/DistUpgradeFetcherCore.py	2018-12-11 23:25:23 +0000
@@ -220,7 +220,7 @@
     def runDistUpgrader(self):
         args = [self.script] + self.run_options
         if os.getuid() != 0:
-            os.execv("/usr/bin/sudo", ["sudo"] + args)
+            os.execv("/usr/bin/sudo", ["sudo", "-E"] + args)
         else:
             os.execv(self.script, args)
 

=== modified file 'debian/changelog'
--- debian/changelog	2018-11-28 22:11:44 +0000
+++ debian/changelog	2018-12-11 23:25:23 +0000
@@ -1,10 +1,22 @@
-ubuntu-release-upgrader (1:19.04.7) UNRELEASED; urgency=medium
+ubuntu-release-upgrader (1:19.04.7ubuntu1) UNRELEASED; urgency=medium
 
   * DistUpgrade/DistUpgradeCache.py: When calculating free space needed for
     mount points don't use a negative number as the buffer. (LP: #1786484)
   * fix more typos re RELEASE_UPRADER_ALLOW_THIRD_PARTY
+  * DistUpgrade/DistUpgradeController.py: When rewriting sources.list entries
+    check to see if the source provides packages for the release to which the
+    upgrade is occurring. If it doesn't the entry is disabled thereby
+    improving upgrades with PPAs. (LP: #1807043)
+  * do-release-upgrade: add a parameter to allow third party mirrors and
+    repositories, additionally pass along the environmental variable
+    RELEASE_UPGRADER_ALLOW_THIRD_PARTY via pkexec and sudo. (LP: #1807032)
+  * DistUpgrade/DistUpgradeCache.py: improve messaging regarding failures to
+    calculate the upgrade and suggest using ppa-purge if PPAs are an issue.
+  * DistUpgrade/DistUpgradeCache.py: in the event there is a failure to
+    calculate the upgrade provide information about the log files in
+    /var/log/dist-upgrade. (LP: #1771387)
 
- -- Brian Murray <[e-mail]>  Wed, 28 Nov 2018 14:11:10 -0800
+ -- Brian Murray <[e-mail]>  Tue, 11 Dec 2018 15:12:19 -0800
 
 ubuntu-release-upgrader (1:19.04.6) disco; urgency=medium
 

=== modified file 'do-release-upgrade'
--- do-release-upgrade	2018-11-28 18:02:37 +0000
+++ do-release-upgrade	2018-12-11 23:25:23 +0000
@@ -103,6 +103,11 @@
                      help=_("Check only if a new distribution release is "
                             "available and report the result via the "
                             "exit code"))
+  parser.add_option ("--allow-third-party", default=False,
+                     action="store_true", dest="allow_third_party",
+                     help=_("Try the upgrade with third party "
+                            "mirrors and repositories enabled "
+                            "instead of commenting them out."))
   parser.add_option ("-q", "--quiet", default=False, action="store_true",
                      dest="quiet")
 
@@ -195,12 +200,19 @@
                 "requires a reboot. Please reboot before upgrading."))
       sys.exit(1)
 
+  if options.allow_third_party:
+    # the env variable is used by code in the dist-upgrader tarball
+    os.environ["RELEASE_UPGRADER_ALLOW_THIRD_PARTY"] = "True"
+
   # GTK 3 fetcher starts setting up the the GUI and KDE GUI needs to start GUI
   # later, too.
   if (options.frontend in {"DistUpgradeViewGtk3", "DistUpgradeViewKDE"} and
       os.getuid() != 0):
     if 'WAYLAND_DISPLAY' in os.environ:
         subprocess.run(['xhost', '+si:localuser:root'])
+    # this is needed because pkexec doesn't pass on the env
+    if 'RELEASE_UPGRADER_ALLOW_THIRD_PARTY' in os.environ:
+        sys.argv.append('--allow-third-party')
     os.execv("/usr/bin/pkexec", ["pkexec"] + sys.argv)
 
   fetcher = get_fetcher(options.frontend, m.new_dist, options.data_dir)

=== modified file 'tests/test_sources_list.py'
--- tests/test_sources_list.py	2018-11-28 22:11:44 +0000
+++ tests/test_sources_list.py	2018-12-11 23:25:23 +0000
@@ -92,8 +92,9 @@
 deb http://security.ubuntu.com/ubuntu/ gutsy-security main restricted
 """)
 
+    @mock.patch("DistUpgrade.DistUpgradeController.DistUpgradeController._sourcesListEntryDownloadable")
     @mock.patch("DistUpgrade.DistUpgradeController.get_distro")
-    def test_sources_list_rewrite(self, mock_get_distro):
+    def test_sources_list_rewrite(self, mock_get_distro, mock_sourcesListEntryDownloadable):
         """
         test regular sources.list rewrite
         """
@@ -107,8 +108,10 @@
                                                           "Ubuntu Feisty Fawn",
                                                           "7.04")
         d.openCache(lock=False)
+        mock_sourcesListEntryDownloadable.return_value = True
         res = d.updateSourcesList()
         self.assertTrue(mock_get_distro.called)
+        self.assertTrue(mock_sourcesListEntryDownloadable.called)
         self.assertTrue(res)
 
         # now test the result
@@ -127,7 +130,7 @@
              apt_pkg.config.find_file("Dir::Etc::sourcelist") + ".in",
              apt_pkg.config.find_file("Dir::Etc::sourcelist") + ".distUpgrade"
              ]))
-             
+
     @mock.patch("DistUpgrade.DistUpgradeController.DistUpgradeController.abort")
     @mock.patch("DistUpgrade.DistUpgradeController.get_distro")
     def test_double_check_source_distribution_reject(self, mock_abort, mock_get_distro):
@@ -145,10 +148,10 @@
         mock_get_distro.return_value = UbuntuDistribution("Ubuntu", "feisty",
                                                           "Ubuntu Feisty Fawn",
                                                           "7.04")
-        
+
         class AbortException(Exception):
             """Exception"""
-                
+
         mock_abort.side_effect = AbortException
         d.openCache(lock=False)
         with self.assertRaises(AbortException):
@@ -157,8 +160,9 @@
         self.assertTrue(mock_get_distro.called)
         self.assertTrue(v.askYesNoQuestion.called)
 
+    @mock.patch("DistUpgrade.DistUpgradeController.DistUpgradeController._sourcesListEntryDownloadable")
     @mock.patch("DistUpgrade.DistUpgradeController.get_distro")
-    def test_double_check_source_distribution_continue(self, mock_get_distro):
+    def test_double_check_source_distribution_continue(self, mock_get_distro, mock_sourcesListEntryDownloadable):
         """
         test that an upgrade from feisty with a sources.list containing
         hardy asks a question, and if continued, does something.
@@ -174,13 +178,15 @@
                                                           "Ubuntu Feisty Fawn",
                                                           "7.04")
         d.openCache(lock=False)
+        mock_sourcesListEntryDownloadable.return_value = True
         res = d.updateSourcesList()
         self.assertTrue(mock_get_distro.called)
+        self.assertTrue(mock_sourcesListEntryDownloadable.called)
         self.assertTrue(res)
 
         # now test the result
         #print(open(os.path.join(self.testdir,"sources.list")).read())
-        
+
         # The result here is not really all that helpful, hence we
         # added the question in the first place. But still better to
         # check what it does than to not check it.
@@ -195,6 +201,7 @@
 # deb http://security.ubuntu.com/ubuntu/ hardy-security universe
 
 deb http://archive.ubuntu.com/ubuntu/ gutsy main
+
 """)
         # check that the backup file was created correctly
         self.assertEqual(0, subprocess.call(
@@ -203,8 +210,9 @@
              apt_pkg.config.find_file("Dir::Etc::sourcelist") + ".distUpgrade"
              ]))
 
+    @mock.patch("DistUpgrade.DistUpgradeController.DistUpgradeController._sourcesListEntryDownloadable")
     @mock.patch("DistUpgrade.DistUpgradeController.get_distro")
-    def test_sources_list_inactive_mirror(self, mock_get_distro):
+    def test_sources_list_inactive_mirror(self, mock_get_distro, mock_sourcesListEntryDownloadable):
         """
         test sources.list rewrite of an obsolete mirror
         """
@@ -218,8 +226,10 @@
                                                           "Ubuntu Feisty Fawn",
                                                           "7.04")
         d.openCache(lock=False)
+        mock_sourcesListEntryDownloadable.return_value = True
         res = d.updateSourcesList()
         self.assertTrue(mock_get_distro.called)
+        self.assertTrue(mock_sourcesListEntryDownloadable.called)
         self.assertTrue(res)
 
         # now test the result
@@ -311,8 +321,9 @@
 deb http://archive.canonical.com/ubuntu gutsy partner
 """)
 
+    @mock.patch("DistUpgrade.DistUpgradeController.DistUpgradeController._sourcesListEntryDownloadable")
     @mock.patch("DistUpgrade.DistUpgradeController.get_distro")
-    def test_extras_removal(self, mock_get_distro):
+    def test_extras_removal(self, mock_get_distro, mock_sourcesListEntryDownloadable):
         """
         test removal of extras.ubuntu.com archives
         """
@@ -328,7 +339,9 @@
                                                           "Ubuntu Feisty Fawn",
                                                           "7.04")
         d.openCache(lock=False)
+        mock_sourcesListEntryDownloadable.return_value = True
         res = d.updateSourcesList()
+        self.assertTrue(mock_sourcesListEntryDownloadable.called)
         self.assertTrue(res)
 
         sources_file = apt_pkg.config.find_file("Dir::Etc::sourcelist")
@@ -338,7 +351,8 @@
 
 """)
 
-    def test_powerpc_transition(self):
+    @mock.patch("DistUpgrade.DistUpgradeController.DistUpgradeController._sourcesListEntryDownloadable")
+    def test_powerpc_transition(self, mock_sourcesListEntryDownloadable):
         """
         test transition of powerpc to ports.ubuntu.com
         """
@@ -351,7 +365,9 @@
         v = DistUpgradeViewNonInteractive()
         d = DistUpgradeController(v, datadir=self.testdir)
         d.openCache(lock=False)
+        mock_sourcesListEntryDownloadable.return_value = True
         res = d.updateSourcesList()
+        self.assertTrue(mock_sourcesListEntryDownloadable.called)
         self.assertTrue(res)
         # now test the result
         self._verifySources("""
@@ -362,7 +378,8 @@
 """)
         apt_pkg.config.set("APT::Architecture", arch)
 
-    def test_sparc_transition(self):
+    @mock.patch("DistUpgrade.DistUpgradeController.DistUpgradeController._sourcesListEntryDownloadable")
+    def test_sparc_transition(self, mock_sourcesListEntryDownloadable):
         """
         test transition of sparc to ports.ubuntu.com
         """
@@ -377,7 +394,9 @@
         d.fromDist = "gutsy"
         d.toDist = "hardy"
         d.openCache(lock=False)
+        mock_sourcesListEntryDownloadable.return_value = True
         res = d.updateSourcesList()
+        self.assertTrue(mock_sourcesListEntryDownloadable.called)
         self.assertTrue(res)
         # now test the result
         self._verifySources("""
@@ -480,7 +499,8 @@
 deb http://archive.ubuntu.com/ubuntu precise-security main restricted universe multiverse
 """)
 
-    def test_partner_update(self):
+    @mock.patch("DistUpgrade.DistUpgradeController.DistUpgradeController._sourcesListEntryDownloadable")
+    def test_partner_update(self, mock_sourcesListEntryDownloadable):
         """
         test transition partner repository updates
         """
@@ -491,7 +511,9 @@
         v = DistUpgradeViewNonInteractive()
         d = DistUpgradeController(v, datadir=self.testdir)
         d.openCache(lock=False)
+        mock_sourcesListEntryDownloadable.return_value = True
         res = d.updateSourcesList()
+        self.assertTrue(mock_sourcesListEntryDownloadable.called)
         self.assertTrue(res)
 
         # now test the result
@@ -504,7 +526,8 @@
 deb http://archive.canonical.com/ubuntu gutsy partner
 """)
 
-    def test_private_ppa_transition(self):
+    @mock.patch("DistUpgrade.DistUpgradeController.DistUpgradeController._sourcesListEntryDownloadable")
+    def test_private_ppa_transition(self, mock_sourcesListEntryDownloadable):
         if "RELEASE_UPGRADER_ALLOW_THIRD_PARTY" in os.environ:
             del os.environ["RELEASE_UPGRADER_ALLOW_THIRD_PARTY"]
         shutil.copy(
@@ -516,7 +539,9 @@
         v = DistUpgradeViewNonInteractive()
         d = DistUpgradeController(v, datadir=self.testdir)
         d.openCache(lock=False)
+        mock_sourcesListEntryDownloadable.return_value = True
         res = d.updateSourcesList()
+        self.assertTrue(mock_sourcesListEntryDownloadable.called)
         self.assertTrue(res)
 
         # now test the result
@@ -533,7 +558,8 @@
 deb https://user:[e-mail]/commercial-ppa-uploaders gutsy main
 """)
 
-    def test_apt_cacher_and_apt_bittorent(self):
+    @mock.patch("DistUpgrade.DistUpgradeController.DistUpgradeController._sourcesListEntryDownloadable")
+    def test_apt_cacher_and_apt_bittorent(self, mock_sourcesListEntryDownloadable):
         """
         test transition of apt-cacher/apt-torrent uris
         """
@@ -544,7 +570,9 @@
         v = DistUpgradeViewNonInteractive()
         d = DistUpgradeController(v, datadir=self.testdir)
         d.openCache(lock=False)
+        mock_sourcesListEntryDownloadable.return_value = True
         res = d.updateSourcesList()
+        self.assertTrue(mock_sourcesListEntryDownloadable.called)
         self.assertTrue(res)
 
         # now test the result
@@ -593,7 +621,8 @@
 # deb http://ppa.launchpad.net/random-ppa quantal main # ppa of Víctor R. Ruiz (vrruiz) disabled on upgrade to gutsy
 """)
 
-    def test_local_mirror(self):
+    @mock.patch("DistUpgrade.DistUpgradeController.DistUpgradeController._sourcesListEntryDownloadable")
+    def test_local_mirror(self, mock_sourcesListEntryDownloadable):
         """
         test that a local mirror with official -backports works (LP:# 1067393)
         """
@@ -603,7 +632,9 @@
         v = DistUpgradeViewNonInteractive()
         d = DistUpgradeController(v, datadir=self.testdir)
         d.openCache(lock=False)
+        mock_sourcesListEntryDownloadable.return_value = True
         res = d.updateSourcesList()
+        self.assertTrue(mock_sourcesListEntryDownloadable.called)
         self.assertTrue(res)
 
         # verify it
@@ -614,7 +645,8 @@
 deb http://archive.ubuntu.com/ubuntu gutsy-backports main restricted universe multiverse
 """)
 
-    def test_disable_proposed(self):
+    @mock.patch("DistUpgrade.DistUpgradeController.DistUpgradeController._sourcesListEntryDownloadable")
+    def test_disable_proposed(self, mock_sourcesListEntryDownloadable):
         """
         Test that proposed is disabled when upgrading to a development
         release.
@@ -628,7 +660,9 @@
         options.devel_release = True
         d = DistUpgradeController(v, options, datadir=self.testdir)
         d.openCache(lock=False)
+        mock_sourcesListEntryDownloadable.return_value = True
         res = d.updateSourcesList()
+        self.assertTrue(mock_sourcesListEntryDownloadable.called)
         self.assertTrue(res)
 
         self._verifySources("""