Bug 69589 – [chromium] Let rule_binding use os.execvp() instead of subprocess.call() to spawn fewer processes.

• Component: New Bugs
• Status: RESOLVED FIXED

[Nico Weber]

[chromium] Let rule_binding use os.execvp() instead of subprocess.call() to spawn fewer processes.

[Nico Weber]

Created attachment 110073 [details]
Patch

[WebKit Review Bot]

Comment on attachment 110073 [details]
Patch

Clearing flags on attachment: 110073

Committed r96892: <http://trac.webkit.org/changeset/96892>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Pavel Podivilov]

Broke chromium win build.

rule_binding.py passes *.idl file as last argument to generate-bindings.pl, however generate-bindings.pl expects *.idl file to be first argument. This all look very mysterious, and I have no idea how did it work before, but it stopped to work with subprocess on windows.

[Pavel Podivilov]

s/with subprocess/without subprocess/

[Nico Weber]

Do you have error logs anywhere?

[Pavel Podivilov]

http://build.chromium.org/p/chromium.webkit/builders/Webkit%20Win%20%28dbg%29%281%29/builds/7136/steps/compile/logs/stdio

Can't call method "fileName" without a package or object reference at ../bindings/scripts/IDLParser.pm line 102.

[Pavel Podivilov]

Not sure if it is related, but you call os.execvp('perl', command) where command is ['perl', '-w', ...].

[Nico Weber]

That's how exec works, argv[0] is the command name. I'll try to find a windows box and debug, thanks

[Ilya Tikhonovsky]

