From b9939f7d4d5c4d4ee22ebd77092abf6b1c2ef1c5 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 20 Apr 2018 14:42:33 -0400 Subject: [PATCH 01/29] link to inotify wikipedia page --- docs/proposed/magic-folder/filesystem-integration.rst | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/proposed/magic-folder/filesystem-integration.rst b/docs/proposed/magic-folder/filesystem-integration.rst index 6ed0671ed..589db2322 100644 --- a/docs/proposed/magic-folder/filesystem-integration.rst +++ b/docs/proposed/magic-folder/filesystem-integration.rst @@ -62,13 +62,15 @@ to the target folder cease for a sufficiently long period of time. *Detecting filesystem changes* -For the Linux implementation, we will use the inotify Linux kernel +For the Linux implementation, we will use the `inotify`_ Linux kernel subsystem to gather events on the local Magic Folder directory tree. This implementation was already present in Tahoe-LAFS 1.9.0, but needs to be changed to gather directory creation and move events, in addition to the events indicating that a file has been written that are gathered by the current code. +.. _`inotify`: https://en.wikipedia.org/wiki/Inotify + For the Windows implementation, we will use the ``ReadDirectoryChangesW`` Win32 API. The prototype implementation simulates a Python interface to the inotify API in terms of ``ReadDirectoryChangesW``, allowing most of From 06a1ada6247b449ba30c1e0179ae81d70751f5c4 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 20 Apr 2018 14:43:34 -0400 Subject: [PATCH 02/29] Remove double-encoding of magic-folder params --- src/allmydata/client.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/allmydata/client.py b/src/allmydata/client.py index 341ada081..395b65f56 100644 --- a/src/allmydata/client.py +++ b/src/allmydata/client.py @@ -602,8 +602,8 @@ class _Client(node.Node, pollmixin.PollMixin): s = magic_folder.MagicFolder( client=self, - upload_dircap=mf_config["upload_dircap"].encode('ascii'), - collective_dircap=mf_config["collective_dircap"].encode('ascii'), + upload_dircap=mf_config["upload_dircap"], + collective_dircap=mf_config["collective_dircap"], local_path_u=abspath_expanduser_unicode(local_dir_config, base=self.basedir), dbfile=abspath_expanduser_unicode(db_filename), umask=self.get_config("magic_folder", "download.umask", 0077), From edf3c7aac70a29157340f671a3ebf3372273354f Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 20 Apr 2018 15:59:35 -0400 Subject: [PATCH 03/29] reformat to fit within 80 cols --- src/allmydata/webish.py | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/src/allmydata/webish.py b/src/allmydata/webish.py index 416e34dfd..5e52dbd28 100644 --- a/src/allmydata/webish.py +++ b/src/allmydata/webish.py @@ -114,16 +114,20 @@ class MyRequest(appserver.NevowRequest): if self._tahoe_request_had_error: error = " [ERROR]" - log.msg(format="web: %(clientip)s %(method)s %(uri)s %(code)s %(length)s%(error)s", - clientip=self.getClientIP(), - method=self.method, - uri=uri, - code=self.code, - length=(self.sentLength or "-"), - error=error, - facility="tahoe.webish", - level=log.OPERATIONAL, - ) + log.msg( + format=( + "web: %(clientip)s %(method)s %(uri)s %(code)s " + "%(length)s%(error)s" + ), + clientip=self.getClientIP(), + method=self.method, + uri=uri, + code=self.code, + length=(self.sentLength or "-"), + error=error, + facility="tahoe.webish", + level=log.OPERATIONAL, + ) class WebishServer(service.MultiService): @@ -218,4 +222,3 @@ class IntroducerWebishServer(WebishServer): service.MultiService.__init__(self) self.root = introweb.IntroducerRoot(introducer) self.buildServer(webport, nodeurl_path, staticdir) - From 8eb83bbfb98653a22ad676be9450b818db81ff42 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 20 Apr 2018 16:03:19 -0400 Subject: [PATCH 04/29] avoid about-to-be-deprecated getClientIP if we can Use the replacement, getClientAddress. But have a fallback to getClientIP to keep supporting older versions of Twisted. --- src/allmydata/webish.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/allmydata/webish.py b/src/allmydata/webish.py index 5e52dbd28..d1a60f495 100644 --- a/src/allmydata/webish.py +++ b/src/allmydata/webish.py @@ -2,6 +2,10 @@ import re, time from twisted.application import service, strports, internet from twisted.web import http, static from twisted.internet import defer +from twisted.internet.address import ( + IPv4Address, + IPv6Address, +) from nevow import appserver, inevow from allmydata.util import log, fileutil @@ -119,7 +123,7 @@ class MyRequest(appserver.NevowRequest): "web: %(clientip)s %(method)s %(uri)s %(code)s " "%(length)s%(error)s" ), - clientip=self.getClientIP(), + clientip=_get_client_ip(self), method=self.method, uri=uri, code=self.code, @@ -130,6 +134,18 @@ class MyRequest(appserver.NevowRequest): ) +def _get_client_ip(request): + try: + get = request.getClientAddress + except AttributeError: + return request.getClientIP() + else: + client_addr = get() + if isinstance(client_addr, (IPv4Address, IPv6Address)): + return client_addr.host + return None + + class WebishServer(service.MultiService): name = "webish" From 0bdabacce3d540c8b7b872fc65d125cb5c14df32 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 20 Apr 2018 14:47:06 -0400 Subject: [PATCH 05/29] document the node_directory parameter --- src/allmydata/frontends/magic_folder.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/allmydata/frontends/magic_folder.py b/src/allmydata/frontends/magic_folder.py index 5727a5343..da98ad019 100644 --- a/src/allmydata/frontends/magic_folder.py +++ b/src/allmydata/frontends/magic_folder.py @@ -119,6 +119,7 @@ def load_magic_folders(node_directory): old-style to new-style config (but WILL read old-style config and return in the same way as if it was new-style). + :param node_directory: path where node data is stored :returns: dict mapping magic-folder-name to its config (also a dict) """ yaml_fname = os.path.join(node_directory, u"private", u"magic_folders.yaml") From ac6269dd2dbba8c02e8a84bb85fda5998430d98a Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 20 Apr 2018 15:11:55 -0400 Subject: [PATCH 06/29] Only read magic-folder config from config reader Also, fix the umask feature which was completely broken previously due to failure to parse the umask string into an integer. --- src/allmydata/client.py | 2 +- src/allmydata/frontends/magic_folder.py | 15 +++++++++++++++ src/allmydata/test/test_magic_folder.py | 18 ++++++++++++++++++ 3 files changed, 34 insertions(+), 1 deletion(-) diff --git a/src/allmydata/client.py b/src/allmydata/client.py index 395b65f56..50935745d 100644 --- a/src/allmydata/client.py +++ b/src/allmydata/client.py @@ -606,7 +606,7 @@ class _Client(node.Node, pollmixin.PollMixin): collective_dircap=mf_config["collective_dircap"], local_path_u=abspath_expanduser_unicode(local_dir_config, base=self.basedir), dbfile=abspath_expanduser_unicode(db_filename), - umask=self.get_config("magic_folder", "download.umask", 0077), + umask=mf_config["umask"], name=name, downloader_delay=poll_interval, ) diff --git a/src/allmydata/frontends/magic_folder.py b/src/allmydata/frontends/magic_folder.py index da98ad019..9750df525 100644 --- a/src/allmydata/frontends/magic_folder.py +++ b/src/allmydata/frontends/magic_folder.py @@ -30,6 +30,9 @@ from allmydata.immutable.upload import FileName, Data from allmydata import magicfolderdb, magicpath +# Mask off all non-owner permissions for magic-folders files by default. +_DEFAULT_DOWNLOAD_UMASK = 0o077 + IN_EXCL_UNLINK = 0x04000000L def get_inotify_module(): @@ -157,11 +160,17 @@ def load_magic_folders(node_directory): ) ) + if config.has_option("magic_folder", "download.umask"): + umask = int(config.get("magic_folder", "download.umask"), 8) + else: + umask = _DEFAULT_DOWNLOAD_UMASK + folders[u"default"] = { u"directory": directory, u"upload_dircap": fileutil.read(up_fname), u"collective_dircap": fileutil.read(coll_fname), u"poll_interval": interval, + u"umask": umask, } else: # without any YAML file AND no local.directory option it's @@ -213,6 +222,12 @@ def load_magic_folders(node_directory): if isinstance(mf_config[k], unicode): mf_config[k] = mf_config[k].encode('ascii') + if not isinstance( + mf_config.setdefault(u"umask", _DEFAULT_DOWNLOAD_UMASK), + int, + ): + raise Exception("magic-folder download umask must be an integer") + return folders diff --git a/src/allmydata/test/test_magic_folder.py b/src/allmydata/test/test_magic_folder.py index 451545584..0f26c99f2 100644 --- a/src/allmydata/test/test_magic_folder.py +++ b/src/allmydata/test/test_magic_folder.py @@ -65,6 +65,7 @@ class NewConfigUtilTests(unittest.TestCase): def test_load(self): folders = magic_folder.load_magic_folders(self.basedir) self.assertEqual(['default'], list(folders.keys())) + self.assertEqual(folders['default'][u'umask'], 0o077) def test_both_styles_of_config(self): os.unlink(join(self.basedir, u"private", u"magic_folders.yaml")) @@ -115,6 +116,23 @@ class NewConfigUtilTests(unittest.TestCase): str(ctx.exception) ) + def test_wrong_umask_obj(self): + """ + If a umask is given for a magic-folder that is not an integer, an + exception is raised. + """ + self.folders[u"default"][u"umask"] = "0077" + yaml_fname = join(self.basedir, u"private", u"magic_folders.yaml") + with open(yaml_fname, "w") as f: + f.write(yamlutil.safe_dump({u"magic-folders": self.folders})) + + with self.assertRaises(Exception) as ctx: + magic_folder.load_magic_folders(self.basedir) + self.assertIn( + "umask must be an integer", + str(ctx.exception) + ) + def test_wrong_sub_obj(self): yaml_fname = join(self.basedir, u"private", u"magic_folders.yaml") with open(yaml_fname, "w") as f: From 8d104dab1c1e90a4c5e295673a5662ff05405a7a Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 23 Apr 2018 10:59:33 -0400 Subject: [PATCH 07/29] Move the complicated MagicFolder constructor All that complexity can be part of MagicFolder itself. --- src/allmydata/client.py | 18 +----------- src/allmydata/frontends/magic_folder.py | 37 +++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 17 deletions(-) diff --git a/src/allmydata/client.py b/src/allmydata/client.py index 50935745d..c9150c79b 100644 --- a/src/allmydata/client.py +++ b/src/allmydata/client.py @@ -593,23 +593,7 @@ class _Client(node.Node, pollmixin.PollMixin): for (name, mf_config) in magic_folders.items(): self.log("Starting magic_folder '{}'".format(name)) - db_filename = os.path.join(self.basedir, "private", "magicfolder_{}.sqlite".format(name)) - local_dir_config = mf_config['directory'] - try: - poll_interval = int(mf_config["poll_interval"]) - except ValueError: - raise ValueError("'poll_interval' option must be an int") - - s = magic_folder.MagicFolder( - client=self, - upload_dircap=mf_config["upload_dircap"], - collective_dircap=mf_config["collective_dircap"], - local_path_u=abspath_expanduser_unicode(local_dir_config, base=self.basedir), - dbfile=abspath_expanduser_unicode(db_filename), - umask=mf_config["umask"], - name=name, - downloader_delay=poll_interval, - ) + s = magic_folder.MagicFolder.from_config(self.basedir, mf_config) self._magic_folders[name] = s s.setServiceParent(self) s.startService() diff --git a/src/allmydata/frontends/magic_folder.py b/src/allmydata/frontends/magic_folder.py index 9750df525..1579babff 100644 --- a/src/allmydata/frontends/magic_folder.py +++ b/src/allmydata/frontends/magic_folder.py @@ -244,6 +244,43 @@ def save_magic_folders(node_directory, folders): class MagicFolder(service.MultiService): + @classmethod + def from_config(cls, client_node, config): + """ + Create a ``MagicFolder`` from a client node and magic-folder + configuration. + + :param _Client client_node: The client node the magic-folder is + attached to. + + :param dict config: Magic-folder configuration like that in the list + returned by ``load_magic_folders``. + """ + db_filename = os.path.join( + client_node.basedir, + "private", + "magicfolder_{}.sqlite".format(name), + ) + local_dir_config = config['directory'] + try: + poll_interval = int(config["poll_interval"]) + except ValueError: + raise ValueError("'poll_interval' option must be an int") + + return cls( + client=client_node, + upload_dircap=config["upload_dircap"], + collective_dircap=config["collective_dircap"], + local_path_u=abspath_expanduser_unicode( + local_dir_config, + base=client_node.basedir, + ), + dbfile=abspath_expanduser_unicode(db_filename), + umask=config["umask"], + name=name, + downloader_delay=poll_interval, + ) + def __init__(self, client, upload_dircap, collective_dircap, local_path_u, dbfile, umask, name, uploader_delay=1.0, clock=None, downloader_delay=60): precondition_abspath(local_path_u) From cfa33332a5a70f43441075ca7e0cd241b86da186 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 23 Apr 2018 11:09:24 -0400 Subject: [PATCH 08/29] Add missing information/import --- src/allmydata/client.py | 2 +- src/allmydata/frontends/magic_folder.py | 9 +++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/allmydata/client.py b/src/allmydata/client.py index c9150c79b..3479da0c3 100644 --- a/src/allmydata/client.py +++ b/src/allmydata/client.py @@ -593,7 +593,7 @@ class _Client(node.Node, pollmixin.PollMixin): for (name, mf_config) in magic_folders.items(): self.log("Starting magic_folder '{}'".format(name)) - s = magic_folder.MagicFolder.from_config(self.basedir, mf_config) + s = magic_folder.MagicFolder.from_config(self.basedir, name, mf_config) self._magic_folders[name] = s s.setServiceParent(self) s.startService() diff --git a/src/allmydata/frontends/magic_folder.py b/src/allmydata/frontends/magic_folder.py index 1579babff..24fd9cb0f 100644 --- a/src/allmydata/frontends/magic_folder.py +++ b/src/allmydata/frontends/magic_folder.py @@ -18,7 +18,12 @@ from zope.interface import Interface, Attribute, implementer from allmydata.util import fileutil, configutil, yamlutil from allmydata.interfaces import IDirectoryNode from allmydata.util import log -from allmydata.util.fileutil import precondition_abspath, get_pathinfo, ConflictError +from allmydata.util.fileutil import ( + precondition_abspath, + get_pathinfo, + ConflictError, + abspath_expanduser_unicode, +) from allmydata.util.assertutil import precondition, _assert from allmydata.util.deferredutil import HookMixin from allmydata.util.progress import PercentProgress @@ -245,7 +250,7 @@ def save_magic_folders(node_directory, folders): class MagicFolder(service.MultiService): @classmethod - def from_config(cls, client_node, config): + def from_config(cls, client_node, name, config): """ Create a ``MagicFolder`` from a client node and magic-folder configuration. From e1c469e3b67fd3b41204eddf7c82d24ead6c7a20 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 23 Apr 2018 11:41:29 -0400 Subject: [PATCH 09/29] make sure we pass the client node --- src/allmydata/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/client.py b/src/allmydata/client.py index 3479da0c3..71397f961 100644 --- a/src/allmydata/client.py +++ b/src/allmydata/client.py @@ -593,7 +593,7 @@ class _Client(node.Node, pollmixin.PollMixin): for (name, mf_config) in magic_folders.items(): self.log("Starting magic_folder '{}'".format(name)) - s = magic_folder.MagicFolder.from_config(self.basedir, name, mf_config) + s = magic_folder.MagicFolder.from_config(self, name, mf_config) self._magic_folders[name] = s s.setServiceParent(self) s.startService() From 035dc6dc76a3d96c285559e90fd9dbbf3b414a1a Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 23 Apr 2018 11:41:36 -0400 Subject: [PATCH 10/29] reduce fragility of tests .. maybe? only trivially, at best, of course. --- src/allmydata/test/test_client.py | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/src/allmydata/test/test_client.py b/src/allmydata/test/test_client.py index cf1d92b7b..b99769d3c 100644 --- a/src/allmydata/test/test_client.py +++ b/src/allmydata/test/test_client.py @@ -401,11 +401,18 @@ class Basic(testutil.ReallyEqualMixin, testutil.NonASCIIPathMixin, unittest.Test _check("helper.furl = pb://blah\n", "pb://blah") def test_create_magic_folder_service(self): - class MockMagicFolder(service.MultiService): + boom = False + class Boom(Exception): + pass + + class MockMagicFolder(allmydata.frontends.magic_folder.MagicFolder): name = 'magic-folder' def __init__(self, client, upload_dircap, collective_dircap, local_path_u, dbfile, umask, name, inotify=None, uploader_delay=1.0, clock=None, downloader_delay=3): + if boom: + raise Boom() + service.MultiService.__init__(self) self.client = client self._umask = umask @@ -415,6 +422,12 @@ class Basic(testutil.ReallyEqualMixin, testutil.NonASCIIPathMixin, unittest.Test self.dbfile = dbfile self.inotify = inotify + def startService(self): + self.running = True + + def stopService(self): + self.running = False + def ready(self): pass @@ -461,12 +474,8 @@ class Basic(testutil.ReallyEqualMixin, testutil.NonASCIIPathMixin, unittest.Test self.failUnless(magicfolder.inotify is None, magicfolder.inotify) self.failUnless(magicfolder.running) - class Boom(Exception): - pass - def BoomMagicFolder(client, upload_dircap, collective_dircap, local_path_u, dbfile, name, - umask, inotify=None, uploader_delay=1.0, clock=None, downloader_delay=3): - raise Boom() - self.patch(allmydata.frontends.magic_folder, 'MagicFolder', BoomMagicFolder) + # See above. + boom = True basedir2 = "test_client.Basic.test_create_magic_folder_service2" os.mkdir(basedir2) From 2bc4aa7d14d7253182caa366716b59d8dc989c75 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 24 Apr 2018 09:04:45 -0400 Subject: [PATCH 11/29] Add recommended lgtm configuration. --- .lgtm.yml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .lgtm.yml diff --git a/.lgtm.yml b/.lgtm.yml new file mode 100644 index 000000000..0e0f0db13 --- /dev/null +++ b/.lgtm.yml @@ -0,0 +1,6 @@ +extraction: + python: + after_prepare: + - | + # https://discuss.lgtm.com/t/determination-of-python-requirements/974/4 + sed -i 's/\("pyOpenSSL\)/\# Dependency removed for lgtm (see .lgtm.yml): \1/g' src/allmydata/_auto_deps.py From be0317632fc47a79bc77a22c6bf7de21b58c876b Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 26 Apr 2018 14:50:25 -0400 Subject: [PATCH 12/29] Turn off a query that generates spurious errors --- .lgtm.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.lgtm.yml b/.lgtm.yml index 0e0f0db13..240bc62c5 100644 --- a/.lgtm.yml +++ b/.lgtm.yml @@ -4,3 +4,9 @@ extraction: - | # https://discuss.lgtm.com/t/determination-of-python-requirements/974/4 sed -i 's/\("pyOpenSSL\)/\# Dependency removed for lgtm (see .lgtm.yml): \1/g' src/allmydata/_auto_deps.py + +queries: + # This generates spurious errors for calls by interface because of the + # zope.interface choice to exclude self from method signatures. So, turn it + # off. + - exclude: "py/call/wrong-arguments" From cc6006dcb3695531582c3c1e466598a0184281da Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 26 Apr 2018 14:57:45 -0400 Subject: [PATCH 13/29] Replace use of deprecated sha module --- misc/simulators/simulator.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/misc/simulators/simulator.py b/misc/simulators/simulator.py index c1f9e9b54..0cf568eaf 100644 --- a/misc/simulators/simulator.py +++ b/misc/simulators/simulator.py @@ -1,6 +1,6 @@ #! /usr/bin/env python -import sha as shamodule +import hashlib import os, random from pkg_resources import require @@ -10,7 +10,7 @@ from pyrrd.rrd import DataSource, RRD, RRA def sha(s): - return shamodule.new(s).digest() + return hashlib.sha1(s).digest() def randomid(): return os.urandom(20) From f99b3bdbda4ee89caa3186dcc13f3102063d1805 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 26 Apr 2018 14:58:40 -0400 Subject: [PATCH 14/29] Remove complicated and dead code --- misc/simulators/simulator.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/misc/simulators/simulator.py b/misc/simulators/simulator.py index 0cf568eaf..a01c5c8c8 100644 --- a/misc/simulators/simulator.py +++ b/misc/simulators/simulator.py @@ -70,10 +70,7 @@ class Node: return False def decide(self, sharesize): - if sharesize > self.capacity: - return False return False - return random.random() > 0.5 def make_space(self, sharesize): assert sharesize <= self.capacity From da9d0ded94eabba0c42d1a332cf2e3cfc6f9f004 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 26 Apr 2018 14:59:18 -0400 Subject: [PATCH 15/29] Remove pointless conditional --- src/allmydata/mutable/publish.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/allmydata/mutable/publish.py b/src/allmydata/mutable/publish.py index ee17847e4..fe01efa01 100644 --- a/src/allmydata/mutable/publish.py +++ b/src/allmydata/mutable/publish.py @@ -976,8 +976,7 @@ class Publish: i += 1 if i >= len(serverlist): i = 0 - if True: - self.log_goal(self.goal, "after update: ") + self.log_goal(self.goal, "after update: ") def _got_write_answer(self, answer, writer, started): From 6d9f0c59b7bfcd2cd52d6a1a866a76fc93f50382 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 26 Apr 2018 14:59:47 -0400 Subject: [PATCH 16/29] Remove pointless conditional --- src/allmydata/mutable/publish.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/allmydata/mutable/publish.py b/src/allmydata/mutable/publish.py index fe01efa01..50db1e2e3 100644 --- a/src/allmydata/mutable/publish.py +++ b/src/allmydata/mutable/publish.py @@ -909,9 +909,7 @@ class Publish: level=log.NOISY) def update_goal(self): - # if log.recording_noisy - if True: - self.log_goal(self.goal, "before update: ") + self.log_goal(self.goal, "before update: ") # first, remove any bad servers from our goal self.goal = set([ (server, shnum) From 206ab732e6fa306af6cf19d6cc0d78f8dc42587a Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 26 Apr 2018 15:00:36 -0400 Subject: [PATCH 17/29] Replace use of deprecated sha module --- misc/coding_tools/make-canary-files.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/misc/coding_tools/make-canary-files.py b/misc/coding_tools/make-canary-files.py index 57f900a5c..4ba06cd9c 100644 --- a/misc/coding_tools/make-canary-files.py +++ b/misc/coding_tools/make-canary-files.py @@ -50,7 +50,7 @@ system where Tahoe is installed, or in a source tree with setup.py like this: setup.py run_with_pythonpath -p -c 'misc/make-canary-files.py ARGS..' """ -import os, sha +import os, hashlib from twisted.python import usage from allmydata.immutable import upload from allmydata.util import base32 @@ -96,7 +96,7 @@ convergence = base32.a2b(convergence_s) def get_permuted_peers(key): results = [] for nodeid in nodes: - permuted = sha.new(key + nodeid).digest() + permuted = hashlib.sha1(key + nodeid).digest() results.append((permuted, nodeid)) results.sort(lambda a,b: cmp(a[0], b[0])) return [ r[1] for r in results ] From 0f5d0e3131b814bbcf80683797503fad1af1ae7b Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 26 Apr 2018 15:07:09 -0400 Subject: [PATCH 18/29] Comment out "testing" code... --- misc/operations_helpers/munin/tahoe_nodememory | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/misc/operations_helpers/munin/tahoe_nodememory b/misc/operations_helpers/munin/tahoe_nodememory index fd3f8b0a4..1ecf53fc9 100644 --- a/misc/operations_helpers/munin/tahoe_nodememory +++ b/misc/operations_helpers/munin/tahoe_nodememory @@ -6,10 +6,9 @@ import os, sys, re -if 0: - # for testing - os.environ["nodememory_warner1"] = "run/warner1" - os.environ["nodememory_warner2"] = "run/warner2" +# for testing +# os.environ["nodememory_warner1"] = "run/warner1" +# os.environ["nodememory_warner2"] = "run/warner2" nodedirs = [] for k,v in os.environ.items(): From 64243527eb112e2d7d5c436b64dbd3ef5d442c76 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 26 Apr 2018 15:08:14 -0400 Subject: [PATCH 19/29] Remove the strange option to not use flog --- src/allmydata/frontends/sftpd.py | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/src/allmydata/frontends/sftpd.py b/src/allmydata/frontends/sftpd.py index 028ea7ee5..83d1e540a 100644 --- a/src/allmydata/frontends/sftpd.py +++ b/src/allmydata/frontends/sftpd.py @@ -38,24 +38,9 @@ from allmydata.dirnode import update_metadata from allmydata.util.fileutil import EncryptedTemporaryFile noisy = True -use_foolscap_logging = True from allmydata.util.log import NOISY, OPERATIONAL, WEIRD, \ - msg as _msg, err as _err, PrefixingLogMixin as _PrefixingLogMixin - -if use_foolscap_logging: - (logmsg, logerr, PrefixingLogMixin) = (_msg, _err, _PrefixingLogMixin) -else: # pragma: no cover - def logmsg(s, level=None): - print s - def logerr(s, level=None): - print s - class PrefixingLogMixin: - def __init__(self, facility=None, prefix=''): - self.prefix = prefix - def log(self, s, level=None): - print "%r %s" % (self.prefix, s) - + msg as logmsg, PrefixingLogMixin def eventually_callback(d): return lambda res: eventually(d.callback, res) From 7609fd18617c02043c278551fc7c7172fad90838 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 26 Apr 2018 15:09:01 -0400 Subject: [PATCH 20/29] Remove impossible third codepath --- src/allmydata/immutable/layout.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/allmydata/immutable/layout.py b/src/allmydata/immutable/layout.py index c634702ab..320f742e0 100644 --- a/src/allmydata/immutable/layout.py +++ b/src/allmydata/immutable/layout.py @@ -438,9 +438,7 @@ class ReadBucketProxy(object): def _get_share_hashes(self, unused=None): if hasattr(self, '_share_hashes'): return self._share_hashes - else: - return self._get_share_hashes_the_old_way() - return self._share_hashes + return self._get_share_hashes_the_old_way() def get_share_hashes(self): d = self._start_if_needed() From 9f8c90393faed664a0b97ef65e761a8c31151058 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 26 Apr 2018 15:10:02 -0400 Subject: [PATCH 21/29] Remove dead `synopsis` definition --- src/allmydata/scripts/runner.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/allmydata/scripts/runner.py b/src/allmydata/scripts/runner.py index d616cf199..70ff75bf2 100644 --- a/src/allmydata/scripts/runner.py +++ b/src/allmydata/scripts/runner.py @@ -47,7 +47,6 @@ class Options(usage.Options): stdout = sys.stdout stderr = sys.stderr - synopsis = "\nUsage: tahoe [command options]" subCommands = ( GROUP("Administration") + create_node.subCommands + stats_gatherer.subCommands From 6b16afaa2eae926b95b6b7c325cf24e7139d93df Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 26 Apr 2018 15:16:00 -0400 Subject: [PATCH 22/29] Avoid using the list comprehension loop variable It works fine but it relies on leaky scopes. --- src/allmydata/util/encodingutil.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/allmydata/util/encodingutil.py b/src/allmydata/util/encodingutil.py index 65f5911a1..11ab942b6 100644 --- a/src/allmydata/util/encodingutil.py +++ b/src/allmydata/util/encodingutil.py @@ -335,8 +335,8 @@ def listdir_unicode_fallback(path): try: return [unicode(fn, filesystem_encoding) for fn in os.listdir(byte_path)] - except UnicodeDecodeError: - raise FilenameEncodingError(fn) + except UnicodeDecodeError as e: + raise FilenameEncodingError(e.object) def listdir_unicode(path): """ From 8d4d0001326415113b0bd35346c2a07857602d36 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 26 Apr 2018 15:20:27 -0400 Subject: [PATCH 23/29] Fix pre-release matching regex character class Previously matched any single character from `abc|r` (with duplicate specification of `c`). Now matches any single character from `abc` or the two character sequence `rc`. I guess this was the intent, anyway. --- src/allmydata/util/verlib.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/allmydata/util/verlib.py b/src/allmydata/util/verlib.py index 6d8d8e254..619f1a845 100644 --- a/src/allmydata/util/verlib.py +++ b/src/allmydata/util/verlib.py @@ -254,7 +254,7 @@ def suggest_normalized_version(s): # if we have something like "b-2" or "a.2" at the end of the # version, that is pobably beta, alpha, etc # let's remove the dash or dot - rs = re.sub(r"([abc|rc])[\-\.](\d+)$", r"\1\2", rs) + rs = re.sub(r"([abc]|rc)[\-\.](\d+)$", r"\1\2", rs) # 1.0-dev-r371 -> 1.0.dev371 # 0.1-dev-r79 -> 0.1.dev79 @@ -324,4 +324,3 @@ def suggest_normalized_version(s): except IrrationalVersionError: pass return None - From b6d33c92ff08b86cbcbad53039fa197a7c3bcaf3 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 26 Apr 2018 15:26:17 -0400 Subject: [PATCH 24/29] Remove disabled ad hoc debug logging --- src/allmydata/web/directory.py | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/allmydata/web/directory.py b/src/allmydata/web/directory.py index f21e1dffa..1cfd7a17b 100644 --- a/src/allmydata/web/directory.py +++ b/src/allmydata/web/directory.py @@ -74,8 +74,6 @@ class DirectoryNodeHandler(RenderMixin, rend.Page, ReplaceMeMixin): return d def got_child(self, node_or_failure, ctx, name): - DEBUG = False - if DEBUG: print "GOT_CHILD", name, node_or_failure req = IRequest(ctx) method = req.method nonterminal = len(req.postpath) > 1 @@ -84,24 +82,18 @@ class DirectoryNodeHandler(RenderMixin, rend.Page, ReplaceMeMixin): f = node_or_failure f.trap(NoSuchChildError) # No child by this name. What should we do about it? - if DEBUG: print "no child", name - if DEBUG: print "postpath", req.postpath if nonterminal: - if DEBUG: print " intermediate" if should_create_intermediate_directories(req): # create intermediate directories - if DEBUG: print " making intermediate directory" d = self.node.create_subdirectory(name) d.addCallback(make_handler_for, self.client, self.node, name) return d else: - if DEBUG: print " terminal" # terminal node if (method,t) in [ ("POST","mkdir"), ("PUT","mkdir"), ("POST", "mkdir-with-children"), ("POST", "mkdir-immutable") ]: - if DEBUG: print " making final directory" # final directory kids = {} if t in ("mkdir-with-children", "mkdir-immutable"): @@ -122,14 +114,12 @@ class DirectoryNodeHandler(RenderMixin, rend.Page, ReplaceMeMixin): self.client, self.node, name) return d if (method,t) in ( ("PUT",""), ("PUT","uri"), ): - if DEBUG: print " PUT, making leaf placeholder" # we were trying to find the leaf filenode (to put a new # file in its place), and it didn't exist. That's ok, # since that's the leaf node that we're about to create. # We make a dummy one, which will respond to the PUT # request by replacing itself. return PlaceHolderNodeHandler(self.client, self.node, name) - if DEBUG: print " 404" # otherwise, we just return a no-such-child error return f @@ -138,11 +128,9 @@ class DirectoryNodeHandler(RenderMixin, rend.Page, ReplaceMeMixin): if not IDirectoryNode.providedBy(node): # we would have put a new directory here, but there was a # file in the way. - if DEBUG: print "blocking" raise WebError("Unable to create directory '%s': " "a file was in the way" % name, http.CONFLICT) - if DEBUG: print "good child" return make_handler_for(node, self.client, self.node, name) def render_DELETE(self, ctx): From 3705264740d90f244f4b6d819d22a84ac36366fb Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 26 Apr 2018 15:27:00 -0400 Subject: [PATCH 25/29] Use preferred exception raising syntax. Also, make the `WindowsError` class "reachable". --- src/allmydata/windows/registry.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/windows/registry.py b/src/allmydata/windows/registry.py index 2b87689b8..4801310d3 100644 --- a/src/allmydata/windows/registry.py +++ b/src/allmydata/windows/registry.py @@ -5,9 +5,9 @@ _AMD_KEY = r"Software\Allmydata" _BDIR_KEY = 'Base Dir Path' if sys.platform not in ('win32'): - raise ImportError, "registry cannot be used on non-windows systems" class WindowsError(Exception): # stupid voodoo to appease pyflakes pass + raise ImportError("registry cannot be used on non-windows systems") def get_registry_setting(key, name, _topkey=None): """ From 87daa3ec5a5113e2f790c35409214e94ab0492df Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 26 Apr 2018 15:32:02 -0400 Subject: [PATCH 26/29] Remove dead debug logging code. --- src/allmydata/immutable/upload.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/allmydata/immutable/upload.py b/src/allmydata/immutable/upload.py index cef226a8e..26d0af30d 100644 --- a/src/allmydata/immutable/upload.py +++ b/src/allmydata/immutable/upload.py @@ -533,7 +533,6 @@ class Tahoe2ServerSelector(log.PrefixingLogMixin): # we haven't improved over the last iteration; give up break; if errors_before == self._query_stats.bad: - if False: print("no more errors; break") break; last_happiness = effective_happiness # print("write trackers left: {}".format(len(write_trackers))) From b623a4a199d6d8c3e856866c9ba94e337eef7138 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 26 Apr 2018 15:32:27 -0400 Subject: [PATCH 27/29] Remove dead Tor TCP control port setup code. If someone wants this I bet they can figure it out. --- src/allmydata/util/tor_provider.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/allmydata/util/tor_provider.py b/src/allmydata/util/tor_provider.py index ccb963775..14d871cd0 100644 --- a/src/allmydata/util/tor_provider.py +++ b/src/allmydata/util/tor_provider.py @@ -72,13 +72,9 @@ def _launch_tor(reactor, tor_executable, private_dir, txtorcon): tor_config = txtorcon.TorConfig() tor_config.DataDirectory = data_directory(private_dir) - if True: # unix-domain control socket - tor_config.ControlPort = "unix:" + os.path.join(private_dir, "tor.control") - tor_control_endpoint_desc = tor_config.ControlPort - else: - # we allocate a new TCP control port each time - tor_config.ControlPort = allocate_tcp_port() - tor_control_endpoint_desc = "tcp:127.0.0.1:%d" % tor_config.ControlPort + # unix-domain control socket + tor_config.ControlPort = "unix:" + os.path.join(private_dir, "tor.control") + tor_control_endpoint_desc = tor_config.ControlPort tor_config.SOCKSPort = allocate_tcp_port() From 60bebd0659fe195a62217d89cfa31b87d30bb214 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 26 Apr 2018 15:35:56 -0400 Subject: [PATCH 28/29] Turn off another lgtm query. It's not good. --- .lgtm.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.lgtm.yml b/.lgtm.yml index 240bc62c5..07b5ff461 100644 --- a/.lgtm.yml +++ b/.lgtm.yml @@ -10,3 +10,8 @@ queries: # zope.interface choice to exclude self from method signatures. So, turn it # off. - exclude: "py/call/wrong-arguments" + + # The premise of this query is broken. The errors it produces are nonsense. + # There is no such thing as a "procedure" in Python and "None" is not + # meaningless. + - exclude: "py/procedure-return-value-used" From 97aff20cfde2445310dc9bac31691bbb5a11f2fa Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 26 Apr 2018 15:41:38 -0400 Subject: [PATCH 29/29] Disable another lgtm query. --- .lgtm.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.lgtm.yml b/.lgtm.yml index 07b5ff461..efc2479ca 100644 --- a/.lgtm.yml +++ b/.lgtm.yml @@ -15,3 +15,8 @@ queries: # There is no such thing as a "procedure" in Python and "None" is not # meaningless. - exclude: "py/procedure-return-value-used" + + # It is true that this query identifies things which are sometimes mistakes. + # However, it also identifies things which are entirely valid. Therefore, + # it produces noisy results. + - exclude: "py/implicit-string-concatenation-in-list" \ No newline at end of file