The build tools fail when the default python is python 3. This is currently worked around by telling the user to change their environment: https://trac.webkit.org/browser/trunk/Tools/gtk/install-dependencies#L324 A better solution is to either: - Change the shebangs and such everywhere to #!/usr/bin/env python2, which should work on all systems - Make the Python code work on both Python 2 and Python 3 Currently working on patches that use the first solution, and then will consider patches that work towards the second solution.
FWIW, there is no python2 on OS X.
Created attachment 335141 [details] First pass I needed to be able to build WebKit using python 3, so I made a patch (running 2to3 along with some manual changes). In some cases, it wasn't clear to me whether files needed to be opened with "wb", since they appear to be text, though I figured that the "b" might be there for a reason and left things as they were, though opening the file as text would allow a str to be passed to write(). Similarly, I'm guessing that there's a better way to handle jsmin.py.
Comment on attachment 335141 [details] First pass View in context: https://bugs.webkit.org/attachment.cgi?id=335141&action=review Is SUSE dropping python2 early? Anyway, I think this patch is good. Maintaining compatibility with both python2 and python3 is going to be painful, but it's obvious that we can't delay this transition much longer, and I assume Apple wants to stick with python2 for now. > Source/JavaScriptCore/generate-bytecode-files:230 > + #dowrite(bytecodeHFile, (cCopyrightMsg % bytecodeJSONFile) This shouldn't be commented out!
(In reply to Mike Gorse from comment #2) > In some cases, it wasn't clear to me whether files needed to be opened with > "wb", since they appear to be text, though I figured that the "b" might be > there for a reason and left things as they were, though opening the file as > text would allow a str to be passed to write(). Should probably take the opportunity to switch to "wb", indeed. > Similarly, I'm guessing that there's a better way to handle jsmin.py. I'm not a python expert. Maybe Konstantin has some opinion on this. I'm OK with uglifying the code in order to support both versions of python at the same time. Anyway, I'll run this through the early warning service to see how the Mac and Windows bots react... it would be impressive if the first attempt doesn't break anything.
Attachment 335141 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_model.py:159: missing whitespace after ',' [pep8/E231] [5] ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_model.py:159: [BuiltinFunction.__lt__] Method should have "self" as first argument [pylint/E0213] [5] ERROR: Source/JavaScriptCore/inspector/scripts/codegen/generate_cpp_frontend_dispatcher_header.py:49: [CppFrontendDispatcherHeaderGenerator.domains_to_generate] Instance of 'CppFrontendDispatcherHeaderGenerator' has no 'events_for_domain' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/inspector/scripts/codegen/generate_objc_backend_dispatcher_header.py:50: [ObjCBackendDispatcherHeaderGenerator.domains_to_generate] Instance of 'ObjCBackendDispatcherHeaderGenerator' has no 'should_generate_commands_for_domain' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/inspector/scripts/codegen/generate_objc_frontend_dispatcher_implementation.py:48: [ObjCFrontendDispatcherImplementationGenerator.domains_to_generate] Instance of 'ObjCFrontendDispatcherImplementationGenerator' has no 'should_generate_events_for_domain' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/Scripts/generate-js-builtins.py:56: expected 2 blank lines, found 1 [pep8/E302] [5] ERROR: Source/JavaScriptCore/Scripts/generateYarrUnicodePropertyTables.py:96: no spaces around keyword / parameter equals [pep8/E251] [5] ERROR: Source/JavaScriptCore/Scripts/generateYarrUnicodePropertyTables.py:220: expected 2 blank lines, found 1 [pep8/E302] [5] ERROR: Source/JavaScriptCore/inspector/scripts/codegen/generate_objc_protocol_type_conversions_header.py:54: [ObjCProtocolTypeConversionsHeaderGenerator.domains_to_generate] Instance of 'ObjCProtocolTypeConversionsHeaderGenerator' has no 'should_generate_types_for_domain' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/inspector/scripts/codegen/generate_cpp_backend_dispatcher_header.py:49: [CppBackendDispatcherHeaderGenerator.domains_to_generate] Instance of 'CppBackendDispatcherHeaderGenerator' has no 'commands_for_domain' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/inspector/scripts/codegen/generate_cpp_protocol_types_header.py:346: [CppProtocolTypesHeaderGenerator._generate_forward_declarations_for_binding_traits] Instance of 'CppProtocolTypesHeaderGenerator' has no 'type_needs_shape_assertions' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/inspector/scripts/codegen/generate_cpp_backend_dispatcher_implementation.py:48: [CppBackendDispatcherImplementationGenerator.domains_to_generate] Instance of 'CppBackendDispatcherImplementationGenerator' has no 'commands_for_domain' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_combined_header.py:161: [BuiltinsCombinedHeaderGenerator.generate_section_for_global_private_code_name_macro] Instance of 'BuiltinsCombinedHeaderGenerator' has no 'model' member [pylint/E1101] [5] Total errors found: 13 in 39 files If any of these errors are false positives, please file a bug against check-webkit-style.
You'll need to fix the EWS bots to not be red. First problem is: Traceback (most recent call last): File "JavaScriptCore/Scripts/generate-js-builtins.py", line 41, in <module> from builtins_model import * ImportError: No module named builtins_model
(In reply to Michael Catanzaro from comment #3) > Is SUSE dropping python2 early? Not for openSUSE that I know of. For SLE 15, it will be part of the legacy module. > > Source/JavaScriptCore/generate-bytecode-files:230 > > + #dowrite(bytecodeHFile, (cCopyrightMsg % bytecodeJSONFile) > > This shouldn't be commented out! Thanks for catching that. I've fixed it in my copy--I probably commented it out while debugging the syntax error and forgot to uncomment. I need to research EWS--I'd been testing against webkitgtk, but there are problems that I hadn't come across.
The EWS is the bunch of green and red bubbles above the comments on this Bugzilla page. You want them to be green. :) You can attach new speculative patches to try to get closer to successful builds on macOS/iOS/Windows without needing to build for those platforms yourself.
Created attachment 346638 [details] Revised patch. Rebased/updated. Fix erroneously-commented line to write out a copyright. Fixed a few style issues. Hoping I fixed the build on MacOS; renamed builtins to wkbuiltins to avoid a name conflict in python 3. This makes the patch larger than it should be.
Please also use Tools/Scripts/prepare-ChangeLog -b 156674 to generate appropriate ChangeLog entries
(In reply to Michael Catanzaro from comment #10) > Please also use Tools/Scripts/prepare-ChangeLog -b 156674 to generate > appropriate ChangeLog entries And mark the patch with the r? cq? Bugzilla flags to request review and commit-queue, respectively.
Created attachment 347950 [details] Updated patch.
Comment on attachment 347950 [details] Updated patch. Hm, looks like none of the bots were able to apply the patch: https://webkit-queues.webkit.org/results/8962750 Make sure to use 'git add' to add all moved files to the patch before using 'webkit-patch upload'
Created attachment 348154 [details] Patch
Attachment 348154 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_wrapper_header.py:41: [BuiltinsWrapperHeaderGenerator.output_filename] Instance of 'BuiltinsWrapperHeaderGenerator' has no 'model' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_wrapper_header.py:45: [BuiltinsWrapperHeaderGenerator.generate_output] Instance of 'BuiltinsWrapperHeaderGenerator' has no 'model' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_wrapper_header.py:49: [BuiltinsWrapperHeaderGenerator.generate_output] Instance of 'BuiltinsWrapperHeaderGenerator' has no 'generate_license' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_wrapper_header.py:66: [BuiltinsWrapperHeaderGenerator.generate_secondary_header_includes] Instance of 'BuiltinsWrapperHeaderGenerator' has no 'model' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_wrapper_header.py:69: [BuiltinsWrapperHeaderGenerator.generate_secondary_header_includes] Instance of 'BuiltinsWrapperHeaderGenerator' has no 'generate_includes_from_entries' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_wrapper_header.py:94: [BuiltinsWrapperHeaderGenerator.generate_constructor] Instance of 'BuiltinsWrapperHeaderGenerator' has no 'model' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_wrapper_header.py:98: [BuiltinsWrapperHeaderGenerator.generate_constructor] Instance of 'BuiltinsWrapperHeaderGenerator' has no 'model' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_wrapper_header.py:108: [BuiltinsWrapperHeaderGenerator.generate_accessors] Instance of 'BuiltinsWrapperHeaderGenerator' has no 'model' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_wrapper_header.py:116: [BuiltinsWrapperHeaderGenerator.generate_members] Instance of 'BuiltinsWrapperHeaderGenerator' has no 'model' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_separate_header.py:49: [BuiltinsSeparateHeaderGenerator.macro_prefix] Instance of 'BuiltinsSeparateHeaderGenerator' has no 'model' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_separate_header.py:53: [BuiltinsSeparateHeaderGenerator.generate_output] Instance of 'BuiltinsSeparateHeaderGenerator' has no 'model' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_separate_header.py:62: [BuiltinsSeparateHeaderGenerator.generate_output] Instance of 'BuiltinsSeparateHeaderGenerator' has no 'generate_license' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_separate_header.py:74: [BuiltinsSeparateHeaderGenerator.generate_output] Instance of 'BuiltinsSeparateHeaderGenerator' has no 'model' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_separate_header.py:108: [BuiltinsSeparateHeaderGenerator.generate_secondary_header_includes] Instance of 'BuiltinsSeparateHeaderGenerator' has no 'generate_includes_from_entries' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_separate_header.py:166: [BuiltinsSeparateHeaderGenerator.generate_section_for_code_table_macro] Instance of 'BuiltinsSeparateHeaderGenerator' has no 'model' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_internals_wrapper_header.py:64: whitespace before ']' [pep8/E202] [5] ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_internals_wrapper_header.py:42: [BuiltinsInternalsWrapperHeaderGenerator.output_filename] Instance of 'BuiltinsInternalsWrapperHeaderGenerator' has no 'model' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_internals_wrapper_header.py:46: [BuiltinsInternalsWrapperHeaderGenerator.generate_output] Instance of 'BuiltinsInternalsWrapperHeaderGenerator' has no 'model' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_internals_wrapper_header.py:50: [BuiltinsInternalsWrapperHeaderGenerator.generate_output] Instance of 'BuiltinsInternalsWrapperHeaderGenerator' has no 'generate_license' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_internals_wrapper_header.py:69: [BuiltinsInternalsWrapperHeaderGenerator.generate_secondary_header_includes] Instance of 'BuiltinsInternalsWrapperHeaderGenerator' has no 'generate_includes_from_entries' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/inspector/scripts/codegen/generate_cpp_protocol_types_implementation.py:182: [CppProtocolTypesImplementationGenerator._generate_builders_for_domain] Instance of 'CppProtocolTypesImplementationGenerator' has no 'type_needs_shape_assertions' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/Scripts/generateYarrUnicodePropertyTables.py:96: no spaces around keyword / parameter equals [pep8/E251] [5] ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_separate_implementation.py:49: [BuiltinsSeparateImplementationGenerator.macro_prefix] Instance of 'BuiltinsSeparateImplementationGenerator' has no 'model' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_separate_implementation.py:53: [BuiltinsSeparateImplementationGenerator.generate_output] Instance of 'BuiltinsSeparateImplementationGenerator' has no 'model' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_separate_implementation.py:62: [BuiltinsSeparateImplementationGenerator.generate_output] Instance of 'BuiltinsSeparateImplementationGenerator' has no 'generate_license' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_separate_implementation.py:64: [BuiltinsSeparateImplementationGenerator.generate_output] Instance of 'BuiltinsSeparateImplementationGenerator' has no 'generate_primary_header_includes' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_separate_implementation.py:70: [BuiltinsSeparateImplementationGenerator.generate_output] Instance of 'BuiltinsSeparateImplementationGenerator' has no 'generate_embedded_code_string_section_for_function' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_separate_implementation.py:71: [BuiltinsSeparateImplementationGenerator.generate_output] Instance of 'BuiltinsSeparateImplementationGenerator' has no 'model' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_separate_implementation.py:73: [BuiltinsSeparateImplementationGenerator.generate_output] Instance of 'BuiltinsSeparateImplementationGenerator' has no 'model' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_separate_implementation.py:112: [BuiltinsSeparateImplementationGenerator.generate_secondary_header_includes] Instance of 'BuiltinsSeparateImplementationGenerator' has no 'generate_includes_from_entries' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:45: [BuiltinsCombinedImplementationGenerator.output_filename] Instance of 'BuiltinsCombinedImplementationGenerator' has no 'model' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:49: [BuiltinsCombinedImplementationGenerator.generate_output] Instance of 'BuiltinsCombinedImplementationGenerator' has no 'model' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:50: [BuiltinsCombinedImplementationGenerator.generate_output] Instance of 'BuiltinsCombinedImplementationGenerator' has no 'model' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:54: [BuiltinsCombinedImplementationGenerator.generate_output] Instance of 'BuiltinsCombinedImplementationGenerator' has no 'generate_license' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:56: [BuiltinsCombinedImplementationGenerator.generate_output] Instance of 'BuiltinsCombinedImplementationGenerator' has no 'generate_primary_header_includes' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:59: [BuiltinsCombinedImplementationGenerator.generate_output] Instance of 'BuiltinsCombinedImplementationGenerator' has no 'model' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:60: [BuiltinsCombinedImplementationGenerator.generate_output] Instance of 'BuiltinsCombinedImplementationGenerator' has no 'generate_embedded_code_string_section_for_function' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:61: [BuiltinsCombinedImplementationGenerator.generate_output] Instance of 'BuiltinsCombinedImplementationGenerator' has no 'model' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:63: [BuiltinsCombinedImplementationGenerator.generate_output] Instance of 'BuiltinsCombinedImplementationGenerator' has no 'model' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:100: [BuiltinsCombinedImplementationGenerator.generate_secondary_header_includes] Instance of 'BuiltinsCombinedImplementationGenerator' has no 'generate_includes_from_entries' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/inspector/scripts/codegen/generate_objc_backend_dispatcher_header.py:50: [ObjCBackendDispatcherHeaderGenerator.domains_to_generate] Instance of 'ObjCBackendDispatcherHeaderGenerator' has no 'should_generate_commands_for_domain' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_internals_wrapper_implementation.py:42: [BuiltinsInternalsWrapperImplementationGenerator.output_filename] Instance of 'BuiltinsInternalsWrapperImplementationGenerator' has no 'model' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_internals_wrapper_implementation.py:46: [BuiltinsInternalsWrapperImplementationGenerator.generate_output] Instance of 'BuiltinsInternalsWrapperImplementationGenerator' has no 'model' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_internals_wrapper_implementation.py:50: [BuiltinsInternalsWrapperImplementationGenerator.generate_output] Instance of 'BuiltinsInternalsWrapperImplementationGenerator' has no 'generate_license' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_internals_wrapper_implementation.py:52: [BuiltinsInternalsWrapperImplementationGenerator.generate_output] Instance of 'BuiltinsInternalsWrapperImplementationGenerator' has no 'generate_primary_header_includes' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_internals_wrapper_implementation.py:82: [BuiltinsInternalsWrapperImplementationGenerator.generate_secondary_header_includes] Instance of 'BuiltinsInternalsWrapperImplementationGenerator' has no 'generate_includes_from_entries' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generator.py:135: whitespace before '}' [pep8/E202] [5] ERROR: Source/JavaScriptCore/inspector/scripts/codegen/generate_objc_protocol_type_conversions_header.py:54: [ObjCProtocolTypeConversionsHeaderGenerator.domains_to_generate] Instance of 'ObjCProtocolTypeConversionsHeaderGenerator' has no 'should_generate_types_for_domain' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/inspector/scripts/codegen/generate_cpp_backend_dispatcher_header.py:49: [CppBackendDispatcherHeaderGenerator.domains_to_generate] Instance of 'CppBackendDispatcherHeaderGenerator' has no 'commands_for_domain' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/inspector/scripts/codegen/generate_cpp_protocol_types_header.py:347: [CppProtocolTypesHeaderGenerator._generate_forward_declarations_for_binding_traits] Instance of 'CppProtocolTypesHeaderGenerator' has no 'type_needs_shape_assertions' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/inspector/scripts/codegen/generate_cpp_backend_dispatcher_implementation.py:48: [CppBackendDispatcherImplementationGenerator.domains_to_generate] Instance of 'CppBackendDispatcherImplementationGenerator' has no 'commands_for_domain' member [pylint/E1101] [5] ERROR: Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] Total errors found: 52 in 56 files If any of these errors are false positives, please file a bug against check-webkit-style.
Used webkit-patch upload this time after running git add. Hoping that the bots will be able to apply it. Also fixed a few PEP errors.
IMO, this patch is hard to review because it's not clear where files are moved verbatimly and where they are modified. If there are any changes in moved files, it would be better to split patch in two parts: move stuff verbatimly, and then apply modifications. (There are review systems which are able to track file renames and visualize modifications appropriately; unfortunately our tooling is inferior)
If you re-create the patch with `git format-patch -M<n>%` you can crank up <n> to adjust the rename detection threshold. It defaults to 50%.
Well a git-formatted patch probably won't work, because it's going to be applied with SVN, not git. Anyway: thanks a bunch for working on this, Mike! I agree with Konstantin that splitting out the renames into a separate bug is a good idea. The style checker can be ignored in the rename bug, but in the bug for the code changes it will need to be placated. You can click on the red EWS bubbles to see why the build is failing on Apple's ports. Looks like: make: *** No rule to make target `JavaScriptCore/Scripts/builtins/__init__.py', needed by `JSCBuiltins.h'. Stop. make: *** Waiting for unfinished jobs....
A git formatted patch will just make it easier to review, that's all :)
git formatted patch should work well with commit queue as well
Created attachment 350121 [details] Updated patch.
Attachment 350121 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/inspector/scripts/codegen/generate_cpp_frontend_dispatcher_header.py:49: [CppFrontendDispatcherHeaderGenerator.domains_to_generate] Instance of 'CppFrontendDispatcherHeaderGenerator' has no 'events_for_domain' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/inspector/scripts/codegen/generate_objc_backend_dispatcher_header.py:50: [ObjCBackendDispatcherHeaderGenerator.domains_to_generate] Instance of 'ObjCBackendDispatcherHeaderGenerator' has no 'should_generate_commands_for_domain' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/inspector/scripts/codegen/generate_objc_frontend_dispatcher_implementation.py:48: [ObjCFrontendDispatcherImplementationGenerator.domains_to_generate] Instance of 'ObjCFrontendDispatcherImplementationGenerator' has no 'should_generate_events_for_domain' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/inspector/scripts/codegen/generate_objc_protocol_type_conversions_header.py:54: [ObjCProtocolTypeConversionsHeaderGenerator.domains_to_generate] Instance of 'ObjCProtocolTypeConversionsHeaderGenerator' has no 'should_generate_types_for_domain' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/inspector/scripts/codegen/generate_cpp_backend_dispatcher_header.py:49: [CppBackendDispatcherHeaderGenerator.domains_to_generate] Instance of 'CppBackendDispatcherHeaderGenerator' has no 'commands_for_domain' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/inspector/scripts/codegen/generate_cpp_protocol_types_header.py:347: [CppProtocolTypesHeaderGenerator._generate_forward_declarations_for_binding_traits] Instance of 'CppProtocolTypesHeaderGenerator' has no 'type_needs_shape_assertions' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_header.py:161: [BuiltinsCombinedHeaderGenerator.generate_section_for_global_private_code_name_macro] Instance of 'BuiltinsCombinedHeaderGenerator' has no 'model' member [pylint/E1101] [5] Total errors found: 7 in 45 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 350121 [details] Updated patch. View in context: https://bugs.webkit.org/attachment.cgi?id=350121&action=review > Source/JavaScriptCore/Scripts/cssmin.py:51 > + sys.stdout = io.TextIOWrapper(sys.stdout.buffer, encoding='UTF-8') This is duplication of code above > Source/JavaScriptCore/Scripts/generate-js-builtins.py:50 > +def doopen(file, mode): do_open?
Comment on attachment 350121 [details] Updated patch. View in context: https://bugs.webkit.org/attachment.cgi?id=350121&action=review > Source/JavaScriptCore/inspector/scripts/codegen/cpp_generator.py:32 > +from .models import PrimitiveType, ObjectType, ArrayType, EnumType, AliasedType, Frameworks Mac bots say: ValueError: Attempted relative import in non-package It's probably easier to test locally with python2 to make sure it still works there, rather than relying on the Mac bots to do it.
Created attachment 350303 [details] Updated patch. It looks like the codegen directory is flattened on MacOS/IOS but not on gtk, so imports need to work in both cases.
Attachment 350303 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/inspector/scripts/codegen/generate_cpp_frontend_dispatcher_header.py:55: [CppFrontendDispatcherHeaderGenerator.domains_to_generate] Instance of 'CppFrontendDispatcherHeaderGenerator' has no 'events_for_domain' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/inspector/scripts/codegen/generate_objc_backend_dispatcher_header.py:57: [ObjCBackendDispatcherHeaderGenerator.domains_to_generate] Instance of 'ObjCBackendDispatcherHeaderGenerator' has no 'should_generate_commands_for_domain' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/inspector/scripts/codegen/generate_objc_frontend_dispatcher_implementation.py:54: [ObjCFrontendDispatcherImplementationGenerator.domains_to_generate] Instance of 'ObjCFrontendDispatcherImplementationGenerator' has no 'should_generate_events_for_domain' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/inspector/scripts/codegen/generate_objc_protocol_type_conversions_header.py:60: [ObjCProtocolTypeConversionsHeaderGenerator.domains_to_generate] Instance of 'ObjCProtocolTypeConversionsHeaderGenerator' has no 'should_generate_types_for_domain' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/inspector/scripts/codegen/generate_cpp_backend_dispatcher_header.py:55: [CppBackendDispatcherHeaderGenerator.domains_to_generate] Instance of 'CppBackendDispatcherHeaderGenerator' has no 'commands_for_domain' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/inspector/scripts/codegen/generate_cpp_protocol_types_header.py:353: [CppProtocolTypesHeaderGenerator._generate_forward_declarations_for_binding_traits] Instance of 'CppProtocolTypesHeaderGenerator' has no 'type_needs_shape_assertions' member [pylint/E1101] [5] ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_header.py:161: [BuiltinsCombinedHeaderGenerator.generate_section_for_global_private_code_name_macro] Instance of 'BuiltinsCombinedHeaderGenerator' has no 'model' member [pylint/E1101] [5] Total errors found: 7 in 45 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 350303 [details] Updated patch. View in context: https://bugs.webkit.org/attachment.cgi?id=350303&action=review OK, thanks! The try/except imports are a bit yucky, but I guess that's what we're stuck with if we have XCode doing something different than CMake. > Source/JavaScriptCore/Scripts/wkbuiltins/builtins_model.py:159 > + def __lt__(self, other): > + return self.function_name < other.function_name Why is it needed? > Source/JavaScriptCore/Scripts/wkbuiltins/builtins_model.py:309 > + return list(map(BuiltinFunction.fromString, functionStrings)) You've done this in a couple other places. Why do you need to wrap the map in a list?
(In reply to Michael Catanzaro from comment #28) > Comment on attachment 350303 [details] > Updated patch. > > View in context: > https://bugs.webkit.org/attachment.cgi?id=350303&action=review > > OK, thanks! > > The try/except imports are a bit yucky, but I guess that's what we're stuck > with if we have XCode doing something different than CMake. > > > Source/JavaScriptCore/Scripts/wkbuiltins/builtins_model.py:159 > > + def __lt__(self, other): > > + return self.function_name < other.function_name > > Why is it needed? I added that in order to get sort to work on python 3. http://python3porting.com/problems.html#comparisons > > Source/JavaScriptCore/Scripts/wkbuiltins/builtins_model.py:309 > > + return list(map(BuiltinFunction.fromString, functionStrings)) > > You've done this in a couple other places. Why do you need to wrap the map > in a list? Map returns an iterator in python 3, not a list as in python 2. Those lines were changed automatically when I ran 2to3, though, so it's possible that the wrapping isn't needed in every place where I have it.
Comment on attachment 350303 [details] Updated patch. Clearing flags on attachment: 350303 Committed r236321: <https://trac.webkit.org/changeset/236321>
All reviewed patches have been landed. Closing bug.
<rdar://problem/44682001>
Looks like https://trac.webkit.org/changeset/236321/webkit has caused a builtins-generators failure: https://build.webkit.org/builders/Apple%20Sierra%20Release%20WK1%20%28Tests%29/builds/12934 Log: https://build.webkit.org/builders/Apple%20Sierra%20Release%20WK1%20%28Tests%29/builds/12934/steps/builtins-generator-tests/logs/stdio
The only one test failure is caused by changed order of lines where order is not significant
Rebaselined tests using run-builtins-generator-tests --reset-results Committed: https://trac.webkit.org/changeset/236351/webkit
It still fails to compile without python2 installed because of: find_package(PythonInterp 2.7.0 REQUIRED) in Source/cmake/WebKitCommon.cmake Mike, can you create a patch for that?
I also got this error: Traceback (most recent call last): File "/builddir/build/BUILD/webkitgtk-2.22.2/Source/JavaScriptCore/Scripts/generateYarrUnicodePropertyTables.py", line 953, in <module> propertyDataHFile.write(header) TypeError: a bytes-like object is required, not 'str'
(In reply to Tomas Popela from comment #37) > I also got this error: > > Traceback (most recent call last): > File > "/builddir/build/BUILD/webkitgtk-2.22.2/Source/JavaScriptCore/Scripts/ > generateYarrUnicodePropertyTables.py", line 953, in <module> > propertyDataHFile.write(header) > TypeError: a bytes-like object is required, not 'str' I wonder if you have the changes to that file in your build, since it has been moved upstream, along with hasher.py, so you would have needed to explicitly tell patch where to find it. Specifically, the header should now be opened with "w", rather than "wb". If you do, in fact, have that change, then I'm puzzled as to why python would be wanting a bytes object.
Created attachment 350816 [details] Patch for cmake to use its python module Tomas, could you try this patch, for cmake? I see that PythonInterp is deprecated in favor of Python/Python2/Python3. Also, is there a minimum version of cmake that we should support?
We declare the minimum at the top of the toplevel CMakeLists.txt. Currently it's at 3.3. I think we can bump it as high as CMake 3.5 (the version in Ubuntu 16.04) as I assume that's old enough for other ports to depend on.
Best create a new bug for the new patch, and be sure to set the r? and cq? flags.
I just checked cmake 3.10.2, and it doesn't have those modules, so I suspect that my last patch would bump the requirement to 3.12.0. Might be a good thing to do in the long term, but I guess we don't want to commit it right now.
(In reply to Mike Gorse from comment #39) > Created attachment 350816 [details] > Patch for cmake to use its python module > > Tomas, could you try this patch, for cmake? I see that PythonInterp is > deprecated in favor of Python/Python2/Python3. > Also, is there a minimum version of cmake that we should support? As you already said later, I do have the cmake 3.11 here and the build fails because of the missing FindPython.cmake module. Requiring 3.12 is a NO-GO. We have to find a solution for the cmake < 3.3 (or 3.5 if we raise the requirement).
I've filed bug 190075 for the cmake issue. It seems like we can just add set(Python_ADDITIONAL_VERSIONS 3) before the find_package line.
I've noticed that, if LANG and LC_TYPE are unset, then the build still fails with python 3. A few more changes are needed. Filed bug 190258.
(In reply to Mike Gorse from comment #45) > I've noticed that, if LANG and LC_TYPE are unset, then the build still fails > with python 3. A few more changes are needed. Filed bug 190258. Python uses these flags to determine the file system encoding. In its absence, it assumes ASCII (which is never what you want). Therefore it's important that bots or any software running Python set it appropriately.
Also, even when you have WebKit in a full ASCII path, the default text file encoding is derived from these environment variables too; so it's very easy that a script fails while reading any file containing non ASCII characters e.g. contributors.json or the Changelog unless it does so in binary mode or with a explicit encoding (e.g. `file = io.open("path", "r", encoding="UTF-8")`). http://python-notes.curiousefficiency.org/en/latest/python3/text_file_processing.html