Merge ~cjwatson/launchpad-buildd:flake8 into launchpad-buildd:master

[Colin Watson]

Add basic pre-commit configuration and apply flake8

[Jürgen Gmach (jugmac00)]

LGTM

A couple of re-formats made the code less readable imho - let's have a quick discussion at standup whether the line length limit is set in stone or whether we could just # noqa this rare occasions.

review: Approve

[Colin Watson (cjwatson)]

As mentioned at standup I have no particular objection to the occasional noqa comment, but in this case I found some alternative approaches that avoid line length issues while (IMO) not compromising readability.

diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
 new file mode 100644
index 0000000..8970c21
--- /dev/null
+++ b/.pre-commit-config.yaml
@@ -0,0 +1,13 @@
+repos:
+-   repo: https://github.com/pre-commit/pre-commit-hooks
+    rev: v4.0.1
+    hooks:
+    -   id: check-added-large-files
+    -   id: check-merge-conflict
+    -   id: check-xml
+    -   id: check-yaml
+    -   id: debug-statements
+-   repo: https://github.com/PyCQA/flake8
+    rev: 4.0.1
+    hooks:
+    -   id: flake8
diff --git a/bin/buildrecipe b/bin/buildrecipe
index 4d04c4f..5dcab4a 100755
--- a/bin/buildrecipe
+++ b/bin/buildrecipe
@@ -114,8 +114,8 @@ class RecipeBuilder:
         # using buildd@<hostname>.
         hostname = socket.gethostname()
         email = 'buildd@%s' % hostname
-        lsb_release = subprocess.Popen(['sudo',
-            '/usr/sbin/chroot', self.chroot_path, 'lsb_release',
+        lsb_release = subprocess.Popen([
+            'sudo', '/usr/sbin/chroot', self.chroot_path, 'lsb_release',
             '-r', '-s'], stdout=subprocess.PIPE, universal_newlines=True)
         distroseries_version = lsb_release.communicate()[0].rstrip()
         assert lsb_release.returncode == 0
@@ -190,12 +190,13 @@ class RecipeBuilder:
         control = self.getSourceControl()
         with open(os.path.join(
                 self.apt_dir, "%s.dsc" % package), "w") as dummy_dsc:
-            print(dedent("""\
-                Format: 1.0
-                Source: %(package)s
-                Architecture: any
-                Version: 99:0
-                Maintainer: [e-mail]""") % {"package": package},
+            print(
+                dedent("""\
+                    Format: 1.0
+                    Source: %(package)s
+                    Architecture: any
+                    Version: 99:0
+                    Maintainer: [e-mail]""") % {"package": package},
                 file=dummy_dsc)
             for field in (
                     "Build-Depends", "Build-Depends-Indep",
@@ -208,17 +209,18 @@ class RecipeBuilder:
     def runAptFtparchive(self):
         conf_path = os.path.join(self.apt_dir, "ftparchive.conf")
         with open(conf_path, "w") as conf:
-            print(dedent("""\
-                Dir::ArchiveDir "%(apt_dir)s";
-                Default::Sources::Compress ". bzip2";
-                BinDirectory "%(apt_dir)s" { Sources "Sources"; };
-                APT::FTPArchive::Release {
-                    Origin "buildrecipe-archive";
-                    Label "buildrecipe-archive";
-                    Suite "invalid";
-                    Codename "invalid";
-                    Description "buildrecipe temporary archive";
-                };""") % {"apt_dir": self.apt_dir},
+            print(
+                dedent("""\
+                    Dir::ArchiveDir "%(apt_dir)s";
+                    Default::Sources::Compress ". bzip2";
+                    BinDirectory "%(apt_dir)s" { Sources "Sources"; };
+                    APT::FTPArchive::Release {
+                        Origin "buildrecipe-archive";
+                        Label "buildrecipe-archive";
+                        Suite "invalid";
+                        Codename "invalid";
+                        Description "buildrecipe temporary archive";
+                    };""") % {"apt_dir": self.apt_dir},
                 file=conf)
         ftparchive_env = dict(os.environ)
         ftparchive_env.pop("APT_CONFIG", None)
diff --git a/bin/test_buildd_generatetranslationtemplates b/bin/test_buildd_generatetranslationtemplates
index 85ddc2e..2282f1d 100755
--- a/bin/test_buildd_generatetranslationtemplates
+++ b/bin/test_buildd_generatetranslationtemplates
@@ -28,8 +28,3 @@ build_type = 'translation-templates'
 filemap = {}
 args = {'branch_url': 'no-branch-here-sorry'}
 print(proxy.build(buildid, build_type, chroot_sha1, filemap, args))
-#status = proxy.status()
-#for filename, sha1 in status[3].iteritems():
-#    print(filename)
-#proxy.clean()
-
diff --git a/bin/test_buildd_recipe b/bin/test_buildd_recipe
index 849b28b..d4f5ed6 100755
--- a/bin/test_buildd_recipe
+++ b/bin/test_buildd_recipe
@@ -17,12 +17,14 @@ distroseries_name = 'maverick'
 recipe_text = """# bzr-builder format 0.2 deb-version {debupstream}-0~{revno}
 http://bazaar.launchpad.dev/~ppa-user/+junk/wakeonlan"""
 
+
 def deb_line(host, suites):
     prefix = 'deb http://'
-    if apt_cacher_ng_host != None:
+    if apt_cacher_ng_host is not None:
         prefix += '%s:3142/' % apt_cacher_ng_host
     return '%s%s %s %s' % (prefix, host, distroseries_name, suites)
 
+
 proxy = ServerProxy('http://localhost:8221/rpc')
 print(proxy.echo('Hello World'))
 print(proxy.info())
@@ -44,8 +46,5 @@ print(proxy.build(
             deb_line('%s.archive.ubuntu.com/ubuntu' % country_code,
                      'main universe'),
             deb_line('ppa.launchpad.net/launchpad/bzr-builder-dev/ubuntu',
-                     'main'),]}))
-#status = proxy.status()
-#for filename, sha1 in status[3].iteritems():
-#    print(filename)
-#proxy.clean()
+                     'main'),
+            ]}))
diff --git a/buildd-genconfig b/buildd-genconfig
index 2615b88..8195ef7 100755
--- a/buildd-genconfig
+++ b/buildd-genconfig
@@ -5,42 +5,47 @@
 
 from __future__ import print_function
 