(In reply to comment #6)
> Do you have error logs anywhere?

good version of rule_binding.py (subprocess.call) is passing 36 arguments
bad version (os.execvp) is passing 111 arguments 
the difference is in the list of defines. execvp is splitting single long list of defines into separate arguments.

-----------
1>  --defines
1>  ENABLE_3D_PLUGIN=1 ENABLE_BLOB=1 ENABLE_BLOB_SLICE=1 ENABLE_CHANNEL_MESSAGING=1 ENABLE_CLIENT_BASED_GEOLOCATION=1 ENABLE_DASHBOARD_SUPPORT=0 ENABLE_DATA_TRANSFER_ITEMS=1 ENABLE_DETAILS=1 ENABLE_DEVICE_ORIENTATION=1 ENABLE_DIRECTORY_UPLOAD=1 ENABLE_DOM_STORAGE=1 ENABLE_DOWNLOAD_ATTRIBUTE=1 ENABLE_FILE_SYSTEM=1 ENABLE_FILTERS=1 ENABLE_FULLSCREEN_API=1 ENABLE_GAMEPAD=1 ENABLE_GEOLOCATION=1 ENABLE_GESTURE_EVENTS=1 ENABLE_GESTURE_RECOGNIZER=1 ENABLE_ICONDATABASE=0 ENABLE_INDEXED_DATABASE=1 ENABLE_INPUT_SPEECH=1 ENABLE_INPUT_TYPE_COLOR=0 ENABLE_INPUT_TYPE_DATE=0 ENABLE_INPUT_TYPE_DATETIME=0 ENABLE_INPUT_TYPE_DATETIMELOCAL=0 ENABLE_INPUT_TYPE_MONTH=0 ENABLE_INPUT_TYPE_TIME=0 ENABLE_INPUT_TYPE_WEEK=0 ENABLE_JAVASCRIPT_DEBUGGER=1 ENABLE_JAVASCRIPT_I18N_API=1 ENABLE_LINK_PREFETCH=1 ENABLE_MEDIA_STATISTICS=1 ENABLE_MEDIA_STREAM=1 ENABLE_METER_TAG=1 ENABLE_MHTML=1 ENABLE_MUTATION_OBSERVERS=0 ENABLE_NOTIFICATIONS=1 ENABLE_ORIENTATION_EVENTS=0 ENABLE_PAGE_VISIBILITY_API=1 ENABLE_PROGRESS_TAG=1 ENABLE_QUOTA=1 ENABLE_REQUEST_ANIMATION_FRAME=1 ENABLE_RUBY=1 ENABLE_SANDBOX=1 ENABLE_SHARED_WORKERS=1 ENABLE_SKIA_GPU=0 ENABLE_SMOOTH_SCROLLING=1 ENABLE_SQL_DATABASE=1 ENABLE_SVG=1 ENABLE_SVG_FONTS=1 ENABLE_TOUCH_EVENTS=1 ENABLE_TOUCH_ICON_LOADING=0 ENABLE_V8_SCRIPT_DEBUG_SERVER=1 ENABLE_VIDEO=1 ENABLE_VIDEO_TRACK=1 ENABLE_WEBGL=1 ENABLE_WEB_SOCKETS=1 ENABLE_WEB_TIMING=1 ENABLE_WORKERS=1 ENABLE_XHR_RESPONSE_BLOB=1 ENABLE_XPATH=1 ENABLE_XSLT=1 WTF_USE_LEVELDB=1 WTF_USE_BUILTIN_UTF8_CODEC=1 WTF_USE_OPENTYPE_SANITIZER=1 WTF_USE_WEBP=1 WTF_USE_WEBKIT_IMAGE_DECODERS=1 ENABLE_WEB_AUDIO=1 WTF_USE_ACCELERATED_COMPOSITING=1 ENABLE_3D_RENDERING=1 ENABLE_ACCELERATED_2D_CANVAS=1 WTF_USE_WEBAUDIO_FFMPEG=1 ENABLE_REGISTER_PROTOCOL_HANDLER=1 LANGUAGE_JAVASCRIPT V8_BINDING
1>  --generator
1>  V8
1>  --inclus
1>  --include
1>  ../dom
1>  --include
1>  ../fileapi
1>  --include
1>  ../html
1>  --include
1>  ../notifications
1>  --include
1>  ../page
1>  --include
1>  ../plugins
1>  --include
1>  ../storage
1>  --include
1>  ../svg
1>  --include
1>  ../testing
1>  --include
1>  ../webaudio
1>  --include
1>  ../websockets
1>  --include
1>  ../workers
1>  --include
1>  ../xml
1>  --outputHeadersDir
1>  E:/src/chromium/src/build/Release//obj/global_intermediate/webkit/bindings
1>  --outputDir
1>  E:/src/chromium/src/build/Release//obj/global_intermediate/webcore/bindings
1>  ..\html\HTMLTitleElement.idl
----------

[Ilya Tikhonovsky]

the bad version

before #ARGS=111
-------------
1>  --defines
1>  ENABLE_3D_PLUGIN=1
1>  ENABLE_BLOB=1
1>  ENABLE_BLOB_SLICE=1
1>  ENABLE_CHANNEL_MESSAGING=1
1>  ENABLE_CLIENT_BASED_GEOLOCATION=1
1>  ENABLE_DASHBOARD_SUPPORT=0
1>  ENABLE_DATA_TRANSFER_ITEMS=1
1>  ENABLE_DETAILS=1
1>  ENABLE_DEVICE_ORIENTATION=1
1>  ENABLE_DIRECTORY_UPLOAD=1
1>  ENABLE_DOM_STORAGE=1
1>  ENABLE_DOWNLOAD_ATTRIBUTE=1
1>  ENABLE_FILE_SYSTEM=1
1>  ENABLE_FILTERS=1
1>  ENABLE_FULLSCREEN_API=1
1>  ENABLE_GAMEPAD=1
1>  ENABLE_GEOLOCATION=1
1>  ENABLE_GESTURE_EVENTS=1
1>  ENABLE_GESTURE_RECOGNIZER=1
1>  ENABLE_ICONDATABASE=0
1>  ENABLE_INDEXED_DATABASE=1
1>  ENABLE_INPUT_SPEECH=1
1>  ENABLE_INPUT_TYPE_COLOR=0
1>  ENABLE_INPUT_TYPE_DATE=0
1>  ENABLE_INPUT_TYPE_DATETIME=0
1>  ENABLE_INPUT_TYPE_DATETIMELOCAL=0
1>  ENABLE_INPUT_TYPE_MONTH=0
1>  ENABLE_INPUT_TYPE_TIME=0
1>  ENABLE_INPUT_TYPE_WEEK=0
1>  ENABLE_JAVASCRIPT_DEBUGGER=1
1>  ENABLE_JAVASCRIPT_I18N_API=1
1>  ENABLE_LINK_PREFETCH=1
1>  ENABLE_MEDIA_STATISTICS=1
1>  ENABLE_MEDIA_STREAM=1
1>  ENABLE_METER_TAG=1
1>  ENABLE_MHTML=1
1>  ENABLE_MUTATION_OBSERVERS=0
1>  ENABLE_NOTIFICATIONS=1
1>  ENABLE_ORIENTATION_EVENTS=0
1>  ENABLE_PAGE_VISIBILITY_API=1
1>  ENABLE_PROGRESS_TAG=1
1>  ENABLE_QUOTA=1
1>  ENABLE_REQUEST_ANIMATION_FRAME=1
1>  ENABLE_RUBY=1
1>  ENABLE_SANDBOX=1
1>  ENABLE_SHARED_WORKERS=1
1>  ENABLE_SKIA_GPU=0
1>  ENABLE_SMOOTH_SCROLLING=1
1>  ENABLE_SQL_DATABASE=1
1>  ENABLE_SVG=1
1>  ENABLE_SVG_FONTS=1
1>  ENABLE_TOUCH_EVENTS=1
1>  ENABLE_TOUCH_ICON_LOADING=0
1>  ENABLE_V8_SCRIPT_DEBUG_SERVER=1
1>  ENABLE_VIDEO=1
1>  ENABLE_VIDEO_TRACK=1
1>  ENABLE_WEBGL=1
1>  ENABLE_WEB_SOCKETS=1
1>  ENABLE_WEB_TIMING=1
1>  ENABLE_WORKERS=1
1>  ENABLE_XHR_RESPONSE_BLOB=1
1>  ENABLE_XPATH=1
1>  ENABLE_XSLT=1
1>  WTF_USE_LEVELDB=1
1>  WTF_USE_BUILTIN_UTF8_CODEC=1
1>  WTF_USE_OPENTYPE_SANITIZER=1
1>  WTF_USE_WEBP=1
1>  WTF_USE_WEBKIT_IMAGE_DECODERS=1
1>  ENABLE_WEB_AUDIO=1
1>  WTF_USE_ACCELERATED_COMPOSITING=1
1>  ENABLE_3D_RENDERING=1
1>  ENABLE_ACCELERATED_2D_CANVAS=1
1>  WTF_USE_WEBAUDIO_FFMPEG=1
1>  ENABLE_REGISTER_PROTOCOL_HANDLER=1
1>  LANGUAGE_JAVASCRIPT
1>  V8_BINDING
1>  --generator
1>  V8
1>  --include
1>  ../css
1>  --include
1>  ../dom
1>  --include
1>  ../fileapi
1>  --include
1>  ../html
1>  --include
1>  ../notifications
1>  --include
1>  ../page
1>  --include
1>  ../plugins
1>  --include
1>  ../storage
1>  --include
1>  ../svg
1>  --include
1>  ../testing
1>  --include
1>  ../webaudio
1>  --include
1>  ../websockets
1>  --include
1>  ../workers
1>  --include
1>  ../xml
1>  --outputHeadersDir
1>  E:/src/chromium/src/build/Release//obj/global_intermediate/webkit/bindings
1>  --outputDir
1>  E:/src/chromium/src/build/Release//obj/global_intermediate/webcore/bindings
1> ..\css\RGBColor.idl


after calling GetOptions almost all the arguments still in the list
------------------------
1>  after ENABLE_BLOB=1 ENABLE_BLOB_SLICE=1 ENABLE_CHANNEL_MESSAGING=1 ENABLE_CLIENT_BASED_GEOLOCATION=1 ENABLE_DASHBOARD_SUPPORT=0 ENABLE_DATA_TRANSFER_ITEMS=1 ENABLE_DETAILS=1 ENABLE_DEVICE_ORIENTATION=1 ENABLE_DIRECTORY_UPLOAD=1 ENABLE_DOM_STORAGE=1 ENABLE_DOWNLOAD_ATTRIBUTE=1 ENABLE_FILE_SYSTEM=1 ENABLE_FILTERS=1 ENABLE_FULLSCREEN_API=1 ENABLE_GAMEPAD=1 ENABLE_GEOLOCATION=1 ENABLE_GESTURE_EVENTS=1 ENABLE_GESTURE_RECOGNIZER=1 ENABLE_ICONDATABASE=0 ENABLE_INDEXED_DATABASE=1 ENABLE_INPUT_SPEECH=1 ENABLE_INPUT_TYPE_COLOR=0 ENABLE_INPUT_TYPE_DATE=0 ENABLE_INPUT_TYPE_DATETIME=0 ENABLE_INPUT_TYPE_DATETIMELOCAL=0 ENABLE_INPUT_TYPE_MONTH=0 ENABLE_INPUT_TYPE_TIME=0 ENABLE_INPUT_TYPE_WEEK=0 ENABLE_JAVASCRIPT_DEBUGGER=1 ENABLE_JAVASCRIPT_I18N_API=1 ENABLE_LINK_PREFETCH=1 ENABLE_MEDIA_STATISTICS=1 ENABLE_MEDIA_STREAM=1 ENABLE_METER_TAG=1 ENABLE_MHTML=1 ENABLE_MUTATION_OBSERVERS=0 ENABLE_NOTIFICATIONS=1 ENABLE_ORIENTATION_EVENTS=0 ENABLE_PAGE_VISIBILITY_API=1 ENABLE_PROGRESS_TAG=1 ENABLE_QUOTA=1 ENABLE_REQUEST_ANIMATION_FRAME=1 ENABLE_RUBY=1 ENABLE_SANDBOX=1 ENABLE_SHARED_WORKERS=1 ENABLE_SKIA_GPU=0 ENABLE_SMOOTH_SCROLLING=1 ENABLE_SQL_DATABASE=1 ENABLE_SVG=1 ENABLE_SVG_FONTS=1 ENABLE_TOUCH_EVENTS=1 ENABLE_TOUCH_ICON_LOADING=0 ENABLE_V8_SCRIPT_DEBUG_SERVER=1 ENABLE_VIDEO=1 ENABLE_VIDEO_TRACK=1 ENABLE_WEBGL=1 ENABLE_WEB_SOCKETS=1 ENABLE_WEB_TIMING=1 ENABLE_WORKERS=1 ENABLE_XHR_RESPONSE_BLOB=1 ENABLE_XPATH=1 ENABLE_XSLT=1 WTF_USE_LEVELDB=1 WTF_USE_BUILTIN_UTF8_CODEC=1 WTF_USE_OPENTYPE_SANITIZER=1 WTF_USE_WEBP=1 WTF_USE_WEBKIT_IMAGE_DECODERS=1 ENABLE_WEB_AUDIO=1 WTF_USE_ACCELERATED_COMPOSITING=1 ENABLE_3D_RENDERING=1 ENABLE_ACCELERATED_2D_CANVAS=1 WTF_USE_WEBAUDIO_FFMPEG=1 ENABLE_REGISTER_PROTOCOL_HANDLER=1 LANGUAGE_JAVASCRIPT V8_BINDING ..\css\RGBColor.idl

[Nico Weber]

When running python on windows (not under cygwin), os.execv doesn't preserve spaces:

test.py is this: """
#!/usr/bin/python
import sys
print sys.argv
"""

Called through execv:

>>> os.execv('python', ['python', 'test.py', 'a', 'b c'])                       
                                                                                
['test.py', 'a', 'b', 'c']

subprocess mentions that it correctly does quoting for this: http://docs.python.org/library/subprocess.html#converting-argument-sequence

execv() doesn't do this. Let's just use subprocess on win32 as a workaround.

[Nico Weber]

Created attachment 110605 [details]
Patch

[Nico Weber]

new patch

[Eric Seidel (no email)]

Comment on attachment 110605 [details]
Patch

rs=me.

[Dirk Pranke]

Comment on attachment 110605 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=110605&action=review

> Source/WebCore/WebCore.gyp/scripts/rule_binding.py:132
> +    else:

since os.execvp() doesn't return, you can delete this else and outdent the following code. You can also delete the trailing else, which is just incorrect.

[Nico Weber]

Created attachment 110611 [details]
Patch

[WebKit Review Bot]

Comment on attachment 110611 [details]
Patch

Clearing flags on attachment: 110611

Committed r97204: <http://trac.webkit.org/changeset/97204>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.