Convert on-the-pull to Python3

It now also passes pytype and flake8 checks.

Also, improve logging in the case of errors

1 files changed, 80 insertions, 90 deletions

diff --git a/modules/gitmirror/templates/on-the-pull b/modules/gitmirror/templates/on-the-pull
index 0427bbff..7d3ede29 100755
--- a/modules/gitmirror/templates/on-the-pull
+++ b/modules/gitmirror/templates/on-the-pull
@@ -1,45 +1,18 @@
-#!/usr/bin/python
+#!/usr/bin/python3
 
-import sys
+import cgi
+import http.server
 import os
 import pwd
-import BaseHTTPServer
-import cgi
 import re
 import subprocess
-# For Python 2.4 compatibility, favour optparse
+import sys
 from optparse import OptionParser
-from time import sleep
+from queue import Queue
 from threading import Thread
-from Queue import Queue
-
-
-class UpdaterQueue(Queue):
-    # Python 2.4 Queue compatibility methods
 
-    def task_done(self):
-        """(Wrapper for Python 2.4 compatibility) Indicate that a formerly enqueued task is complete. Used for join().
 
-        WARNING: Does nothing in Python 2.4 for now
-        """
-        # TODO: Make this do something useful in Python 2.4
-        if hasattr(Queue, 'task_done'):
-            return Queue.task_done(self)
-
-    def join(self):
-        """(Wrapper for Python 2.4 compatibility) Blocks until all items in the Queue have been gotten and processed.
-
-        WARNING: Does nothing in Python 2.4 for now.
-        """
-        # TODO: Make this do something useful in Python 2.4
-        if hasattr(Queue, 'join'):
-            return Queue.join(self)
-
-    def wait(self):
-        "DEPRECATED: Use `join()` instead. Block until all jobs are completed."
-        self.join()
-
-GitUpdaterQueue = UpdaterQueue(0)
+GitUpdaterQueue = Queue(0)
 
 
 # NB The following class and bits for running git commands were "liberated"
@@ -54,11 +27,12 @@ class CommandError(Exception):
             'Command "%s" failed with retcode %s' % (' '.join(cmd), retcode,)
             )
 
+
 # It is assumed in many places that the encoding is uniformly UTF-8,
 # so changing these constants is unsupported.  But define them here
 # anyway, to make it easier to find (at least most of) the places
 # where the encoding is important.
-(ENCODING, CHARSET) = ('UTF-8', 'utf-8')
+ENCODING = 'UTF-8'
 
 
 # The "git" program (this could be changed to include a full path):
@@ -86,40 +60,38 @@ def choose_git_command():
             # do it.
             cmd = [GIT_EXECUTABLE, '-c', 'foo.bar=baz', '--version']
             read_output(cmd)
-            GIT_CMD = [GIT_EXECUTABLE, '-c', 'i18n.logoutputencoding=%s' % (ENCODING,)]
+            GIT_CMD = [GIT_EXECUTABLE, '-c', f'i18n.logoutputencoding={ENCODING}']
         except CommandError:
             GIT_CMD = [GIT_EXECUTABLE]
 
 
-def read_git_output(args, input=None, keepends=False, **kw):
+def read_git_output(args, inp=None, keepends=False, **kw):
     """Read the output of a Git command."""
 
     if GIT_CMD is None:
         choose_git_command()
 
-    return read_output(GIT_CMD + args, input=input, keepends=keepends, **kw)
+    return read_output(GIT_CMD + args, inp=inp, keepends=keepends, **kw)
 
 
-def read_output(cmd, input=None, keepends=False, **kw):
-    if input:
+# NOTE: output is in bytes, not a string
+def read_output(cmd, inp=None, keepends=False, **kw):
+    if inp:
         stdin = subprocess.PIPE
     else:
         stdin = None
     p = subprocess.Popen(
         cmd, stdin=stdin, stdout=subprocess.PIPE, stderr=subprocess.PIPE, **kw
         )
-    (out, err) = p.communicate(input)
+    (out, err) = p.communicate(inp)
     retcode = p.wait()
     if retcode:
         raise CommandError(cmd, retcode)
     if not keepends:
-        out = out.rstrip('\n\r')
+        out = out.rstrip(b'\n\r')
     return out
 
 
-
-
-
 def run_git_command(args, **kw):
     """Runs a git command, ignoring the output.
     """
