Fix slash direction sensitivity in git cl owners

Slashes and backslashes are interchangeable on Windows in many contexts.
But not all. In particular, if you add a path to a dictionary and then
try looking it up with an equivalent path that has different slashes
then - not surprisingly - the lookup will fail. Therefore it is
important to always use slashes when populating _paths_to_owners. This
is now done, and documented.

Additionally, while fnmatch.fnmatch handles comparing slash separated
paths to backslash separated paths, fnmatch.translate does not. This is
arguably a bug in fnmatch.translate, or at least something that should
be documented. This bug is worked around by having _fnmatch sanitize
file names to always use slashes, and asserting that the patterns do
not contain backslashes.

With these changes this command:
  git cl owners --ignore-current --show-all chrome/browser/BUILD.gn
now correctly gives the same results as this command on Windows:
  git cl owners --ignore-current --show-all chrome\browser\BUILD.gn

The modified test fails without the other changes, passes with them.

Bug: 1009104
Change-Id: I416c7131281f00e352c1d2b85c30fcc463417fa5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1915410
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>

owners.py

@@ -59,6 +59,7 @@ Examples for all of these combinations can be found in tests/owners_unittest.py.
 
 import collections
 import fnmatch
+import os
 import random
 import re
 
@@ -133,6 +134,7 @@ class Database(object):
 
     # Mappings of directories -> globs in the directory -> owners
     # Example: "chrome/browser" -> "chrome/browser/*.h" -> ("john", "maria")
+    # Paths used as keys must use slashes as the separator, even on Windows.
     self._paths_to_owners = {}
 
     # Mapping reviewers to the preceding comment per file in the OWNERS files.
@@ -234,6 +236,8 @@ class Database(object):
     self._read_global_comments()
     visited_dirs = set()
     for f in files:
+      # Always use slashes as separators.
+      f = f.replace(os.sep, '/')
       dirpath = self.os_path.dirname(f)
       while dirpath not in visited_dirs:
         visited_dirs.add(dirpath)
@@ -413,6 +417,9 @@ class Database(object):
           'cannot parse status entry: "%s"' % line.strip())
 
   def _add_entry(self, owned_paths, directive, owners_path, lineno, comment):
+    # Consistently uses paths with slashes as the keys or else Windows will
+    # break in surprising and untested ways.
+    owned_paths = owned_paths.replace(os.sep, '/')
     if directive == 'set noparent':
       self._stop_looking.setdefault(
         self._get_root_affected_dir(owned_paths), set()).add(owned_paths)
@@ -567,6 +574,8 @@ class Database(object):
     all_possible_owners_for_dir_or_file_cache = {}
     all_possible_owners = {}
     for current_dir in dirs_and_files:
+      # Always use slashes as separators.
+      current_dir = current_dir.replace(os.sep, '/')
       dir_owners = self._all_possible_owners_for_dir_or_file(
         current_dir, author,
         all_possible_owners_for_dir_or_file_cache)
@@ -580,6 +589,11 @@ class Database(object):
 
   def _fnmatch(self, filename, pattern):
     """Same as fnmatch.fnmatch(), but internally caches the compiled regexes."""
+    # Make sure backslashes are never used in the filename. The regex
+    # expressions generated by fnmatch.translate don't handle matching slashes
+    # to backslashes.
+    filename = filename.replace(os.sep, '/')
+    assert pattern.count('\\') == 0, 'Backslashes found in %s' % pattern
     matcher = self._fnmatch_cache.get(pattern)
     if matcher is None:
       matcher = re.compile(fnmatch.translate(pattern)).match

presubmit_support.py

@@ -634,7 +634,8 @@ class InputApi(object):
   def AffectedFiles(self, include_deletes=True, file_filter=None):
     """Same as input_api.change.AffectedFiles() except only lists files
     (and optionally directories) in the same directory as the current presubmit
-    script, or subdirectories thereof.
+    script, or subdirectories thereof. Note that files are listed using the OS
+    path separator, so backslashes are used as separators on Windows.
     """
     dir_with_slash = normpath('%s/' % self.PresubmitLocalPath())
     if len(dir_with_slash) == 1:

tests/owners_finder_test.py

@@ -150,6 +150,17 @@ class OwnersFinderTests(_BaseTestCase):
     finder = self.ownersFinder(files, author=brett)
     self.assertEqual(finder.unreviewed_files, {'content/bar/foo.cc'})
 
+  def test_native_path_sep(self):
+    # Create a path with backslashes on Windows to make sure these are handled.
+    # This test is a harmless duplicate on other platforms.
+    native_slashes_path = 'chrome/browser/defaults.h'.replace('/', os.sep)
+    files = [
+        native_slashes_path,          # owned by brett
+        'content/bar/foo.cc',         # not owned by brett
+    ]
+    finder = self.ownersFinder(files, reviewers=[brett])
+    self.assertEqual(finder.unreviewed_files, {'content/bar/foo.cc'})
+
   def test_reset(self):
     finder = self.defaultFinder()
     for _ in range(2):