Bug 33365 – webkit-patch's use of auto-install is dropping autoinstall directories everywhere

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Eric Seidel (no email)]

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.

[Chris Jerdonek]

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.

[Eric Seidel (no email)]

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).

[Chris Jerdonek]

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.

[Eric Seidel (no email)]

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/

[Chris Jerdonek]

Comment on attachment 47447 [details]
Proposed patch

cq- to use webkit-patch.

[Chris Jerdonek]

Comment on attachment 47447 [details]
Proposed patch

Clearing flags on attachment: 47447

Committed r53887: <http://trac.webkit.org/changeset/53887>

[Chris Jerdonek]

All reviewed patches have been landed.  Closing bug.

[Tony Chang]

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

[Tony Chang]

(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

[Chris Jerdonek]

(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?

[Tony Chang]

(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).

[Chris Jerdonek]

(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.

[Tony Chang]

(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.

[Eric Seidel (no email)]

This broek the commit-queue too. r olling out.

[Eric Seidel (no email)]

hmm...  looks liek chris is on it...

[Chris Jerdonek]

Attempted fix r53888:

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