Some more PEP

src/modules/packages/main.py

@@ -34,26 +34,29 @@ class PackageManager:
         """ Installs packages.
 
         :param pkgs:
+        :param from_local:
         """
         if self.backend == "packagekit":
             for pkg in pkgs:
                 check_chroot_call(["pkcon", "-py", "install", pkg])
         elif self.backend == "zypp":
-            check_chroot_call(
+            check_chroot_call(["zypper", "--non-interactive", "--quiet-install", "install",
-                ["zypper", "--non-interactive", "--quiet-install", "install",
+                               "--auto-agree-with-licenses", "install"] + pkgs)
-                 "--auto-agree-with-licenses", "install"] + pkgs)
         elif self.backend == "yum":
             check_chroot_call(["yum", "install", "-y"] + pkgs)
         elif self.backend == "dnf":
             check_chroot_call(["dnf", "install", "-y"] + pkgs)
         elif self.backend == "urpmi":
-            check_chroot_call(
+            check_chroot_call(["urpmi", "--download-all", "--no-suggests", "--no-verify-rpm",
-                ["urpmi", "--download-all", "--no-suggests", "--no-verify-rpm",
+                               "--fastunsafe", "--ignoresize", "--nolock", "--auto"] + pkgs)
-                 "--fastunsafe", "--ignoresize", "--nolock", "--auto"] + pkgs)
         elif self.backend == "apt":
             check_chroot_call(["apt-get", "-q", "-y", "install"] + pkgs)
         elif self.backend == "pacman":
-            pacman_flags = "-U" if from_local else "-Sy"
+            if from_local:
+                pacman_flags = "-U"
+            else:
+                pacman_flags = "-Sy"
+
             check_chroot_call(["pacman", pacman_flags, "--noconfirm"] + pkgs)
         elif self.backend == "portage":
             check_chroot_call(["emerge", "-v"] + pkgs)
@@ -110,17 +113,19 @@ def run():
     :return:
     """
     backend = libcalamares.job.configuration.get("backend")
+
     if backend not in ("packagekit", "zypp", "yum", "dnf", "urpmi", "apt", "pacman", "portage", "entropy"):
-        return ("Bad backend", "backend=\"{}\"".format(backend))
+        return "Bad backend", "backend=\"{}\"".format(backend)
 
     pkgman = PackageManager(backend)
-
     operations = libcalamares.job.configuration.get("operations", [])
+
     for entry in operations:
         run_operations(pkgman, entry)
 
     if libcalamares.globalstorage.contains("packageOperations"):
         operations = libcalamares.globalstorage.value("packageOperations")
+
         for entry in operations:
             run_operations(pkgman, entry)
 

src/modules/removeuser/main.py

@@ -24,12 +24,11 @@ import libcalamares
 
 def run():
     """ Remove live user from target system """
+    username = libcalamares.job.configuration["username"]
 
-    username = libcalamares.job.configuration[("username")]
     try:
         libcalamares.utils.check_chroot_call(["userdel", "-f", "-r", username])
     except subprocess.CalledProcessError as e:
-        libcalamares.utils.debug( "Cannot remove user. " +
+        libcalamares.utils.debug("Cannot remove user.", "'userdel' terminated with exit code {}.".format(e.returncode))
-                                  "userdel terminated with exit code {}.".format(e.returncode))
 
     return None

src/modules/services/main.py

@@ -24,38 +24,31 @@ import libcalamares
 
 def run():
     """ Setup systemd services """
-
     services = libcalamares.job.configuration['services']
     targets = libcalamares.job.configuration['targets']
 
     # enable services
     for svc in services:
-        ec = libcalamares.utils.chroot_call(['systemctl',
+        ec = libcalamares.utils.chroot_call(['systemctl', 'enable', '{}.service'.format(svc['name'])])
-                                             'enable',
+
-                                             '{}.service'.format(svc['name'])])
         if ec != 0:
             if svc['mandatory']:
                 return "Cannot enable systemd service {}".format(svc['name']), \
                        "systemctl enable call in chroot returned error code {}".format(ec)
             else:
-                libcalamares.utils.debug(
+                libcalamares.utils.debug("Cannot enable systemd service {}".format(svc['name']))
-                    "Cannot enable systemd service {}".format(svc['name']))
+                libcalamares.utils.debug("systemctl enable call in chroot returned error code {}".format(ec))
-                libcalamares.utils.debug(
-                    "systemctl enable call in chroot returned error code {}".format(ec))
 
     # enable targets
     for tgt in targets:
-        ec = libcalamares.utils.chroot_call(['systemctl',
+        ec = libcalamares.utils.chroot_call(['systemctl', 'enable', '{}.target'.format(tgt['name'])])
-                                             'enable',
+
-                                             '{}.target'.format(tgt['name'])])
         if ec != 0:
             if tgt['mandatory']:
                 return "Cannot enable systemd target {}".format(tgt['name']), \
                        "systemctl enable call in chroot returned error code {}".format(ec)
             else:
-                libcalamares.utils.debug(
+                libcalamares.utils.debug("Cannot enable systemd target {}".format(tgt['name']))
-                    "Cannot enable systemd target {}".format(tgt['name']))
+                libcalamares.utils.debug("systemctl enable call in chroot returned error code {}".format(ec))
-                libcalamares.utils.debug(
-                    "systemctl enable call in chroot returned error code {}".format(ec))
 
     return None

src/modules/umount/main.py

@@ -31,10 +31,13 @@ def list_mounts(root_mount_point):
     :return:
     """
     lst = []