+from optparse import OptionParser
 import os
 
 archtag = os.popen("dpkg --print-architecture").read().strip()
 
-from optparse import OptionParser
-
 parser = OptionParser()
-parser.add_option("-n", "--name", dest="NAME",
-                  help="the name for this buildd",
-                  metavar="NAME",
-                  default="default")
-
-parser.add_option("-H", "--host", dest="BINDHOST",
-                  help="the IP/host this buildd binds to",
-                  metavar="HOSTNAME",
-                  default="localhost")
-
-parser.add_option("-p", "--port", dest="BINDPORT",
-                  help="the port this buildd binds to",
-                  metavar="PORT",
-                  default="8221")
-
-parser.add_option("-a", "--arch", dest="ARCHTAG",
-                  help="the arch tag this buildd claims",
-                  metavar="ARCHTAG",
-                  default=archtag)
-
-parser.add_option("-t", "--template", dest="TEMPLATE",
-                  help="the template file to use",
-                  metavar="FILE",
-                  default="/usr/share/launchpad-buildd/template-buildd-slave.conf")
-
-parser.add_option("--proxy-port", dest="PROXYPORT",
-                  help="the port the local builder proxy binds to",
-                  metavar="PORT",
-                  default="8222")
+parser.add_option(
+    "-n", "--name", dest="NAME",
+    help="the name for this buildd",
+    metavar="NAME",
+    default="default")
+
+parser.add_option(
+    "-H", "--host", dest="BINDHOST",
+    help="the IP/host this buildd binds to",
+    metavar="HOSTNAME",
+    default="localhost")
+
+parser.add_option(
+    "-p", "--port", dest="BINDPORT",
+    help="the port this buildd binds to",
+    metavar="PORT",
+    default="8221")
+
+parser.add_option(
+    "-a", "--arch", dest="ARCHTAG",
+    help="the arch tag this buildd claims",
+    metavar="ARCHTAG",
+    default=archtag)
+
+parser.add_option(
+    "-t", "--template", dest="TEMPLATE",
+    help="the template file to use",
+    metavar="FILE",
+    default="/usr/share/launchpad-buildd/template-buildd-slave.conf")
+
+parser.add_option(
+    "--proxy-port", dest="PROXYPORT",
+    help="the port the local builder proxy binds to",
+    metavar="PORT",
+    default="8222")
 
 (options, args) = parser.parse_args()
 
