Accidentally modify the whole script accidentally

README.md

@@ -34,3 +34,8 @@ optional arguments:
 | `sudo ubports-qa remove xenial_-_somebranch` | Remove the `xenial_-_somebranch` ppa and upgrade all packages |
 | `ubports-qa list` | List all installed testing-PPAs |
 | `sudo ubports-qa update` | Upgrade all packages |
+
+
+### Contributing
+
+When modifying the ubports-qa script, run [black](https://github.com/ambv/black) before you commit. This helps to reduce the diffs between commits and keeps formatting simple.

debian/changelog

@@ -1,3 +1,10 @@
+ubports-qa-scripts (0.2) xenial; urgency=medium
+
+  * Update code style
+  * Fix script not mounting root filesystem read-write on several subcommands
+
+ -- Dalton Durst <dalton@ubports.com>  Fri, 7 Sep 2018 14:09:03 -0500
+
 ubports-qa-scripts (0.1) xenial; urgency=medium
 
   * Update to unified script mode

ubports-qa

@@ -5,150 +5,241 @@ 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/{repo}/pulls/{num}"
+JENKINS_API_BUILD = "https://ci.ubports.com/blue/rest/organizations/jenkins/pipelines/{repo}/branches/{ref}"
 
-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"
+IS_ROOT = os.geteuid() == 0
 
-is_root = (os.geteuid() == 0)
 
 class Status(Enum):
     SUCCESS = 1
     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, exc_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:
+    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))
 
+
 def list_lists():
-    return [f.split("ubports-")[1].split(".list")[0]
-           for f in os.listdir("/etc/apt/preferences.d")
-           if os.path.isfile(f) and f.startswith("ubports-")]
+    return [
+        f.split("ubports-")[1].split(".list")[0]
+        for f in os.listdir("/etc/apt/preferences.d")
+        if os.path.isfile(f) and f.startswith("ubports-")
+    ]
+
 
 def add_list(branch):
     if list_exists(branch):
         return
     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)
+    with open(get_list_file(branch), "w+") as repo_list:
+        repo_list.write("deb http://repo.ubports.com/ {} main".format(branch))
+
 
 def remove_list(branch):
     # If it does not exist, just ignore
     if not list_exists(branch):
         return
     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()
 
+
 def get_issue_status(repo, ref):
     build = get_jenkins_build(repo, ref)["latestRun"]
     print(build)
     if build["result"] == "SUCCESS":
         return Status.SUCCESS
     elif build["result"] == "BULDING":
         return Status.BULDING
-    else:
-        return Status.FAILED
+
+    return Status.FAILED
+
 
 def get_issue_branch(repo, num):
     return get_github_pr(repo, num)["head"]["ref"]
 
-def die(m):
-    print(m)
-    exit()
 
-def install(args):
-    ensure_root()
+def print_error(error_message):
+    """Prints error_message in red"""
+    print("\033[91m" + error_message + "\033[0m")
+
+
+def die(error_message):
+    """Prints error_message in red and exits with status 3"""
+    print_error(error_message)
+    exit(3)
+
+
+def install_command(args):
+    """Install a PPA or Pull Request"""
     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()
-    if not list_exists(args.repo):
-        die("Repo %s is not installed" % args.repo)
-    remove_list(args.repo)
-    update(args)
-
-def list(args):
-    print(" ".join(list_lists()))
 
-def update(args):
-    ensure_root()
-    mount()
-    apt_update()
-    apt_upgrade()
-    unmount()
-
-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='')
+def remove_command(args):
+    """Remove and uninstall a PPA"""
+    if not list_exists(args.repo):
+        die("Repo {} is not installed".format(args.repo))
+    with WritableRootFS():
+        remove_list(args.repo)
+        apt_update()
+        apt_upgrade()
 
-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_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)
+def list_command(args):
+    """List installed PPAs"""
+    print(" ".join(list_lists()))
 
-parser_list = subparsers.add_parser('list', help='List installed PPAs', description='List installed PPAs')
-parser_list.set_defaults(func=list)
 
-parser_update = subparsers.add_parser('update', help='Update all packages using apt', description='Update all packages using apt')
-parser_update.set_defaults(func=update)
+def update_command(args):
+    """Update all packages using apt"""
+    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_command.__doc__,
+    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_command)
+
+parser_remove = subparsers.add_parser(
+    "remove", help=remove_command.__doc__, description="Remove and uninstall a ppa"
+)
+parser_remove.add_argument("repo", type=str, help="Name of the ppa")
+parser_remove.set_defaults(func=remove_command)
+
+parser_list = subparsers.add_parser(
+    "list", help=list_command.__doc__, description="List installed PPAs"
+)
+parser_list.set_defaults(func=list_command)
+
+parser_update = subparsers.add_parser(
+    "update", help=update_command.__doc__, description="Update all packages using apt"
+)
+parser_update.set_defaults(func=update_command)
 
 try:
-    args = parser.parse_args()
-    args.func(args)
+    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)