Merge lp:~sil2100/ubuntu-archive-tools/migration-assistant-minor-clean into lp:ubuntu-archive-tools

[Łukasz Zemczak]

Refactor the migration-assistant script slightly, with indent changes and removing some of the global variables that should have been local.

Refactor the migration-assistant script slightly, with indent changes and removing some of the global variables that should have been local.

[lp:~sil2100/ubuntu-archive-tools/migration-assistant-minor-clean]

1172. By Łukasz Zemczak on 2018-03-08

Fix some brokenness caused by the cleanup.

[Łukasz Zemczak (sil2100)]

+1 by Matt.

review: Approve

=== modified file 'migration-assistant.py'
--- migration-assistant.py	2018-03-08 10:28:01 +0000
+++ migration-assistant.py	2018-03-08 11:12:37 +0000
@@ -38,23 +38,13 @@
 import logging
 
 from enum import Enum
-
 from urllib.request import FancyURLopener
-
 from launchpadlib.launchpad import Launchpad
 
 DEBIAN_CURRENT_SERIES = 'sid'
 ARCHIVE_PAGES = 'https://people.canonical.com/~ubuntu-archive/'
 MAX_CACHE_AGE = 14400   # excuses cache should not be older than 4 hours
 
-lp_cachedir = os.path.expanduser(os.path.join("~", ".launchpadlib/cache"))
-lp = Launchpad.login_anonymously(
-    'what-next', 'production', lp_cachedir, version='devel')
-
-ubuntu = lp.distributions["ubuntu"]
-archive = ubuntu.main_archive
-series = ubuntu.current_series
-
 excuses = {}
 
 level = 0
@@ -137,7 +127,7 @@
         return None
 
 
-def find_excuses(src, level, seen):
+def find_excuses(lp, src, level, seen):
     if src in seen:
         return
 
@@ -145,7 +135,7 @@
         item_name = item.get('item-name')
 
         if item_name == src:
-            process(item, level, seen)
+            process(lp, item, level, seen)
 
 
 def get_pkg_archive_path(package):
@@ -181,12 +171,12 @@
     return None
 
 
-def package_in_distro(package, distro='ubuntu', proposed=False):
+def package_in_distro(package, distro='ubuntu', distroseries='bionic',
+                      proposed=False):
     assistant = StatusAdapter(assistant_logger, {'depth': 0})
 
     # TODO: This operation is pretty costly, do caching?
 
-    distroseries = series.name
     if distro == 'debian':
         distroseries = DEBIAN_CURRENT_SERIES
     if proposed:
@@ -204,7 +194,7 @@
         if " {} ".format(package) not in line:
             continue
         package_line = line.split(' | ')
- 
+
         series_component = package_line[2].split('/')
         component = 'main'
         if len(series_component) > 1:
@@ -226,7 +216,7 @@
     return {}
 
 
-def process_lp_build_results(source, level, uploads, failed):
+def process_lp_build_results(source, archive, series, level, uploads, failed):
     assistant = StatusAdapter(assistant_logger, {'depth': level + 1})
 
     source_name = source.get('source')
@@ -270,11 +260,12 @@
             }
 
 
-def process_unsatisfiable_depends(source, level, seen):
+def process_unsatisfiable_depends(source, series, level, seen):
     assistant = StatusAdapter(assistant_logger, {'depth': level + 1})
 
     affected_sources = {}
     unsatisfiable = {}
+    distroseries = series.name
 
     depends = source.get('dependencies').get('unsatisfiable-dependencies')
     for arch, signatures in depends.items():
@@ -287,12 +278,15 @@
                 pkg = get_source_package(binary_name)
                 affected_sources[arch].add(pkg)
             except Exception:
-                # FIXME: we might be dealing with a new package in proposed here,
-                #        but using the binary name instead of the source name.
-                if any(package_in_distro(binary_name, distro='ubuntu')):
+                # FIXME: we might be dealing with a new package in proposed
+                #        here, but using the binary name instead of the source
+                #        name.
+                if any(package_in_distro(binary_name, distro='ubuntu',
+                                         distroseries=distroseries)):
                     affected_sources[arch].add(binary_name)
                 elif any(package_in_distro(binary_name,
                                            distro='ubuntu',
+                                           distroseries=distroseries,
                                            proposed=True)):
                     affected_sources[arch].append(binary_name)
 
@@ -328,8 +322,10 @@
             depends = signature.split(' ')[0]
             assistant.error("{} can not be satisfied".format(signature),
                             status=ExcuseValue.FAIL)
