[Merge] lp:~brian-murray/ubuntu-release-upgrader/patching-get-distro into lp:ubuntu-release-upgrader

Barry Warsaw barry at canonical.com
Wed Nov 18 14:44:28 UTC 2015


Review: Needs Fixing

LGTM, with just a few style nits.  However, there's a merge conflict that needs to be fixed first.

Diff comments:

> === modified file 'DistUpgrade/DistUpgradeController.py'
> --- DistUpgrade/DistUpgradeController.py	2015-10-27 01:51:21 +0000
> +++ DistUpgrade/DistUpgradeController.py	2015-11-17 23:15:11 +0000
> @@ -525,12 +528,15 @@
>              mirror_check=False
>  
>          # check if we need to enable main
> +        main_was_missing = False
>          if mirror_check == True and self.useNetwork:
>              # now check if the base-meta pkgs are available in
>              # the archive or only available as "now"
>              # -> if not that means that "main" is missing and we
> -            #    need to  enable it
> +            #    need to enable it
> +            logging.debug(self.config.getlist("Distro","BaseMetaPkgs"))

Should add a space after the comma.

>              for pkgname in self.config.getlist("Distro","BaseMetaPkgs"):
> +                logging.debug("Checking pkg: %s" % pkgname)
>                  if ((not pkgname in self.cache or
>                       not self.cache[pkgname].candidate or
>                       len(self.cache[pkgname].candidate.origins) == 0)
> @@ -544,17 +550,24 @@
>                          distro = get_distro()
>                          distro.get_sources(self.sources)
>                          distro.enable_component("main")
> -                    except NoDistroTemplateException:
> +                        main_was_missing = True
> +                    except NoDistroTemplateException as e:
> +                        logging.debug('NoDistroTemplateException raised: %s' % e)

Maybe use logging.exception() instead?

>                          # fallback if everything else does not work,
> -                        # we replace the sources.list with a single
> -                        # line to ubuntu-main
> -                        logging.warning('get_distro().enable_component("main") failed, overwriting sources.list instead as last resort')
> -                        s =  "# auto generated by ubuntu-release-upgrader"
> -                        s += "deb http://archive.ubuntu.com/ubuntu %s main restricted" % self.toDist
> -                        s += "deb http://archive.ubuntu.com/ubuntu %s-updates main restricted" % self.toDist
> -                        s += "deb http://security.ubuntu.com/ubuntu %s-security main restricted" % self.toDist
> -                        with open("/etc/apt/sources.list","w") as f:
> -                            f.write(s)
> +                        # we replace the sources.list with lines to
> +                        # main and restricted
> +                        logging.debug('get_distro().enable_component("main") failed, overwriting sources.list instead as last resort')
> +                        comment = " auto generated by ubuntu-release-upgrader"
> +                        comps = ["main", "restricted"]
> +                        uri = "http://archive.ubuntu.com/ubuntu"
> +                        self.sources.add("deb", uri, self.toDist, comps,
> +                                         comment)
> +                        self.sources.add("deb", uri, self.toDist+"-updates",
> +                                         comps, comment)
> +                        self.sources.add("deb",
> +                                         "http://security.ubuntu.com/ubuntu",
> +                                         self.toDist+"-security", comps,
> +                                         comment)
>                      break
>  
>          # this must map, i.e. second in "from" must be the second in "to"
> @@ -731,12 +745,34 @@
>                  entry.disabled = True
>                  self.sources_disabled = True
>                  logging.debug("entry '%s' was disabled (unknown mirror)" % get_string_with_no_auth_from_source_entry(entry))
> +                # if its not a valid mirror and we manually added main, be
> +                # nice and add pockets and components corresponding to what we
> +                # disabled.
> +                if main_was_missing:
> +                    if entry.dist in fromDists:
> +                        entry.dist = toDists[fromDists.index(entry.dist)]
> +                    # gather what components are enabled and are inconsistent
> +                    for d in ["%s" % self.toDist,
> +                              "%s-updates" % self.toDist,
> +                              "%s-security" % self.toDist]:
> +                        # create entry if needed, ignore deb-src entries
> +                        self.found_components.setdefault(d,set())

Space after comma.

> +                        if (entry.dist == d and entry.type == "deb"):

In this case, you don't need the parentheses.

> +                            for comp in entry.comps:
> +                                # only sync components we know about
> +                                if not comp in sync_components:

`if comp not in sync_components:`

> +                                    continue
> +                                self.found_components[d].add(comp)
> +                    logging.debug("Adding entry: %s %s %s" % (entry.type, entry.dist, entry.comps))
> +                    uri = "http://archive.ubuntu.com/ubuntu"
> +                    comment = " auto generated by ubuntu-release-upgrader"
> +                    self.sources.add(entry.type, uri, entry.dist, entry.comps, comment)
>  
>          # now go over the list again and check for missing components
>          # in $dist-updates and $dist-security and add them
>          for entry in self.sources.list[:]:
>              # skip all comps that are not relevant (including e.g. "hardy")
> -            if (entry.invalid or entry.disabled or entry.type == "deb-src" or 
> +            if (entry.invalid or entry.disabled or entry.type == "deb-src" or
>                  entry.uri.startswith("cdrom:") or entry.dist == self.toDist):
>                  continue
>              # now check for "$dist-updates" and "$dist-security" and add any inconsistencies
> 
> === modified file 'debian/changelog'
> --- debian/changelog	2015-11-02 18:10:59 +0000
> +++ debian/changelog	2015-11-17 23:15:11 +0000
> @@ -1,4 +1,8 @@
> +<<<<<<< TREE
>  ubuntu-release-upgrader (1:16.04.2) wily; urgency=medium
> +=======
> +ubuntu-release-upgrader (1:16.04.2) xenial; urgency=medium
> +>>>>>>> MERGE-SOURCE
>  

Oops... conflict!

>    * DistUpgrade/DistUpgradeQuirks.py: protect against the linux metapackage
>      not being available in the cache. (LP: #1511831)
> 
> === modified file 'tests/data-sources-list-test/DistUpgrade.cfg'
> --- tests/data-sources-list-test/DistUpgrade.cfg	2010-05-25 19:23:56 +0000
> +++ tests/data-sources-list-test/DistUpgrade.cfg	2015-11-17 23:15:11 +0000
> @@ -4,7 +4,7 @@
>  # Distro contains global information about the upgrade
>  [Distro]
>  MetaPkgs=ubuntu-desktop
> -;BaseMetaPkgs=ubuntu-minimal
> +BaseMetaPkgs=ubuntu-minimal

Was this just a typo in the original code?

>  
>  [ubuntu-desktop]
>  KeyDependencies=gdm, gnome-panel, ubuntu-artwork


-- 
https://code.launchpad.net/~brian-murray/ubuntu-release-upgrader/patching-get-distro/+merge/277770
Your team Ubuntu Core Development Team is subscribed to branch lp:ubuntu-release-upgrader.



More information about the Ubuntu-reviews mailing list