@@ -149,14 +121,14 @@ class GitUpdater(Thread):
             if repo is None:
                 break
             try:
-                print >> sys.stderr, "Got update request for '%s'" % repo
+                print(f"Got update request for '{repo}'", file=sys.stderr)
                 clonefolder = os.path.join(self.basedir, repo)
                 if self.repoprefix:
                     if not repo.startswith(self.repoprefix):
-                       print >> sys.stderr, "Ignoring repo '%s' due to invalid prefix" % repo
-                       GitUpdaterQueue.task_done()
-                       continue
-                    clonefolder = os.path.join(self.basedir, repo[len(self.repoprefix):]) 
+                        print(f"Ignoring repo '{repo}' due to invalid prefix", file=sys.stderr)
+                        GitUpdaterQueue.task_done()
+                        continue
+                    clonefolder = os.path.join(self.basedir, repo[len(self.repoprefix):])
                 command = []
                 treeish = ''
                 changed = True
@@ -170,18 +142,18 @@ class GitUpdater(Thread):
                         command.append('--mirror')
                     command.append(cloneurl)
                     command.append(clonefolder)
-                    print >> sys.stderr, "Cloning repo '%s' ('%s' -> '%s')" % (repo, cloneurl, clonefolder)
+                    print(f"Cloning repo '{repo}' ('{cloneurl}' -> '{clonefolder}')", file=sys.stderr)
 
                     run_git_command(command)
                     if not os.path.isdir(clonefolder):
-                        raise Exception("Clone folder '%s' is not a directory. Cloning failed or file in it's place?" % clonefolder)
+                        raise Exception(f"Clone folder '{clonefolder}' is not a directory. Cloning failed or file in it's place?")
                     os.chdir(clonefolder)
                     if '--mirror' != self.branch and 'master' != self.branch:
                         command = ['checkout', '-t', 'origin/' + self.branch]
                         run_git_command(command)
                 elif os.path.isdir(clonefolder):
                     os.chdir(clonefolder)
-                    print >> sys.stderr, "Updating existing repo '%s' ('%s')" % (repo, clonefolder)
+                    print(f"Updating existing repo '{repo}' ({clonefolder})", file=sys.stderr)
                     command = ['remote', 'update']
                     run_git_command(command)
                     if '--mirror' != self.branch:
@@ -190,18 +162,18 @@ class GitUpdater(Thread):
                         if sha1before and sha1after:
                             if sha1before == sha1after:
                                 changed = False
-                                print >> sys.stderr, "Repo '%s' update on branch '%s': No changed detected" % (repo, self.branch)
+                                print(f"Repo '{repo}' update on branch '{self.branch}': No changed detected", file=sys.stderr)
                             else:
-                                treeish = sha1before + '..' + sha1after
-                                print >> sys.stderr, "Repo '%s' update on branch '%s': Treeish '%s'" % (repo, self.branch, treeish)
+                                treeish = sha1before.decode(ENCODING) + '..' + sha1after.decode(ENCODING)
+                                print(f"Repo '{repo}' update on branch '{self.branch}': Treeish '{treeish}'", file=sys.stderr)
                         else:
-                            print >> sys.stderr, "Repo '%s' update on branch '%s': Before or after sha1 could not be extracted." % (repo, self.branch)
+                            print(f"Repo '{repo}' update on branch '{self.branch}': Before or after sha1 could not be extracted.", file=sys.stderr)
                         command = ['update-ref', 'refs/heads/' + self.branch, 'refs/remotes/origin/' + self.branch]
                         run_git_command(command)
                         command = ['checkout', '-f', self.branch]
                         run_git_command(command)
                 else:
-                    raise Exception("Clone folder '%s' is appears to be a file :s" % clonefolder)
+                    raise Exception(f"Clone folder '{clonefolder}' appears to be a file :s")
 
                 if changed and self.cmd:
                     # Udate the info/web/last-modified file as used by cgit
@@ -211,45 +183,58 @@ class GitUpdater(Thread):
                         command += [treeish]
                     run_command(command)
 
-                print >> sys.stderr, "Update for '%s' complete." % repo
-            except Exception, e:
-                print >> sys.stderr, "Error processing repo '%s'" % repo
-                print >> sys.stderr, str(e)
+                print(f"Update for '{repo}' complete.", file=sys.stderr)
+            except Exception as e:
+                print(f"Error processing repo '{repo}'", file=sys.stderr)
+                print(str(e), file=sys.stderr)
 
             GitUpdaterQueue.task_done()
 