diff --git a/debian/changelog b/debian/changelog
index 92cb241..a10074a 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,9 @@
+launchpad-buildd (206) UNRELEASED; urgency=medium
+
+  * Fix flake8 violations.
+
+ -- Colin Watson <[e-mail]>  Wed, 08 Dec 2021 15:42:26 +0000
+
 launchpad-buildd (205) bionic; urgency=medium
 
   * Ignore NotAutomatic flag for -proposed and -backports (LP: #1016776).
diff --git a/debian/upgrade-config b/debian/upgrade-config
index 3145a89..db10239 100755
--- a/debian/upgrade-config
+++ b/debian/upgrade-config
@@ -31,6 +31,9 @@ apt_pkg.init()
 
 (old_version, conf_file) = sys.argv[1:]
 
+bin_path = "/usr/share/launchpad-buildd/slavebin"
+
+
 def upgrade_to_12():
     print("Upgrading %s to version 12" % conf_file)
     subprocess.call(["mv", conf_file, conf_file+"-prev12~"])
@@ -38,13 +41,14 @@ def upgrade_to_12():
     out_file = open(conf_file, "w")
     for line in in_file:
         if line.startswith("[debianmanager]"):
-            line += "ogrepath = /usr/share/launchpad-buildd/slavebin/apply-ogre-model\n"
+            line += "ogrepath = %s/apply-ogre-model\n" % bin_path
         if line.startswith("sbuildargs"):
             line = line.replace("-A", "")
         out_file.write(line)
     in_file.close()
     out_file.close()
 
+
 def upgrade_to_34():
     print("Upgrading %s to version 34" % conf_file)
     subprocess.call(["mv", conf_file, conf_file+"-prev34~"])
@@ -52,11 +56,12 @@ def upgrade_to_34():
     out_file = open(conf_file, "w")
     for line in in_file:
         if line.startswith("[debianmanager]"):
-            line += "sourcespath = /usr/share/launchpad-buildd/slavebin/override-sources-list\n"
+            line += "sourcespath = %s/override-sources-list\n" % bin_path
         out_file.write(line)
     in_file.close()
     out_file.close()
 
+
 def upgrade_to_39():
     print("Upgrading %s to version 39" % conf_file)
     subprocess.call(["mv", conf_file, conf_file+"-prev39~"])
@@ -64,13 +69,14 @@ def upgrade_to_39():
     out_file = open(conf_file, "w")
     for line in in_file:
         if line.startswith("sbuildargs"):
-            line = line.replace("-dautobuild ","")
+            line = line.replace("-dautobuild ", "")
         if line.startswith("[slave]"):
             line += "ntphost = ntp.buildd\n"
         out_file.write(line)
     in_file.close()
     out_file.close()
 
+
 def upgrade_to_57():
     print("Upgrading %s to version 57" % conf_file)
     subprocess.call(["mv", conf_file, conf_file+"-prev57~"])
@@ -90,6 +96,7 @@ def upgrade_to_57():
     in_file.close()
     out_file.close()
 
+
 def upgrade_to_58():
     print("Upgrading %s to version 58" % conf_file)
     subprocess.call(["mv", conf_file, conf_file+"-prev58~"])
@@ -98,8 +105,8 @@ def upgrade_to_58():
     out_file.write(in_file.read())
     out_file.write(
         '\n[sourcepackagerecipemanager]\n'
-        'buildrecipepath = /usr/share/launchpad-buildd'
-        '/slavebin/buildrecipe\n')
+        'buildrecipepath = %s/buildrecipe\n' % bin_path)
+
 
 def upgrade_to_59():
     print("Upgrading %s to version 59" % conf_file)
@@ -109,8 +116,9 @@ def upgrade_to_59():
     out_file.write(in_file.read())
     out_file.write(
         '\n[translationtemplatesmanager]\n'
-        'generatepath = /usr/share/launchpad-buildd/slavebin/generate-translation-templates\n'
-        'resultarchive = translation-templates.tar.gz\n')
+        'generatepath = %s/generate-translation-templates\n'
+        'resultarchive = translation-templates.tar.gz\n' % bin_path)
+
 
 def upgrade_to_63():
     print("Upgrading %s to version 63" % conf_file)
@@ -121,6 +129,7 @@ def upgrade_to_63():
         if not line.startswith('ogrepath'):
             out_file.write(line)
 
+
 def upgrade_to_110():
     print("Upgrading %s to version 110" % conf_file)
     subprocess.call(["mv", conf_file, conf_file+"-prev110~"])
@@ -128,11 +137,12 @@ def upgrade_to_110():
     out_file = open(conf_file, "w")
     for line in in_file:
         if line.startswith("[allmanagers]"):
-            line += "preppath = /usr/share/launchpad-buildd/slavebin/slave-prep\n"
+            line += "preppath = %s/slave-prep\n" % bin_path
         out_file.write(line)
     in_file.close()
     out_file.close()
 
+
 def upgrade_to_115():
     print("Upgrading %s to version 115" % conf_file)
     subprocess.call(["mv", conf_file, conf_file+"-prev115~"])
@@ -143,13 +153,15 @@ def upgrade_to_115():
         if line.startswith("[allmanagers]"):
             in_allmanagers = True
         elif in_allmanagers and (line.startswith("[") or not line.strip()):
-            out_file.write("processscanpath = /usr/share/launchpad-buildd/slavebin/scan-for-processes\n")
+            out_file.write(
+                "processscanpath = %s/scan-for-processes\n" % bin_path)
             in_allmanagers = False
         if not line.startswith("processscanpath = "):
             out_file.write(line)
     in_file.close()
     out_file.close()
 
+
 def upgrade_to_120():
     print("Upgrading %s to version 120" % conf_file)
     subprocess.call(["mv", conf_file, conf_file+"-prev120~"])