-            in_archive = package_in_distro(depends, distro='ubuntu')
+            in_archive = package_in_distro(depends, distro='ubuntu',
+                                           distroseries=distroseries)
             in_proposed = package_in_distro(depends, distro='ubuntu',
+                                            distroseries=distroseries,
                                             proposed=True)
 
             if any(in_archive) and not any(in_proposed):
@@ -350,7 +346,8 @@
                                "but not in Debian?",
                                status=ExcuseValue.INFO)
             elif not any(in_archive) and not any(in_proposed):
-                in_debian = package_in_distro(depends, distro='debian')
+                in_debian = package_in_distro(depends, distro='debian',
+                                              distroseries=distroseries)
                 if any(in_debian):
                     assistant.warning("{} only exists in Debian".format(depends),
                                       status=ExcuseValue.FAIL)
@@ -426,7 +423,7 @@
                         status=ExcuseValue.INFO)
 
 
-def process_blocking(source, level):
+def process_blocking(source, lp, level):
     assistant = StatusAdapter(assistant_logger, {'depth': level + 1})
 
     bugs = source.get('policy_info').get('block-bugs')
@@ -506,7 +503,7 @@
             find_excuses(blocker, level+2, seen)
 
 
-def process_missing_builds(source, level):
+def process_missing_builds(source, archive, series, level):
     assistant = StatusAdapter(assistant_logger, {'depth': level + 1})
 
     source_name = source.get('source')
@@ -569,7 +566,7 @@
     anais = []
     new_binaries = set()
 
-    process_lp_build_results(source, level, uploads, failed)
+    process_lp_build_results(source, archive, series, level, uploads, failed)
 
     if new_version in uploads:
         for arch, item in uploads[new_version].items():
@@ -578,7 +575,7 @@
                 new_binaries.add(binary_name)
                 if binary.get('is_new'):
                     new.append(binary)
-                     
+
     if not any(failed):
         assistant.error("No failed builds found", status=ExcuseValue.PASS)
 
@@ -598,9 +595,9 @@
                                   "Admin to run:",
                                   status=ExcuseValue.INFO)
                 assistant.info("remove-package %(arches)s -b %(bins)s"
-                               % ({ 'arches': " ".join(arch_o),
-                                    'bins': " ".join(old_binaries),
-                               }), status=ExcuseValue.NONE)
+                               % ({'arches': " ".join(arch_o),
+                                   'bins': " ".join(old_binaries),
+                                   }), status=ExcuseValue.NONE)
         except AttributeError:
             # Ignore a failure here, it just means we don't have
             # missing-builds to process after all.
