Reland "Migrate //PRESUBMIT*.py to Python3."
This reverts commit abee50a262a41ae335bb74b6093d8610349118d3.
Reason for revert: Underlying fix to
tools/translation/helper/git_helper has landed.
Original change's description:
> Revert "Migrate //PRESUBMIT*.py to Python3."
>
> This reverts commit 8b59f97def70a451cb491d0bdd1819ba4840deda.
>
> Reason for revert:
> Breaks uploading CLs which change localized strings. See
> https://bugs.chromium.org/p/chromium/issues/detail?id=1209392.
>
> Original change's description:
> > Migrate //PRESUBMIT*.py to Python3.
> >
> > This CL migrates the root-level PRESUBMIT checks, their associated
> > tests, and and one other file that got pulled along for the ride)
> > to run under Python3 instead of Python2.
> >
> > Bug: 816629
> > Change-Id: I75f3edb34ca72458432ba54f3afa022e027f8eb9
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2877897
> > Commit-Queue: Dirk Pranke <[email protected]>
> > Reviewed-by: Andrew Grieve <[email protected]>
> > Cr-Commit-Position: refs/heads/master@{#883002}
>
> Bug: 816629
> Fixed: 1209392
> Change-Id: Icf0c084f1db3d20ca76600eb67cc3f6e81b3c9f9
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2899404
> Auto-Submit: Kyle Horimoto <[email protected]>
> Commit-Queue: Rubber Stamper <[email protected]>
> Bot-Commit: Rubber Stamper <[email protected]>
> Owners-Override: Yutaka Hirano <[email protected]>
> Cr-Commit-Position: refs/heads/master@{#883359}
Bug: 816629
Change-Id: Ie70748076c06e3ebcacd18e3d69f105d94a5e5ac
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2901242
Bot-Commit: Rubber Stamper <[email protected]>
Commit-Queue: Dirk Pranke <[email protected]>
Cr-Commit-Position: refs/heads/master@{#884028}
diff --git a/PRESUBMIT.py b/PRESUBMIT.py
index ecec493..032995cb 100644
--- a/PRESUBMIT.py
+++ b/PRESUBMIT.py
@@ -9,6 +9,10 @@
"""
PRESUBMIT_VERSION = '2.0.0'
+# This line is 'magic' in that git-cl looks for it to decide whether to
+# use Python3 instead of Python2 when running the code in this file.
+USE_PYTHON3 = True
+
_EXCLUDED_PATHS = (
# Generated file.
(r"^components[\\/]variations[\\/]proto[\\/]devtools[\\/]"
@@ -1929,7 +1933,7 @@
for f in input_api.AffectedFiles():
# checkperms.py file/directory arguments must be relative to the
# repository.
- file_list.write(f.LocalPath() + '\n')
+ file_list.write((f.LocalPath() + '\n').encode('utf8'))
file_list.close()
args += ['--file-list', file_list.name]
try:
@@ -2142,7 +2146,7 @@
if rule.startswith('+') or rule.startswith('!')
])
for _, rules in parsed_deps.get('specific_include_rules',
- {}).iteritems():
+ {}).items():
add_rules.update([
rule[1:] for rule in rules
if rule.startswith('+') or rule.startswith('!')
@@ -2171,12 +2175,7 @@
'Str': str,
}
- # TODO(crbug.com/1207012): We need to strip the BOM because it isn't
- # legal in Python source files. We can remove this check once the CL
- # that actually removes the BOM from //third_party/crashpad/DEPS lands.
- if contents.startswith(u'\ufeff'):
- contents = contents[1:]
- exec contents in global_scope, local_scope
+ exec(contents, global_scope, local_scope)
return local_scope
@@ -2907,9 +2906,9 @@
# Go through the OWNERS files to check, filtering out rules that are already
# present in that OWNERS file.
- for owners_file, patterns in to_check.iteritems():
+ for owners_file, patterns in to_check.items():
try:
- with file(owners_file) as f:
+ with open(owners_file) as f:
lines = set(f.read().splitlines())
for entry in patterns.values():
entry['rules'] = [rule for rule in entry['rules'] if rule not in lines
@@ -2920,10 +2919,10 @@
# All the remaining lines weren't found in OWNERS files, so emit an error.
errors = []
- for owners_file, patterns in to_check.iteritems():
+ for owners_file, patterns in to_check.items():
missing_lines = []
files = []
- for _, entry in patterns.iteritems():
+ for _, entry in patterns.items():
missing_lines.extend(entry['rules'])
files.extend([' %s' % f.LocalPath() for f in entry['files']])
if missing_lines:
@@ -3739,7 +3738,7 @@
return []
error_descriptions = []
- for file_path, bad_lines in bad_files.iteritems():
+ for file_path, bad_lines in bad_files.items():
error_description = file_path
for line in bad_lines:
error_description += '\n ' + line
@@ -4290,9 +4289,16 @@
# The PRESUBMIT.py file (and the directory containing it) might
# have been affected by being moved or removed, so only try to
# run the tests if they still exist.
+ use_python3 = False
+ with open(f.LocalPath()) as fp:
+ use_python3 = any(line.startswith('USE_PYTHON3 = True')
+ for line in fp.readlines())
+
results.extend(input_api.canned_checks.RunUnitTestsInDirectory(
input_api, output_api, full_path,
- files_to_check=[r'^PRESUBMIT_test\.py$']))
+ files_to_check=[r'^PRESUBMIT_test\.py$'],
+ run_on_python2=not use_python3,
+ run_on_python3=use_python3))
return results
@@ -5032,18 +5038,18 @@
if file_path.endswith('.grdp'):
if f.OldContents():
old_id_to_msg_map = grd_helper.GetGrdpMessagesFromString(
- unicode('\n'.join(f.OldContents())))
+ '\n'.join(f.OldContents()))
if f.NewContents():
new_id_to_msg_map = grd_helper.GetGrdpMessagesFromString(
- unicode('\n'.join(f.NewContents())))
+ '\n'.join(f.NewContents()))
else:
file_dir = input_api.os_path.dirname(file_path) or '.'
if f.OldContents():
old_id_to_msg_map = grd_helper.GetGrdMessages(
- StringIO(unicode('\n'.join(f.OldContents()))), file_dir)
+ StringIO('\n'.join(f.OldContents())), file_dir)
if f.NewContents():
new_id_to_msg_map = grd_helper.GetGrdMessages(
- StringIO(unicode('\n'.join(f.NewContents()))), file_dir)
+ StringIO('\n'.join(f.NewContents())), file_dir)
grd_name, ext = input_api.os_path.splitext(
input_api.os_path.basename(file_path))
@@ -5161,7 +5167,7 @@
# ui/webui/resoucres/tools/generate_grd.py.
ignore_path = input_api.os_path.join(
'ui', 'webui', 'resources', 'tools', 'tests')
- grd_files = filter(lambda p: ignore_path not in p, grd_files)
+ grd_files = [p for p in grd_files if ignore_path not in p]
try:
translation_helper.get_translatable_grds(repo_root, grd_files,
diff --git a/PRESUBMIT_test.py b/PRESUBMIT_test.py
index 11e8f69..b490d2a 100755
--- a/PRESUBMIT_test.py
+++ b/PRESUBMIT_test.py
@@ -290,17 +290,17 @@
test_data = [
('invalid_json_1.json',
['{ x }'],
- 'Expecting property name:'),
+ 'Expecting property name'),
('invalid_json_2.json',
['// Hello world!',
'{ "hello": "world }'],
'Unterminated string starting at:'),
('invalid_json_3.json',
['{ "a": "b", "c": "d", }'],
- 'Expecting property name:'),
+ 'Expecting property name'),
('invalid_json_4.json',
['{ "a": "b" "c": "d" }'],
- 'Expecting , delimiter:'),
+ "Expecting ',' delimiter:"),
]
input_api.files = [MockFile(filename, contents)
@@ -330,10 +330,10 @@
MockFile(file_without_comments,
contents_without_comments)]
- self.assertEqual('No JSON object could be decoded',
- str(PRESUBMIT._GetJSONParseError(input_api,
- file_with_comments,
- eat_comments=False)))
+ self.assertNotEqual(None,
+ str(PRESUBMIT._GetJSONParseError(input_api,
+ file_with_comments,
+ eat_comments=False)))
self.assertEqual(None,
PRESUBMIT._GetJSONParseError(input_api,
file_without_comments,
@@ -2218,9 +2218,9 @@
self._mockChangeOwnerAndReviewers(
mock_input_api, '[email protected]', ['[email protected]'])
result = PRESUBMIT.CheckSecurityChanges(mock_input_api, mock_output_api)
- self.assertEquals(1, len(result))
- self.assertEquals(result[0].type, 'notify')
- self.assertEquals(result[0].message,
+ self.assertEqual(1, len(result))
+ self.assertEqual(result[0].type, 'notify')
+ self.assertEqual(result[0].message,
'The following files change calls to security-sensive functions\n' \
'that need to be reviewed by ipc/SECURITY_OWNERS.\n'
' file.cc\n'
@@ -2237,9 +2237,9 @@
self._mockChangeOwnerAndReviewers(
mock_input_api, '[email protected]', ['[email protected]'])
result = PRESUBMIT.CheckSecurityChanges(mock_input_api, mock_output_api)
- self.assertEquals(1, len(result))
- self.assertEquals(result[0].type, 'error')
- self.assertEquals(result[0].message,
+ self.assertEqual(1, len(result))
+ self.assertEqual(result[0].type, 'error')
+ self.assertEqual(result[0].message,
'The following files change calls to security-sensive functions\n' \
'that need to be reviewed by ipc/SECURITY_OWNERS.\n'
' file.cc\n'
@@ -2256,7 +2256,7 @@
mock_input_api, '[email protected]',
['[email protected]', '[email protected]'])
result = PRESUBMIT.CheckSecurityChanges(mock_input_api, mock_output_api)
- self.assertEquals(0, len(result))
+ self.assertEqual(0, len(result))
def testChangeOwnerIsSecurityOwner(self):
mock_input_api = MockInputApi()
@@ -2268,7 +2268,7 @@
self._mockChangeOwnerAndReviewers(
mock_input_api, '[email protected]', ['[email protected]'])
result = PRESUBMIT.CheckSecurityChanges(mock_input_api, mock_output_api)
- self.assertEquals(1, len(result))
+ self.assertEqual(1, len(result))
class BannedTypeCheckTest(unittest.TestCase):
@@ -2674,8 +2674,8 @@
MockFile('dir/jumbo.h', ['#include "sphelper.h"']),
]
results = PRESUBMIT._CheckNoStrCatRedefines(mock_input_api, MockOutputApi())
- self.assertEquals(1, len(results))
- self.assertEquals(4, len(results[0].items))
+ self.assertEqual(1, len(results))
+ self.assertEqual(4, len(results[0].items))
self.assertTrue('StrCat' in results[0].message)
self.assertTrue('foo_win.cc' in results[0].items[0])
self.assertTrue('bar.h' in results[0].items[1])
@@ -2689,7 +2689,7 @@
MockFile('dir/baz-win.h', ['#include "base/win/atl.h"']),
]
results = PRESUBMIT._CheckNoStrCatRedefines(mock_input_api, MockOutputApi())
- self.assertEquals(0, len(results))
+ self.assertEqual(0, len(results))
def testAllowsToCreateWrapper(self):
mock_input_api = MockInputApi()
@@ -2699,7 +2699,7 @@
'#include "base/win/windows_defines.inc"']),
]
results = PRESUBMIT._CheckNoStrCatRedefines(mock_input_api, MockOutputApi())
- self.assertEquals(0, len(results))
+ self.assertEqual(0, len(results))
class StringTest(unittest.TestCase):
diff --git a/PRESUBMIT_test_mocks.py b/PRESUBMIT_test_mocks.py
index f8143ae..eb78f61 100644
--- a/PRESUBMIT_test_mocks.py
+++ b/PRESUBMIT_test_mocks.py
@@ -126,7 +126,7 @@
if file_.LocalPath() == filename:
return '\n'.join(file_.NewContents())
# Otherwise, file is not in our mock API.
- raise IOError, "No such file or directory: '%s'" % filename
+ raise IOError("No such file or directory: '%s'" % filename)
class MockOutputApi(object):
diff --git a/tools/translation/helper/translation_helper.py b/tools/translation/helper/translation_helper.py
index 6bbf650..299e1df 100644
--- a/tools/translation/helper/translation_helper.py
+++ b/tools/translation/helper/translation_helper.py
@@ -9,8 +9,12 @@
import ast
import os
import re
+import sys
import xml.etree.cElementTree as ElementTree
+if sys.version_info.major != 2:
+ basestring = str # pylint: disable=redefined-builtin
+
class GRDFile(object):
"""Class representing a grd xml file.
@@ -95,7 +99,7 @@
# the translation expectations.
grds_with_expectations = set(grd_to_langs.keys()).union(untranslated_grds)
all_grds = {p: GRDFile(os.path.join(repo_root, p)) for p in all_grd_paths}
- for path, grd in all_grds.iteritems():
+ for path, grd in all_grds.items():
if grd.appears_translatable:
if path not in grds_with_expectations:
errors.append('%s appears to be translatable (because it contains '
@@ -113,7 +117,7 @@
(translation_expectations_path, '\n - '.join(errors)))
translatable_grds = []
- for path, expected_languages_list in grd_to_langs.iteritems():
+ for path, expected_languages_list in grd_to_langs.items():
grd = all_grds[path]
grd.expected_languages = expected_languages_list
grd._populate_lang_to_xtb_path(errors)