@@ -158,10 +170,11 @@ def upgrade_to_120():
     out_file.write(in_file.read())
     out_file.write(
         "\n[livefilesystemmanager]\n"
-        "buildlivefspath = /usr/share/launchpad-buildd/slavebin/buildlivefs\n")
+        "buildlivefspath = %s/buildlivefs\n" % bin_path)
     in_file.close()
     out_file.close()
 
+
 def upgrade_to_126():
     print("Upgrading %s to version 126" % conf_file)
     subprocess.call(["mv", conf_file, conf_file+"-prev126~"])
@@ -176,6 +189,7 @@ def upgrade_to_126():
     in_file.close()
     out_file.close()
 
+
 def upgrade_to_127():
     print("Upgrading %s to version 127" % conf_file)
     os.rename(conf_file, conf_file + "-prev127~")
@@ -209,6 +223,7 @@ def upgrade_to_127():
     in_file.close()
     out_file.close()
 
+
 def upgrade_to_162():
     print("Upgrading %s to version 162" % conf_file)
     os.rename(conf_file, conf_file + "-prev162~")
@@ -220,6 +235,7 @@ def upgrade_to_162():
                 "\n[snapmanager]\n"
                 "proxyport = 8222\n")
 
+
 def upgrade_to_190():
     print("Upgrading %s to version 190" % conf_file)
     os.rename(conf_file, conf_file + "-prev190~")
@@ -231,6 +247,7 @@ def upgrade_to_190():
                     line = "[builder]\n"
                 out_file.write(line)
 
+
 def upgrade_to_200():
     print("Upgrading %s to version 200" % conf_file)
 
@@ -273,6 +290,7 @@ def upgrade_to_200():
                         (line.startswith("[") or not line.strip())):
                     in_snapmanager = False
 
+
 if __name__ == "__main__":
     old_version = re.sub(r'[~-].*', '', old_version)
     if apt_pkg.version_compare(old_version, "12") < 0:
