Merge branch '3448.convert-only-unicode-to-str'

Trac: fixes #3448

Makefile

@@ -19,8 +19,10 @@ PYTHON=python
 export PYTHON
 PYFLAKES=flake8
 export PYFLAKES
+VIRTUAL_ENV=./.tox/py27
 SOURCES=src/allmydata static misc setup.py
 APPNAME=tahoe-lafs
+TEST_SUITE=allmydata
 
 
 # Top-level, phony targets
@@ -45,6 +47,17 @@ test: .tox/create-venvs.log
 	tox --develop -e codechecks
 # Run all the test environments in parallel to reduce run-time
 	tox --develop -p auto -e 'py27,py36,pypy27'
+.PHONY: test-venv-coverage
+## Run all tests with coverage collection and reporting.
+test-venv-coverage:
+# Special handling for reporting coverage even when the test run fails
+	test_exit=
+	$(VIRTUAL_ENV)/bin/coverage run -m twisted.trial --rterrors --reporter=timing \
+		$(TEST_SUITE) || test_exit="$$?"
+	$(VIRTUAL_ENV)/bin/coverage combine
+	$(VIRTUAL_ENV)/bin/coverage xml || true
+	$(VIRTUAL_ENV)/bin/coverage report
+	if [ ! -z "$$test_exit" ]; then exit "$$test_exit"; fi
 .PHONY: test-py3-all
 ## Run all tests under Python 3
 test-py3-all: .tox/create-venvs.log

misc/python3/Makefile

@@ -0,0 +1,53 @@
+# BBB: Python 3 porting targets
+#
+# NOTE: this Makefile requires GNU make
+
+### Defensive settings for make:
+#     https://tech.davis-hansson.com/p/make/
+SHELL := bash
+.ONESHELL:
+.SHELLFLAGS := -xeu -o pipefail -c
+.SILENT:
+.DELETE_ON_ERROR:
+MAKEFLAGS += --warn-undefined-variables
+MAKEFLAGS += --no-builtin-rules
+
+
+# Top-level, phony targets
+
+.PHONY: default
+default:
+	@echo "no default target"
+
+.PHONY: test-py3-all-before
+## Log the output of running all tests under Python 3 before changes
+test-py3-all-before: ../../.tox/make-test-py3-all-old.log
+.PHONY: test-py3-all-diff
+## Compare the output of running all tests under Python 3 after changes
+test-py3-all-diff: ../../.tox/make-test-py3-all.diff
+
+
+# Real targets
+
+# Gauge the impact of changes on Python 3 compatibility
+# Compare the output from running all tests under Python 3 before and after changes.
+# Before changes:
+# `$ rm -f .tox/make-test-py3-all-*.log && make .tox/make-test-py3-all-old.log`
+# After changes:
+# `$ make .tox/make-test-py3-all.diff`
+$(foreach side,old new,../../.tox/make-test-py3-all-$(side).log):
+	cd "../../"
+	tox --develop --notest -e py36-coverage
+	(make VIRTUAL_ENV=./.tox/py36-coverage TEST_SUITE=allmydata \
+		test-venv-coverage || true) | \
+		sed -E 's/\([0-9]+\.[0-9]{3} secs\)/(#.### secs)/' | \
+		tee "./misc/python3/$(@)"
+../../.tox/make-test-py3-all.diff: ../../.tox/make-test-py3-all-new.log
+	(diff -u "$(<:%-new.log=%-old.log)" "$(<)" || true) | tee "$(@)"
+
+# Locate modules that are candidates for naively converting `unicode` -> `str`.
+# List all Python source files that reference `unicode` but don't reference `str`
+../../.tox/py3-unicode-no-str.ls:
+	cd "../../"
+	find src -type f -iname '*.py' -exec grep -l -E '\Wunicode\W' '{}' ';' | \
+		xargs grep -L '\Wstr\W' | xargs ls -ld | tee "./misc/python3/$(@)"

newsfragments/3448.minor

@@ -0,0 +1 @@
+Convert modules that only reference `unicode` to use `str`.

src/allmydata/scripts/common.py

@@ -2,9 +2,16 @@ from __future__ import print_function
 
 import os, sys, urllib, textwrap
 import codecs
-from six.moves.configparser import NoSectionError
 from os.path import join
+
+# BBB: Python 2 compatibility
+from future.utils import PY2
+if PY2:
+    from future.builtins import str  # noqa: F401
+from six.moves.configparser import NoSectionError
+
 from twisted.python import usage
+
 from allmydata.util.assertutil import precondition
 from allmydata.util.encodingutil import unicode_to_url, quote_output, \
     quote_local_unicode_path, argv_to_abspath