@@ -608,7 +605,7 @@
 
     if any(new):
         assistant.error("This package has NEW binaries to process:",
-                         status=ExcuseValue.INFO)
+                        status=ExcuseValue.INFO)
         for binary in new:
             assistant.warning("[{}] {}/{}".format(
                                   binary.get('architecture'),
@@ -617,11 +614,13 @@
                               status=ExcuseValue.FAIL)
 
 
-
-
-def process(source, level, seen):
+def process(lp, source, level, seen):
     assistant = StatusAdapter(assistant_logger, {'depth': level})
 
+    ubuntu = lp.distributions["ubuntu"]
+    archive = ubuntu.main_archive
+    series = ubuntu.current_series
+
     source_name = source.get('source')
     reasons = source.get('reason')
 
@@ -644,15 +643,15 @@
     missing_builds = source.get('missing-builds')
     if missing_builds is not None or 'no-binaries' in reasons:
         work_needed = True
-        process_missing_builds(source, level)
+        process_missing_builds(source, archive, series, level)
 
     if 'depends' in reasons:
         work_needed = True
-        process_unsatisfiable_depends(source, level, seen)
+        process_unsatisfiable_depends(source, series, level, seen)
 
     if 'block' in reasons:
         work_needed = True
-        process_blocking(source, level)
+        process_blocking(source, lp, level)
 
     if 'autopkgtest' in reasons:
         work_needed = True
@@ -691,7 +690,7 @@
         src_num += 1
 
     while True:
-        print (options)
+        print(options)
         print("\n".join(wrapper.wrap(msg)))
         num = input("\nWhich package do you want to look at?")
 
@@ -705,66 +704,71 @@
                 return num
 
     return options[choice]
-        
-    
-
-parser = argparse.ArgumentParser(
-    description='Evaluate next steps for proposed migration')
-parser.add_argument('-s', '--source', dest='source',
-                    help='the package to evaluate')
-parser.add_argument('--no-cache', dest='do_not_cache', action='store_const',
-                    const=True, default=False,
-                    help='Do not cache excuses')
-parser.add_argument('--refresh', action='store_const',
-                    const=True, default=False,
-                    help='Force refresh of cached excuses')
-parser.add_argument('--debug', action='store_const',
-                    const=True, default=False,
-                    help='Show debugging information for this tool.')
-
-args = parser.parse_args()
-
-if args.debug:
-    logging.basicConfig(level=logging.DEBUG, format="%(message)s")
-else:
-    logging.basicConfig(level=logging.INFO, format="%(message)s")
-
-refresh_due = False
-xdg_cache = os.getenv('XDG_CACHE_HOME', '~/.cache')
-excuses_path = os.path.expanduser(os.path.join(xdg_cache, 'excuses.yaml'))
-if args.do_not_cache:
-    fp = tempfile.NamedTemporaryFile()
-else:
-    try:
-        fp = open(excuses_path, 'r')
-    except FileNotFoundError:
-        refresh_due = True
-        pass
-    finally:
-        fp = open(excuses_path, 'a+')
-
-    file_state = os.stat(excuses_path)
-    mtime = file_state.st_mtime
-    now = time.time()
-    if (now - mtime) > MAX_CACHE_AGE:
-        refresh_due = True
-
-with fp:
-    if args.refresh or refresh_due:
-        url_opener = FancyURLopener()
-        excuses_url = ARCHIVE_PAGES + 'proposed-migration/update_excuses.yaml'
-        excuses_data = url_opener.retrieve(excuses_url,
-                                           fp.name,
-                                           report_download)
-    fp.seek(0)
-
-    # Use the C implementation of the SafeLoader, it's noticeably faster, and
-    # here we're dealing with large input files.
-    excuses = yaml.load(fp, Loader=yaml.CSafeLoader)
-
-    if args.source is None:
-        print("No source package name was provided. The following packages are "
-              "blocked in proposed:\n")
-        args.source = choose_blocked_source(excuses)
-
-    find_excuses(args.source, 0, seen)
+
+
+if __name__ == '__main__':
+
+    parser = argparse.ArgumentParser(
+        description='Evaluate next steps for proposed migration')
+    parser.add_argument('-s', '--source', dest='source',
+                        help='the package to evaluate')
+    parser.add_argument('--no-cache', dest='do_not_cache', action='store_const',
+                        const=True, default=False,
+                        help='Do not cache excuses')
+    parser.add_argument('--refresh', action='store_const',
+                        const=True, default=False,
+                        help='Force refresh of cached excuses')
+    parser.add_argument('--debug', action='store_const',
+                        const=True, default=False,
+                        help='Show debugging information for this tool.')
+
+    args = parser.parse_args()
+
+    if args.debug:
+        logging.basicConfig(level=logging.DEBUG, format="%(message)s")
+    else:
+        logging.basicConfig(level=logging.INFO, format="%(message)s")
+
+    lp_cachedir = os.path.expanduser(os.path.join("~", ".launchpadlib/cache"))
+    lp = Launchpad.login_anonymously(
+        'what-next', 'production', lp_cachedir, version='devel')
+
+    refresh_due = False
+    xdg_cache = os.getenv('XDG_CACHE_HOME', '~/.cache')
+    excuses_path = os.path.expanduser(os.path.join(xdg_cache, 'excuses.yaml'))
+    if args.do_not_cache:
+        fp = tempfile.NamedTemporaryFile()
+    else:
+        try:
+            fp = open(excuses_path, 'r')
+        except FileNotFoundError:
+            refresh_due = True
+            pass
+        finally:
+            fp = open(excuses_path, 'a+')
+
+        file_state = os.stat(excuses_path)
+        mtime = file_state.st_mtime
+        now = time.time()
+        if (now - mtime) > MAX_CACHE_AGE:
+            refresh_due = True
+
+    with fp:
+        if args.refresh or refresh_due:
+            url_opener = FancyURLopener()
+            excuses_url = ARCHIVE_PAGES + 'proposed-migration/update_excuses.yaml'
+            excuses_data = url_opener.retrieve(excuses_url,
+                                               fp.name,
+                                               report_download)
+        fp.seek(0)
+
+        # Use the C implementation of the SafeLoader, it's noticeably faster, and
+        # here we're dealing with large input files.
+        excuses = yaml.load(fp, Loader=yaml.CSafeLoader)
+
+        if args.source is None:
+            print("No source package name was provided. The following packages are "
+                  "blocked in proposed:\n")
+            args.source = choose_blocked_source(excuses)
+
+        find_excuses(lp, args.source, 0, seen)