From 5a08cf3b9606bf77ef02a37dca1e2bc76f74558f Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Fri, 2 Sep 2016 00:22:22 +0300 Subject: [PATCH 01/12] Fix test_start_requests_errors for PyPy Twisted prints errors in DebugInfo.__del__, but PyPy does not run gc.collect() on exit: http://doc.pypy.org/en/latest/cpython_differences.html?highlight=gc.collect#differences-related-to-garbage-collection-strategies --- scrapy/cmdline.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/scrapy/cmdline.py b/scrapy/cmdline.py index e4dc7f2de..b546d030e 100644 --- a/scrapy/cmdline.py +++ b/scrapy/cmdline.py @@ -4,6 +4,7 @@ import optparse import cProfile import inspect import pkg_resources +import gc import scrapy from scrapy.crawler import CrawlerProcess @@ -165,4 +166,9 @@ def _run_command_profiled(cmd, args, opts): p.dump_stats(opts.profile) if __name__ == '__main__': - execute() + try: + execute() + finally: + # Twisted prints errors in DebugInfo.__del__, but PyPy does not run gc.collect() + # on exit: http://doc.pypy.org/en/latest/cpython_differences.html?highlight=gc.collect#differences-related-to-garbage-collection-strategies + gc.collect() From 6014856df5717496b57aaac6c8a2e64b125ac32b Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Thu, 1 Sep 2016 18:39:25 +0300 Subject: [PATCH 02/12] Fix test_output_processor_error undere PyPy For float(u'$10') PyPy includes "u'" in the error message, and it's more fair to check error message on input we are really passing. --- tests/test_loader.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_loader.py b/tests/test_loader.py index 2693a18d9..9d07eb95b 100644 --- a/tests/test_loader.py +++ b/tests/test_loader.py @@ -290,7 +290,7 @@ class BasicItemLoaderTest(unittest.TestCase): il = TestItemLoader() il.add_value('name', [u'$10']) try: - float('$10') + float(u'$10') except Exception as e: expected_exc_str = str(e) From c3d17659b33432ba8cd2e5b2da57105c1cc0da22 Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Thu, 1 Sep 2016 15:39:16 +0300 Subject: [PATCH 03/12] Fix queue serialization test on PyPy It is not affected by Twisted bug #7989 and is more permissive with pickling (especially with protocol=2). --- tests/test_squeues.py | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/tests/test_squeues.py b/tests/test_squeues.py index 48871ceeb..3a24348b4 100644 --- a/tests/test_squeues.py +++ b/tests/test_squeues.py @@ -1,3 +1,5 @@ +import pickle + from queuelib.tests import test_queue as t from scrapy.squeues import MarshalFifoDiskQueue, MarshalLifoDiskQueue, PickleFifoDiskQueue, PickleLifoDiskQueue from scrapy.item import Item, Field @@ -14,6 +16,22 @@ class TestLoader(ItemLoader): default_item_class = TestItem name_out = staticmethod(_test_procesor) +def nonserializable_object_test(self): + try: + pickle.dumps(lambda x: x) + except Exception: + # Trigger Twisted bug #7989 + import twisted.persisted.styles # NOQA + q = self.queue() + self.assertRaises(ValueError, q.push, lambda x: x) + else: + # Use a different unpickleable object + class A(object): pass + a = A() + a.__reduce__ = a.__reduce_ex__ = None + q = self.queue() + self.assertRaises(ValueError, q.push, a) + class MarshalFifoDiskQueueTest(t.FifoDiskQueueTest): chunksize = 100000 @@ -30,11 +48,7 @@ class MarshalFifoDiskQueueTest(t.FifoDiskQueueTest): self.assertEqual(q.pop(), 123) self.assertEqual(q.pop(), {'a': 'dict'}) - def test_nonserializable_object(self): - # Trigger Twisted bug #7989 - import twisted.persisted.styles # NOQA - q = self.queue() - self.assertRaises(ValueError, q.push, lambda x: x) + test_nonserializable_object = nonserializable_object_test class ChunkSize1MarshalFifoDiskQueueTest(MarshalFifoDiskQueueTest): chunksize = 1 @@ -110,11 +124,7 @@ class MarshalLifoDiskQueueTest(t.LifoDiskQueueTest): self.assertEqual(q.pop(), 123) self.assertEqual(q.pop(), 'a') - def test_nonserializable_object(self): - # Trigger Twisted bug #7989 - import twisted.persisted.styles # NOQA - q = self.queue() - self.assertRaises(ValueError, q.push, lambda x: x) + test_nonserializable_object = nonserializable_object_test class PickleLifoDiskQueueTest(MarshalLifoDiskQueueTest): From 5abb70c8d71365adbf3a035b1f6d07f5fbbbf446 Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Thu, 1 Sep 2016 15:43:57 +0300 Subject: [PATCH 04/12] Fix test_weakkeycache on PyPy: run gc.collect() One gc.collect() seems to be enough, but it's more reliable to run it several times (at most 100), until all objects are collected. --- tests/test_utils_python.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/test_utils_python.py b/tests/test_utils_python.py index 9a0cc975d..e22bd8eb6 100644 --- a/tests/test_utils_python.py +++ b/tests/test_utils_python.py @@ -1,3 +1,4 @@ +import gc import functools import operator import unittest @@ -144,6 +145,9 @@ class UtilsPythonTestCase(unittest.TestCase): self.assertNotEqual(v, wk[_Weakme()]) self.assertEqual(v, wk[k]) del k + for _ in range(100): + if wk._weakdict: + gc.collect() self.assertFalse(len(wk._weakdict)) @unittest.skipUnless(six.PY2, "deprecated function") From 7c67047e77914dfe0d6666e4dc535c684aa77090 Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Thu, 1 Sep 2016 18:35:57 +0300 Subject: [PATCH 05/12] Fix get_func_args tests under PyPy On CPython get_func_args does not work correctly for built-in methods. --- tests/test_utils_python.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/tests/test_utils_python.py b/tests/test_utils_python.py index e22bd8eb6..8becca0f1 100644 --- a/tests/test_utils_python.py +++ b/tests/test_utils_python.py @@ -3,6 +3,7 @@ import functools import operator import unittest from itertools import count +import platform import six from scrapy.utils.python import ( @@ -212,10 +213,16 @@ class UtilsPythonTestCase(unittest.TestCase): self.assertEqual(get_func_args(cal), ['a', 'b', 'c']) self.assertEqual(get_func_args(object), []) - # TODO: how do we fix this to return the actual argument names? - self.assertEqual(get_func_args(six.text_type.split), []) - self.assertEqual(get_func_args(" ".join), []) - self.assertEqual(get_func_args(operator.itemgetter(2)), []) + if platform.python_implementation() == 'CPython': + # TODO: how do we fix this to return the actual argument names? + self.assertEqual(get_func_args(six.text_type.split), []) + self.assertEqual(get_func_args(" ".join), []) + self.assertEqual(get_func_args(operator.itemgetter(2)), []) + else: + self.assertEqual(get_func_args(six.text_type.split), ['sep', 'maxsplit']) + self.assertEqual(get_func_args(" ".join), ['list']) + self.assertEqual(get_func_args(operator.itemgetter(2)), ['obj']) + def test_without_none_values(self): self.assertEqual(without_none_values([1, None, 3, 4]), [1, 3, 4]) From 19ca986aa1340b08658caaac0ce677aa22be9814 Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Fri, 2 Sep 2016 00:32:33 +0300 Subject: [PATCH 06/12] Move garbage_collect to scrapy.utils.python --- scrapy/cmdline.py | 4 ++-- scrapy/utils/python.py | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/scrapy/cmdline.py b/scrapy/cmdline.py index b546d030e..dc6b59fe0 100644 --- a/scrapy/cmdline.py +++ b/scrapy/cmdline.py @@ -4,7 +4,6 @@ import optparse import cProfile import inspect import pkg_resources -import gc import scrapy from scrapy.crawler import CrawlerProcess @@ -12,6 +11,7 @@ from scrapy.commands import ScrapyCommand from scrapy.exceptions import UsageError from scrapy.utils.misc import walk_modules from scrapy.utils.project import inside_project, get_project_settings +from scrapy.utils.python import garbage_collect from scrapy.settings.deprecated import check_deprecated_settings def _iter_command_classes(module_name): @@ -171,4 +171,4 @@ if __name__ == '__main__': finally: # Twisted prints errors in DebugInfo.__del__, but PyPy does not run gc.collect() # on exit: http://doc.pypy.org/en/latest/cpython_differences.html?highlight=gc.collect#differences-related-to-garbage-collection-strategies - gc.collect() + garbage_collect() diff --git a/scrapy/utils/python.py b/scrapy/utils/python.py index 4c500abf4..d28d71bd3 100644 --- a/scrapy/utils/python.py +++ b/scrapy/utils/python.py @@ -1,6 +1,7 @@ """ This module contains essential stuff that should've come with Python itself ;) """ +import gc import os import re import inspect @@ -8,6 +9,8 @@ import weakref import errno import six from functools import partial, wraps +import sys +import time from scrapy.utils.decorators import deprecated @@ -355,3 +358,20 @@ def global_object_name(obj): 'scrapy.http.request.Request' """ return "%s.%s" % (obj.__module__, obj.__name__) + + +if sys.platform.startswith('java'): + def garbage_collect(): + # Some JVM GCs will execute finalizers in a different thread, meaning + # we need to wait for that to complete before we go on looking for the + # effects of that. + gc.collect() + time.sleep(0.1) +elif hasattr(sys, "pypy_version_info"): + def garbage_collect(): + # Collecting weakreferences can take two collections on PyPy. + gc.collect() + gc.collect() +else: + def garbage_collect(): + gc.collect() From b4eb60e5270498a64200332587a065ec9978a333 Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Thu, 15 Jun 2017 13:24:06 +0300 Subject: [PATCH 07/12] Install PyPyDispatcher for PyPy tests Using https://github.com/lopuhin/pydispatcher, pypy branch. This is executed as a separate step to avoid changing default requirements.txt and setup.py. If just added to "deps" in tox, this install command will be executed as one command and PyPyDispatcher will not override PyDispatcher. --- tox.ini | 1 + 1 file changed, 1 insertion(+) diff --git a/tox.ini b/tox.ini index 6987847f8..c559f1e47 100644 --- a/tox.ini +++ b/tox.ini @@ -57,6 +57,7 @@ commands = [testenv:pypy] basepython = pypy commands = + pip install PyPyDispatcher>=2.0.6 py.test {posargs:scrapy tests} [testenv:py33] From a8df0900713a1a56cffca8774bc137a6e781877e Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Thu, 15 Jun 2017 14:29:28 +0300 Subject: [PATCH 08/12] Fix httpcache leveldb tests: gc.collect after del LevelDB does not have "official" close method, so we have to rely on garbage collection to close it. --- scrapy/extensions/httpcache.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scrapy/extensions/httpcache.py b/scrapy/extensions/httpcache.py index 2fb4b6a15..648b32ec7 100644 --- a/scrapy/extensions/httpcache.py +++ b/scrapy/extensions/httpcache.py @@ -13,7 +13,7 @@ from scrapy.responsetypes import responsetypes from scrapy.utils.request import request_fingerprint from scrapy.utils.project import data_path from scrapy.utils.httpobj import urlparse_cached -from scrapy.utils.python import to_bytes, to_unicode +from scrapy.utils.python import to_bytes, to_unicode, garbage_collect logger = logging.getLogger(__name__) @@ -362,6 +362,7 @@ class LeveldbCacheStorage(object): # avoid them being removed in storages with timestamp-based autoremoval. self.db.CompactRange() del self.db + garbage_collect() def retrieve_response(self, spider, request): data = self._read_data(spider, request) From ea08b952801b9af9bc45ae9918fcf5ab9bcb5aaf Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Mon, 19 Jun 2017 16:45:29 +0300 Subject: [PATCH 09/12] Remove Jython gc branch: it's not supported --- scrapy/utils/python.py | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/scrapy/utils/python.py b/scrapy/utils/python.py index d28d71bd3..72f8f4311 100644 --- a/scrapy/utils/python.py +++ b/scrapy/utils/python.py @@ -360,14 +360,7 @@ def global_object_name(obj): return "%s.%s" % (obj.__module__, obj.__name__) -if sys.platform.startswith('java'): - def garbage_collect(): - # Some JVM GCs will execute finalizers in a different thread, meaning - # we need to wait for that to complete before we go on looking for the - # effects of that. - gc.collect() - time.sleep(0.1) -elif hasattr(sys, "pypy_version_info"): +if hasattr(sys, "pypy_version_info"): def garbage_collect(): # Collecting weakreferences can take two collections on PyPy. gc.collect() From 271b3a485cbdce1b0a866d9fb9938d46d7ddb497 Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Mon, 19 Jun 2017 16:46:16 +0300 Subject: [PATCH 10/12] Require pypy build to pass --- .travis.yml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index 906115096..449eee96b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -11,6 +11,8 @@ matrix: env: TOXENV=py27 - python: 2.7 env: TOXENV=jessie + - python: 2.7 + env: TOXENV=pypy - python: 3.3 env: TOXENV=py33 - python: 3.5 @@ -21,9 +23,6 @@ matrix: env: TOXENV=pypy - python: 3.6 env: TOXENV=docs - allow_failures: - - python: 2.7 - env: TOXENV=pypy install: - | if [ "$TOXENV" = "pypy" ]; then From 5ba8e5adc0e1aa7d9430b63a0094c6a71c51564a Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Mon, 19 Jun 2017 17:45:28 +0300 Subject: [PATCH 11/12] Remove duplicate PyPy toxenv from Travis config Thanks for the catch @redapple --- .travis.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 449eee96b..4f44d1e6d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -19,8 +19,6 @@ matrix: env: TOXENV=py35 - python: 3.6 env: TOXENV=py36 - - python: 2.7 - env: TOXENV=pypy - python: 3.6 env: TOXENV=docs install: From b0a9236357dd74381b5a014e3de15a3a52de4f7d Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Mon, 19 Jun 2017 19:16:50 +0300 Subject: [PATCH 12/12] Use environment markers for custom PyPy requirements --- setup.py | 23 ++++++++++++++++++++++- tox.ini | 1 - 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/setup.py b/setup.py index 086ab8142..c03f0b9f7 100644 --- a/setup.py +++ b/setup.py @@ -1,11 +1,31 @@ from os.path import dirname, join -from setuptools import setup, find_packages +from pkg_resources import parse_version +from setuptools import setup, find_packages, __version__ as setuptools_version with open(join(dirname(__file__), 'scrapy/VERSION'), 'rb') as f: version = f.read().decode('ascii').strip() +def has_environment_marker_platform_impl_support(): + """Code extracted from 'pytest/setup.py' + https://github.com/pytest-dev/pytest/blob/7538680c/setup.py#L31 + + The first known release to support environment marker with range operators + it is 18.5, see: + https://setuptools.readthedocs.io/en/latest/history.html#id235 + """ + return parse_version(setuptools_version) >= parse_version('18.5') + + +extras_require = {} + +if has_environment_marker_platform_impl_support(): + extras_require[':platform_python_implementation == "PyPy"'] = [ + 'PyPyDispatcher>=2.1.0', + ] + + setup( name='Scrapy', version=version, @@ -53,4 +73,5 @@ setup( 'PyDispatcher>=2.0.5', 'service_identity', ], + extras_require=extras_require, ) diff --git a/tox.ini b/tox.ini index c559f1e47..6987847f8 100644 --- a/tox.ini +++ b/tox.ini @@ -57,7 +57,6 @@ commands = [testenv:pypy] basepython = pypy commands = - pip install PyPyDispatcher>=2.0.6 py.test {posargs:scrapy tests} [testenv:py33]