From f2d73522b8d989a4a325ddefdf4dc88f39eaeedb Mon Sep 17 00:00:00 2001
From: mbjorge <mbjorge@chromium.org>
Date: Thu, 14 Jul 2016 13:28:59 -0700
Subject: [PATCH] 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
---
 owners.py                          | 4 ++--
 testing_support/filesystem_mock.py | 6 +++---
 tests/owners_unittest.py           | 9 +++++++++
 3 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/owners.py b/owners.py
index 563675a15..56311e40e 100644
--- a/owners.py
+++ b/owners.py
@@ -99,7 +99,7 @@ class Database(object):
       root: the path to the root of the Repository
       open: function callback to open a text file for reading
       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
           (i.e., glob.glob)
     """
@@ -292,7 +292,7 @@ class Database(object):
       include_path = path[2:]
     else:
       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)
 
     owners_path = self.os_path.join(self.root, include_path)
diff --git a/testing_support/filesystem_mock.py b/testing_support/filesystem_mock.py
index b45ea8e37..7a44dabb3 100644
--- a/testing_support/filesystem_mock.py
+++ b/testing_support/filesystem_mock.py
@@ -86,9 +86,9 @@ class MockFileSystem(object):
       _RaiseNotFound(path)
     return self.files[path]
 
-  @staticmethod
-  def relpath(path, base):
+  def relpath(self, path, base):
     # 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 base.endswith('/')
     return path[len(base):]
diff --git a/tests/owners_unittest.py b/tests/owners_unittest.py
index 5cb177411..5cc5371e6 100755
--- a/tests/owners_unittest.py
+++ b/tests/owners_unittest.py
@@ -228,6 +228,15 @@ class OwnersDatabaseTest(_BaseTestCase):
     self.assert_files_not_covered_by(['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):
     self.files['/content/qux/OWNERS'] = owners_file(peter,
         lines=['per-file foo.*=file://content/baz/OWNERS'])