Commit f2d73522 authored by mbjorge's avatar mbjorge Committed by Commit bot

Fix relative file: paths in OWNERS with roots other than '/'

If an OWNERS file used the file: directive with a relative file
path, but was using a root other than '/' (e.g.
'/path/to/my/real/root'), then the include resolver would incorrectly
leave a leading '/' on the include path. When os_path.join was then
called, the leading '/' meant the path was treated as an absolute path
and the join did not behave as expected.

Review-Url: https://codereview.chromium.org/2148683003
parent 9a0de0b9
...@@ -99,7 +99,7 @@ class Database(object): ...@@ -99,7 +99,7 @@ class Database(object):
root: the path to the root of the Repository root: the path to the root of the Repository
open: function callback to open a text file for reading open: function callback to open a text file for reading
os_path: module/object callback with fields for 'abspath', 'dirname', os_path: module/object callback with fields for 'abspath', 'dirname',
'exists', and 'join' 'exists', 'join', and 'relpath'
glob: function callback to list entries in a directory match a glob glob: function callback to list entries in a directory match a glob
(i.e., glob.glob) (i.e., glob.glob)
""" """
...@@ -292,7 +292,7 @@ class Database(object): ...@@ -292,7 +292,7 @@ class Database(object):
include_path = path[2:] include_path = path[2:]
else: else:
assert start.startswith(self.root) assert start.startswith(self.root)
start = self.os_path.dirname(start[len(self.root):]) start = self.os_path.dirname(self.os_path.relpath(start, self.root))
include_path = self.os_path.join(start, path) include_path = self.os_path.join(start, path)
owners_path = self.os_path.join(self.root, include_path) owners_path = self.os_path.join(self.root, include_path)
......
...@@ -86,9 +86,9 @@ class MockFileSystem(object): ...@@ -86,9 +86,9 @@ class MockFileSystem(object):
_RaiseNotFound(path) _RaiseNotFound(path)
return self.files[path] return self.files[path]
@staticmethod def relpath(self, path, base):
def relpath(path, base):
# This implementation is wrong in many ways; assert to check them for now. # This implementation is wrong in many ways; assert to check them for now.
if not base.endswith(self.sep):
base += self.sep
assert path.startswith(base) assert path.startswith(base)
assert base.endswith('/')
return path[len(base):] return path[len(base):]
...@@ -228,6 +228,15 @@ class OwnersDatabaseTest(_BaseTestCase): ...@@ -228,6 +228,15 @@ class OwnersDatabaseTest(_BaseTestCase):
self.assert_files_not_covered_by(['content/garply/baz.cc'], self.assert_files_not_covered_by(['content/garply/baz.cc'],
[tom], ['content/garply/baz.cc']) [tom], ['content/garply/baz.cc'])
def test_file_include_relative_path_non_empty_root(self):
old_root = self.root
self.root = '/content'
self.assert_files_not_covered_by(['garply/foo.cc'], [peter], [])
self.assert_files_not_covered_by(['garply/bar.cc'], [darin], [])
self.assert_files_not_covered_by(['garply/baz.cc'],
[tom], ['garply/baz.cc'])
self.root = old_root
def test_file_include_per_file_absolute_path(self): def test_file_include_per_file_absolute_path(self):
self.files['/content/qux/OWNERS'] = owners_file(peter, self.files['/content/qux/OWNERS'] = owners_file(peter,
lines=['per-file foo.*=file://content/baz/OWNERS']) lines=['per-file foo.*=file://content/baz/OWNERS'])
......
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