diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 000000000..5a3900722 --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,6 @@ +repos: +- repo: https://gitlab.com/pycqa/flake8 + # TODO: update rev periodically to keep up with tox + rev: 3.8.3 + hooks: + - id: flake8 diff --git a/docs/developer-guide.rst b/docs/developer-guide.rst new file mode 100644 index 000000000..9b40bb283 --- /dev/null +++ b/docs/developer-guide.rst @@ -0,0 +1,44 @@ +Developer Guide +=============== + + +Pre-commit Checks +----------------- + +This project is configured for use with `pre-commit `_ to perform some static code analysis checks. By default, pre-commit behavior is disabled. To enable pre-commit in a local checkout, first install pre-commit (consider using `pipx `_), then install the hooks with ``pre-commit install``. + +For example:: + + tahoe-lafs $ pre-commit install + pre-commit installed at .git/hooks/pre-commit + tahoe-lafs $ python -c "import pathlib; pathlib.Path('src/allmydata/tabbed.py').write_text('def foo():\\n\\tpass\\n')" + tahoe-lafs $ git add src/allmydata/tabbed.py + tahoe-lafs $ git commit -a -m "Add a file that violates flake8" + flake8...................................................................Failed + - hook id: flake8 + - exit code: 1 + + src/allmydata/tabbed.py:2:1: W191 indentation contains tabs + +To uninstall:: + + tahoe-lafs $ pre-commit uninstall + pre-commit uninstalled + + +Some find running linters on every commit to be a nuisance. To avoid the checks triggering during commits, but to check before pushing to the CI, install the hook for pre-push instead:: + + tahoe-lafs $ pre-commit install -t pre-push + pre-commit installed at .git/hooks/pre-push + tahoe-lafs $ git commit -a -m "Add a file that violates flake8" + [3398.pre-commit 29f8f43d2] Add a file that violates flake8 + 1 file changed, 2 insertions(+) + create mode 100644 src/allmydata/tabbed.py + tahoe-lafs $ git push + flake8...................................................................Failed + - hook id: flake8 + - exit code: 1 + + src/allmydata/tabbed.py:2:1: W191 indentation contains tabs + + error: failed to push some refs to 'github.com:jaraco/tahoe-lafs.git' diff --git a/docs/index.rst b/docs/index.rst index 581e74bbb..3d0a41302 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -39,6 +39,8 @@ Contents: write_coordination backupdb + developer-guide + anonymity-configuration nodekeys diff --git a/newsfragments/3398.minor b/newsfragments/3398.minor new file mode 100644 index 000000000..477c141fd --- /dev/null +++ b/newsfragments/3398.minor @@ -0,0 +1 @@ +Added pre-commit config to run flake8 checks on commit/push. diff --git a/newsfragments/3427.minor b/newsfragments/3427.minor new file mode 100644 index 000000000..e69de29bb diff --git a/newsfragments/3431.minor b/newsfragments/3431.minor new file mode 100644 index 000000000..e69de29bb diff --git a/newsfragments/3436.minor b/newsfragments/3436.minor new file mode 100644 index 000000000..e69de29bb diff --git a/newsfragments/3437.minor b/newsfragments/3437.minor new file mode 100644 index 000000000..e69de29bb diff --git a/newsfragments/3438.minor b/newsfragments/3438.minor new file mode 100644 index 000000000..e69de29bb diff --git a/newsfragments/3439.minor b/newsfragments/3439.minor new file mode 100644 index 000000000..e69de29bb diff --git a/newsfragments/3443.minor b/newsfragments/3443.minor new file mode 100644 index 000000000..e69de29bb diff --git a/newsfragments/3446.minor b/newsfragments/3446.minor new file mode 100644 index 000000000..e69de29bb diff --git a/src/allmydata/immutable/downloader/share.py b/src/allmydata/immutable/downloader/share.py index 7ec2935fd..15f55d409 100644 --- a/src/allmydata/immutable/downloader/share.py +++ b/src/allmydata/immutable/downloader/share.py @@ -464,7 +464,7 @@ class Share(object): # there was corruption somewhere in the given range reason = "corruption in share[%d-%d): %s" % (start, start+offset, str(f.value)) - self._rref.callRemoteOnly("advise_corrupt_share", reason) + self._rref.callRemoteOnly("advise_corrupt_share", reason.encode("utf-8")) def _satisfy_block_hash_tree(self, needed_hashes): o_bh = self.actual_offsets["block_hashes"] diff --git a/src/allmydata/immutable/offloaded.py b/src/allmydata/immutable/offloaded.py index e04e94e8f..fb8c706a3 100644 --- a/src/allmydata/immutable/offloaded.py +++ b/src/allmydata/immutable/offloaded.py @@ -203,7 +203,7 @@ class CHKUploadHelper(Referenceable, upload.CHKUploader): def _finished(self, ur): assert interfaces.IUploadResults.providedBy(ur), ur vcapstr = ur.get_verifycapstr() - precondition(isinstance(vcapstr, str), vcapstr) + precondition(isinstance(vcapstr, bytes), vcapstr) v = uri.from_string(vcapstr) f_times = self._fetcher.get_times() @@ -492,9 +492,9 @@ class Helper(Referenceable): # helper at random. name = "helper" - VERSION = { "http://allmydata.org/tahoe/protocols/helper/v1" : + VERSION = { b"http://allmydata.org/tahoe/protocols/helper/v1" : { }, - "application-version": str(allmydata.__full_version__), + b"application-version": allmydata.__full_version__.encode("utf-8"), } MAX_UPLOAD_STATUSES = 10 diff --git a/src/allmydata/immutable/upload.py b/src/allmydata/immutable/upload.py index 1ab312ab6..5e38ba31a 100644 --- a/src/allmydata/immutable/upload.py +++ b/src/allmydata/immutable/upload.py @@ -1816,15 +1816,15 @@ class Uploader(service.MultiService, log.PrefixingLogMixin): def _got_helper(self, helper): self.log("got helper connection, getting versions") - default = { "http://allmydata.org/tahoe/protocols/helper/v1" : + default = { b"http://allmydata.org/tahoe/protocols/helper/v1" : { }, - "application-version": b"unknown: no get_version()", + b"application-version": b"unknown: no get_version()", } d = add_version_to_remote_reference(helper, default) d.addCallback(self._got_versioned_helper) def _got_versioned_helper(self, helper): - needed = "http://allmydata.org/tahoe/protocols/helper/v1" + needed = b"http://allmydata.org/tahoe/protocols/helper/v1" if needed not in helper.version: raise InsufficientVersionError(needed, helper.version) self._helper = helper diff --git a/src/allmydata/interfaces.py b/src/allmydata/interfaces.py index 43565dc4f..49dcf7646 100644 --- a/src/allmydata/interfaces.py +++ b/src/allmydata/interfaces.py @@ -2,13 +2,15 @@ Interfaces for Tahoe-LAFS. Ported to Python 3. + +Note that for RemoteInterfaces, the __remote_name__ needs to be a native string because of https://github.com/warner/foolscap/blob/43f4485a42c9c28e2c79d655b3a9e24d4e6360ca/src/foolscap/remoteinterface.py#L67 """ from __future__ import absolute_import from __future__ import division from __future__ import print_function from __future__ import unicode_literals -from future.utils import PY2 +from future.utils import PY2, native_str if PY2: # Don't import object/str/dict/etc. types, so we don't break any # interfaces. Not importing open() because it triggers bogus flake8 error. @@ -105,7 +107,7 @@ ReadData = ListOf(ShareData) class RIStorageServer(RemoteInterface): - __remote_name__ = b"RIStorageServer.tahoe.allmydata.com" + __remote_name__ = native_str("RIStorageServer.tahoe.allmydata.com") def get_version(): """ @@ -2836,17 +2838,17 @@ class RIControlClient(RemoteInterface): # debug stuff - def upload_random_data_from_file(size=int, convergence=str): + def upload_random_data_from_file(size=int, convergence=bytes): return str - def download_to_tempfile_and_delete(uri=str): + def download_to_tempfile_and_delete(uri=bytes): return None def get_memory_usage(): """Return a dict describes the amount of memory currently in use. The keys are 'VmPeak', 'VmSize', and 'VmData'. The values are integers, measuring memory consupmtion in bytes.""" - return DictOf(str, int) + return DictOf(bytes, int) def speed_test(count=int, size=int, mutable=Any()): """Write 'count' tempfiles to disk, all of the given size. Measure @@ -2871,11 +2873,11 @@ class RIControlClient(RemoteInterface): return DictOf(str, float) -UploadResults = Any() #DictOf(str, str) +UploadResults = Any() #DictOf(bytes, bytes) class RIEncryptedUploadable(RemoteInterface): - __remote_name__ = b"RIEncryptedUploadable.tahoe.allmydata.com" + __remote_name__ = native_str("RIEncryptedUploadable.tahoe.allmydata.com") def get_size(): return Offset @@ -2884,33 +2886,33 @@ class RIEncryptedUploadable(RemoteInterface): return (int, int, int, long) def read_encrypted(offset=Offset, length=ReadSize): - return ListOf(str) + return ListOf(bytes) def close(): return None class RICHKUploadHelper(RemoteInterface): - __remote_name__ = b"RIUploadHelper.tahoe.allmydata.com" + __remote_name__ = native_str("RIUploadHelper.tahoe.allmydata.com") def get_version(): """ Return a dictionary of version information. """ - return DictOf(str, Any()) + return DictOf(bytes, Any()) def upload(reader=RIEncryptedUploadable): return UploadResults class RIHelper(RemoteInterface): - __remote_name__ = b"RIHelper.tahoe.allmydata.com" + __remote_name__ = native_str("RIHelper.tahoe.allmydata.com") def get_version(): """ Return a dictionary of version information. """ - return DictOf(str, Any()) + return DictOf(bytes, Any()) def upload_chk(si=StorageIndex): """See if a file with a given storage index needs uploading. The @@ -2931,7 +2933,7 @@ class RIHelper(RemoteInterface): class RIStatsProvider(RemoteInterface): - __remote_name__ = b"RIStatsProvider.tahoe.allmydata.com" + __remote_name__ = native_str("RIStatsProvider.tahoe.allmydata.com") """ Provides access to statistics and monitoring information. """ @@ -2944,16 +2946,16 @@ class RIStatsProvider(RemoteInterface): stats are instantaneous measures (potentially time averaged internally) """ - return DictOf(str, DictOf(str, ChoiceOf(float, int, long, None))) + return DictOf(bytes, DictOf(bytes, ChoiceOf(float, int, long, None))) class RIStatsGatherer(RemoteInterface): - __remote_name__ = b"RIStatsGatherer.tahoe.allmydata.com" + __remote_name__ = native_str("RIStatsGatherer.tahoe.allmydata.com") """ Provides a monitoring service for centralised collection of stats """ - def provide(provider=RIStatsProvider, nickname=str): + def provide(provider=RIStatsProvider, nickname=bytes): """ @param provider: a stats collector instance that should be polled periodically by the gatherer to collect stats. @@ -2965,7 +2967,7 @@ class RIStatsGatherer(RemoteInterface): class IStatsProducer(Interface): def get_stats(): """ - returns a dictionary, with str keys representing the names of stats + returns a dictionary, with bytes keys representing the names of stats to be monitored, and numeric values. """ diff --git a/src/allmydata/mutable/filenode.py b/src/allmydata/mutable/filenode.py index 48ce5d8b7..849dc4c88 100644 --- a/src/allmydata/mutable/filenode.py +++ b/src/allmydata/mutable/filenode.py @@ -280,13 +280,14 @@ class MutableFileNode(object): def __hash__(self): return hash((self.__class__, self._uri)) - def __cmp__(self, them): - if cmp(type(self), type(them)): - return cmp(type(self), type(them)) - if cmp(self.__class__, them.__class__): - return cmp(self.__class__, them.__class__) - return cmp(self._uri, them._uri) + def __eq__(self, them): + if type(self) != type(them): + return False + return self._uri == them._uri + + def __ne__(self, them): + return not (self == them) ################################# # ICheckable @@ -946,7 +947,7 @@ class MutableFileVersion(object): """ c = consumer.MemoryConsumer(progress=progress) d = self.read(c, fetch_privkey=fetch_privkey) - d.addCallback(lambda mc: "".join(mc.chunks)) + d.addCallback(lambda mc: b"".join(mc.chunks)) return d diff --git a/src/allmydata/mutable/retrieve.py b/src/allmydata/mutable/retrieve.py index b2d234a15..a7785e4c5 100644 --- a/src/allmydata/mutable/retrieve.py +++ b/src/allmydata/mutable/retrieve.py @@ -39,7 +39,7 @@ class RetrieveStatus(object): self.size = None self.status = "Not started" self.progress = 0.0 - self.counter = self.statusid_counter.next() + self.counter = next(self.statusid_counter) self.started = time.time() def get_started(self): @@ -321,7 +321,7 @@ class Retrieve(object): self._active_readers = [] # list of active readers for this dl. self._block_hash_trees = {} # shnum => hashtree - for i in xrange(self._total_shares): + for i in range(self._total_shares): # So we don't have to do this later. self._block_hash_trees[i] = hashtree.IncompleteHashTree(self._num_segments) @@ -743,7 +743,7 @@ class Retrieve(object): block_and_salt, blockhashes, sharehashes = results block, salt = block_and_salt - _assert(type(block) is str, (block, salt)) + _assert(isinstance(block, bytes), (block, salt)) blockhashes = dict(enumerate(blockhashes)) self.log("the reader gave me the following blockhashes: %s" % \ @@ -847,7 +847,7 @@ class Retrieve(object): # d.items()[0] is like (shnum, (block, salt)) # d.items()[0][1] is like (block, salt) # d.items()[0][1][1] is the salt. - salt = blocks_and_salts.items()[0][1][1] + salt = list(blocks_and_salts.items())[0][1][1] # Next, extract just the blocks from the dict. We'll use the # salt in the next step. share_and_shareids = [(k, v[0]) for k, v in blocks_and_salts.items()] @@ -870,7 +870,7 @@ class Retrieve(object): else: d = defer.maybeDeferred(self._segment_decoder.decode, shares, shareids) def _process(buffers): - segment = "".join(buffers) + segment = b"".join(buffers) self.log(format="now decoding segment %(segnum)s of %(numsegs)s", segnum=segnum, numsegs=self._num_segments, diff --git a/src/allmydata/mutable/servermap.py b/src/allmydata/mutable/servermap.py index 8dba6d8b5..32962c495 100644 --- a/src/allmydata/mutable/servermap.py +++ b/src/allmydata/mutable/servermap.py @@ -33,7 +33,7 @@ class UpdateStatus(object): self.mode = "?" self.status = "Not started" self.progress = 0.0 - self.counter = self.statusid_counter.next() + self.counter = next(self.statusid_counter) self.started = time.time() self.finished = None diff --git a/src/allmydata/storage/server.py b/src/allmydata/storage/server.py index 14eb08f84..8a8138f26 100644 --- a/src/allmydata/storage/server.py +++ b/src/allmydata/storage/server.py @@ -697,7 +697,7 @@ class StorageServer(service.MultiService, Referenceable): # This is a remote API, I believe, so this has to be bytes for legacy # protocol backwards compatibility reasons. assert isinstance(share_type, bytes) - assert isinstance(reason, bytes) + assert isinstance(reason, bytes), "%r is not bytes" % (reason,) fileutil.make_dirs(self.corruption_advisory_dir) now = time_format.iso_utc(sep="T") si_s = si_b2a(storage_index) diff --git a/src/allmydata/test/common.py b/src/allmydata/test/common.py index b69d58ab9..6b0059cb4 100644 --- a/src/allmydata/test/common.py +++ b/src/allmydata/test/common.py @@ -781,7 +781,7 @@ def create_mutable_filenode(contents, mdmf=False, all_contents=None): return filenode -TEST_DATA="\x02"*(Uploader.URI_LIT_SIZE_THRESHOLD+1) +TEST_DATA=b"\x02"*(Uploader.URI_LIT_SIZE_THRESHOLD+1) class WebErrorMixin(object): @@ -958,12 +958,12 @@ def _corrupt_offset_of_block_hashes_to_truncate_crypttext_hashes(data, debug=Fal assert sharevernum in (1, 2), "This test is designed to corrupt immutable shares of v1 or v2 in specific ways." if sharevernum == 1: curval = struct.unpack(">L", data[0x0c+0x18:0x0c+0x18+4])[0] - newval = random.randrange(0, max(1, (curval/hashutil.CRYPTO_VAL_SIZE)/2))*hashutil.CRYPTO_VAL_SIZE + newval = random.randrange(0, max(1, (curval//hashutil.CRYPTO_VAL_SIZE)//2))*hashutil.CRYPTO_VAL_SIZE newvalstr = struct.pack(">L", newval) return data[:0x0c+0x18]+newvalstr+data[0x0c+0x18+4:] else: curval = struct.unpack(">Q", data[0x0c+0x2c:0x0c+0x2c+8])[0] - newval = random.randrange(0, max(1, (curval/hashutil.CRYPTO_VAL_SIZE)/2))*hashutil.CRYPTO_VAL_SIZE + newval = random.randrange(0, max(1, (curval//hashutil.CRYPTO_VAL_SIZE)//2))*hashutil.CRYPTO_VAL_SIZE newvalstr = struct.pack(">Q", newval) return data[:0x0c+0x2c]+newvalstr+data[0x0c+0x2c+8:] diff --git a/src/allmydata/test/common_py3.py b/src/allmydata/test/common_py3.py index 5b791fd0a..c3a84189b 100644 --- a/src/allmydata/test/common_py3.py +++ b/src/allmydata/test/common_py3.py @@ -159,9 +159,12 @@ class FakeCanary(object): if self.ignore: return del self.disconnectors[marker] + def getRemoteTubID(self): + return None + def getPeer(self): + return "" class LoggingServiceParent(service.MultiService): def log(self, *args, **kwargs): return log.msg(*args, **kwargs) - diff --git a/src/allmydata/test/common_util.py b/src/allmydata/test/common_util.py index e1943e60a..6b964368d 100644 --- a/src/allmydata/test/common_util.py +++ b/src/allmydata/test/common_util.py @@ -8,12 +8,10 @@ from twisted.internet import reactor, defer from twisted.trial import unittest from ..util.assertutil import precondition +from ..scripts import runner from allmydata.util.encodingutil import get_io_encoding -from future.utils import PY2 -if PY2: # XXX this is a hack that makes some tests pass on Python3, remove - # in the future - from ..scripts import runner # Imported for backwards compatibility: +from future.utils import bord, bchr, binary_type from .common_py3 import ( SignalMixin, skip_if_cannot_represent_filename, ReallyEqualMixin, ShouldFailMixin ) @@ -57,9 +55,10 @@ class DevNullDictionary(dict): return def insecurerandstr(n): - return ''.join(map(chr, map(randrange, [0]*n, [256]*n))) + return b''.join(map(bchr, map(randrange, [0]*n, [256]*n))) def flip_bit(good, which): + # TODO Probs need to update with bchr/bord as with flip_one_bit, below. # flip the low-order bit of good[which] if which == -1: pieces = good[:which], good[-1:], "" @@ -70,10 +69,11 @@ def flip_bit(good, which): def flip_one_bit(s, offset=0, size=None): """ flip one random bit of the string s, in a byte greater than or equal to offset and less than offset+size. """ + precondition(isinstance(s, binary_type)) if size is None: size=len(s)-offset i = randrange(offset, offset+size) - result = s[:i] + chr(ord(s[i])^(0x01<