diff --git a/lpbuildd/binarypackage.py b/lpbuildd/binarypackage.py
index 7312473..1bbf831 100644
--- a/lpbuildd/binarypackage.py
+++ b/lpbuildd/binarypackage.py
@@ -48,7 +48,11 @@ APT_DUBIOUS_DEP_PATTERNS = [
 
 
 class BuildLogRegexes:
-    """Build log regexes for performing actions based on regexes, and extracting dependencies for auto dep-waits"""
+    """Various build log regexes.
+
+    These allow performing actions based on regexes, and extracting
+    dependencies for auto dep-waits.
+    """
     GIVENBACK = [
         (r"^E: There are problems and -y was used without --force-yes"),
         ]
@@ -131,15 +135,16 @@ class BinaryPackageBuildManager(DebianBuildManager):
             # Use the "plain" chroot type because we do the necessary setup
             # and teardown ourselves: it's easier to do this the same way
             # for all build types.
-            print(dedent('''\
-                [build-{buildid}]
-                description=build-{buildid}
-                groups=sbuild,root
-                root-groups=sbuild,root
-                type=plain
-                directory={chroot_path}
-                ''').format(
-                    buildid=self._buildid, chroot_path=self.chroot_path),
+            print(
+                dedent('''\
+                    [build-{buildid}]
+                    description=build-{buildid}
+                    groups=sbuild,root
+                    root-groups=sbuild,root
+                    type=plain
+                    directory={chroot_path}
+                    ''').format(
+                        buildid=self._buildid, chroot_path=self.chroot_path),
                 file=schroot_file, end='')
             schroot_file.flush()
             subprocess.check_call(
@@ -285,9 +290,8 @@ class BinaryPackageBuildManager(DebianBuildManager):
                 return True
         dep_restrictions = dep.get("restrictions")
         if dep_restrictions is not None:
-            if all(
-                any(restriction.enabled for restriction in restrlist)
-                for restrlist in dep_restrictions):
+            if all(any(restriction.enabled for restriction in restrlist)
+                   for restrlist in dep_restrictions):
                 # This dependency "matches" in the sense that it's ignored
                 # when no build profiles are enabled.
                 return True
diff --git a/lpbuildd/builder.py b/lpbuildd/builder.py
index b8362d3..ad08639 100644
--- a/lpbuildd/builder.py
+++ b/lpbuildd/builder.py
@@ -193,7 +193,8 @@ class BuildManager(object):
             if notify:
                 iterate = partial(self.iterateReap, state)
             else:
-                iterate = lambda success: None
+                def iterate(success):
+                    pass
             self.runTargetSubProcess("scan-for-processes", iterate=iterate)
 
     def doCleanup(self):
@@ -232,9 +233,8 @@ class BuildManager(object):
 
         os.mkdir("%s/build-%s" % (self.home, self._buildid))
         for f in files:
-            os.symlink( self._builder.cachePath(files[f]),
-                        "%s/build-%s/%s" % (self.home,
-                                            self._buildid, f))
+            os.symlink(self._builder.cachePath(files[f]),
+                       "%s/build-%s/%s" % (self.home, self._buildid, f))
         self._chroottarfile = self._builder.cachePath(chroot)
 
         self.image_type = extra_args.get('image_type', 'chroot')
diff --git a/lpbuildd/check_implicit_pointer_functions.py b/lpbuildd/check_implicit_pointer_functions.py
index 7f28338..78df89f 100755
--- a/lpbuildd/check_implicit_pointer_functions.py
+++ b/lpbuildd/check_implicit_pointer_functions.py
@@ -42,12 +42,12 @@ implicit_pattern = re.compile(
 pointer_pattern = re.compile(
     br"([^:]*):(\d+):(\d+:)? warning: "
     br"("
-     br"(assignment"
-     br"|initialization"
-     br"|return"
-     br"|passing arg \d+ of `[^']*'"
-     br"|passing arg \d+ of pointer to function"
-     br") makes pointer from integer without a cast"
+    br"(assignment"
+    br"|initialization"
+    br"|return"
+    br"|passing arg \d+ of `[^']*'"
+    br"|passing arg \d+ of pointer to function"
+    br") makes pointer from integer without a cast"
     br"|"
     br"cast to pointer from integer of different size)")
 
@@ -77,7 +77,7 @@ def filter_log(in_file, out_file, in_line=False):
                 pointer_filename = m.group(1)
                 pointer_linenum = int(m.group(2))
                 if (last_implicit_filename == pointer_filename
-                    and last_implicit_linenum == pointer_linenum):
+                        and last_implicit_linenum == pointer_linenum):
                     err = (
                         b"Function `%s' implicitly converted to pointer at "
                         b"%s:%d" % (
diff --git a/lpbuildd/oci.py b/lpbuildd/oci.py
index aa28a03..c6e103c 100644
--- a/lpbuildd/oci.py
+++ b/lpbuildd/oci.py
@@ -242,7 +242,6 @@ class OCIBuildManager(BuildManagerProxyMixin, DebianBuildManager):
                     source_name, target_name))
             shutil.copy(source_name, target_name)
 
-
         # We need these mapping files
         sha_directory = tempfile.mkdtemp()
         # This can change depending on the kernel options / docker package
@@ -255,7 +254,7 @@ class OCIBuildManager(BuildManagerProxyMixin, DebianBuildManager):
         # we will have no contents from it.
         if self.backend.path_exists(sha_path):
             sha_files = [x for x in self.backend.listdir(sha_path)
-                        if not x.startswith('.')]
+                         if not x.startswith('.')]
             for file in sha_files:
                 self.backend.copy_out(
                     os.path.join(sha_path, file),
diff --git a/lpbuildd/proxy.py b/lpbuildd/proxy.py
index 908770d..de6e3c7 100644
--- a/lpbuildd/proxy.py
+++ b/lpbuildd/proxy.py
@@ -128,15 +128,15 @@ class BuilderProxyRequest(http.Request):
         if self.startedWriting:
             return
         self.startedWriting = 1
-        l = []
-        l.append(
+        lines = []
+        lines.append(
             self.clientproto + b" " + intToBytes(self.code) + b" " +
             self.code_message + b"\r\n")
         for name, values in self.responseHeaders.getAllRawHeaders():
             for value in values:
-                l.extend([name, b": ", value, b"\r\n"])
-        l.append(b"\r\n")
-        self.transport.writeSequence(l)
+                lines.extend([name, b": ", value, b"\r\n"])
+        lines.append(b"\r\n")
+        self.transport.writeSequence(lines)
 
     def write(self, data):
         if self.channel is not None:
diff --git a/lpbuildd/sourcepackagerecipe.py b/lpbuildd/sourcepackagerecipe.py
index b2f46cb..4e237a6 100644
--- a/lpbuildd/sourcepackagerecipe.py
+++ b/lpbuildd/sourcepackagerecipe.py
@@ -117,9 +117,8 @@ class SourcePackageRecipeBuildManager(DebianBuildManager):
                     print("Returning build status: Build failed")
                     self._builder.buildFail()
             self.alreadyfailed = True
-        elif (
-            retcode >= RETCODE_FAILURE_INSTALL and
-            retcode <= RETCODE_FAILURE_BUILD_SOURCE_PACKAGE):
+        elif (retcode >= RETCODE_FAILURE_INSTALL and
+              retcode <= RETCODE_FAILURE_BUILD_SOURCE_PACKAGE):
             # XXX AaronBentley 2009-01-13: We should handle depwait separately
             if not self.alreadyfailed:
                 self._builder.buildFail()
diff --git a/lpbuildd/target/build_charm.py b/lpbuildd/target/build_charm.py
index d5ade71..5d0b3ab 100644
--- a/lpbuildd/target/build_charm.py
+++ b/lpbuildd/target/build_charm.py
@@ -136,4 +136,3 @@ class BuildCharm(BuilderProxyOperationMixin, VCSOperationMixin,
             logger.exception('Build failed')
             return RETCODE_FAILURE_BUILD
         return 0
-
diff --git a/lpbuildd/target/lxd.py b/lpbuildd/target/lxd.py
index 6ddd1d0..163176d 100644
--- a/lpbuildd/target/lxd.py
+++ b/lpbuildd/target/lxd.py
@@ -402,8 +402,8 @@ class LXD(Backend):
         if os.path.islink(resolv_conf):
             resolv_conf = os.path.realpath(resolv_conf)
             if (resolv_conf == "/run/systemd/resolve/stub-resolv.conf" and
-                os.path.isfile("/run/systemd/resolve/resolv.conf")):
-                    resolv_conf = "/run/systemd/resolve/resolv.conf"
+                    os.path.isfile("/run/systemd/resolve/resolv.conf")):
+                resolv_conf = "/run/systemd/resolve/resolv.conf"
 
         self.copy_in(resolv_conf, "/etc/resolv.conf")
 
diff --git a/lpbuildd/target/operation.py b/lpbuildd/target/operation.py
index 7020786..2f9fe64 100644
--- a/lpbuildd/target/operation.py
+++ b/lpbuildd/target/operation.py
@@ -5,8 +5,6 @@ from __future__ import print_function
 
 __metaclass__ = type
 
-import os
-
 from lpbuildd.target.backend import make_backend
 
 
diff --git a/lpbuildd/target/tests/test_lxd.py b/lpbuildd/target/tests/test_lxd.py
index 38d3337..35be3bb 100644
--- a/lpbuildd/target/tests/test_lxd.py
+++ b/lpbuildd/target/tests/test_lxd.py
@@ -312,8 +312,9 @@ class TestLXD(TestCase):
                 ("lxc.network.0.ipv4.gateway", "10.10.10.1"),
                 ])
 
-        raw_lxc_config = "".join("{key}={val}\n".format(key=key, val=val)
-                for key, val in sorted(raw_lxc_config + extra_raw_lxc_config))
+        raw_lxc_config = "".join(
+            "{key}={val}\n".format(key=key, val=val)
+            for key, val in sorted(raw_lxc_config + extra_raw_lxc_config))
 
         expected_config = {
             "security.privileged": "true",
diff --git a/lpbuildd/target/vcs.py b/lpbuildd/target/vcs.py
index 4ddd9e5..f02e540 100644
--- a/lpbuildd/target/vcs.py
+++ b/lpbuildd/target/vcs.py
@@ -57,7 +57,8 @@ class VCSOperationMixin:
         else:
             return ["git"]
 
-    def vcs_fetch(self, name, cwd, env=None, quiet=False, git_shallow_clone=False):
+    def vcs_fetch(self, name, cwd, env=None, quiet=False,
+                  git_shallow_clone=False):
         full_env = OrderedDict()
         full_env["LANG"] = "C.UTF-8"
         full_env["SHELL"] = "/bin/sh"
diff --git a/lpbuildd/tests/fakebuilder.py b/lpbuildd/tests/fakebuilder.py
index 7731ea2..ef20810 100644
--- a/lpbuildd/tests/fakebuilder.py
+++ b/lpbuildd/tests/fakebuilder.py
@@ -108,10 +108,10 @@ class FakeBuilder:
         self._config = FakeConfig()
         self.waitingfiles = {}
         for fake_method in (
-            "emptyLog", "log",
-            "chrootFail", "buildFail", "builderFail", "depFail", "buildOK",
-            "buildComplete",
-            ):
+                "emptyLog", "log",
+                "chrootFail", "buildFail", "builderFail", "depFail", "buildOK",
+                "buildComplete",
+                ):
             setattr(self, fake_method, FakeMethod())
 
     def cachePath(self, file):
diff --git a/lpbuildd/tests/oci_tarball.py b/lpbuildd/tests/oci_tarball.py
index 21ec013..35860ad 100644
--- a/lpbuildd/tests/oci_tarball.py
+++ b/lpbuildd/tests/oci_tarball.py
@@ -17,8 +17,9 @@ class OCITarball:
     @property
     def config(self):
         return self._makeFile(
-            {"rootfs": {"diff_ids":
-                ["sha256:diff1", "sha256:diff2", "sha256:diff3"]}},
+            {"rootfs": {
+                "diff_ids": [
+                    "sha256:diff1", "sha256:diff2", "sha256:diff3"]}},
             'config.json')
 
     @property
@@ -57,10 +58,7 @@ class OCITarball:
         source = os.path.join(source_layer_directory, "layer.tar")
 
         os.mkdir(target_layer_directory)
-        os.symlink(
-            os.path.relpath(source, target_layer_directory),
-            os.path.join(target_layer_directory, "layer.tar")
-        )
+        os.symlink(os.path.relpath(source, target_layer_directory), target)
         return target_layer_directory
 
     def build_tar_file(self):
diff --git a/lpbuildd/tests/test_binarypackage.py b/lpbuildd/tests/test_binarypackage.py
index c88bdb2..22c4b0a 100644
--- a/lpbuildd/tests/test_binarypackage.py
+++ b/lpbuildd/tests/test_binarypackage.py
@@ -134,11 +134,11 @@ class TestBinaryPackageBuildManagerIteration(TestCase):
         self.assertState(
             BinaryPackageBuildState.SBUILD,
             [
-            'sharepath/bin/sbuild-package', 'sbuild-package',
-            self.buildid, 'i386', 'warty',
-            '-c', 'chroot:build-' + self.buildid,
-            '--arch=i386', '--dist=warty', '--nolog',
-            'foo_1.dsc',
+                'sharepath/bin/sbuild-package', 'sbuild-package',
+                self.buildid, 'i386', 'warty',
+                '-c', 'chroot:build-' + self.buildid,
+                '--arch=i386', '--dist=warty', '--nolog',
+                'foo_1.dsc',
             ], final=True)
         self.assertFalse(self.builder.wasCalled('chrootFail'))
 
@@ -490,12 +490,12 @@ class TestBinaryPackageBuildManagerIteration(TestCase):
     def test_relationMatches_versioned(self):
         # relationMatches handles versioned dependencies correctly.
         for version, expected in (
-            (("<<", "1"), False), (("<<", "1.1"), True),
-            (("<=", "0.9"), False), (("<=", "1"), True),
-            (("=", "1"), True), (("=", "2"), False),
-            ((">=", "1"), True), ((">=", "1.1"), False),
-            ((">>", "0.9"), True), ((">>", "1"), False),
-            ):
+                (("<<", "1"), False), (("<<", "1.1"), True),
+                (("<=", "0.9"), False), (("<=", "1"), True),
+                (("=", "1"), True), (("=", "2"), False),
+                ((">=", "1"), True), ((">=", "1.1"), False),
+                ((">>", "0.9"), True), ((">>", "1"), False),
+                ):
             assert_method = self.assertTrue if expected else self.assertFalse
             assert_method(self.buildmanager.relationMatches(
                 {"name": "foo", "version": version}, {"foo": set(["1"])}),
@@ -505,10 +505,10 @@ class TestBinaryPackageBuildManagerIteration(TestCase):
         # If multiple versions of a package are present, relationMatches
         # returns True for dependencies that match any of them.
         for version, expected in (
-            (("=", "1"), True),
-            (("=", "1.1"), True),
-            (("=", "2"), False),
-            ):
+                (("=", "1"), True),
+                (("=", "1.1"), True),
+                (("=", "2"), False),
+                ):
             assert_method = self.assertTrue if expected else self.assertFalse
             assert_method(self.buildmanager.relationMatches(
                 {"name": "foo", "version": version},
diff --git a/lpbuildd/tests/test_buildrecipe.py b/lpbuildd/tests/test_buildrecipe.py
index 2faa985..de7b50a 100644
--- a/lpbuildd/tests/test_buildrecipe.py
+++ b/lpbuildd/tests/test_buildrecipe.py
@@ -277,12 +277,13 @@ class TestRecipeBuilder(TestCase):
         os.makedirs(os.path.dirname(control_path))
         os.makedirs(self.builder.apt_dir)
         with open(control_path, "w") as control:
-            print(dedent("""\
-                Source: foo
-                Build-Depends: debhelper (>= 9~), libfoo-dev
+            print(
+                dedent("""\
+                    Source: foo
+                    Build-Depends: debhelper (>= 9~), libfoo-dev
 
-                Package: foo
-                Depends: ${shlibs:Depends}"""),
+                    Package: foo
+                    Depends: ${shlibs:Depends}"""),
                 file=control)
         self.builder.makeDummyDsc("foo")
         with open(os.path.join(self.builder.apt_dir, "foo.dsc")) as dsc:
@@ -308,15 +309,16 @@ class TestRecipeBuilder(TestCase):
         os.makedirs(os.path.dirname(control_path))
         os.makedirs(self.builder.apt_dir)
         with open(control_path, "w") as control:
-            print(dedent("""\
-                Source: foo
-                Build-Depends: debhelper (>= 9~),
-                               libfoo-dev,
-                # comment line
-                               pkg-config
+            print(
+                dedent("""\
+                    Source: foo
+                    Build-Depends: debhelper (>= 9~),
+                                   libfoo-dev,
+                    # comment line
+                                   pkg-config
 
-                Package: foo
-                Depends: ${shlibs:Depends}"""),
+                    Package: foo
+                    Depends: ${shlibs:Depends}"""),
                 file=control)
         self.builder.makeDummyDsc("foo")
         with open(os.path.join(self.builder.apt_dir, "foo.dsc")) as dsc:
@@ -337,13 +339,14 @@ class TestRecipeBuilder(TestCase):
     def test_runAptFtparchive(self):
         os.makedirs(self.builder.apt_dir)
         with open(os.path.join(self.builder.apt_dir, "foo.dsc"), "w") as dsc:
-            print(dedent("""\
-                Format: 1.0
-                Source: foo
-                Architecture: any
-                Version: 99:0
-                Maintainer: [e-mail]
-                Build-Depends: debhelper (>= 9~), libfoo-dev"""),
+            print(
+                dedent("""\
+                    Format: 1.0
+                    Source: foo
+                    Architecture: any
+                    Version: 99:0
+                    Maintainer: [e-mail]
+                    Build-Depends: debhelper (>= 9~), libfoo-dev"""),
                 file=dsc)
         self.assertEqual(0, self.builder.runAptFtparchive())
         self.assertEqual(
@@ -379,12 +382,13 @@ class TestRecipeBuilder(TestCase):
             # Not a valid changelog, but only the first line matters here.
             print("foo (1.0-1) bionic; urgency=medium", file=changelog)
         with open(control_path, "w") as control:
-            print(dedent("""\
-                Source: foo
-                Build-Depends: debhelper (>= 9~), libfoo-dev
+            print(
+                dedent("""\
+                    Source: foo
+                    Build-Depends: debhelper (>= 9~), libfoo-dev
 
-                Package: foo
-                Depends: ${shlibs:Depends}"""),
+                    Package: foo
+                    Depends: ${shlibs:Depends}"""),
                 file=control)
         self.assertEqual(0, self.builder.installBuildDeps())
         self.assertThat(
diff --git a/lpbuildd/tests/test_charm.py b/lpbuildd/tests/test_charm.py
index 18e01a1..97b4a15 100644
--- a/lpbuildd/tests/test_charm.py
+++ b/lpbuildd/tests/test_charm.py
@@ -101,7 +101,8 @@ class TestCharmBuildManagerIteration(TestCase):
             "git_path": "master",
             }
         expected_options = [
-            "--git-repository", "https://git.launchpad.dev/~example/+git/charm",
+            "--git-repository",
+            "https://git.launchpad.dev/~example/+git/charm",
             "--git-path", "master",
             ]
         yield self.startBuild(args, expected_options)
diff --git a/lpbuildd/tests/test_harness.py b/lpbuildd/tests/test_harness.py
index 049ecd1..0ea496b 100644
--- a/lpbuildd/tests/test_harness.py
+++ b/lpbuildd/tests/test_harness.py
@@ -5,6 +5,6 @@ __metaclass__ = type
 
 import doctest
 
+
 def test_suite():
     return doctest.DocTestSuite('lpbuildd.tests.harness')
-
diff --git a/lpbuildd/tests/test_sourcepackagerecipe.py b/lpbuildd/tests/test_sourcepackagerecipe.py
index dda21d4..7a2cb48 100644
--- a/lpbuildd/tests/test_sourcepackagerecipe.py
+++ b/lpbuildd/tests/test_sourcepackagerecipe.py
@@ -78,7 +78,7 @@ class TestSourcePackageRecipeBuildManagerIteration(TestCase):
             'archives': [
                 'deb http://archive.ubuntu.com/ubuntu maverick main universe',
                 'deb http://ppa.launchpad.net/launchpad/bzr-builder-dev/'
-                    'ubuntu main',
+                'ubuntu main',
                 ],
             }
         if git:
diff --git a/lpbuildd/tests/test_util.py b/lpbuildd/tests/test_util.py
index 3eb92f9..8b04d44 100644
--- a/lpbuildd/tests/test_util.py
+++ b/lpbuildd/tests/test_util.py
@@ -21,7 +21,8 @@ class TestShellEscape(TestCase):
         self.assertEqual("'  '", shell_escape("  "))
 
     def test_single_quotes(self):
-        self.assertEqual("'shell'\"'\"'s great'", shell_escape("shell's great"))
+        self.assertEqual(
+            "'shell'\"'\"'s great'", shell_escape("shell's great"))
 
     def test_bytes(self):
         self.assertEqual(
diff --git a/setup.py b/setup.py
index 32e4498..aa05864 100755
--- a/setup.py
+++ b/setup.py
@@ -57,7 +57,7 @@ setup(
         # XXX cjwatson 2015-11-04: This does in fact require python-apt, but
         # that's normally shipped as a system package and specifying it here
         # causes problems for Launchpad's build system.
-        #'python-apt',
+        # 'python-apt',
         'python-debian>=0.1.23',
         'requests',
         'six',