Revamp script for a more Pythonic experience

ubports-qa had a few papercuts. Namely:
1. It wouldn't remount the system when installing or updating
2. It was easy for a developer to forget to mount or unmount
   (which caused point 1)
3. Failure exits return 0
4. It used the old style of formatting strings which is generally
   regarded as bad practice
5. Remounting read-only will fail if anything wakes up and grabs a file
   on the rootfs. Handle that and return an error.
6. It didn't lint.
  a. Some of the function names clobbered built-in keywords.

This fixes that.

Author: UniversalSuperBox

ubports-qa

@@ -5,13 +5,14 @@ The UBports QA scripts allow you to efficiently manage PPAs from repo.ubports.co
 Copyright 2018 UBports Foundation
 Licensed GPL v. 3 or later
 """
-
-import requests, subprocess, os, argparse
+import subprocess
+import os
+import argparse
 from enum import Enum
+import requests
 
-GITHUB_API_PULLREQUEST="https://api.github.com/repos/ubports/%s/pulls/%s"
-JENKINS_API_BUILD="https://ci.ubports.com/blue/rest/organizations/jenkins/pipelines/%s/branches/%s"
-REPO="http://repo.ubports.com/dists/%s/main/binary-armhf/Packages"
+GITHUB_API_PULLREQUEST="https://api.github.com/repos/ubports/{repo}/pulls/{num}"
+JENKINS_API_BUILD="https://ci.ubports.com/blue/rest/organizations/jenkins/pipelines/{repo}/branches/{ref}"
 
 is_root = (os.geteuid() == 0)
 
@@ -20,28 +21,60 @@ class Status(Enum):
     BULDING = 2
     FAILED = 3
 
+class WritableRootFS():
+    """
+    A class to be used with the `with` statement to mount `/` read-write, for example::
+
+        with WritableRootFS():
+            write_file(/good_file)
+
+    `/` will be remounted read-only on close, unless the file /userdata/.writable_image
+    exists.
+    """
+    def __enter__(self):
+        self.attempt_writable_mount()
+
+    def __exit__(self, type, value, traceback):
+        self.attempt_unmount()
+
+    @classmethod
+    def attempt_writable_mount(cls):
+        """Tries to mount the rootfs read-write"""
+        ensure_root()
+        subprocess.run(["mount", "-o", "rw,remount", "/"])
+
+    @classmethod
+    def attempt_unmount(cls):
+        if not os.path.exists("/userdata/.writable_image"):
+            ensure_root()
+            os.sync()
+            try:
+                subprocess.run(["mount", "-o", "ro,remount", "/"], check=True)
+            except subprocess.CalledProcessError:
+                print_error("Failed to remount root filesystem read-only.")
+                print_error("Please consider rebooting your device.")
+
 def ensure_root():
     if not is_root:
         die("Insufficient permissions, please run with sudo.")
 
-def mount():
-    subprocess.call(["mount", "-o", "rw,remount", "/"])
-
-def unmount():
-    if not os.path.exists("/userdata/.writable_image"):
-        subprocess.call(["mount", "-o", "ro,remount", "/"])
-
 def apt_update():
-    subprocess.call(["apt", "update"])
+    try:
+        subprocess.run(["apt", "update"], check=True)
+    except subprocess.CalledProcessError:
+        print_error("Failed to run 'apt update'. See the output above for details.")
 
 def apt_upgrade():
-    subprocess.call(["apt", "upgrade"])
+    try:
+        subprocess.run(["apt", "upgrade"], check=True)
+    except subprocess.CalledProcessError:
+        print_error("Failed to run 'apt upgrade'. See the output above for details.")
 
 def get_list_file(branch):
-    return "/etc/apt/sources.list.d/ubports-%s.list" % branch
+    return "/etc/apt/sources.list.d/ubports-{}.list".format(branch)
 
 def get_pref_file(branch):
-    return "/etc/apt/preferences.d/ubports-%s.pref" % branch
+    return "/etc/apt/preferences.d/ubports-{}.pref".format(branch)
 
 def list_exists(branch):
     return os.path.isfile(get_list_file(branch))
@@ -57,7 +90,7 @@ def add_list(branch):
     if requests.get("http://repo.ubports.com/dists/" + branch).status_code != 200:
         die("PPA not found")
     with open(get_list_file(branch),"w+") as list:
-        list.write("deb http://repo.ubports.com/ %s main" % branch)
+        list.write("deb http://repo.ubports.com/ {} main".format(branch))
 
 def remove_list(branch):
     # If it does not exist, just ignore
@@ -66,13 +99,13 @@ def remove_list(branch):
     os.remove(get_list_file(branch))
 
 def get_github_pr(repo, num):
-    ret = requests.get(GITHUB_API_PULLREQUEST % (repo, num))
+    ret = requests.get(GITHUB_API_PULLREQUEST.format(repo=repo, num=num))
     if ret.status_code != 200:
         die("Pull-Request not found")
     return ret.json()
 
 def get_jenkins_build(repo, ref):
-    ret = requests.get(JENKINS_API_BUILD % (repo, ref))
+    ret = requests.get(JENKINS_API_BUILD.format(repo=repo, ref=ref))
     if ret.status_code != 200:
         die("Jenkins build not found")
     return ret.json()
@@ -90,65 +123,72 @@ def get_issue_status(repo, ref):
 def get_issue_branch(repo, num):
     return get_github_pr(repo, num)["head"]["ref"]
 
+def print_error(error):
+    """Prints red text found in error"""
+    print('\033[91m' + error + '\033[0m')
+
 def die(m):
-    print(m)
-    exit()
+    """Prints red text m and exits with status 3"""
+    print_error(m)
+    exit(3)
 
-def install(args):
-    ensure_root()
+def install_command(args):
     if args.pr != -1:
         args.repo.replace("ubports/", "")
-        ref = get_issue_branch(args.repo, args.pr);
-        status = get_issue_status(args.repo, ref);
+        ref = get_issue_branch(args.repo, args.pr)
+        status = get_issue_status(args.repo, ref)
         if status == Status.FAILED:
             die("Issue failed to build")
         if status == Status.BUILDING:
             die("Issue is currently building")
-    add_list(args.repo)
-    apt_update()
-    apt_upgrade()
+    with WritableRootFS():
+        add_list(args.repo)
+        apt_update()
+        apt_upgrade()
 
-def remove(args):
-    ensure_root()
+def remove_command(args):
     if not list_exists(args.repo):
-        die("Repo %s is not installed" % args.repo)
-    remove_list(args.repo)
-    update(args)
+        die("Repo {} is not installed".format(args.repo))
+    with WritableRootFS():
+        remove_list(args.repo)
+        apt_update()
+        apt_upgrade()
 
-def list(args):
+def list_command(args):
     print(" ".join(list_lists()))
 
-def update(args):
-    ensure_root()
-    mount()
-    apt_update()
-    apt_upgrade()
-    unmount()
+def update_command(args):
+    with WritableRootFS():
+        apt_update()
+        apt_upgrade()
 
 parser = argparse.ArgumentParser(description='The UBports QA scripts allow you to efficiently manage PPAs from repo.ubports.com for testing deb components. See http://docs.ubports.com/en/latest/about/process/ppa.html.')
 subparsers = parser.add_subparsers(help='')
 
 parser_install = subparsers.add_parser('install', help='Install a ppa or pull-request', description='Install a ppa or pull-request. See http://docs.ubports.com/en/latest/about/process/ppa.html.')
 parser_install.add_argument('repo', type=str, help='Name of a PPA on repo.ubports.com. Alternatively, if the \'pr\' argument is provided, the name of a git repository can be specified to automatically add the PPA from a pull-request.')
 parser_install.add_argument('pr', type=int, help='Numeric ID of a pull-request on the git repository specified in the \'repo\' argument. If \'repo\' is supposed to be the name of a ppa, the \'pr\' argument should not be specified.', nargs='?', default=-1)
-parser_install.set_defaults(func=install)
+parser_install.set_defaults(func=install_command)
 
 parser_remove = subparsers.add_parser('remove', help='Remove and uninstall a PPA', description='Remove and uninstall a ppa')
 parser_remove.add_argument('repo', type=str, help='Name of the ppa')
-parser_remove.set_defaults(func=remove)
+parser_remove.set_defaults(func=remove_command)
 
 parser_list = subparsers.add_parser('list', help='List installed PPAs', description='List installed PPAs')
-parser_list.set_defaults(func=list)
+parser_list.set_defaults(func=list_command)
 
 parser_update = subparsers.add_parser('update', help='Update all packages using apt', description='Update all packages using apt')
-parser_update.set_defaults(func=update)
+parser_update.set_defaults(func=update_command)
 
 try:
     args = parser.parse_args()
     args.func(args)
 except IOError as e:
+    # We weren't allowed to do something with a file.
+    # Either we aren't root or the disk is read-only.
     ensure_root()
     die(e)
 except AttributeError as e:
+    # The user typed an incorrect command
     parser.print_help()
-    exit()
+    exit(4)