diff --git a/presubmit_support.py b/presubmit_support.py index 0feba2c86..b5d10704e 100755 --- a/presubmit_support.py +++ b/presubmit_support.py @@ -741,11 +741,26 @@ class InputApi(object): self.change.AffectedFiles(include_deletes, file_filter))) def LocalPaths(self): - """Returns local paths of input_api.AffectedFiles().""" + """Returns platform-native local paths of .AffectedFiles(). + + E.g. on Windows the paths will contain backslashes. The paths + will be relative to the client root, which might be different + than the CWD, so use this for platform-appropriate error messages, + but not for accessing the files. Use .AbsoluteLocalPaths() when you + need to actually access the files.""" paths = [af.LocalPath() for af in self.AffectedFiles()] logging.debug('LocalPaths: %s', paths) return paths + def UnixLocalPaths(self): + """Returns unix-style local paths of input_api.AffectedFiles(). + + I.e., on Windows the path will contain forward slashes, + not backslashes. Other platforms are unchanged. + + Use this when you need to check paths in a platform-independent way.""" + return [p.replace(os.path.sep, '/') for p in self.LocalPaths()] + def AbsoluteLocalPaths(self): """Returns absolute local paths of input_api.AffectedFiles().""" return [af.AbsoluteLocalPath() for af in self.AffectedFiles()] @@ -784,6 +799,9 @@ class InputApi(object): '/' path separators should be used in the regular expressions and will work on Windows as well as other platforms. + For backwards-compatibility, '\' path separators will + also work correctly on Windows (but not on other platforms). + Note: Copy-paste this function to suit your needs or use a lambda function. """ if files_to_check is None: @@ -793,13 +811,11 @@ class InputApi(object): def Find(affected_file, items): local_path = affected_file.LocalPath() + unix_local_path = affected_file.UnixLocalPath() for item in items: if self.re.match(item, local_path): return True - # Handle the cases where the files regex only handles /, but the - # local path uses \. - if self.is_windows and self.re.match( - item, local_path.replace('\\', '/')): + if self.re.match(item, unix_local_path): return True return False @@ -1035,14 +1051,24 @@ class AffectedFile(object): def LocalPath(self): """Returns the path of this file on the local disk relative to client root. - This should be used for error messages but not for accessing files, + This should be used for error messages but not for accessing the file, because presubmit checks are run with CWD=PresubmitLocalPath() (which is - often != client root). + often != client root). Use .AbsoluteLocalPath() to access the file. """ return normpath(self._path) + def UnixLocalPath(self): + """Returns LocalPath() normalized to use forward slashes. + + On Windows, this means that any backslashes in the path will be + replaced with forward slashes. On other platforms the behavior + is unchanged.""" + return self.LocalPath().replace(os.path.sep, '/') + def AbsoluteLocalPath(self): - """Returns the absolute path of this file on the local disk.""" + """Returns the absolute path of this file on the local disk. + + Use this when you need to actually access the file.""" return os.path.abspath(os.path.join(self._local_root, self.LocalPath())) def Action(self): @@ -1387,9 +1413,24 @@ class Change(object): return self.AffectedTestableFiles(include_deletes=include_deletes) def LocalPaths(self): - """Convenience function.""" + """Returns platform-native local paths of .AffectedFiles(). + + E.g.., on Windows the paths will contain backslashes. The paths + will be relative to the client root, which might be different + than the CWD, so use this for platform-appropriate error messages, + but not for accessing the files. Use .AbsoluteLocalPaths() when + you need to actually access the files.""" return [af.LocalPath() for af in self.AffectedFiles()] + def UnixLocalPaths(self): + """Returns unix-style local paths of .AffectedFiles(). + + I.e., on Windows the path will contain forward slashes, + not backslashes. Other platforms are unchanged. + + Use this when you need to check paths in a platform-independent way.""" + return [p.replace(os.path.sep, '/') for p in self.LocalPaths()] + def LocalSubmodules(self): """Returns local paths for affected submodules.""" return [af.LocalPath() for af in self.AffectedSubmodules()] diff --git a/tests/presubmit_support_test.py b/tests/presubmit_support_test.py index e6fe25260..62a14827c 100755 --- a/tests/presubmit_support_test.py +++ b/tests/presubmit_support_test.py @@ -98,6 +98,13 @@ class ProvidedDiffChangeTest(fake_repos.FakeReposTestBase): self._test('somewhere/else', ['not a top level file!'], ['still not a top level file!']) + def test_unix_local_paths(self): + if sys.platform == 'win32': + self.assertIn(r'somewhere\else', self.change.LocalPaths()) + else: + self.assertIn('somewhere/else', self.change.LocalPaths()) + self.assertIn('somewhere/else', self.change.UnixLocalPaths()) + class TestGenerateDiff(fake_repos.FakeReposTestBase): """ Tests for --generate_diff. diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index 1c4db0a56..03b534753 100755 --- a/tests/presubmit_unittest.py +++ b/tests/presubmit_unittest.py @@ -1353,6 +1353,12 @@ class InputApiUnittest(PresubmitTestsBase): self.assertEqual(got_files[3].LocalPath(), presubmit.normpath(files[7][1])) + self.assertIn('foo/blat.cc', input_api.UnixLocalPaths()) + if sys.platform == 'win32': + self.assertIn(r'foo\blat.cc', input_api.LocalPaths()) + else: + self.assertIn('foo/blat.cc', input_api.LocalPaths()) + def testDefaultFilesToCheckFilesToSkipFilters(self): def f(x): return presubmit.AffectedFile(x, 'M', self.fake_root_dir, None)