Bug 50901 – Add remote zip file handling to webkitpy.

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[James Kozianski]

Add remote zip file handling to webkitpy.

[James Kozianski]

Created attachment 76349 [details]
Patch

Add DirAsZip and RemoteZip for providing a unified interface for directories and remote zip files.

[Eric Seidel (no email)]

Comment on attachment 76349 [details]
Patch

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

So I feel like you're trying to make a directory act like a zip, when you really want a zip to act like a directory.  I'm confused as to the overarching design here.

You want some abstraction to treat both local results as well as remote results the same, right?  As well as zipped results vs. non-zipped, right?

Why not just always download and unzip and then they're all teh same?  I'm confused.

> WebKitTools/ChangeLog:7
> +

The ChangeLog should talk more about the "why" of these changes.  Why are these two classes needed?

> WebKitTools/ChangeLog:9
> +        * Scripts/webkitpy/common/diraszip.py: Added.
> +        * Scripts/webkitpy/common/net/remotezip.py: Added.

Every change should have a test.  In python it's super easy.  Just name a file with _unittest.py in the webkitpy directory and test-webkitpy will find it and run all the tests in it.

> WebKitTools/Scripts/webkitpy/common/diraszip.py:36
> +    """Provides a zipfile-like interface to a local directory"""

So I'm confused why this is needed?

> WebKitTools/Scripts/webkitpy/common/diraszip.py:39
> +        self._filesystem = filesystem if filesystem != None else FileSystem()

Using "None" to mean "default value" is often written as:
if not filesystem:
    filesystem = FileSystem()

and then later:
self._filesystem = filesystem

But your solution works too.  I wouldn't have bothered checking explicit != None.  I probably would have "cheated" with or and just written: self._filesystem = filesystem or FileSystem()

> WebKitTools/Scripts/webkitpy/common/diraszip.py:41
> +        if not self._path.endswith(os.path.sep):
> +            self._path += os.path.sep

Unclear why this is necessary?

> WebKitTools/Scripts/webkitpy/common/diraszip.py:58
> +        return ospath.relpath(os.path.join(dir, filename), dir) is not None

