From cd59b94ded971613c8b46d02fea0ad7b1e966d0c Mon Sep 17 00:00:00 2001
From: Lars Karlitski <lars@karlitski.net>
Date: Tue, 24 Sep 2019 16:11:53 +0200
Subject: [PATCH] tree-wide: always explicitly pass `check` to subprocess.run

pylint recently started recommending this.
---
 osbuild-run               | 10 ++++++----
 osbuild/buildroot.py      |  5 ++++-
 stages/org.osbuild.dnf    | 10 +++++-----
 stages/org.osbuild.script |  2 +-
 stages/org.osbuild.yum    | 12 ++++++------
 5 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/osbuild-run b/osbuild-run
index eda46430..da8245c2 100755
--- a/osbuild-run
+++ b/osbuild-run
@@ -28,7 +28,9 @@ def update_ca_trust():
     os.makedirs("/etc/pki/ca-trust/extracted/pem")
     os.makedirs("/etc/pki/tls/certs")
     os.symlink("/etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem", "/etc/pki/tls/certs/ca-bundle.crt")
-    subprocess.run(["update-ca-trust"])
+
+    # allow to fail, because it sometimes mysteriously does
+    subprocess.run(["update-ca-trust", "extract"], check=False)
 
 
 def append_certs(cert_conf, dir_fd, parents=b""):
@@ -51,13 +53,13 @@ def update_ca_certificates():
     os.makedirs("/etc/ssl/certs")
     with open("/etc/ca-certificates.conf", "wb") as f:
         append_certs(f, os.open("/usr/share/ca-certificates", os.O_DIRECTORY))
-    subprocess.run(["update-ca-certificates"])
+    subprocess.run(["update-ca-certificates"], check=True)
 
 
 def tmpfiles():
     # Allow systemd-tmpfiles to return non-0. Some packages want to create
     # directories owned by users that are not set up with systemd-sysusers.
-    subprocess.run(["systemd-tmpfiles", "--create"])
+    subprocess.run(["systemd-tmpfiles", "--create"], check=False)
 
 
 def nsswitch():
@@ -78,5 +80,5 @@ if __name__ == "__main__":
     tmpfiles()
     nsswitch()
 
-    r = subprocess.run(sys.argv[1:])
+    r = subprocess.run(sys.argv[1:], check=False)
     sys.exit(r.returncode)
diff --git a/osbuild/buildroot.py b/osbuild/buildroot.py
index 11f98533..b03a767b 100644
--- a/osbuild/buildroot.py
+++ b/osbuild/buildroot.py
@@ -67,6 +67,9 @@ class BuildRoot:
         Its arguments mean the same as those for subprocess.run().
         """
 
+        # pylint suggests to epxlicitly pass `check` to subprocess.run()
+        check = kwargs.pop("check", False)
+
         return subprocess.run([
             "systemd-nspawn",
             "--quiet",
@@ -77,7 +80,7 @@ class BuildRoot:
             f"--directory={self.root}",
             *[f"--bind={b}" for b in (binds or [])],
             *[f"--bind-ro={b}" for b in [f"{self.api}:/run/osbuild/api"] + (readonly_binds or [])],
-            ] + argv, **kwargs)
+            ] + argv, check=check, **kwargs)
 
     @contextlib.contextmanager
     def bound_socket(self, name):
diff --git a/stages/org.osbuild.dnf b/stages/org.osbuild.dnf
index 47c2d81b..a186e753 100755
--- a/stages/org.osbuild.dnf
+++ b/stages/org.osbuild.dnf
@@ -36,11 +36,11 @@ def main(tree, options):
         mount -o bind /sys {tree}/sys
         mount -o bind /proc {tree}/proc
     """
-    returncode = subprocess.run(["/bin/sh", "-c", script]).returncode
-
-    if returncode != 0:
-        print(f"setting up API VFS in target tree failed: {returncode}")
-        return returncode
+    try:
+        subprocess.run(["/bin/sh", "-c", script], check=True)
+    except subprocess.CalledProcessError as err:
+        print(f"setting up API VFS in target tree failed: {err.returncode}")
+        return err.returncode
 
     cmd = [
         "dnf", "-yv",
diff --git a/stages/org.osbuild.script b/stages/org.osbuild.script
index b7a51cc9..b9950e77 100755
--- a/stages/org.osbuild.script
+++ b/stages/org.osbuild.script
@@ -18,7 +18,7 @@ def main(tree, options):
     os.chmod(scriptfile, 0o550)
     atexit.register(lambda: os.unlink(scriptfile))
 
-    return subprocess.run(["chroot", tree, "/osbuild-script"]).returncode
+    return subprocess.run(["chroot", tree, "/osbuild-script"], check=False).returncode
 
 
 if __name__ == '__main__':
diff --git a/stages/org.osbuild.yum b/stages/org.osbuild.yum
index ec36e57f..55b2bd59 100755
--- a/stages/org.osbuild.yum
+++ b/stages/org.osbuild.yum
@@ -36,11 +36,11 @@ def main(tree, options):
         mount -t sysfs none {tree}/sys
         mount -t proc none {tree}/proc
     """
-    returncode = subprocess.run(["/bin/sh", "-c", script]).returncode
-
-    if returncode != 0:
-        print(f"setting up API VFS in target tree failed: {returncode}")
-        return returncode
+    try:
+        subprocess.run(["/bin/sh", "-c", script], check=True)
+    except subprocess.CalledProcessError as err:
+        print(f"setting up API VFS in target tree failed: {err.returncode}")
+        return err.returncode
 
     cmd = [
         "yum", "-y", "-v",
@@ -53,7 +53,7 @@ def main(tree, options):
     ] + packages
 
     print(" ".join(cmd), flush=True)
-    return subprocess.run(cmd).returncode
+    return subprocess.run(cmd, check=False).returncode
 
 
 if __name__ == '__main__':