From 7f75c0e92e6aa75cfb773d06201b919ec196faf0 Mon Sep 17 00:00:00 2001 From: John Budorick Date: Fri, 23 Aug 2019 22:51:00 +0000 Subject: [PATCH] owners: fix inline comment support in included files. The parsing logic for OWNERS files included via "file:" lines currently rejects inline comments (e.g. "foo@example.com # for foo.cc"), while the normal OWNERS parsing logic correctly ignores such inline comments. This CL makes the inline case ignore inline comments too. Bug: 995474 Change-Id: I6f30554daf0a5f63b81719dced44f59187707eaa Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1769603 Reviewed-by: Edward Lesmes Commit-Queue: John Budorick --- owners.py | 4 ++++ tests/owners_unittest.py | 7 +++++++ 2 files changed, 11 insertions(+) diff --git a/owners.py b/owners.py index e264b59460..602fd31149 100644 --- a/owners.py +++ b/owners.py @@ -478,6 +478,10 @@ class Database(object): line.startswith('per-file')): continue + # If the line ends with a comment, strip the comment. + line, _delim, _comment = line.partition('#') + line = line.strip() + if self.email_regexp.match(line) or line == EVERYONE: owners.add(line) continue diff --git a/tests/owners_unittest.py b/tests/owners_unittest.py index 2b308da704..5998c46d92 100755 --- a/tests/owners_unittest.py +++ b/tests/owners_unittest.py @@ -48,10 +48,13 @@ def test_repo(): '/chrome/browser/defaults.h': '', '/chrome/gpu/OWNERS': owners_file(ken), '/chrome/gpu/gpu_channel.h': '', + '/chrome/comment/OWNERS': owners_file(file='//content/comment/OWNERS'), '/chrome/renderer/OWNERS': owners_file(peter), '/chrome/renderer/gpu/gpu_channel_host.h': '', '/chrome/renderer/safe_browsing/scorer.h': '', '/content/OWNERS': owners_file(john, darin, comment='foo', noparent=True), + '/content/comment/OWNERS': owners_file(john + ' # for comments', + darin + ' # for everything else'), '/content/content.gyp': '', '/content/bar/foo.cc': '', '/content/baz/OWNERS': owners_file(brett), @@ -321,6 +324,10 @@ class OwnersDatabaseTest(_BaseTestCase): except owners.SyntaxErrorInOwnersFile as e: self.assertTrue(str(e).startswith('/ipc/OWNERS:1')) + def test_file_include_with_comment(self): + # See crbug.com/995474 for context. + self.assert_files_not_covered_by(['chrome/comment/comment.cc'], [darin], []) + def assert_syntax_error(self, owners_file_contents): db = self.db() self.files['/foo/OWNERS'] = owners_file_contents