+
     for line in open("/etc/mtab").readlines():
         device, mount_point, _ = line.split(" ", 2)
+
         if mount_point.startswith(root_mount_point):
             lst.append((device, mount_point))
+
     return lst
 
 
@@ -44,10 +47,12 @@ def run():
     :return:
     """
     root_mount_point = libcalamares.globalstorage.value("rootMountPoint")
+
     if not root_mount_point:
         return ("No mount point for root partition in globalstorage",
                 "globalstorage does not contain a \"rootMountPoint\" key, "
                 "doing nothing")
+
     if not os.path.exists(root_mount_point):
         return ("Bad mount point for root partition in globalstorage",
                 "globalstorage[\"rootMountPoint\"] is \"{}\", which does not "
@@ -62,4 +67,5 @@ def run():
         subprocess.check_call(["umount", "-lv", mount_point])
 
     os.rmdir(root_mount_point)
+
     return None

src/modules/unpackfs/main.py

@@ -59,10 +59,13 @@ def list_excludes(destination):
     """
     lst = []
     extra_mounts = globalstorage.value("extraMounts")
+
     for extra_mount in extra_mounts:
         mount_point = extra_mount["mountPoint"]
+
         if mount_point:
             lst.extend(['--exclude', mount_point + '/'])
+
     return lst
 
 
@@ -87,11 +90,7 @@ def file_copy(source, dest, progress_cb):
     args = ['rsync', '-aHAXr']
     args.extend(list_excludes(dest))
     args.extend(['--progress', source, dest])
-    process = subprocess.Popen(args,
+    process = subprocess.Popen(args, env=at_env, bufsize=1, stdout=subprocess.PIPE, close_fds=ON_POSIX)
-                               env=at_env,
-                               bufsize=1,
-                               stdout=subprocess.PIPE,
-                               close_fds=ON_POSIX)
 
     for line in iter(process.stdout.readline, b''):
         # small comment on this regexp.
@@ -107,6 +106,7 @@ def file_copy(source, dest, progress_cb):
         # therefore we can easily subtract x from y in order to get real files
         # copied / processed count.
         m = re.findall(r'xfr#(\d+), ir-chk=(\d+)/(\d+)', line.decode())
+
         if m:
             # we've got a percentage update
             num_files_remaining = int(m[0][1])
@@ -117,9 +117,12 @@ def file_copy(source, dest, progress_cb):
             # I guess we're updating every 100 files...
             if num_files_copied % 100 == 0:
                 progress_cb(num_files_copied)
+
     process.wait()
+
     if process.returncode != 0:
         return "rsync failed with error code {}.".format(process.returncode)
+
     return None
 
 
