mirror of
https://github.com/mozilla/gecko-dev.git
synced 2024-10-10 20:05:49 +00:00
Bug 971272 - Part 1: Guard against adding {foo, foo/bar} to FileRegistry. r=gps
This already raised if the order was [foo, foo/bar]. But it didn't prevent adding [foo/bar, foo]. The only sub-classes of FileRegistry are FileCopier and Jarrer. FileCopier.copy threw in the previously unhandled case: the order of creation is the same as the order of addition, so that foo is created after foo/bar. A zip file index can contain both foo and foo/bar. I don't think we should rely on this property in our use of Jarrer, but if we already do, I guess we need to move these guards into FileCopier. Let's hope that's not the case! (For the record: On my Mac OS X system, unzipping such a zip file prompts the user for what to do, depending on the order of the entries in the zip index.)
This commit is contained in:
parent
e929711433
commit
a45bc98b33
@ -13,6 +13,7 @@ from mozpack.files import (
|
||||
import mozpack.path
|
||||
import errno
|
||||
from collections import (
|
||||
Counter,
|
||||
OrderedDict,
|
||||
)
|
||||
|
||||
@ -31,6 +32,19 @@ class FileRegistry(object):
|
||||
|
||||
def __init__(self):
|
||||
self._files = OrderedDict()
|
||||
self._required_directories = Counter()
|
||||
|
||||
def _partial_paths(self, path):
|
||||
'''
|
||||
Turn "foo/bar/baz/zot" into ["foo/bar/baz", "foo/bar", "foo"].
|
||||
'''
|
||||
partial_paths = []
|
||||
partial_path = path
|
||||
while partial_path:
|
||||
partial_path = mozpack.path.dirname(partial_path)
|
||||
if partial_path:
|
||||
partial_paths.append(partial_path)
|
||||
return partial_paths
|
||||
|
||||
def add(self, path, content):
|
||||
'''
|
||||
@ -39,14 +53,17 @@ class FileRegistry(object):
|
||||
assert isinstance(content, BaseFile)
|
||||
if path in self._files:
|
||||
return errors.error("%s already added" % path)
|
||||
if self._required_directories[path] > 0:
|
||||
return errors.error("Can't add %s: it is a required directory" %
|
||||
path)
|
||||
# Check whether any parent of the given path is already stored
|
||||
partial_path = path
|
||||
while partial_path:
|
||||
partial_path = mozpack.path.dirname(partial_path)
|
||||
partial_paths = self._partial_paths(path)
|
||||
for partial_path in partial_paths:
|
||||
if partial_path in self._files:
|
||||
return errors.error("Can't add %s: %s is a file" %
|
||||
(path, partial_path))
|
||||
self._files[path] = content
|
||||
self._required_directories.update(partial_paths)
|
||||
|
||||
def match(self, pattern):
|
||||
'''
|
||||
@ -76,6 +93,7 @@ class FileRegistry(object):
|
||||
"not matching anything previously added"))
|
||||
for i in items:
|
||||
del self._files[i]
|
||||
self._required_directories.subtract(self._partial_paths(i))
|
||||
|
||||
def paths(self):
|
||||
'''
|
||||
|
@ -88,6 +88,34 @@ class TestFileRegistry(MatchTestTemplate, unittest.TestCase):
|
||||
self.add('foo/.foo')
|
||||
self.assertTrue(self.registry.contains('foo/.foo'))
|
||||
|
||||
def test_registry_paths(self):
|
||||
self.registry = FileRegistry()
|
||||
|
||||
# Can't add a file if it requires a directory in place of a
|
||||
# file we also require.
|
||||
self.registry.add('foo', GeneratedFile('foo'))
|
||||
self.assertRaises(ErrorMessage, self.registry.add, 'foo/bar',
|
||||
GeneratedFile('foobar'))
|
||||
|
||||
# Can't add a file if we already have a directory there.
|
||||
self.registry.add('bar/baz', GeneratedFile('barbaz'))
|
||||
self.assertRaises(ErrorMessage, self.registry.add, 'bar',
|
||||
GeneratedFile('bar'))
|
||||
|
||||
# Bump the count of things that require bar/ to 2.
|
||||
self.registry.add('bar/zot', GeneratedFile('barzot'))
|
||||
self.assertRaises(ErrorMessage, self.registry.add, 'bar',
|
||||
GeneratedFile('bar'))
|
||||
|
||||
# Drop the count of things that require bar/ to 1.
|
||||
self.registry.remove('bar/baz')
|
||||
self.assertRaises(ErrorMessage, self.registry.add, 'bar',
|
||||
GeneratedFile('bar'))
|
||||
|
||||
# Drop the count of things that require bar/ to 0.
|
||||
self.registry.remove('bar/zot')
|
||||
self.registry.add('bar/zot', GeneratedFile('barzot'))
|
||||
|
||||
|
||||
class TestFileCopier(TestWithTmpDir):
|
||||
def all_dirs(self, base):
|
||||
|
Loading…
Reference in New Issue
Block a user