Update twisted version in webkitpy.thirdparty.autoinstalled module.
Created attachment 272171 [details] Patch
Attachment 272171 [details] did not pass style-queue: ERROR: Tools/ChangeLog:1: ChangeLog entry has no bug number [changelog/bugnumber] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 272171 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=272171&action=review > Tools/ChangeLog:11 > + As far as I know, only benchmark runner uses twisted module in webkitpy module.
Comment on attachment 272171 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=272171&action=review >> Tools/ChangeLog:11 >> + > > As far as I know, only benchmark runner uses twisted module in webkitpy module. How can this be confirmed?
Comment on attachment 272171 [details] Patch Buildbot master uses an ancient buildbot version, which needs this old twisted. I added exactly this version to autoinstall to be able to run buildbot master.cfg unittest out of the box.
See https://bugs.webkit.org/show_bug.cgi?id=142219 for details.
Additionally this change is incorrect, because after this change autoinstaller still installs the old twisted version: $ Tools/Scripts/test-webkitpy Suppressing most webkitpy logging while running unit tests. Skipping tests in the following modules or packages because they are really, really, slow: webkitpy.common.checkout.scm.scm_unittest (https://bugs.webkit.org/show_bug.cgi?id=31818; use --all to include) Checking autoinstalled packages ...Auto-installing package: jinja2 Auto-installing package: sqlalchemy Auto-installing package: twisted Auto-installing package: buildbot Auto-installing package: coverage Auto-installing package: eliza.py Auto-installing package: mechanize Auto-installing package: pep8.py Auto-installing package: logilab.common Auto-installing package: logilab.astng Auto-installing package: pylint Auto-installing package: twisted $ cat Tools/Scripts/webkitpy/thirdparty/autoinstalled/twisted/.twisted.url https://pypi.python.org/packages/source/T/Twisted/Twisted-12.1.0.tar.bz2#md5=f396f1d6f5321e869c2f89b2196a9eb5 The root of this problem is https://trac.webkit.org/changeset/187925 which took no account of already installed twisted by _install_buildbot
Created attachment 274048 [details] Patch
Attachment 274048 [details] did not pass style-queue: ERROR: Tools/Scripts/webkitpy/benchmark_runner/http_server_driver/http_server/twisted_http_server.py:12: No name 'new_twisted' in module 'webkitpy.thirdparty.autoinstalled' [pylint/E0611] [5] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 274048 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=274048&action=review > Tools/Scripts/webkitpy/benchmark_runner/http_server_driver/http_server/twisted_http_server.py:-32 > - reactor.stop() You should explain this change in the change log.
Created attachment 274052 [details] Patch for landing
Comment on attachment 274052 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=274052&action=review > Tools/ChangeLog:8 > + Add lasted twisted to webkitpy.thirdparty.autoinstalled. lasted? latest? > Tools/Scripts/webkitpy/benchmark_runner/http_server_driver/http_server/twisted_http_server.py:12 > + from webkitpy.thirdparty.autoinstalled.new_twisted import twisted Using "new" in code is a mistake. In a year or two, it's no longer new and the read doesn't really know what changed. If it has a version number, say that explicitly. > Tools/Scripts/webkitpy/thirdparty/__init__.py:160 > + def _install_new_twisted(self): Don't say "new".
Created attachment 274062 [details] Patch
Attachment 274062 [details] did not pass style-queue: ERROR: Tools/Scripts/webkitpy/benchmark_runner/http_server_driver/http_server/twisted_http_server.py:12: No name 'twisted_15_5_0' in module 'webkitpy.thirdparty.autoinstalled' [pylint/E0611] [5] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 274062 [details] Patch Clearing r+ flag, since this has been landed and rolled out. Additionally, it's not clear from bug comments if Ossy's concerns have been addressed.
Created attachment 291211 [details] Patch
*** Bug 163261 has been marked as a duplicate of this bug. ***
(In reply to comment #6) > See https://bugs.webkit.org/show_bug.cgi?id=142219 for details. Updated the patch. Could you take a look? Thanks.
I have tested this. It works fine on the GTK+ platform. But I see that it won't auto-install twisted 15.0 or use it if the system already has python-twisted installed. If you want to ensure this script always uses twisted 15.0 you can try something like: --- a/Tools/Scripts/webkitpy/benchmark_runner/http_server_driver/http_server/twisted_http_server.py +++ b/Tools/Scripts/webkitpy/benchmark_runner/http_server_driver/http_server/twisted_http_server.py @@ -5,11 +5,8 @@ import logging import os import sys -try: - import twisted -except ImportError: - sys.path.append(os.path.abspath(os.path.join(os.path.dirname(os.path.abspath(__file__)), '../../../..'))) - from webkitpy.thirdparty.autoinstalled.twisted import twisted +sys.path.append(os.path.abspath(os.path.join(os.path.dirname(os.path.abspath(__file__)), '../../../..'))) +from webkitpy.thirdparty.autoinstalled.twisted_15_5_0 import twisted from twisted.web import static, server from twisted.web.resource import Resource By the way.. what is the context for requiring twisted 15.0 here? Is it necessary for some run-benchmark test?
(In reply to comment #19) > > By the way.. what is the context for requiring twisted 15.0 here? > Is it necessary for some run-benchmark test? Yeah, older versions of twisted has a bug whereby which some benchmark page fails to load.
(In reply to comment #19) > I have tested this. It works fine on the GTK+ platform. > > But I see that it won't auto-install twisted 15.0 or use it if the system > already has python-twisted installed. > > If you want to ensure this script always uses twisted 15.0 you can try > something like: > > > --- > a/Tools/Scripts/webkitpy/benchmark_runner/http_server_driver/http_server/ > twisted_http_server.py > +++ > b/Tools/Scripts/webkitpy/benchmark_runner/http_server_driver/http_server/ > twisted_http_server.py > @@ -5,11 +5,8 @@ import logging > import os > import sys > > -try: > - import twisted > -except ImportError: > - > sys.path.append(os.path.abspath(os.path.join(os.path.dirname(os.path. > abspath(__file__)), '../../../..'))) > - from webkitpy.thirdparty.autoinstalled.twisted import twisted > +sys.path.append(os.path.abspath(os.path.join(os.path.dirname(os.path. > abspath(__file__)), '../../../..'))) > +from webkitpy.thirdparty.autoinstalled.twisted_15_5_0 import twisted > > from twisted.web import static, server > from twisted.web.resource import Resource That sounds good to me. I think it's even better to check the version of twisted, if it is a version earlier than 15.5.0(this version is working for sure), we should always use autoinstalled module in webkitpy.thirdparty. > > > > > By the way.. what is the context for requiring twisted 15.0 here? > Is it necessary for some run-benchmark test?
Created attachment 293481 [details] Patch
Created attachment 293504 [details] Patch for landing
Comment on attachment 293504 [details] Patch for landing Clearing flags on attachment: 293504 Committed r208205: <http://trac.webkit.org/changeset/208205>
All reviewed patches have been landed. Closing bug.