bool(EXPRESSION) or "not not EXPRESSION" are common ways of converting from a value to a bool (which are slightly less verbose than "is not None"

[James Kozianski]

(In reply to comment #2)
> (From update of attachment 76349 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=76349&action=review
> 
> So I feel like you're trying to make a directory act like a zip, when you really want a zip to act like a directory.  I'm confused as to the overarching design here.
> 
> You want some abstraction to treat both local results as well as remote results the same, right?  As well as zipped results vs. non-zipped, right?

Yes, you're right - and calling this abstraction 'zip' is definitely confusing. The point of these classes it to provide a simple interface for talking about collections of files (they just happen to crib the Python zipfile.ZipFile interface). What do you think about renaming them to ZipFileBundle and DirectoryFileBundle? (Or some noun better than FileBundle :-) )

> 
> Why not just always download and unzip and then they're all teh same?  I'm confused.

It's true that ZipFileBundle could be implemented by downloading and extracting a zip and then delegating to a DirectoryFileBundle, but I think that the current implementation is good because it is more efficient (it will only read files that it needs to, which can be significant on chromium builders which store passing and failing test results) and it is quite a short class anyway (<30 lines).

> 
> > WebKitTools/ChangeLog:7
> > +
> 
> The ChangeLog should talk more about the "why" of these changes.  Why are these two classes needed?
> 
> > WebKitTools/ChangeLog:9
> > +        * Scripts/webkitpy/common/diraszip.py: Added.
> > +        * Scripts/webkitpy/common/net/remotezip.py: Added.
> 
> Every change should have a test.  In python it's super easy.  Just name a file with _unittest.py in the webkitpy directory and test-webkitpy will find it and run all the tests in it.
> 
> > WebKitTools/Scripts/webkitpy/common/diraszip.py:36
> > +    """Provides a zipfile-like interface to a local directory"""
> 
> So I'm confused why this is needed?

I'll try and add something more concise to the ChangeLog, but briefly the reason why I want these interfaces is because they are much easier to deal with than raw strings representing paths and filenames. It's not that there's any use specifically in having a directory behave like a zip file (as discussed above), this is more just a way to define a common interface for dealing with collections of files in a simple and easy to test way.

For instance, FakeZip is a trivial class to implement and leads to test code that looks like:

local_results = FakeZip()
local_results.insert('platform/mac/some-test-expected.txt', 'old baseline')

remote_results = FakeZip()
remote_results.insert('platform/mac/some-test-actual.txt', 'new baseline')

> 
> > WebKitTools/Scripts/webkitpy/common/diraszip.py:39
> > +        self._filesystem = filesystem if filesystem != None else FileSystem()
> 
> Using "None" to mean "default value" is often written as:
> if not filesystem:
>     filesystem = FileSystem()
> 
> and then later:
> self._filesystem = filesystem
> 
> But your solution works too.  I wouldn't have bothered checking explicit != None.  I probably would have "cheated" with or and just written: self._filesystem = filesystem or FileSystem()
> 
> > WebKitTools/Scripts/webkitpy/common/diraszip.py:41
> > +        if not self._path.endswith(os.path.sep):
> > +            self._path += os.path.sep
> 
> Unclear why this is necessary?

If the user provides us with a path like "a/b", the results we get from an os.walk() will all have prefixes of "a/b/". We normalise our path at the beginning so we know how much to drop off the resulting os.walk() filenames. I'll add a comment to the code to make this more clear.

> 
> > WebKitTools/Scripts/webkitpy/common/diraszip.py:58
> > +        return ospath.relpath(os.path.join(dir, filename), dir) is not None
> 
> bool(EXPRESSION) or "not not EXPRESSION" are common ways of converting from a value to a bool (which are slightly less verbose than "is not None"

[James Kozianski]

(In reply to comment #2)
> (From update of attachment 76349 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=76349&action=review
> 
> So I feel like you're trying to make a directory act like a zip, when you really want a zip to act like a directory.  I'm confused as to the overarching design here.
> 
> You want some abstraction to treat both local results as well as remote results the same, right?  As well as zipped results vs. non-zipped, right?
> 
> Why not just always download and unzip and then they're all teh same?  I'm confused.
> 
> > WebKitTools/ChangeLog:7
> > +
> 
> The ChangeLog should talk more about the "why" of these changes.  Why are these two classes needed?

Done.

> 
> > WebKitTools/ChangeLog:9
> > +        * Scripts/webkitpy/common/diraszip.py: Added.
> > +        * Scripts/webkitpy/common/net/remotezip.py: Added.
> 
> Every change should have a test.  In python it's super easy.  Just name a file with _unittest.py in the webkitpy directory and test-webkitpy will find it and run all the tests in it.

Done.

> 
> > WebKitTools/Scripts/webkitpy/common/diraszip.py:36
> > +    """Provides a zipfile-like interface to a local directory"""
> 
> So I'm confused why this is needed?
> 
> > WebKitTools/Scripts/webkitpy/common/diraszip.py:39
> > +        self._filesystem = filesystem if filesystem != None else FileSystem()
> 
> Using "None" to mean "default value" is often written as:
> if not filesystem:
>     filesystem = FileSystem()
> 
> and then later:
> self._filesystem = filesystem
> 
> But your solution works too.  I wouldn't have bothered checking explicit != None.  I probably would have "cheated" with or and just written: self._filesystem = filesystem or FileSystem()

Actually, I like your way the best, so I've changed it to that :-)

> 
> > WebKitTools/Scripts/webkitpy/common/diraszip.py:41
> > +        if not self._path.endswith(os.path.sep):
> > +            self._path += os.path.sep
> 
> Unclear why this is necessary?
> 
> > WebKitTools/Scripts/webkitpy/common/diraszip.py:58
> > +        return ospath.relpath(os.path.join(dir, filename), dir) is not None
> 
> bool(EXPRESSION) or "not not EXPRESSION" are common ways of converting from a value to a bool (which are slightly less verbose than "is not None"

Done.

[James Kozianski]

Created attachment 77865 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 77865 [details]
Patch

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

> Tools/Scripts/webkitpy/common/system/filesystem_mock.py:116
> +    def files_under(self, path):
> +        results = []
> +        for file in self.files:
> +            if file.startswith(path):
> +                results.append(file)
> +        return results

This is  just [file for file in self.files if file.startswith(path)]

[Eric Seidel (no email)]

Comment on attachment 77865 [details]
Patch

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

> Tools/Scripts/webkitpy/common/directoryfileset_unittest.py:51
> +

two blank lines i believe.

> Tools/Scripts/webkitpy/common/directoryfileset_unittest.py:53
> +if __name__ == '__main__':
> +    unittest.main()

I generally omit these since the way we do imports makes it impossible to run these by hand anywa.

> Tools/Scripts/webkitpy/common/net/zipfileset.py:1
> +#!/usr/bin/env python

Please omit.

> Tools/Scripts/webkitpy/common/net/zipfileset.py:80
> +        if filename is None:
> +            self._fileset.extract(self._filename, path)

I would have early returned here.

[Eric Seidel (no email)]

Comment on attachment 77865 [details]
Patch

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

You could just make this a new module common.fileset.  YOu could also just slap this all into one file common/fileset.py.  I'm not sure which is cleaner.  Making it a module is better long-term if we plan to expand on this.

> Tools/Scripts/webkitpy/common/directoryfileset_unittest.py:33
> +class DirectoryFileSetTest(unittest.TestCase):

Seems we should test the whole api here, no?

> Tools/Scripts/webkitpy/common/net/zipfileset.py:63
> +class FileSetFileHandle(object):

Since this is used by more than one file seems it should be in its own file.

[Eric Seidel (no email)]

Comment on attachment 77865 [details]
Patch

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

> Tools/Scripts/webkitpy/common/directoryfileset.py:36
> +    """Provides a zipfile-like interface to a local directory"""

I'm still confused why we're making directories look like zips instead of zips look like directories.  That seems backwards. :)

[Eric Seidel (no email)]

We would use something like this in common.net.layouttestresults if it existed, I think.  It's still unclear to me why we treat directories like zips.  But I do think it makes sense to have a concept of some sort of file set, so I think we want this change.

I think my major source of confusion is just in the positioning of this class.  Its basically an abstraction around a filesystem tree, which could be backed by a remote file system or a local file system or a zip or a directory.  I'm not sure what we call that.  FileSet is fine, I guess.

[Mihai Parparita]

Comment on attachment 77865 [details]
Patch

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

> Tools/Scripts/webkitpy/common/directoryfileset.py:30
> +from webkitpy.common.net.zipfileset import FileSetFileHandle

Having DirectoryFileSet depend on zipfileset seems wrong. Can there be a (Base)FileSet class/file for code that is used in both?

> Tools/Scripts/webkitpy/common/directoryfileset.py:39
> +        self._filesystem = filesystem if filesystem != None else FileSystem()

This can just be self._filesystem = filesystem or Filesystem()

> Tools/Scripts/webkitpy/common/directoryfileset.py:72
> +        # directory heirarchy exists at the output path.

Typo ("heirarchy")

> Tools/Scripts/webkitpy/common/directoryfileset_unittest.py:35
> +        self._files = {}

Seems like _files doesn't need to be a property of the class (and you don't need an addFile method), given that you're using a MockFileSystem. Instead you could just say:
def setUp(self):
  files = {
   '/test/some-file': 'contents',
  ...
  }
  self._fileset = DirectoryFileSet('/test', MockFileSystem(files))

>> Tools/Scripts/webkitpy/common/system/filesystem_mock.py:116

> 
> This is  just [file for file in self.files if file.startswith(path)]

Though this should also handle directory names that are substrings of one another. I.e. if files is:
/foo/bar
/foo2/bar

Then files_under('/foo') should not include ('foo2/bar). 

(see how listdir is implemented to avoid this)

[Mihai Parparita]

Oops, mid-air review collision.

(In reply to comment #10)
> We would use something like this in common.net.layouttestresults if it existed, I think.  It's still unclear to me why we treat directories like zips.  But I do think it makes sense to have a concept of some sort of file set, so I think we want this change.

Agreed, I think the goal shouldn't be "zip file-like implementation for local directories" but instead "file set abstraction that could be backed by local directories or zip files."

[James Kozianski]

(In reply to comment #6)
> (From update of attachment 77865 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=77865&action=review
> 
> > Tools/Scripts/webkitpy/common/system/filesystem_mock.py:116
> > +    def files_under(self, path):
> > +        results = []
> > +        for file in self.files:
> > +            if file.startswith(path):
> > +                results.append(file)
> > +        return results
> 
> This is  just [file for file in self.files if file.startswith(path)]

Done.

[James Kozianski]

Comment on attachment 77865 [details]
Patch

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

>> Tools/Scripts/webkitpy/common/directoryfileset.py:30

> 
> Having DirectoryFileSet depend on zipfileset seems wrong. Can there be a (Base)FileSet class/file for code that is used in both?

Yep - I'll add a fileset.py file to hold the FileSetHandle class. I guess such a file would be where a general FileSet (ie: one not backed by filesystem or zip file) class could be defined if one is ever needed.

>> Tools/Scripts/webkitpy/common/directoryfileset.py:36

> 
> I'm still confused why we're making directories look like zips instead of zips look like directories.  That seems backwards. :)

I think this comment is out of date. It should read "The set of files under a local directory." That this interface is like a zip file is incidental :-)

The reason this interface looks like the one for zip files is because like a zip file it doesn't require the caller to remember the path to the directory that they are interested in, for example:

d = DirectoryFileSet('/some/path/prefix')
d.namelist()
d.read('some-file')
d.open('some-other-file')

would be

fs = FileSystem()
fs.all_files_under('/some/path/prefix')
fs.read('/some/path/prefix/some-file')
fs.open('/some/path/prefix/some-other-file')

>> Tools/Scripts/webkitpy/common/directoryfileset.py:39

> 
> This can just be self._filesystem = filesystem or Filesystem()

Done.

>> Tools/Scripts/webkitpy/common/directoryfileset.py:72

> 
> Typo ("heirarchy")

Done.

>> Tools/Scripts/webkitpy/common/directoryfileset_unittest.py:33

> 
> Seems we should test the whole api here, no?

Yes, we should :-) In doing so I realised that FileSystemMock lacks a remove() function (though it exists in FileSystem). Thanks.

I also added a test for ZipFileSet (it uses temp directories / files if that's okay).

>> Tools/Scripts/webkitpy/common/directoryfileset_unittest.py:35

> 
> Seems like _files doesn't need to be a property of the class (and you don't need an addFile method), given that you're using a MockFileSystem. Instead you could just say:
> def setUp(self):
>   files = {
>    '/test/some-file': 'contents',
>   ...
>   }
>   self._fileset = DirectoryFileSet('/test', MockFileSystem(files))

Ah yes, true :-)
Done.

>> Tools/Scripts/webkitpy/common/directoryfileset_unittest.py:51

> 
> two blank lines i believe.

Done.

>> Tools/Scripts/webkitpy/common/directoryfileset_unittest.py:53

> 
> I generally omit these since the way we do imports makes it impossible to run these by hand anywa.

I've got a command that adds the webkitpy directory to python's path for running tests like these, which is handy because running all the unit tests with test-webkitpy can take a while.

>> Tools/Scripts/webkitpy/common/net/zipfileset.py:1

> 
> Please omit.

Done.

>> Tools/Scripts/webkitpy/common/net/zipfileset.py:63

> 
> Since this is used by more than one file seems it should be in its own file.

I put it in fileset.py to mean 'common code for file sets'. Would it be better to put it in filesetfilehandle.py?

>> Tools/Scripts/webkitpy/common/net/zipfileset.py:80

> 
> I would have early returned here.

Done.

[James Kozianski]

(In reply to comment #11)
> >> Tools/Scripts/webkitpy/common/system/filesystem_mock.py:116
> 
> > 
> > This is  just [file for file in self.files if file.startswith(path)]
> 
> Though this should also handle directory names that are substrings of one another. I.e. if files is:
> /foo/bar
> /foo2/bar
> 
> Then files_under('/foo') should not include ('foo2/bar). 
> 
> (see how listdir is implemented to avoid this)
Done.

[James Kozianski]

Created attachment 78188 [details]
Patch

[Mihai Parparita]

Comment on attachment 78188 [details]
Patch

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

> Tools/ChangeLog:12
> +        * Scripts/webkitpy/common/directoryfileset.py: Added.

Should be updated to include fileset.py

> Tools/Scripts/webkitpy/common/directoryfileset.py:30
> +from webkitpy.common.system.filesystem import FileSystem

Given that filesystem lives in webkitpy.common.system, perhaps the fileset classes should be in there too.

> Tools/Scripts/webkitpy/common/directoryfileset.py:44
> +        return os.path.join(self._path, filename)

Should use filesystem.join instead of os.path.join.

> Tools/Scripts/webkitpy/common/directoryfileset.py:69
> +        dest = os.path.join(path, filename)

Same here.

> Tools/Scripts/webkitpy/common/system/filesystem_mock.py:117
> +        del(self.files[path])

del is actually a statement, this should be del self.files[path]

[James Kozianski]

Created attachment 78201 [details]
Patch

[James Kozianski]

Comment on attachment 78188 [details]
Patch

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

>> Tools/ChangeLog:12

> 
> Should be updated to include fileset.py

Done.

>> Tools/Scripts/webkitpy/common/directoryfileset.py:30

> 
> Given that filesystem lives in webkitpy.common.system, perhaps the fileset classes should be in there too.

Done.

>> Tools/Scripts/webkitpy/common/directoryfileset.py:44

> 
> Should use filesystem.join instead of os.path.join.

Done.

>> Tools/Scripts/webkitpy/common/directoryfileset.py:69

> 
> Same here.

Done.

>> Tools/Scripts/webkitpy/common/system/filesystem_mock.py:117

> 
> del is actually a statement, this should be del self.files[path]

Done.

[Adam Barth]

@ojan: Another example of a broken context from a comment reply in the review tool ^^^^^^^^^^

[Ojan Vafai]

(In reply to comment #20)
> @ojan: Another example of a broken context from a comment reply in the review tool ^^^^^^^^^^

Whoops. I mis-split my original patch up. Fixed http://trac.webkit.org/changeset/75222.

[Mihai Parparita]

Comment on attachment 78201 [details]
Patch

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

> Tools/Scripts/webkitpy/common/system/filesystem.py:162
> +    def extract(self, zip, filename, path):

As discussed in #webkit, extract doesn't really belong in FileSystem. For the sake of testability, passing in the ZipFile (or a lazy-loading wrapper) to the ZipFileSet constructor is cleaner.

> Tools/Scripts/webkitpy/common/system/filesystem_mock.py:60
> +        return os.path.join(*comps)

The mock filesystem hardcodes / as the separator, so it should keep using '/'.join(comps) as its join() implementation.

[James Kozianski]

(In reply to comment #22)
> (From update of attachment 78201 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=78201&action=review
> 
> > Tools/Scripts/webkitpy/common/system/filesystem.py:162
> > +    def extract(self, zip, filename, path):
> 
> As discussed in #webkit, extract doesn't really belong in FileSystem. For the sake of testability, passing in the ZipFile (or a lazy-loading wrapper) to the ZipFileSet constructor is cleaner.

Yep. I used your suggestion of passing in the constructor.

> 
> > Tools/Scripts/webkitpy/common/system/filesystem_mock.py:60
> > +        return os.path.join(*comps)
> 
> The mock filesystem hardcodes / as the separator, so it should keep using '/'.join(comps) as its join() implementation.

I've changed it to use os.path.join() but then to substitute back to using '/' because os.path.join() collapses slashes, which '/'.join() doesn't.

[James Kozianski]

Created attachment 78206 [details]
Patch

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 78206 [details]:

http/tests/xmlhttprequest/logout.html bug 52047 (author: [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 78206 [details]
Patch

Clearing flags on attachment: 78206

Committed r75233: <http://trac.webkit.org/changeset/75233>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Eric Seidel (no email)]

Comment on attachment 78206 [details]
Patch

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

LayoutTestResults/FlakyTestReporter/CommitQueueTask may want something like this for saving off a copy of the LayoutTestsDirectory (either in zip form for upload, or just for later use after a second layout-test run.

> Tools/Scripts/webkitpy/common/system/directoryfileset.py:64
> +        return self._filesystem.read_text_file(self._full_path(filename))

This won't work for binary files, obviously.

[Csaba Osztrogonác]

It broke Qt and GTK bots, reopen to fix. I think 
the problem caused by different python versions.

test-webkitpy: INFO     Suppressing most webkitpy logging while running unit tests.
test-webkitpy: WARNING  Skipping tests in ./Tools/QueueStatusServer due to failure (No module named dev_appserver).  This module is optional.  The failure is likely due to a missing Google AppEngine install.  (http://code.google.com/appengine/downloads.html)
webkitpy.test.main: INFO     Excluding: webkitpy.common.checkout.scm_unittest (use --all to include)
/home/webkitbuildbot/slaves/release32bit/buildslave/qt-linux-release/build/Tools/Scripts/webkitpy/common/system/fileset.py:49: Warning: 'with' will become a reserved keyword in Python 2.6
Traceback (most recent call last):
  File "./Tools/Scripts/test-webkitpy", line 266, in <module>
    Tester().run_tests(sys.argv, external_package_paths)
  File "/home/webkitbuildbot/slaves/release32bit/buildslave/qt-linux-release/build/Tools/Scripts/webkitpy/test/main.py", line 140, in run_tests
    unittest.main(argv=sys_argv, module=None)
  File "/usr/lib/python2.5/unittest.py", line 767, in __init__
    self.parseArgs(argv)
  File "/usr/lib/python2.5/unittest.py", line 794, in parseArgs
    self.createTests()
  File "/usr/lib/python2.5/unittest.py", line 800, in createTests
    self.module)
  File "/usr/lib/python2.5/unittest.py", line 565, in loadTestsFromNames
    suites = [self.loadTestsFromName(name, module) for name in names]
  File "/usr/lib/python2.5/unittest.py", line 533, in loadTestsFromName
    module = __import__('.'.join(parts_copy))
  File "/home/webkitbuildbot/slaves/release32bit/buildslave/qt-linux-release/build/Tools/Scripts/webkitpy/common/system/directoryfileset_unittest.py", line 28, in <module>
    from webkitpy.common.system.directoryfileset import DirectoryFileSet
  File "/home/webkitbuildbot/slaves/release32bit/buildslave/qt-linux-release/build/Tools/Scripts/webkitpy/common/system/directoryfileset.py", line 29, in <module>
    from webkitpy.common.system.fileset import FileSetFileHandle
  File "/home/webkitbuildbot/slaves/release32bit/buildslave/qt-linux-release/build/Tools/Scripts/webkitpy/common/system/fileset.py", line 49
    with self._filesystem.mkdtemp() as temp_dir:
            ^
SyntaxError: invalid syntax
program finished with exit code 1

[Eric Seidel (no email)]

Oh, the file just needs a:

from __future__ import with_statement

at the top in order to work on python 2.5. :)

[WebKit Review Bot]

http://trac.webkit.org/changeset/75233 might have broken Leopard Intel Release (Tests) and Leopard Intel Debug (Tests)

[Csaba Osztrogonác]

(In reply to comment #30)
> Oh, the file just needs a:
> 
> from __future__ import with_statement
> 
> at the top in order to work on python 2.5. :)

Fixed in http://trac.webkit.org/changeset/75239 . Thx.

[Mihai Parparita]

(In reply to comment #30)
> Oh, the file just needs a:
> 
> from __future__ import with_statement
> 
> at the top in order to work on python 2.5. :)

This happens periodically. Any chance the cq machines can be downgraded to 2.5 to catch this?

[Eric Seidel (no email)]

I'll see about making the commit-queue use python 2.5 to run the python tests if available.

[Eric Seidel (no email)]

filed bug 52066.