@@ -188,7 +195,7 @@ def get_alias(aliases, path_unicode, default):
     and default is not found in aliases, an UnknownAliasError is
     raised.
     """
-    precondition(isinstance(path_unicode, unicode), path_unicode)
+    precondition(isinstance(path_unicode, str), path_unicode)
 
     from allmydata import uri
     path = path_unicode.encode('utf-8').strip(" ")

src/allmydata/scripts/stats_gatherer.py

@@ -1,7 +1,14 @@
 from __future__ import print_function
 
 import os
+
+# BBB: Python 2 compatibility
+from future.utils import PY2
+if PY2:
+    from future.builtins import str  # noqa: F401
+
 from twisted.python import usage
+
 from allmydata.scripts.common import NoDefaultBasedirOptions
 from allmydata.scripts.create_node import write_tac
 from allmydata.util.assertutil import precondition
@@ -62,7 +69,7 @@ def create_stats_gatherer(config):
     err = config.stderr
     basedir = config['basedir']
     # This should always be called with an absolute Unicode basedir.
-    precondition(isinstance(basedir, unicode), basedir)
+    precondition(isinstance(basedir, str), basedir)
 
     if os.path.exists(basedir):
         if listdir_unicode(basedir):

src/allmydata/scripts/tahoe_check.py

@@ -2,7 +2,14 @@ from __future__ import print_function
 
 import urllib
 import json
+
+# BBB: Python 2 compatibility
+from future.utils import PY2
+if PY2:
+    from future.builtins import str  # noqa: F401
+
 from twisted.protocols.basic import LineOnlyReceiver
+
 from allmydata.scripts.common import get_alias, DEFAULT_ALIAS, escape_path, \
                                      UnknownAliasError
 from allmydata.scripts.common_http import do_http, format_http_error
@@ -101,7 +108,7 @@ def check_location(options, where):
 
 def check(options):
     if len(options.locations) == 0:
-        errno = check_location(options, unicode())
+        errno = check_location(options, str())
         if errno != 0:
             return errno
         return 0
@@ -325,7 +332,7 @@ class DeepCheckStreamer(LineOnlyReceiver, object):
 
     def run(self, options):
         if len(options.locations) == 0:
-            errno = self.deepcheck_location(options, unicode())
+            errno = self.deepcheck_location(options, str())
             if errno != 0:
                 return errno
             return 0

src/allmydata/stats.py

@@ -1,13 +1,16 @@
 from __future__ import print_function
 
-from past.builtins import unicode
-
 import json
 import os
 import pprint
 import time
 from collections import deque
 
+# BBB: Python 2 compatibility
+from future.utils import PY2
+if PY2:
+    from future.builtins import str  # noqa: F401
+
 from twisted.internet import reactor
 from twisted.application import service
 from twisted.application.internet import TimerService
@@ -157,7 +160,7 @@ class StatsProvider(Referenceable, service.MultiService):
         service.MultiService.startService(self)
 
     def count(self, name, delta=1):
-        if isinstance(name, unicode):
+        if isinstance(name, str):
             name = name.encode("utf-8")
         val = self.counters.setdefault(name, 0)
         self.counters[name] = val + delta
@@ -178,7 +181,7 @@ class StatsProvider(Referenceable, service.MultiService):
         def to_bytes(d):
             result = {}
             for (k, v) in d.items():
-                if isinstance(k, unicode):
+                if isinstance(k, str):
                     k = k.encode("utf-8")
                 result[k] = v
             return result

src/allmydata/test/check_load.py

@@ -37,6 +37,11 @@ a mean of 10kB and a max of 100MB, so filesize=min(int(1.0/random(.0002)),1e8)
 import os, sys, httplib, binascii
 import urllib, json, random, time, urlparse
 
+# BBB: Python 2 compatibility
+from future.utils import PY2
+if PY2:
+    from future.builtins import str  # noqa: F401
+
 if sys.argv[1] == "--stats":
     statsfiles = sys.argv[2:]
     # gather stats every 10 seconds, do a moving-window average of the last
@@ -116,7 +121,7 @@ def listdir(nodeurl, root, remote_pathname):
     assert nodetype == "dirnode"
     global directories_read
     directories_read += 1
-    children = dict( [(unicode(name),value)
+    children = dict( [(str(name),value)
                       for (name,value)
                       in d["children"].iteritems()] )
     return children

src/allmydata/test/check_memory.py

@@ -1,18 +1,25 @@
 from __future__ import print_function
 
 import os, shutil, sys, urllib, time, stat, urlparse
+
+# BBB: Python 2 compatibility
+from future.utils import PY2
+if PY2:
+    from future.builtins import str  # noqa: F401
 from six.moves import cStringIO as StringIO
+
 from twisted.internet import defer, reactor, protocol, error
 from twisted.application import service, internet
 from twisted.web import client as tw_client
+from twisted.python import log, procutils
+from foolscap.api import Tub, fireEventually, flushEventualQueue
+
 from allmydata import client, introducer
 from allmydata.immutable import upload
 from allmydata.scripts import create_node
 from allmydata.util import fileutil, pollmixin
 from allmydata.util.fileutil import abspath_expanduser_unicode
 from allmydata.util.encodingutil import get_filesystem_encoding
-from foolscap.api import Tub, fireEventually, flushEventualQueue
-from twisted.python import log, procutils
 
 class StallableHTTPGetterDiscarder(tw_client.HTTPPageGetter, object):
     full_speed_ahead = False
@@ -69,7 +76,7 @@ class SystemFramework(pollmixin.PollMixin):
     numnodes = 7
 
     def __init__(self, basedir, mode):
-        self.basedir = basedir = abspath_expanduser_unicode(unicode(basedir))
+        self.basedir = basedir = abspath_expanduser_unicode(str(basedir))
         if not (basedir + os.path.sep).startswith(abspath_expanduser_unicode(u".") + os.path.sep):
             raise AssertionError("safety issue: basedir must be a subdir")
         self.testdir = testdir = os.path.join(basedir, "test")

src/allmydata/test/eliotutil.py

@@ -2,7 +2,10 @@
 Tools aimed at the interaction between tests and Eliot.
 """
 