-class TimeoutServer(BaseHTTPServer.HTTPServer):
+
+class TimeoutServer(http.server.HTTPServer):
     def get_request(self):
         result = self.socket.accept()
         result[0].settimeout(10)
         return result
 
-class PostHandler(BaseHTTPServer.BaseHTTPRequestHandler):
+
+class PostHandler(http.server.BaseHTTPRequestHandler):
     def do_POST(self):
-        ctype, pdict = cgi.parse_header(self.headers.getheader('content-type'))
+        ctype, pdict = cgi.parse_header(self.headers['content-type'])
         repo = ""
         try:
-          if ctype != 'x-git/repo':
-              self.send_response(415)
-              return
-
-          length = int(self.headers.getheader('content-length'))
-          if length < 1:
-              self.send_response(411)
-              return
-          if length > 1024:
-              self.send_response(413)
-              return
-          repo = self.rfile.read(length)
-
-          if re.match("^[-_/a-zA-Z0-9\+\.]+$", repo) is None:
-              self.send_response(400)
-              return
-
-          GitUpdaterQueue.put(repo)
-          self.send_response(202)
-        except:
-          print >> sys.stderr, "Error"
+            if ctype != 'x-git/repo':
+                self.send_response(415)
+                self.end_headers()
+                return
+
+            # chunked mode is a legitimate reason there would be no content-length,
+            # but it's easier to just insist on it
+            length = int(self.headers['content-length']) if self.headers['content-length'] else 0
+            if length < 1:
+                self.send_response(411)
+                self.end_headers()
+                return
+            if length > 1024:
+                self.send_response(413)
+                self.end_headers()
+                return
+            repo = self.rfile.read(length).decode(ENCODING)
+
+            if re.match(r"^[-_/a-zA-Z0-9\+\.]+$", repo) is None:
+                self.send_response(400)
+                self.end_headers()
+                return
+
+            GitUpdaterQueue.put(repo)
+            self.send_response(202)
+            self.end_headers()
+
+        except Exception as e:
+            print("Error processing request", file=sys.stderr)
+            print(str(e), file=sys.stderr)
+            self.send_response(500)
+            self.end_headers()
 
 
 def Demote(pidfile, uid, gid):
@@ -278,7 +263,7 @@ def daemonise(options, serverprefix, basefolder):
     else:
         pw = pwd.getpwnam(os.getlogin())
 
-    user    = pw.pw_name
+    user = pw.pw_name
     dirname = pw.pw_dir
     env = {
       'HOME': dirname,
@@ -332,7 +317,10 @@ e.g. curl --header 'Content-Type: x-git/repo' --data 'my/repo/name' http://local
                       help="The branch to track on clone. If you pass '--mirror' (the default) as the branch name we will clone as a bare mirror")
     parser.add_option("-c", "--cmd",
                       type="string", dest="cmd", default="",
-                      help="Third party command to exectue after updates. It will execute in the folder of the repo and if we're not in mirror mode, a treeish will be passed as the only argument containing the refs that changed otherwise the command will be run without any arguments")
+                      help="Third party command to exectue after updates. It will execute in the "
+                      "folder of the repo and if we're not in mirror mode, a treeish will be "
+                      "passed as the only argument containing the refs that changed otherwise "
+                      "the command will be run without any arguments")
     parser.add_option("-d", "--pid-file",
                       type="string", dest="pidfile", default="",
                       help="Daemonise and write pidfile")
@@ -356,14 +344,16 @@ e.g. curl --header 'Content-Type: x-git/repo' --data 'my/repo/name' http://local
     if options.user:
         parser.error("You can only specify a user if you're also deamonising (with a pid file).")
 
+    print("Server started", file=sys.stderr)
+    srvr = TimeoutServer((options.addr, options.port), PostHandler)
+    GitUpdater(serverprefix, basefolder, options.repoprefix, options.branch, options.cmd).start()
+
     try:
-        print >> sys.stderr, "Server started"
-        srvr = TimeoutServer((options.addr, options.port), PostHandler)
-        GitUpdater(serverprefix, basefolder, options.repoprefix, options.branch, options.cmd).start()
         srvr.serve_forever()
     except KeyboardInterrupt:
         GitUpdaterQueue.put(None)
         srvr.socket.close()
 
+
 if __name__ == "__main__":
     main()