Bug 33365 - webkit-patch's use of auto-install is dropping autoinstall directories everywhere
Summary: webkit-patch's use of auto-install is dropping autoinstall directories everyw...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Chris Jerdonek
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-01-07 22:29 PST by Eric Seidel (no email)
Modified: 2010-01-26 19:59 PST (History)
3 users (show)

See Also:


Attachments
Proposed patch (2.31 KB, patch)
2010-01-26 14:12 PST, Chris Jerdonek
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2010-01-07 22:29:03 PST
webkit-patch's use of auto-install is dropping autoinstall directories everywhere

We need to teach autoinstall to not use the CWD.  Right now depending on where you run the command from, it will drop a new autoinstall direcotry there.  That's bad.
Comment 1 Chris Jerdonek 2010-01-26 10:10:57 PST
This has been annoying to me, too.  I'll change it so that the autoinstall cache directory is put in the same directory as autoinstall.

I also think the cache directory should give some indication of where it came from.  The first time I saw them, I had no way to know what they were or where they were coming from.
Comment 2 Eric Seidel (no email) 2010-01-26 12:06:21 PST
Mostly I've been afraid of python's "logging" module.  I need to learn it. :(

I suspect this fix is a one-lnine change to webkitpy/__init__.py (the one place we use autoinstall.py).
Comment 3 Chris Jerdonek 2010-01-26 14:12:56 PST
Created attachment 47447 [details]
Proposed patch

The cache directory gets created when "import" is called for an external module (when searching for the module to import), rather than when autoinstall.py is first processed.

Other possibilities would be to change the working directory prior to calling "import" for an external module, or to trigger the cache directory to get created while still inside __init__.py, but this seems most straightforward.
Comment 4 Eric Seidel (no email) 2010-01-26 14:22:38 PST
Comment on attachment 47447 [details]
Proposed patch

LGTM.  Thanks for taking care of this.  Eventually we might upstream this change http://code.google.com/p/pyautoinstall/
Comment 5 Chris Jerdonek 2010-01-26 19:02:21 PST
Comment on attachment 47447 [details]
Proposed patch

cq- to use webkit-patch.
Comment 6 Chris Jerdonek 2010-01-26 19:05:09 PST
Comment on attachment 47447 [details]
Proposed patch

Clearing flags on attachment: 47447

Committed r53887: <http://trac.webkit.org/changeset/53887>
Comment 7 Chris Jerdonek 2010-01-26 19:05:16 PST
All reviewed patches have been landed.  Closing bug.
Comment 8 Tony Chang 2010-01-26 19:31:42 PST
This seems to have broken webkit-patch for me.  The error is:

Failed to run "['WebKitTools/Scripts/test-webkitpy']" exit_code: 1
Last 500 characters of output:
.py:201: Warning: 'with' will become a reserved keyword in Python 2.6
Traceback (most recent call last):
  File "WebKitTools/Scripts/test-webkitpy", line 33, in <module>
    from webkitpy.bugzilla_unittest import *
  File "/Users/tc/webkit/WebKit/WebKitTools/Scripts/webkitpy/__init__.py", line 3, in <module>
    import autoinstall
  File "/Users/tc/webkit/WebKit/WebKitTools/Scripts/webkitpy/autoinstall.py", line 201
    with open(readme_path, "w") as f:
            ^
SyntaxError: invalid syntax

Failed to run "['WebKitTools/Scripts/test-webkitpy']" exit_code: 1
Comment 9 Tony Chang 2010-01-26 19:33:09 PST
(In reply to comment #8)
> This seems to have broken webkit-patch for me.  The error is:

This is from me running webkit-patch land-from-bug.  It compiles fine, but seems to crash after it gets to "Running Python unit tests".  From further up the stack:

===========================================================
 WebKit is now built (00m:27s). 
 To run Safari with this newly-built code, use the
 "WebKitTools/Scripts/run-safari" script.
===========================================================
Running Python unit tests
/Users/tc/webkit/WebKit/WebKitTools/Scripts/webkitpy/autoinstall.py:201: Warning: 'with' will become a reserved keyword in Python 2.6
Traceback (most recent call last):
  File "WebKitTools/Scripts/test-webkitpy", line 33, in <module>
    from webkitpy.bugzilla_unittest import *
  File "/Users/tc/webkit/WebKit/WebKitTools/Scripts/webkitpy/__init__.py", line 3, in <module>
    import autoinstall
  File "/Users/tc/webkit/WebKit/WebKitTools/Scripts/webkitpy/autoinstall.py", line 201
    with open(readme_path, "w") as f:
            ^
SyntaxError: invalid syntax
Comment 10 Chris Jerdonek 2010-01-26 19:41:12 PST
(In reply to comment #9)
==========
> Running Python unit tests
> /Users/tc/webkit/WebKit/WebKitTools/Scripts/webkitpy/autoinstall.py:201:
> Warning: 'with' will become a reserved keyword in Python 2.6
> Traceback (most recent call last):
>   File "WebKitTools/Scripts/test-webkitpy", line 33, in <module>
>     from webkitpy.bugzilla_unittest import *
>   File "/Users/tc/webkit/WebKit/WebKitTools/Scripts/webkitpy/__init__.py", line
> 3, in <module>
>     import autoinstall
>   File "/Users/tc/webkit/WebKit/WebKitTools/Scripts/webkitpy/autoinstall.py",
> line 201
>     with open(readme_path, "w") as f:
>             ^
> SyntaxError: invalid syntax

It looks like this requires an extra line of code for Python 2.5 users:

http://docs.python.org/whatsnew/2.6.html#pep-343-the-with-statement

from __future__ import with_statement

What version of Python are you using?
Comment 11 Tony Chang 2010-01-26 19:43:01 PST
(In reply to comment #10)
> It looks like this requires an extra line of code for Python 2.5 users:
> 
> http://docs.python.org/whatsnew/2.6.html#pep-343-the-with-statement
> 
> from __future__ import with_statement
> 
> What version of Python are you using?

pthon2.5.1, the default version on my mac (leopard).
Comment 12 Chris Jerdonek 2010-01-26 19:49:15 PST
(In reply to comment #11)
> (In reply to comment #10)
> > It looks like this requires an extra line of code for Python 2.5 users:
> > 
> > http://docs.python.org/whatsnew/2.6.html#pep-343-the-with-statement
> > 
> > from __future__ import with_statement
> > 
> > What version of Python are you using?
> 
> pthon2.5.1, the default version on my mac (leopard).

I can try fixing this by adding this line to the top of WebKitTools/Scripts/webkitpy/autoinstall.py:

from __future__ import with_statement

Tony, would you be able to try adding this on your machine to see if it resolves the issue? I don't have Python 2.5 installed.
Comment 13 Tony Chang 2010-01-26 19:50:55 PST
(In reply to comment #12)
> I can try fixing this by adding this line to the top of
> WebKitTools/Scripts/webkitpy/autoinstall.py:
> 
> from __future__ import with_statement
> 
> Tony, would you be able to try adding this on your machine to see if it
> resolves the issue? I don't have Python 2.5 installed.

That works for me (adding it above the __version__ line).  Or you could rewrite the code to not use "with".  :)  It's not a lot of code either way.
Comment 14 Eric Seidel (no email) 2010-01-26 19:51:14 PST
This broek the commit-queue too. r olling out.
Comment 15 Eric Seidel (no email) 2010-01-26 19:51:46 PST
hmm...  looks liek chris is on it...
Comment 16 Chris Jerdonek 2010-01-26 19:59:21 PST
Attempted fix r53888:

http://trac.webkit.org/changeset/53888