-from past.builtins import unicode
+# BBB: Python 2 compatibility
+# Can't use `builtins.str` because it's not JSON encodable:
+# `exceptions.TypeError: <class 'future.types.newstr.newstr'> is not JSON-encodeable`
+from past.builtins import unicode as str
 
 __all__ = [
     "RUN_TEST",
@@ -29,7 +32,7 @@ from twisted.internet.defer import (
 
 _NAME = Field.for_types(
     u"name",
-    [unicode],
+    [str],
     u"The name of the test.",
 )
 

src/allmydata/test/mutable/test_version.py

@@ -1,9 +1,16 @@
 from __future__ import print_function
 
 import os
+
+# BBB: Python 2 compatibility
+from future.utils import PY2
+if PY2:
+    from future.builtins import str  # noqa: F401
 from six.moves import cStringIO as StringIO
+
 from twisted.internet import defer
 from twisted.trial import unittest
+
 from allmydata import uri
 from allmydata.interfaces import SDMF_VERSION, MDMF_VERSION
 from allmydata.util import base32, consumer, mathutil
@@ -75,7 +82,7 @@ class Version(GridTestMixin, unittest.TestCase, testutil.ShouldFailMixin, \
             fso = debug.FindSharesOptions()
             storage_index = base32.b2a(n.get_storage_index())
             fso.si_s = storage_index
-            fso.nodedirs = [os.path.dirname(abspath_expanduser_unicode(unicode(storedir)))
+            fso.nodedirs = [os.path.dirname(abspath_expanduser_unicode(str(storedir)))
                             for (i,ss,storedir)
                             in self.iterate_servers()]
             fso.stdout = StringIO()

src/allmydata/test/test_auth.py

@@ -1,3 +1,8 @@
+# BBB: Python 2 compatibility
+from future.utils import PY2
+if PY2:
+    from future.builtins import str  # noqa: F401
+
 from twisted.trial import unittest
 from twisted.python import filepath
 from twisted.cred import error, credentials
@@ -39,7 +44,7 @@ class AccountFileCheckerKeyTests(unittest.TestCase):
     def setUp(self):
         self.account_file = filepath.FilePath(self.mktemp())
         self.account_file.setContent(DUMMY_ACCOUNTS)
-        abspath = abspath_expanduser_unicode(unicode(self.account_file.path))
+        abspath = abspath_expanduser_unicode(str(self.account_file.path))
         self.checker = auth.AccountFileChecker(None, abspath)
 
     def test_unknown_user(self):

src/allmydata/test/test_deepcheck.py

@@ -1,9 +1,22 @@
+import os, json, urllib
+
+# BBB: Python 2 compatibility
+# Can't use `builtins.str` because something deep in Twisted callbacks ends up repr'ing
+# a `future.types.newstr.newstr` as a *Python 3* byte string representation under
+# *Python 2*:
+#   File "/home/rpatterson/src/work/sfu/tahoe-lafs/.tox/py27/lib/python2.7/site-packages/allmydata/util/netstring.py", line 43, in split_netstring
+#    assert data[position] == b","[0], position
+# exceptions.AssertionError: 15
+# ...
+# (Pdb) pp data
+# '334:12:b\'mutable-good\',90:URI:SSK-RO:...
+from past.builtins import unicode as str
 from future.utils import native_str
 
-import os, json, urllib
 from twisted.trial import unittest
 from twisted.internet import defer
 from twisted.internet.defer import inlineCallbacks, returnValue
+
 from allmydata.immutable import upload
 from allmydata.mutable.common import UnrecoverableFileError
 from allmydata.mutable.publish import MutableData
@@ -917,13 +930,13 @@ class DeepCheckWebBad(DeepCheckBase, unittest.TestCase):
         if nodetype == "mutable":
             mutable_uploadable = MutableData("mutable file contents")
             d = self.g.clients[0].create_mutable_file(mutable_uploadable)
-            d.addCallback(lambda n: self.root.set_node(unicode(name), n))
+            d.addCallback(lambda n: self.root.set_node(str(name), n))
         elif nodetype == "large":
             large = upload.Data("Lots of data\n" * 1000 + name + "\n", None)
-            d = self.root.add_file(unicode(name), large)
+            d = self.root.add_file(str(name), large)
         elif nodetype == "small":
             small = upload.Data("Small enough for a LIT", None)
-            d = self.root.add_file(unicode(name), small)
+            d = self.root.add_file(str(name), small)
 
         d.addCallback(self._stash_node, name)