Merge lp:~glyph/divmod.org/combinator-py3 into lp:divmod.org

[Glyph Lefkowitz]

Support enough of python 3 in Combinator so that it doesn't raise an exception when being imported. This is convenient because it allows for Twisted development in a Combinator-managed checkout without having to re-set PYTHONPATH.

[Tristan Seligmann (mithrandi)]

The rest of this looks good to me, but the following code seems problematic:

40 - print 'removing unknown:', unknownFile
41 + print('removing unknown: ' + str(unknownFile))

I'm pretty sure this is going to blow up with some encode or decode error if `unknownFile` contains non-ASCII characters, so not only will Combinator (I guess just chbranch/mkbranch?) explode, but you won't even be able to determine which file is making it explode.

...either that, or the code is already broken, in which case I guess this is a moot point.

review: Needs Fixing

[Jean-Paul Calderone (exarkun)]

That changed line is also without test coverage. As far as I can tell, it's the *only* uncovered changed line in the branch (it is hard to get coverage reporting on `sitecustomize.py` though).

`unknownFile` is clearly a string. It is less clear whether that is a byte string or a unicode string (on Python 3). But on Python 2 it's clearly a byte string. Exercising that branch on Python 2 should be pretty easy, I'll give it a shot.

[Jean-Paul Calderone (exarkun)]

See lp:~exarkun/divmod.org/combinator-py3 where I added test coverage for this. Not sure what the proper Launchpad merge proposal etiquette is for this. Please merge my branches into this branch and your branch into trunk?

review: Approve

[Glyph Lefkowitz (glyph)]

I guess this is approved now?

[Jean-Paul Calderone (exarkun)]

I guess a reviewer is supposed to submit an "Approve" review and also separately edit the ticket state to be "Approved"?

=== modified file 'Combinator/combinator/branchmgr.py'
--- Combinator/combinator/branchmgr.py	2009-11-25 18:48:46 +0000
+++ Combinator/combinator/branchmgr.py	2013-02-16 05:53:19 +0000
@@ -125,11 +125,11 @@
     display its output.
     """
     popenstr = commandString(args)
-    print prompt(popenstr)
+    print(prompt(popenstr))
 
     output, code = runCommand(popenstr)
 
-    print 'C: ' + '\nC: '.join(output.splitlines())
+    print('C: ' + '\nC: '.join(output.splitlines()))
     return checkStatus(popenstr, output, code)
 
 
@@ -339,7 +339,7 @@
             if yth.endswith('.bch'):
                 yth = os.path.join(self.sitePathsPath, yth)
                 projName = os.path.splitext(os.path.split(yth)[-1])[0]
-                branchPath = file(yth).read().strip()
+                branchPath = open(yth).read().strip()
                 yield projName, branchPath
 
 
@@ -378,7 +378,7 @@
 
     def currentBranchFor(self, projectName):
 
-        return file(os.path.join(self.sitePathsPath, projectName)+'.bch'
+        return open(os.path.join(self.sitePathsPath, projectName)+'.bch'
                     ).read().strip()
 
 
@@ -506,7 +506,7 @@
                     for line in statusf.splitlines():
                         if line[0] == '?' or line[0] == 'I':
                             unknownFile = line[7:].strip()
-                            print 'removing unknown:', unknownFile
+                            print('removing unknown: ' + str(unknownFile))
                             if os.path.isdir(unknownFile):
                                 shutil.rmtree(unknownFile)
                             else:
@@ -518,7 +518,7 @@
             if not os.path.exists(self.sitePathsPath):
                 os.makedirs(self.sitePathsPath)
 
-            f = file(os.path.join(self.sitePathsPath, projectName) + '.bch', 'w')
+            f = open(os.path.join(self.sitePathsPath, projectName) + '.bch', 'w')
             f.write(branchRelativePath)
             f.close()
         finally:
@@ -565,9 +565,9 @@
                     if not os.path.exists(dst):
                         stream.write('link: %r => %r\n <on account of %r>\n' %
                                      (dst, src, ent))
-                        file(dst, 'w').write(file(src).read())
+                        open(dst, 'w').write(open(src).read())
                         if os.name != 'nt':
-                            os.chmod(dst, 0755)
+                            os.chmod(dst, 0o755)
 
 
 
@@ -623,21 +623,21 @@
     """
     try:
         return operation(*args)
-    except MissingTrunkLocation, e:
+    except MissingTrunkLocation as e:
         raise SystemExit(
             "The location of %r trunk is not known.  Specify a URI as the "
             "3rd argument to check out a branch (check out trunk to make "
             "this unnecessary)." % e.args)
-    except NonExistentBranch, e:
+    except NonExistentBranch as e:
         raise SystemExit(
             "No such branch: %r" % e.args)
-    except DuplicateBranch, e:
+    except DuplicateBranch as e:
         raise SystemExit(
             "Branch named %r exists already." % e.args)
     except UncleanTrunkWorkingCopy:
         raise SystemExit(
             "Can't unbranch while trunk working copy contains modifications.")
-    except InvalidBranch, e:
+    except InvalidBranch as e:
         raise SystemExit("Cannot merge trunk.")
 
 
@@ -680,10 +680,10 @@
     for k, v in _combinatorMain(theBranchManager.getCurrentBranches):
         if whichBranch is not None:
             if k == whichBranch:
-                print v
+                print(v)
                 break
         else:
-            print k + ":", v
+            print(k + ": " + v)
 
 
 

=== modified file 'Combinator/combinator/xsite.py'
--- Combinator/combinator/xsite.py	2009-03-31 19:25:11 +0000
+++ Combinator/combinator/xsite.py	2013-02-16 05:53:19 +0000
@@ -67,7 +67,7 @@
             if line.startswith("#"):
                 continue
             if line.startswith("import"):
-                exec line
+                exec(line)
                 continue
             line = line.rstrip()
             dir, dircase = makepath(sitedir, line)

=== modified file 'Combinator/sitecustomize.py'
--- Combinator/sitecustomize.py	2009-03-28 18:35:36 +0000
+++ Combinator/sitecustomize.py	2013-02-16 05:53:19 +0000
@@ -1,11 +1,10 @@
 
 import sys
-import os
 
 from combinator.branchmgr import theBranchManager
 theBranchManager.addPaths()
 
-for key in sys.modules.keys():
+for key in list(sys.modules.keys()):
     # Unload all Combinator modules that had to be loaded in order to call
     # addPaths().  Although the very very beginning of this script needs to
     # load the trunk combinator (or whichever one your shell points at), once