Bug 54431 - nrwt multiprocessing: fix various windows-related regressions
Summary: nrwt multiprocessing: fix various windows-related regressions
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Dirk Pranke
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-02-14 20:34 PST by Dirk Pranke
Modified: 2011-02-15 12:13 PST (History)
5 users (show)

See Also:


Attachments
Patch (8.05 KB, patch)
2011-02-14 20:46 PST, Dirk Pranke
tony: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dirk Pranke 2011-02-14 20:34:17 PST
nrwt multiprocessing: fix various windows-related regressions
Comment 1 Dirk Pranke 2011-02-14 20:46:58 PST
Created attachment 82412 [details]
Patch
Comment 2 Dirk Pranke 2011-02-14 20:48:58 PST
I am open to better suggestions if anyone has some ...
Comment 3 Tony Chang 2011-02-15 10:30:12 PST
Comment on attachment 82412 [details]
Patch

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

Seems fine to try.

> Tools/Scripts/new-run-webkit-tests:40
> +    # (which means a file in sys.path that ends in .py). We use the main
> +    # file in the layout_tests subdirectory, and we also make sure that
> +    # sys.path / PYTHONPATH is set and propagating correctly.

Nit: The second sentence seems to be repeating what is already stated in the first sentence.

> Tools/Scripts/new-run-webkit-tests:43
> +    script_dir = os.path.dirname(os.path.abspath(__file__))
> +    env = os.environ
> +    env['PYTHONPATH'] = script_dir

Should we append the old PYTHONPATH to script_dir?

> Tools/Scripts/webkitpy/layout_tests/layout_package/manager_worker_broker_unittest.py:47
> +# In order to reliably control when child workers are starting and stopping,
> +# we a pair of global variables to hold queues used for messaging. Ideally

Nit: we a pair -> we use a pair?

> Tools/Scripts/webkitpy/layout_tests/layout_package/manager_worker_broker_unittest.py:51
>  

Nit: Remove blank line between comment and variables.
Comment 4 Dirk Pranke 2011-02-15 12:12:54 PST
Committed r78601: <http://trac.webkit.org/changeset/78601>
Comment 5 Dirk Pranke 2011-02-15 12:13:49 PST
(In reply to comment #3)
> > Tools/Scripts/new-run-webkit-tests:40
> > +    # (which means a file in sys.path that ends in .py). We use the main
> > +    # file in the layout_tests subdirectory, and we also make sure that
> > +    # sys.path / PYTHONPATH is set and propagating correctly.
> 
> Nit: The second sentence seems to be repeating what is already stated in the first sentence.
>

The intent was to be slightly different, but it probably wasn't different enough to be worth it. removed.
 
> > Tools/Scripts/new-run-webkit-tests:43
> > +    script_dir = os.path.dirname(os.path.abspath(__file__))
> > +    env = os.environ
> > +    env['PYTHONPATH'] = script_dir
> 
> Should we append the old PYTHONPATH to script_dir?
>

Done.
 
> > Tools/Scripts/webkitpy/layout_tests/layout_package/manager_worker_broker_unittest.py:47
> > +# In order to reliably control when child workers are starting and stopping,
> > +# we a pair of global variables to hold queues used for messaging. Ideally
> 
> Nit: we a pair -> we use a pair?
>

Fixed.
 
> > Tools/Scripts/webkitpy/layout_tests/layout_package/manager_worker_broker_unittest.py:51
> >  
> 
> Nit: Remove blank line between comment and variables.

Fixed.