Commit 9f33cba2 authored by dpranke@chromium.org's avatar dpranke@chromium.org

2011-01-30 Dirk Pranke <dpranke@chromium.org>

        Reviewed by Eric Seidel.

        Clean up of the filesystem-related modules used in webkitpy.
        I've added relpath() to the filesystem interface, modified
        ospath.relpath() so that it could work with the filesystem
        interface, and modified the fileset* routines to use the
        filesystem interface consistently.

        This patch also adds a close() routine to the fileset routines
        to indicate that the caller is done with the fileset. This
        allows zipfileset to clean up after itself when it creates
        tempfiles to store downloads.

        https://bugs.webkit.org/show_bug.cgi?id=53326

        * Scripts/webkitpy/common/system/directoryfileset.py:
        * Scripts/webkitpy/common/system/fileset.py:
        * Scripts/webkitpy/common/system/filesystem.py:
        * Scripts/webkitpy/common/system/filesystem_mock.py:
        * Scripts/webkitpy/common/system/filesystem_unittest.py:
        * Scripts/webkitpy/common/system/ospath.py:
        * Scripts/webkitpy/common/system/zipfileset.py:
        * Scripts/webkitpy/common/system/zipfileset_unittest.py:

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@77093 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 9411f803
2011-01-30 Dirk Pranke <dpranke@chromium.org>
Reviewed by Eric Seidel.
Clean up of the filesystem-related modules used in webkitpy.
I've added relpath() to the filesystem interface, modified
ospath.relpath() so that it could work with the filesystem
interface, and modified the fileset* routines to use the
filesystem interface consistently.
This patch also adds a close() routine to the fileset routines
to indicate that the caller is done with the fileset. This
allows zipfileset to clean up after itself when it creates
tempfiles to store downloads.
https://bugs.webkit.org/show_bug.cgi?id=53326
* Scripts/webkitpy/common/system/directoryfileset.py:
* Scripts/webkitpy/common/system/fileset.py:
* Scripts/webkitpy/common/system/filesystem.py:
* Scripts/webkitpy/common/system/filesystem_mock.py:
* Scripts/webkitpy/common/system/filesystem_unittest.py:
* Scripts/webkitpy/common/system/ospath.py:
* Scripts/webkitpy/common/system/zipfileset.py:
* Scripts/webkitpy/common/system/zipfileset_unittest.py:
2011-01-30 Balazs Kelemen <kbalazs@webkit.org>
Reviewed by Csaba Osztrogonác.
......@@ -21,14 +21,8 @@
# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
# THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
from __future__ import with_statement
import os
import shutil
from webkitpy.common.system.fileset import FileSetFileHandle
from webkitpy.common.system.filesystem import FileSystem
import webkitpy.common.system.ospath as ospath
class DirectoryFileSet(object):
......@@ -36,8 +30,8 @@ class DirectoryFileSet(object):
def __init__(self, path, filesystem=None):
self._path = path
self._filesystem = filesystem or FileSystem()
if not self._path.endswith(os.path.sep):
self._path += os.path.sep
if not self._path.endswith(self._filesystem.sep):
self._path += self._filesystem.sep
def _full_path(self, filename):
assert self._is_under(self._path, filename)
......@@ -52,7 +46,7 @@ class DirectoryFileSet(object):
return self._filesystem.files_under(self._path)
def _is_under(self, dir, filename):
return bool(ospath.relpath(self._filesystem.join(dir, filename), dir))
return bool(self._filesystem.relpath(self._filesystem.join(dir, filename), dir))
def open(self, filename):
return FileSetFileHandle(self, filename, self._filesystem)
......@@ -69,7 +63,7 @@ class DirectoryFileSet(object):
dest = self._filesystem.join(path, filename)
# As filename may have slashes in it, we must ensure that the same
# directory hierarchy exists at the output path.
self._filesystem.maybe_make_directory(os.path.split(dest)[0])
self._filesystem.maybe_make_directory(self._filesystem.dirname(dest))
self._filesystem.copyfile(src, dest)
def delete(self, filename):
......
......@@ -22,7 +22,6 @@
# THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
from __future__ import with_statement
import os
from webkitpy.common.system.filesystem import FileSystem
......@@ -38,6 +37,9 @@ class FileSetFileHandle(object):
def __str__(self):
return "%s:%s" % (self._fileset, self._filename)
def close(self):
pass
def contents(self):
if self._contents is None:
self._contents = self._fileset.read(self._filename)
......@@ -61,4 +63,4 @@ class FileSetFileHandle(object):
return self._filename
def splitext(self):
return os.path.splitext(self.name())
return self._filesystem.splitext(self.name())
......@@ -39,13 +39,20 @@ import shutil
import tempfile
import time
from webkitpy.common.system import ospath
class FileSystem(object):
"""FileSystem interface for webkitpy.
Unless otherwise noted, all paths are allowed to be either absolute
or relative."""
def __init__(self):
self.sep = os.sep
self._sep = os.sep
def _get_sep(self):
return self._sep
sep = property(_get_sep, doc="pathname separator")
def abspath(self, path):
return os.path.abspath(path)
......@@ -200,6 +207,9 @@ class FileSystem(object):
with codecs.open(path, 'r', 'utf8') as f:
return f.read()
def relpath(self, path, start='.'):
return ospath.relpath(path, start)
class _WindowsError(exceptions.OSError):
"""Fake exception for Linux and Mac."""
pass
......
......@@ -28,9 +28,11 @@
import errno
import os
import path
import re
from webkitpy.common.system import path
from webkitpy.common.system import ospath
class MockFileSystem(object):
def __init__(self, files=None):
......@@ -44,17 +46,23 @@ class MockFileSystem(object):
"""
self.files = files or {}
self.written_files = {}
self.sep = '/'
self._sep = '/'
self.current_tmpno = 0
def _get_sep(self):
return self._sep
sep = property(_get_sep, doc="pathname separator")
def _raise_not_found(self, path):
raise IOError(errno.ENOENT, path, os.strerror(errno.ENOENT))
def _split(self, path):
idx = path.rfind('/')
return (path[0:idx], path[idx + 1:])
return path.rsplit(self.sep, max=1)
def abspath(self, path):
if path.endswith(self.sep):
return path[:-1]
return path
def basename(self, path):
......@@ -91,10 +99,10 @@ class MockFileSystem(object):
if self.basename(path) in dirs_to_skip:
return []
if not path.endswith('/'):
path += '/'
if not path.endswith(self.sep):
path += self.sep
dir_substrings = ['/' + d + '/' for d in dirs_to_skip]
dir_substrings = [self.sep + d + self.sep for d in dirs_to_skip]
for filename in self.files:
if not filename.startswith(path):
continue
......@@ -118,7 +126,7 @@ class MockFileSystem(object):
return [f for f in self.files if f == path]
def isabs(self, path):
return path.startswith('/')
return path.startswith(self.sep)
def isfile(self, path):
return path in self.files and self.files[path] is not None
......@@ -126,8 +134,8 @@ class MockFileSystem(object):
def isdir(self, path):
if path in self.files:
return False
if not path.endswith('/'):
path += '/'
if not path.endswith(self.sep):
path += self.sep
# We need to use a copy of the keys here in order to avoid switching
# to a different thread and potentially modifying the dict in
......@@ -136,22 +144,24 @@ class MockFileSystem(object):
return any(f.startswith(path) for f in files)
def join(self, *comps):
return re.sub(re.escape(os.path.sep), '/', os.path.join(*comps))
# FIXME: might want tests for this and/or a better comment about how
# it works.
return re.sub(re.escape(os.path.sep), self.sep, os.path.join(*comps))
def listdir(self, path):
if not self.isdir(path):
raise OSError("%s is not a directory" % path)
if not path.endswith('/'):
path += '/'
if not path.endswith(self.sep):
path += self.sep
dirs = []
files = []
for f in self.files:
if self.exists(f) and f.startswith(path):
remaining = f[len(path):]
if '/' in remaining:
dir = remaining[:remaining.index('/')]
if self.sep in remaining:
dir = remaining[:remaining.index(self.sep)]
if not dir in dirs:
dirs.append(dir)
else:
......@@ -165,7 +175,7 @@ class MockFileSystem(object):
def _mktemp(self, suffix='', prefix='tmp', dir=None, **kwargs):
if dir is None:
dir = '/__im_tmp'
dir = self.sep + '__im_tmp'
curno = self.current_tmpno
self.current_tmpno += 1
return self.join(dir, "%s_%u_%s" % (prefix, curno, suffix))
......@@ -224,6 +234,9 @@ class MockFileSystem(object):
self._raise_not_found(path)
return self.files[path]
def relpath(self, path, start='.'):
return ospath.relpath(path, start, self.abspath, self.sep)
def remove(self, path):
if self.files[path] is None:
self._raise_not_found(path)
......@@ -231,8 +244,8 @@ class MockFileSystem(object):
self.written_files[path] = None
def rmtree(self, path):
if not path.endswith('/'):
path += '/'
if not path.endswith(self.sep):
path += self.sep
for f in self.files:
if f.startswith(path):
......
......@@ -167,6 +167,19 @@ class FileSystemTest(unittest.TestCase):
self.assertTrue(fs.remove('filename', remove_with_exception))
self.assertEquals(-1, FileSystemTest._remove_failures)
def test_sep(self):
fs = FileSystem()
self.assertEquals(fs.sep, os.sep)
self.assertEquals(fs.join("foo", "bar"),
os.path.join("foo", "bar"))
def test_sep__is_readonly(self):
def assign_sep():
fs.sep = ' '
fs = FileSystem()
self.assertRaises(AttributeError, assign_sep)
if __name__ == '__main__':
unittest.main()
......@@ -32,7 +32,7 @@ import os
#
# It should behave essentially the same as os.path.relpath(), except for
# returning None on paths not contained in abs_start_path.
def relpath(path, start_path, os_path_abspath=None):
def relpath(path, start_path, os_path_abspath=None, sep=None):
"""Return a path relative to the given start path, or None.
Returns None if the path is not contained in the directory start_path.
......@@ -44,10 +44,12 @@ def relpath(path, start_path, os_path_abspath=None):
os_path_abspath: A replacement function for unit testing. This
function should strip trailing slashes just like
os.path.abspath(). Defaults to os.path.abspath.
sep: Path separator. Defaults to os.path.sep
"""
if os_path_abspath is None:
os_path_abspath = os.path.abspath
sep = sep or os.sep
# Since os_path_abspath() calls os.path.normpath()--
#
......@@ -67,11 +69,11 @@ def relpath(path, start_path, os_path_abspath=None):
if not rel_path:
# Then the paths are the same.
pass
elif rel_path[0] == os.sep:
elif rel_path[0] == sep:
# It is probably sufficient to remove just the first character
# since os.path.normpath() collapses separators, but we use
# lstrip() just to be sure.
rel_path = rel_path.lstrip(os.sep)
rel_path = rel_path.lstrip(sep)
else:
# We are in the case typified by the following example:
#
......
......@@ -33,22 +33,28 @@ class ZipFileSet(object):
"""The set of files in a zip file that resides at a URL (local or remote)"""
def __init__(self, zip_url, filesystem=None, zip_factory=None):
self._zip_url = zip_url
self._temp_file = None
self._zip_file = None
self._filesystem = filesystem or FileSystem()
self._zip_factory = zip_factory or self._retrieve_zip_file
def _retrieve_zip_file(self, zip_url):
temp_file = NetworkTransaction().run(lambda: urllib.urlretrieve(zip_url)[0])
return zipfile.ZipFile(temp_file)
return (temp_file, zipfile.ZipFile(temp_file))
def _load(self):
if self._zip_file is None:
self._zip_file = self._zip_factory(self._zip_url)
self._temp_file, self._zip_file = self._zip_factory(self._zip_url)
def open(self, filename):
self._load()
return FileSetFileHandle(self, filename, self._filesystem)
def close(self):
if self._temp_file:
self._filesystem.remove(self._temp_file)
self._temp_file = None
def namelist(self):
self._load()
return self._zip_file.namelist()
......
......@@ -64,13 +64,17 @@ class ZipFileSetTest(unittest.TestCase):
result = FakeZip(self._filesystem)
result.add_file('some-file', 'contents')
result.add_file('a/b/some-other-file', 'other contents')
return result
return (None, result)
def test_open(self):
file = self._zip.open('a/b/some-other-file')
self.assertEquals('a/b/some-other-file', file.name())
self.assertEquals('other contents', file.contents())
def test_close(self):
zipfileset = ZipFileSet('blah', self._filesystem, self.make_fake_zip)
zipfileset.close()
def test_read(self):
self.assertEquals('contents', self._zip.read('some-file'))
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment