Skip to content

Commit

Permalink
Revert "Migrate //PRESUBMIT*.py to Python3."
Browse files Browse the repository at this point in the history 
This reverts commit 8b59f97.

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 <dpranke@google.com>
> Reviewed-by: Andrew Grieve <agrieve@chromium.org>
> 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 <khorimoto@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Owners-Override: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#883359}
  • Loading branch information
Kyle Horimoto authored and Chromium LUCI CQ committed May 17, 2021
1 parent 58e172f commit abee50a
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 51 deletions.
44 changes: 19 additions & 25 deletions PRESUBMIT.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,6 @@
"""
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[\\/]"
Expand Down Expand Up @@ -1933,7 +1929,7 @@ def CheckFilePermissions(input_api, output_api):
for f in input_api.AffectedFiles():
# checkperms.py file/directory arguments must be relative to the
# repository.
file_list.write((f.LocalPath() + '\n').encode('utf8'))
file_list.write(f.LocalPath() + '\n')
file_list.close()
args += ['--file-list', file_list.name]
try:
Expand Down Expand Up @@ -2146,7 +2142,7 @@ def _ExtractAddRulesFromParsedDeps(parsed_deps):
if rule.startswith('+') or rule.startswith('!')
])
for _, rules in parsed_deps.get('specific_include_rules',
{}).items():
{}).iteritems():
add_rules.update([
rule[1:] for rule in rules
if rule.startswith('+') or rule.startswith('!')
Expand Down Expand Up @@ -2175,7 +2171,12 @@ def Lookup(self, var_name):
'Str': str,
}

exec(contents, global_scope, local_scope)
# 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
return local_scope


Expand Down Expand Up @@ -2906,9 +2907,9 @@ def CheckSecurityOwners(input_api, output_api):

# 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.items():
for owners_file, patterns in to_check.iteritems():
try:
with open(owners_file) as f:
with file(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
Expand All @@ -2919,10 +2920,10 @@ def CheckSecurityOwners(input_api, output_api):

# All the remaining lines weren't found in OWNERS files, so emit an error.
errors = []
for owners_file, patterns in to_check.items():
for owners_file, patterns in to_check.iteritems():
missing_lines = []
files = []
for _, entry in patterns.items():
for _, entry in patterns.iteritems():
missing_lines.extend(entry['rules'])
files.extend([' %s' % f.LocalPath() for f in entry['files']])
if missing_lines:
Expand Down Expand Up @@ -3738,7 +3739,7 @@ def CheckForRelativeIncludes(input_api, output_api):
return []

error_descriptions = []
for file_path, bad_lines in bad_files.items():
for file_path, bad_lines in bad_files.iteritems():
error_description = file_path
for line in bad_lines:
error_description += '\n ' + line
Expand Down Expand Up @@ -4289,16 +4290,9 @@ def ChecksCommon(input_api, output_api):
# 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$'],
run_on_python2=not use_python3,
run_on_python3=use_python3))
files_to_check=[r'^PRESUBMIT_test\.py$']))
return results


Expand Down Expand Up @@ -5038,18 +5032,18 @@ def _ParseIcuVariants(text, offset=0):
if file_path.endswith('.grdp'):
if f.OldContents():
old_id_to_msg_map = grd_helper.GetGrdpMessagesFromString(
'\n'.join(f.OldContents()))
unicode('\n'.join(f.OldContents())))
if f.NewContents():
new_id_to_msg_map = grd_helper.GetGrdpMessagesFromString(
'\n'.join(f.NewContents()))
unicode('\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('\n'.join(f.OldContents())), file_dir)
StringIO(unicode('\n'.join(f.OldContents()))), file_dir)
if f.NewContents():
new_id_to_msg_map = grd_helper.GetGrdMessages(
StringIO('\n'.join(f.NewContents())), file_dir)
StringIO(unicode('\n'.join(f.NewContents()))), file_dir)

grd_name, ext = input_api.os_path.splitext(
input_api.os_path.basename(file_path))
Expand Down Expand Up @@ -5167,7 +5161,7 @@ def CheckTranslationExpectations(input_api, output_api,
# ui/webui/resoucres/tools/generate_grd.py.
ignore_path = input_api.os_path.join(
'ui', 'webui', 'resources', 'tools', 'tests')
grd_files = [p for p in grd_files if ignore_path not in p]
grd_files = filter(lambda p: ignore_path not in p, grd_files)

try:
translation_helper.get_translatable_grds(repo_root, grd_files,
Expand Down
38 changes: 19 additions & 19 deletions PRESUBMIT_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -290,17 +290,17 @@ def testFailure(self):
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)
Expand Down Expand Up @@ -330,10 +330,10 @@ def testNoEatComments(self):
MockFile(file_without_comments,
contents_without_comments)]

self.assertNotEqual(None,
str(PRESUBMIT._GetJSONParseError(input_api,
file_with_comments,
eat_comments=False)))
self.assertEqual('No JSON object could be decoded',
str(PRESUBMIT._GetJSONParseError(input_api,
file_with_comments,
eat_comments=False)))
self.assertEqual(None,
PRESUBMIT._GetJSONParseError(input_api,
file_without_comments,
Expand Down Expand Up @@ -2218,9 +2218,9 @@ def testChangeOwnersMissing(self):
self._mockChangeOwnerAndReviewers(
mock_input_api, 'owner@chromium.org', ['banana@chromium.org'])
result = PRESUBMIT.CheckSecurityChanges(mock_input_api, mock_output_api)
self.assertEqual(1, len(result))
self.assertEqual(result[0].type, 'notify')
self.assertEqual(result[0].message,
self.assertEquals(1, len(result))
self.assertEquals(result[0].type, 'notify')
self.assertEquals(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'
Expand All @@ -2237,9 +2237,9 @@ def testChangeOwnersMissingAtCommit(self):
self._mockChangeOwnerAndReviewers(
mock_input_api, 'owner@chromium.org', ['banana@chromium.org'])
result = PRESUBMIT.CheckSecurityChanges(mock_input_api, mock_output_api)
self.assertEqual(1, len(result))
self.assertEqual(result[0].type, 'error')
self.assertEqual(result[0].message,
self.assertEquals(1, len(result))
self.assertEquals(result[0].type, 'error')
self.assertEquals(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'
Expand All @@ -2256,7 +2256,7 @@ def testChangeOwnersPresent(self):
mock_input_api, 'owner@chromium.org',
['apple@chromium.org', 'banana@chromium.org'])
result = PRESUBMIT.CheckSecurityChanges(mock_input_api, mock_output_api)
self.assertEqual(0, len(result))
self.assertEquals(0, len(result))

def testChangeOwnerIsSecurityOwner(self):
mock_input_api = MockInputApi()
Expand All @@ -2268,7 +2268,7 @@ def testChangeOwnerIsSecurityOwner(self):
self._mockChangeOwnerAndReviewers(
mock_input_api, 'orange@chromium.org', ['pear@chromium.org'])
result = PRESUBMIT.CheckSecurityChanges(mock_input_api, mock_output_api)
self.assertEqual(1, len(result))
self.assertEquals(1, len(result))


class BannedTypeCheckTest(unittest.TestCase):
Expand Down Expand Up @@ -2674,8 +2674,8 @@ def testBlocksDirectIncludes(self):
MockFile('dir/jumbo.h', ['#include "sphelper.h"']),
]
results = PRESUBMIT._CheckNoStrCatRedefines(mock_input_api, MockOutputApi())
self.assertEqual(1, len(results))
self.assertEqual(4, len(results[0].items))
self.assertEquals(1, len(results))
self.assertEquals(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])
Expand All @@ -2689,7 +2689,7 @@ def testAllowsToIncludeWrapper(self):
MockFile('dir/baz-win.h', ['#include "base/win/atl.h"']),
]
results = PRESUBMIT._CheckNoStrCatRedefines(mock_input_api, MockOutputApi())
self.assertEqual(0, len(results))
self.assertEquals(0, len(results))

def testAllowsToCreateWrapper(self):
mock_input_api = MockInputApi()
Expand All @@ -2699,7 +2699,7 @@ def testAllowsToCreateWrapper(self):
'#include "base/win/windows_defines.inc"']),
]
results = PRESUBMIT._CheckNoStrCatRedefines(mock_input_api, MockOutputApi())
self.assertEqual(0, len(results))
self.assertEquals(0, len(results))


class StringTest(unittest.TestCase):
Expand Down
2 changes: 1 addition & 1 deletion PRESUBMIT_test_mocks.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ def ReadFile(self, filename, mode='rU'):
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):
Expand Down
8 changes: 2 additions & 6 deletions tools/translation/helper/translation_helper.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,8 @@
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.
Expand Down Expand Up @@ -99,7 +95,7 @@ def get_translatable_grds(repo_root, all_grd_paths,
# 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.items():
for path, grd in all_grds.iteritems():
if grd.appears_translatable:
if path not in grds_with_expectations:
errors.append('%s appears to be translatable (because it contains '
Expand All @@ -117,7 +113,7 @@ def get_translatable_grds(repo_root, all_grd_paths,
(translation_expectations_path, '\n - '.join(errors)))

translatable_grds = []
for path, expected_languages_list in grd_to_langs.items():
for path, expected_languages_list in grd_to_langs.iteritems():
grd = all_grds[path]
grd.expected_languages = expected_languages_list
grd._populate_lang_to_xtb_path(errors)
Expand Down

0 comments on commit abee50a

Please sign in to comment.