@@ -128,6 +131,7 @@ class UnpackOperation:
 
     :param entries:
     """
+
     def __init__(self, entries):
         self.entries = entries
         self.entry_for_source = dict((x.source, x) for x in self.entries)
@@ -135,6 +139,7 @@ class UnpackOperation:
     def report_progress(self):
         """ Pass progress to user interface """
         progress = float(0)
+
         for entry in self.entries:
             if entry.total == 0:
                 continue
@@ -152,10 +157,10 @@ class UnpackOperation:
         :return:
         """
         source_mount_path = tempfile.mkdtemp()
+
         try:
             for entry in self.entries:
-                imgbasename = os.path.splitext(
+                imgbasename = os.path.splitext(os.path.basename(entry.source))[0]
-                    os.path.basename(entry.source))[0]
                 imgmountdir = os.path.join(source_mount_path, imgbasename)
                 os.mkdir(imgmountdir)
 
@@ -169,20 +174,19 @@ class UnpackOperation:
                                 "Failed to find unsquashfs, make sure you have "
                                 "the squashfs-tools package installed")
 
-                    fslist = subprocess.check_output(["unsquashfs",
+                    fslist = subprocess.check_output(["unsquashfs", "-l", entry.source])
-                                                      "-l",
+
-                                                      entry.source])
                 if entry.sourcefs == "ext4":
-                    fslist = subprocess.check_output(["find",
+                    fslist = subprocess.check_output(["find", imgmountdir, "-type", "f"])
-                                                      imgmountdir,
+
-                                                      "-type", "f"])
                 entry.total = len(fslist.splitlines())
 
                 self.report_progress()
                 error_msg = self.unpack_image(entry, imgmountdir)
+
                 if error_msg:
-                    return ("Failed to unpack image {}".format(entry.source),
+                    return "Failed to unpack image {}".format(entry.source), error_msg
-                            error_msg)
+
             return None
         finally:
             shutil.rmtree(source_mount_path)
@@ -193,12 +197,7 @@ class UnpackOperation:
         :param entry:
         :param imgmountdir:
         """
-        subprocess.check_call(["mount",
+        subprocess.check_call(["mount", entry.source, imgmountdir, "-t", entry.sourcefs, "-o", "loop"])
-                               entry.source,
-                               imgmountdir,
-                               "-t",
-                               entry.sourcefs,
-                               "-o", "loop"])
 
     def unpack_image(self, entry, imgmountdir):
         """ Unpacks image.
@@ -207,7 +206,6 @@ class UnpackOperation:
         :param imgmountdir:
         :return:
         """
-
         def progress_cb(copied):
             """ Copies file to given destination target.
 
@@ -217,9 +215,7 @@ class UnpackOperation:
             self.report_progress()
 
         try:
-            return file_copy(imgmountdir,
+            return file_copy(imgmountdir, entry.destination, progress_cb)
-                             entry.destination,
-                             progress_cb)
         finally:
             subprocess.check_call(["umount", "-l", imgmountdir])
 
@@ -245,14 +241,17 @@ def run():
     PATH_PROCFS = '/proc/filesystems'
 
     root_mount_point = globalstorage.value("rootMountPoint")
+
     if not root_mount_point:
         return ("No mount point for root partition in globalstorage",
                 "globalstorage does not contain a \"rootMountPoint\" key, "
                 "doing nothing")
+
     if not os.path.exists(root_mount_point):
         return ("Bad mount point for root partition in globalstorage",
                 "globalstorage[\"rootMountPoint\"] is \"{}\", which does not "
                 "exist, doing nothing".format(root_mount_point))
+
     unpack = list()
 
     for entry in job.configuration["unpack"]:
@@ -279,12 +278,13 @@ def run():
         destination = os.path.abspath(root_mount_point + entry["destination"])
 
         if not os.path.exists(source) or os.path.isdir(source):
-            return ("Bad source", "source=\"{}\"".format(source))
+            return "Bad source", "source=\"{}\"".format(source)
+
         if not os.path.isdir(destination):
-            return ("Bad destination",
+            return "Bad destination", "destination=\"{}\"".format(destination)
-                    "destination=\"{}\"".format(destination))
 
         unpack.append(UnpackEntry(source, sourcefs, destination))
 
     unpackop = UnpackOperation(unpack)
+
     return unpackop.run()