From efd28c14c284a1594420cbccc106cf48eef55f29 Mon Sep 17 00:00:00 2001 From: Michael Achenbach Date: Thu, 3 Feb 2022 13:27:53 +0100 Subject: [PATCH] [infra] Make various scripts compatible with Python3 This fixes all Python3 problems in scripts and tests running via v8_presubmit.py. It includes: - Test runner - Release tools - Perf runner - Torque formatter - V8's main presubmit On bots, v8_presubmit is run with vpython, hence we also add the required dependencies. After the Python3 migration, most of the transitional code in this CL can be removed again. Bug: chromium:1293709,chromium:1292016 Change-Id: Ic25e5965948b212c047e9d5194d2a4b6db1fa91b Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3432213 Reviewed-by: Liviu Rau Commit-Queue: Michael Achenbach Cr-Commit-Position: refs/heads/main@{#78936} --- .vpython3 | 25 +++++++++++++++++++++++++ tools/release/auto_roll.py | 2 +- tools/release/common_includes.py | 29 ++++++++++++++--------------- tools/release/create_release.py | 11 +++++++++-- tools/run_perf.py | 3 ++- tools/testrunner/local/command.py | 11 +++++++++-- tools/torque/format-torque.py | 12 ++++++++++-- tools/unittests/run_perf_test.py | 10 ++++++---- tools/unittests/run_tests_test.py | 10 ++++++++-- tools/v8_presubmit.py | 26 +++++++++++++++++--------- 10 files changed, 101 insertions(+), 38 deletions(-) diff --git a/.vpython3 b/.vpython3 index 95e52ee59e..d844b4dadd 100644 --- a/.vpython3 +++ b/.vpython3 @@ -44,3 +44,28 @@ wheel: < name: "infra/python/wheels/six-py2_py3" version: "version:1.15.0" > + +wheel: < + name: "infra/python/wheels/coverage/${vpython_platform}" + version: "version:5.5.chromium.2" +> + +wheel: < + name: "infra/python/wheels/pbr-py2_py3" + version: "version:3.0.0" +> + +wheel: < + name: "infra/python/wheels/funcsigs-py2_py3" + version: "version:1.0.2" +> + +wheel: < + name: "infra/python/wheels/mock-py2_py3" + version: "version:2.0.0" +> + +wheel: < + name: "infra/python/wheels/numpy/${vpython_platform}" + version: "version:1.20.3" +> \ No newline at end of file diff --git a/tools/release/auto_roll.py b/tools/release/auto_roll.py index 76247b1fb3..2aa4ff6829 100755 --- a/tools/release/auto_roll.py +++ b/tools/release/auto_roll.py @@ -80,7 +80,7 @@ class DetectRevisionToRoll(Step): version = self.GetVersionTag(revision) assert version, "Internal error. All recent releases should have a tag" - if SortingKey(self["last_version"]) < SortingKey(version): + if LooseVersion(self["last_version"]) < LooseVersion(version): self["roll"] = revision break else: diff --git a/tools/release/common_includes.py b/tools/release/common_includes.py index b61a3e2e27..0ccaa6aa11 100644 --- a/tools/release/common_includes.py +++ b/tools/release/common_includes.py @@ -31,7 +31,7 @@ from __future__ import print_function import argparse import datetime -import httplib +from distutils.version import LooseVersion import glob import imp import json @@ -43,11 +43,20 @@ import sys import textwrap import time import urllib -import urllib2 from git_recipes import GitRecipesMixin from git_recipes import GitFailedException +PYTHON3 = sys.version_info >= (3, 0) + +if PYTHON3: + import http.client as httplib + import urllib.request as urllib2 +else: + import httplib + import urllib2 + + DAY_IN_SECONDS = 24 * 60 * 60 PUSH_MSG_GIT_RE = re.compile(r".* \(based on (?P[a-fA-F0-9]+)\)$") PUSH_MSG_NEW_RE = re.compile(r"^Version \d+\.\d+\.\d+$") @@ -92,16 +101,6 @@ def MSub(rexp, replacement, text): return re.sub(rexp, replacement, text, flags=re.MULTILINE) -def SortingKey(version): - """Key for sorting version number strings: '3.11' > '3.2.1.1'""" - version_keys = map(int, version.split(".")) - # Fill up to full version numbers to normalize comparison. - while len(version_keys) < 4: # pragma: no cover - version_keys.append(0) - # Fill digits. - return ".".join(map("{0:04d}".format, version_keys)) - - # Some commands don't like the pipe, e.g. calling vi from within the script or # from subscripts like git cl upload. def Command(cmd, args="", prefix="", pipe=True, cwd=None): @@ -256,7 +255,7 @@ class GitInterface(VCInterface): lambda s: re.match(r"^branch\-heads/\d+\.\d+$", s), self.step.GitRemotes()) # Remove 'branch-heads/' prefix. - return map(lambda s: s[13:], branches) + return [b[13:] for b in branches] def MainBranch(self): return "main" @@ -557,7 +556,7 @@ class Step(GitRecipesMixin): int(time_now - max_age)).strip() # Filter out revisions who's tag is off by one or more commits. - return filter(lambda r: self.GetVersionTag(r), revisions.splitlines()) + return list(filter(self.GetVersionTag, revisions.splitlines())) def GetLatestVersion(self): # Use cached version if available. @@ -571,7 +570,7 @@ class Step(GitRecipesMixin): only_version_tags = NormalizeVersionTags(all_tags) version = sorted(only_version_tags, - key=SortingKey, reverse=True)[0] + key=LooseVersion, reverse=True)[0] self["latest_version"] = version return version diff --git a/tools/release/create_release.py b/tools/release/create_release.py index d1a066f00b..62c5ed675f 100755 --- a/tools/release/create_release.py +++ b/tools/release/create_release.py @@ -10,10 +10,17 @@ import argparse import os import sys import tempfile -import urllib2 from common_includes import * +PYTHON3 = sys.version_info >= (3, 0) + +if PYTHON3: + import urllib.request as urllib2 +else: + import urllib2 + + class Preparation(Step): MESSAGE = "Preparation." @@ -48,7 +55,7 @@ class IncrementVersion(Step): # Use the highest version from main or from tags to determine the new # version. authoritative_version = sorted( - [main_version, latest_version], key=SortingKey)[1] + [main_version, latest_version], key=LooseVersion)[1] self.StoreVersion(authoritative_version, "authoritative_") # Variables prefixed with 'new_' contain the new version numbers for the diff --git a/tools/run_perf.py b/tools/run_perf.py index 1e22b298a8..b394c3af03 100644 --- a/tools/run_perf.py +++ b/tools/run_perf.py @@ -289,7 +289,8 @@ def RunResultsProcessor(results_processor, output, count): stderr=subprocess.PIPE, ) new_output = copy.copy(output) - new_output.stdout, _ = p.communicate(input=output.stdout) + new_output.stdout = p.communicate( + input=output.stdout.encode('utf-8'))[0].decode('utf-8') logging.info('>>> Processed stdout (#%d):\n%s', count, output.stdout) return new_output diff --git a/tools/testrunner/local/command.py b/tools/testrunner/local/command.py index df603d79d2..6942d1b9a4 100644 --- a/tools/testrunner/local/command.py +++ b/tools/testrunner/local/command.py @@ -19,6 +19,7 @@ from ..local.android import ( from ..local import utils from ..objects import output +PYTHON3 = sys.version_info >= (3, 0) BASE_DIR = os.path.normpath( os.path.join(os.path.dirname(os.path.abspath(__file__)), '..' , '..', '..')) @@ -114,11 +115,17 @@ class BaseCommand(object): timer.cancel() + def convert(stream): + if PYTHON3: + return stream.decode('utf-8', 'replace') + else: + return stream.decode('utf-8', 'replace').encode('utf-8') + return output.Output( process.returncode, timeout_occured[0], - stdout.decode('utf-8', 'replace').encode('utf-8'), - stderr.decode('utf-8', 'replace').encode('utf-8'), + convert(stdout), + convert(stderr), process.pid, duration ) diff --git a/tools/torque/format-torque.py b/tools/torque/format-torque.py index 638ca100fb..98ae087ca1 100755 --- a/tools/torque/format-torque.py +++ b/tools/torque/format-torque.py @@ -15,6 +15,14 @@ import sys import re from subprocess import Popen, PIPE +PYTHON3 = sys.version_info >= (3, 0) + +def maybe_decode(arg, encoding="utf-8"): + return arg.decode(encoding) if PYTHON3 else arg + +def maybe_encode(arg, encoding="utf-8"): + return arg.encode(encoding) if PYTHON3 else arg + kPercentEscape = r'α'; # Unicode alpha kDerefEscape = r'☆'; # Unicode star kAddressofEscape = r'⌂'; # Unicode house @@ -103,8 +111,8 @@ def process(filename, lint, should_format): p = Popen(['clang-format', '-assume-filename=.ts'], stdin=PIPE, stdout=PIPE, stderr=PIPE, shell=True) else: p = Popen(['clang-format', '-assume-filename=.ts'], stdin=PIPE, stdout=PIPE, stderr=PIPE) - output, err = p.communicate(preprocess(content)) - output = postprocess(output) + output, err = p.communicate(maybe_encode(preprocess(content))) + output = postprocess(maybe_decode(output)) rc = p.returncode if (rc != 0): print("error code " + str(rc) + " running clang-format. Exiting...") diff --git a/tools/unittests/run_perf_test.py b/tools/unittests/run_perf_test.py index 28f71b2b33..6d8c5e2a13 100755 --- a/tools/unittests/run_perf_test.py +++ b/tools/unittests/run_perf_test.py @@ -28,6 +28,8 @@ TEST_DATA = os.path.join(BASE_DIR, 'unittests', 'testdata') TEST_WORKSPACE = os.path.join(tempfile.gettempdir(), 'test-v8-run-perf') +SORT_KEY = lambda x: x['graphs'] + V8_JSON = { 'path': ['.'], 'owners': ['username@chromium.org'], @@ -196,8 +198,8 @@ class PerfTest(unittest.TestCase): {'units': units, 'graphs': [suite, trace['name']], 'results': trace['results'], - 'stddev': trace['stddev']} for trace in traces]), - sorted(self._LoadResults(file_name)['traces'])) + 'stddev': trace['stddev']} for trace in traces], key=SORT_KEY), + sorted(self._LoadResults(file_name)['traces'], key=SORT_KEY)) def _VerifyRunnableDurations(self, runs, timeout, file_name=None): self.assertListEqual([ @@ -368,7 +370,7 @@ class PerfTest(unittest.TestCase): 'graphs': ['test', 'DeltaBlue'], 'results': [200.0], 'stddev': ''}, - ]), sorted(self._LoadResults()['traces'])) + ], key=SORT_KEY), sorted(self._LoadResults()['traces'], key=SORT_KEY)) self._VerifyErrors([]) self._VerifyMockMultiple( (os.path.join('out', 'x64.release', 'd7'), '--flag', 'run.js'), @@ -605,7 +607,7 @@ class PerfTest(unittest.TestCase): 'results': [2.1, 2.1], 'stddev': '', }, - ]), sorted(results['traces'])) + ], key=SORT_KEY), sorted(results['traces'], key=SORT_KEY)) def testResultsProcessor(self): results = self._RunPerf('d8_mocked2.py', 'test2.json') diff --git a/tools/unittests/run_tests_test.py b/tools/unittests/run_tests_test.py index 3581996ae4..762d3096ec 100755 --- a/tools/unittests/run_tests_test.py +++ b/tools/unittests/run_tests_test.py @@ -30,7 +30,11 @@ import sys import tempfile import unittest -from cStringIO import StringIO +# TODO(https://crbug.com/1292016): Remove after Python3 migration. +try: + from cStringIO import StringIO +except ImportError: + from io import StringIO TOOLS_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) TEST_DATA_ROOT = os.path.join(TOOLS_ROOT, 'unittests', 'testdata') @@ -277,7 +281,9 @@ class SystemTest(unittest.TestCase): # We need lexicographic sorting here to avoid non-deterministic behaviour # The original sorting key is duration, but in our fake test we have # non-deterministic durations before we reset them to 1 - json_output['slowest_tests'].sort(key= lambda x: str(x)) + def sort_key(x): + return str(sorted(x.items())) + json_output['slowest_tests'].sort(key=sort_key) with open(os.path.join(TEST_DATA_ROOT, expected_results_name)) as f: expected_test_results = json.load(f) diff --git a/tools/v8_presubmit.py b/tools/v8_presubmit.py index 508326cebf..1d2d7b1c6e 100755 --- a/tools/v8_presubmit.py +++ b/tools/v8_presubmit.py @@ -41,20 +41,28 @@ except ImportError as e: import json +import multiprocessing import optparse import os from os.path import abspath, join, dirname, basename, exists import pickle import re -import sys import subprocess -import multiprocessing from subprocess import PIPE +import sys from testrunner.local import statusfile from testrunner.local import testsuite from testrunner.local import utils +PYTHON3 = sys.version_info >= (3, 0) + +def maybe_decode(arg, encoding="utf-8"): + return arg.decode(encoding) if PYTHON3 else arg + +def maybe_encode(arg, encoding="utf-8"): + return arg.encode(encoding) if PYTHON3 else arg + # Special LINT rules diverging from default and reason. # build/header_guard: Our guards have the form "V8_FOO_H_", not "SRC_FOO_H_". # We now run our own header guard check in PRESUBMIT.py. @@ -92,7 +100,7 @@ def CppLintWorker(command): out_lines = "" error_count = -1 while True: - out_line = process.stderr.readline() + out_line = maybe_decode(process.stderr.readline()) if out_line == '' and process.poll() != None: if error_count == -1: print("Failed to process %s" % command.pop()) @@ -118,7 +126,7 @@ def TorqueLintWorker(command): out_lines = "" error_count = 0 while True: - out_line = process.stderr.readline() + out_line = maybe_decode(process.stderr.readline()) if out_line == '' and process.poll() != None: break out_lines += out_line @@ -148,7 +156,7 @@ def JSLintWorker(command): sys.stdout.write("error code " + str(rc) + " running clang-format.\n") return rc - if output != contents: + if maybe_decode(output) != contents: return 1 return 0 @@ -206,7 +214,7 @@ class FileContentsCache(object): for file in files: try: handle = open(file, "r") - file_sum = md5er(handle.read()).digest() + file_sum = md5er(maybe_encode(handle.read())).digest() if not file in self.sums or self.sums[file] != file_sum: changed_or_new.append(file) self.sums[file] = file_sum @@ -490,7 +498,7 @@ class SourceProcessor(SourceFileProcessor): output = subprocess.Popen('git ls-files --full-name', stdout=PIPE, cwd=path, shell=True) result = [] - for file in output.stdout.read().split(): + for file in maybe_decode(output.stdout.read()).split(): for dir_part in os.path.dirname(file).replace(os.sep, '/').split('/'): if self.IgnoreDir(dir_part): break @@ -623,8 +631,8 @@ class SourceProcessor(SourceFileProcessor): violations = 0 for file in files: try: - handle = open(file) - contents = handle.read() + handle = open(file, "rb") + contents = maybe_decode(handle.read(), "ISO-8859-1") if len(contents) > 0 and not self.ProcessContents(file, contents): success = False violations += 1