From cc7a3bc2b54a19ae8e30ad2b54d8af0985df77d6 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Mon, 28 Sep 2020 11:48:51 -0400 Subject: [PATCH 1/5] Audit script to find potentially broken for loops. --- misc/python3/audit-dict-for-loops.py | 43 ++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 misc/python3/audit-dict-for-loops.py diff --git a/misc/python3/audit-dict-for-loops.py b/misc/python3/audit-dict-for-loops.py new file mode 100644 index 000000000..9e34603fa --- /dev/null +++ b/misc/python3/audit-dict-for-loops.py @@ -0,0 +1,43 @@ +""" +The following code is valid in Python 2: + +for x in my_dict.keys(): + if something(x): + del my_dict[x] + +But broken in Python 3. + +One solution is: + +for x in list(my_dict.keys()): + if something(x): + del my_dict[x] + +Some but not all code in Tahoe has been changed to that. In other cases, the code was left unchanged since there was no `del`. + +However, some mistakes may have slept through. + +To help catch cases that were incorrectly ported, this script runs futurize on all ported modules, which should convert it into the `list()` form. +You can then look at git diffs to see if any of the impacted would be buggy without the newly added `list()`. +""" + +import os +from subprocess import check_call + +from allmydata.util import _python3 + + +def fix_potential_issue(): + for module in _python3.PORTED_MODULES + _python3.PORTED_TEST_MODULES: + filename = "src/" + module.replace(".", "/") + ".py" + if not os.path.exists(filename): + # Package, probably + filename = "src/" + module.replace(".", "/") + "/__init__.py" + check_call(["futurize", "-f", "lib2to3.fixes.fix_dict", "-w", filename]) + print( + "All loops converted. Check diff to see if there are any that need to be commitedd." + ) + + +if __name__ == "__main__": + fix_potential_issue() From eb787ae540248f728cf054b5132824bd50689c19 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Mon, 28 Sep 2020 11:55:35 -0400 Subject: [PATCH 2/5] Make items into list, just in case someone tries to merge object into itself. --- src/allmydata/util/dictutil.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/util/dictutil.py b/src/allmydata/util/dictutil.py index 2a80e7e33..3ace8fca4 100644 --- a/src/allmydata/util/dictutil.py +++ b/src/allmydata/util/dictutil.py @@ -24,7 +24,7 @@ class DictOfSets(dict): self[key] = set([value]) def update(self, otherdictofsets): - for key, values in otherdictofsets.items(): + for key, values in list(otherdictofsets.items()): if key in self: self[key].update(values) else: From f578b85f8ce39fe8e5844f999c8ed812f13ccad5 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Mon, 28 Sep 2020 11:56:15 -0400 Subject: [PATCH 3/5] News file. --- newsfragments/3417.minor | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 newsfragments/3417.minor diff --git a/newsfragments/3417.minor b/newsfragments/3417.minor new file mode 100644 index 000000000..e69de29bb From 66b4330ca2ada66632a30c266737d0fa1a0899d5 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Mon, 28 Sep 2020 15:43:42 -0400 Subject: [PATCH 4/5] Remove unused API. --- src/allmydata/util/deferredutil.py | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/allmydata/util/deferredutil.py b/src/allmydata/util/deferredutil.py index 9ce05ef3a..1d13f61e6 100644 --- a/src/allmydata/util/deferredutil.py +++ b/src/allmydata/util/deferredutil.py @@ -180,17 +180,6 @@ class HookMixin(object): log.msg(msg, level=log.NOISY) -def for_items(cb, mapping): - """ - For each (key, value) pair in a mapping, I add a callback to cb(None, key, value) - to a Deferred that fires immediately. I return that Deferred. - """ - d = defer.succeed(None) - for k, v in mapping.items(): - d.addCallback(lambda ign, k=k, v=v: cb(None, k, v)) - return d - - class WaitForDelayedCallsMixin(PollMixin): def _delayed_calls_done(self): # We're done when the only remaining DelayedCalls fire after threshold. From 7aa7716f3af0ab0f0142747c50f364d31163fa16 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Mon, 28 Sep 2020 15:44:29 -0400 Subject: [PATCH 5/5] Wrap with list(), just in case. --- src/allmydata/test/test_storage.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py index b9e3d00c6..30abf9cdc 100644 --- a/src/allmydata/test/test_storage.py +++ b/src/allmydata/test/test_storage.py @@ -547,7 +547,7 @@ class Server(unittest.TestCase): already,writers = self.allocate(ss, b"disconnect", [0,1,2], 75, canary) self.failUnlessEqual(already, set()) self.failUnlessEqual(set(writers.keys()), set([0,1,2])) - for (f,args,kwargs) in canary.disconnectors.values(): + for (f,args,kwargs) in list(canary.disconnectors.values()): f(*args, **kwargs) del already del writers