From 7c3fbe3af69e2cc9d3f806e5f97f8512fc4bc76e Mon Sep 17 00:00:00 2001 From: Brian Clifton Date: Fri, 29 Oct 2021 12:07:39 -0700 Subject: [PATCH] Fixing pylint warnings --- build/android/gyp/bytecode_processor.py | 81 ++++++----- build/commands/scripts/format.py | 40 +++--- build/commands/scripts/lint.py | 178 ++++++++++++------------ build/mac/build_delta_installer.py | 94 ++++++------- build/mac/notarize_dmg_pkg.py | 9 +- installer/mac/copy_x64_binary.py | 9 +- script/apply-patches.py | 4 +- script/build-bisect.py | 6 - 8 files changed, 206 insertions(+), 215 deletions(-) diff --git a/build/android/gyp/bytecode_processor.py b/build/android/gyp/bytecode_processor.py index ed09b3e889b..bba8605070d 100755 --- a/build/android/gyp/bytecode_processor.py +++ b/build/android/gyp/bytecode_processor.py @@ -6,7 +6,6 @@ """Wraps bin/helper/java_bytecode_rewriter and expands @FileArgs.""" import argparse -import os import subprocess import sys @@ -15,54 +14,54 @@ import build_utils def _AddSwitch(parser, val): - parser.add_argument( - val, action='store_const', default='--disabled', const=val) + parser.add_argument( + val, action='store_const', default='--disabled', const=val) def main(argv): - argv = build_utils.ExpandFileArgs(argv[1:]) - parser = argparse.ArgumentParser() - parser.add_argument('--script', required=True, - help='Path to the java binary wrapper script.') - parser.add_argument('--input-jar', required=True) - parser.add_argument('--output-jar', required=True) - parser.add_argument('--direct-classpath-jars', required=True) - parser.add_argument('--sdk-classpath-jars', required=True) - parser.add_argument('--extra-classpath-jars', dest='extra_jars', - action='append', default=[], - help='Extra inputs, passed last to the binary script.') - parser.add_argument('-v', '--verbose', action='store_true') - parser.add_argument('--missing-classes-allowlist') - _AddSwitch(parser, '--is-prebuilt') - _AddSwitch(parser, '--enable-thread-annotations') - _AddSwitch(parser, '--enable-check-class-path') - args = parser.parse_args(argv) + argv = build_utils.ExpandFileArgs(argv[1:]) + parser = argparse.ArgumentParser() + parser.add_argument('--script', required=True, + help='Path to the java binary wrapper script.') + parser.add_argument('--input-jar', required=True) + parser.add_argument('--output-jar', required=True) + parser.add_argument('--direct-classpath-jars', required=True) + parser.add_argument('--sdk-classpath-jars', required=True) + parser.add_argument('--extra-classpath-jars', dest='extra_jars', + action='append', default=[], + help='Extra inputs, passed last to the binary script.') + parser.add_argument('-v', '--verbose', action='store_true') + parser.add_argument('--missing-classes-allowlist') + _AddSwitch(parser, '--is-prebuilt') + _AddSwitch(parser, '--enable-thread-annotations') + _AddSwitch(parser, '--enable-check-class-path') + args = parser.parse_args(argv) - sdk_jars = build_utils.ParseGnList(args.sdk_classpath_jars) - assert len(sdk_jars) > 0 + sdk_jars = build_utils.ParseGnList(args.sdk_classpath_jars) + assert len(sdk_jars) > 0 - direct_jars = build_utils.ParseGnList(args.direct_classpath_jars) - assert len(direct_jars) > 0 + direct_jars = build_utils.ParseGnList(args.direct_classpath_jars) + assert len(direct_jars) > 0 - extra_classpath_jars = [] - for a in args.extra_jars: - extra_classpath_jars.extend(build_utils.ParseGnList(a)) - args.missing_classes_allowlist = build_utils.ParseGnList( - args.missing_classes_allowlist) + extra_classpath_jars = [] + for a in args.extra_jars: + extra_classpath_jars.extend(build_utils.ParseGnList(a)) + args.missing_classes_allowlist = build_utils.ParseGnList( + args.missing_classes_allowlist) - if args.verbose: - verbose = '--verbose' - else: - verbose = '--not-verbose' + if args.verbose: + verbose = '--verbose' + else: + verbose = '--not-verbose' - cmd = ([ - args.script, args.input_jar, args.output_jar, verbose, args.is_prebuilt, - args.enable_thread_annotations, args.enable_check_class_path - ] + [str(len(args.missing_classes_allowlist))] + - args.missing_classes_allowlist + [str(len(sdk_jars))] + sdk_jars + - [str(len(direct_jars))] + direct_jars + extra_classpath_jars) - subprocess.check_call(cmd) + cmd = ([ + args.script, args.input_jar, args.output_jar, verbose, args.is_prebuilt, + args.enable_thread_annotations, args.enable_check_class_path + ] + [str(len(args.missing_classes_allowlist))] + + args.missing_classes_allowlist + [str(len(sdk_jars))] + sdk_jars + + [str(len(direct_jars))] + direct_jars + extra_classpath_jars) + subprocess.check_call(cmd) if __name__ == '__main__': - sys.exit(main(sys.argv)) + sys.exit(main(sys.argv)) diff --git a/build/commands/scripts/format.py b/build/commands/scripts/format.py index 197a3b03b06..376bd4beabf 100644 --- a/build/commands/scripts/format.py +++ b/build/commands/scripts/format.py @@ -17,27 +17,27 @@ import sys import git_cl def main(args): - """Runs clang-format and gn format on the current changelist.""" - parser = git_cl.OptionParser() - parser.parse_args([]) + """Runs clang-format and gn format on the current changelist.""" + parser = git_cl.OptionParser() + parser.parse_args([]) - # Change the current working directory before calling so that it - # shows the correct base. - settings = git_cl.settings - previous_cwd = os.getcwd() - os.chdir(settings.GetRoot()) - try: - cmd = ['cl', 'format'] + args - print('git ' + ' '.join(cmd)) - git_cl.RunGit(cmd) - except: - e = sys.exc_info()[1] - print('Could not run format: %s' % e.message) - return 1 - finally: - os.chdir(previous_cwd) - print('Formatting done.') - return 0 + # Change the current working directory before calling so that it + # shows the correct base. + settings = git_cl.settings + previous_cwd = os.getcwd() + os.chdir(settings.GetRoot()) + try: + cmd = ['cl', 'format'] + args + print('git ' + ' '.join(cmd)) + git_cl.RunGit(cmd) + except: + e = sys.exc_info()[1] + print('Could not run format: %s' % e.message) + return 1 + finally: + os.chdir(previous_cwd) + print('Formatting done.') + return 0 if __name__ == '__main__': sys.exit(main(sys.argv[1:])) diff --git a/build/commands/scripts/lint.py b/build/commands/scripts/lint.py index 01ef2253a70..48f8ad6cb5a 100644 --- a/build/commands/scripts/lint.py +++ b/build/commands/scripts/lint.py @@ -21,108 +21,108 @@ import git_cl import git_common def HasFormatErrors(): - # For more options, see vendor/depot_tools/git_cl.py - cmd = ['cl', 'format', '--diff'] - diff = git_cl.RunGit(cmd).encode('utf-8') - if diff: - # Verify that git cl format generates a diff - if git_common.is_dirty_git_tree('git cl format'): - # Skip verification if there are uncommitted changes - print(diff) - print('Format errors detected. Run npm format locally to fix.') - return True - git_cl.RunGit(['cl', 'format']) - git_diff = git_common.run('diff').encode('utf-8') - if git_diff: - print(git_diff) - print('Format errors have been auto-fixed. Please review and commit these' - ' changes if lint was run locally. Otherwise run npm format to fix.' - ) - return True - return False + # For more options, see vendor/depot_tools/git_cl.py + cmd = ['cl', 'format', '--diff'] + diff = git_cl.RunGit(cmd).encode('utf-8') + if diff: + # Verify that git cl format generates a diff + if git_common.is_dirty_git_tree('git cl format'): + # Skip verification if there are uncommitted changes + print(diff) + print('Format errors detected. Run npm format locally to fix.') + return True + git_cl.RunGit(['cl', 'format']) + git_diff = git_common.run('diff').encode('utf-8') + if git_diff: + print(git_diff) + print('Format errors have been auto-fixed. Please review and commit these' + ' changes if lint was run locally. Otherwise run npm format to fix.' + ) + return True + return False def RunFormatCheck(upstream_branch): # pylint: disable=inconsistent-return-statements - upstream_commit = git_cl.RunGit(['merge-base', 'HEAD', upstream_branch]) - print('Running git cl/gn format on the diff from %s...' % upstream_commit) - try: - if HasFormatErrors(): - return 'Format check failed.' - except Exception: - e = traceback.format_exc() - return 'Error running format check:\n' + e + upstream_commit = git_cl.RunGit(['merge-base', 'HEAD', upstream_branch]) + print('Running git cl/gn format on the diff from %s...' % upstream_commit) + try: + if HasFormatErrors(): + return 'Format check failed.' + except Exception: + e = traceback.format_exc() + return 'Error running format check:\n' + e def main(args): - """Runs cpplint on the current changelist.""" - # Adapted from git_cl.py CMDlint - parser = git_cl.OptionParser() - parser.add_option('--filter', action='append', metavar='-x,+y', - help='Comma-separated list of cpplint\'s category-filters') - parser.add_option('--project_root') - parser.add_option('--base_branch') - options, args = parser.parse_args(args) + """Runs cpplint on the current changelist.""" + # Adapted from git_cl.py CMDlint + parser = git_cl.OptionParser() + parser.add_option('--filter', action='append', metavar='-x,+y', + help='Comma-separated list of cpplint\'s category-filters') + parser.add_option('--project_root') + parser.add_option('--base_branch') + options, args = parser.parse_args(args) - # Access to a protected member _XX of a client class - # pylint: disable=protected-access - try: - import cpplint # pylint: disable=import-outside-toplevel,syntax-error - import cpplint_chromium # pylint: disable=import-outside-toplevel - except ImportError: - print('Your depot_tools is missing cpplint.py and/or cpplint_chromium.py.') - return 1 + # Access to a protected member _XX of a client class + # pylint: disable=protected-access + try: + import cpplint # pylint: disable=import-outside-toplevel,syntax-error + import cpplint_chromium # pylint: disable=import-outside-toplevel + except ImportError: + print('Your depot_tools is missing cpplint.py and/or cpplint_chromium.py.') + return 1 - # Change the current working directory before calling lint so that it - # shows the correct base. - settings = git_cl.settings - previous_cwd = os.getcwd() - os.chdir(settings.GetRoot()) - cl = git_cl.Changelist() - base_branch = options.base_branch + # Change the current working directory before calling lint so that it + # shows the correct base. + settings = git_cl.settings + previous_cwd = os.getcwd() + os.chdir(settings.GetRoot()) + cl = git_cl.Changelist() + base_branch = options.base_branch - try: - print('Running cpplint...') - files = cl.GetAffectedFiles( - git_common.get_or_create_merge_base(cl.GetBranch(), base_branch)) - if not files: - print('Cannot lint an empty CL') - return 0 + try: + print('Running cpplint...') + files = cl.GetAffectedFiles( + git_common.get_or_create_merge_base(cl.GetBranch(), base_branch)) + if not files: + print('Cannot lint an empty CL') + return 0 - # Process cpplints arguments if any. - command = args + files - if options.filter: - command = ['--filter=' + ','.join(options.filter)] + command - if options.project_root: - command = ['--project_root=' + - options.project_root.replace('\\', '/')] + command - filenames = cpplint.ParseArguments(command) + # Process cpplints arguments if any. + command = args + files + if options.filter: + command = ['--filter=' + ','.join(options.filter)] + command + if options.project_root: + command = ['--project_root=' + + options.project_root.replace('\\', '/')] + command + filenames = cpplint.ParseArguments(command) - white_regex = re.compile(settings.GetLintRegex()) - black_regex = re.compile(settings.GetLintIgnoreRegex()) - extra_check_functions = [cpplint_chromium.CheckPointerDeclarationWhitespace] - for filename in filenames: - if white_regex.match(filename): - if black_regex.match(filename): - print('Ignoring file %s' % filename) - else: - cpplint.ProcessFile(filename, cpplint._cpplint_state.verbose_level, - extra_check_functions) - else: - print('Skipping file %s' % filename) + white_regex = re.compile(settings.GetLintRegex()) + black_regex = re.compile(settings.GetLintIgnoreRegex()) + extra_check_functions = [cpplint_chromium.CheckPointerDeclarationWhitespace] + for filename in filenames: + if white_regex.match(filename): + if black_regex.match(filename): + print('Ignoring file %s' % filename) + else: + cpplint.ProcessFile(filename, cpplint._cpplint_state.verbose_level, + extra_check_functions) + else: + print('Skipping file %s' % filename) - # Run format checks - format_output = RunFormatCheck(base_branch or cl.GetUpstreamBranch()) - finally: - os.chdir(previous_cwd) + # Run format checks + format_output = RunFormatCheck(base_branch or cl.GetUpstreamBranch()) + finally: + os.chdir(previous_cwd) - if format_output: - print(format_output) - return 1 - if cpplint._cpplint_state.error_count != 0: - print('cpplint errors found: %d\n' % cpplint._cpplint_state.error_count) - return 1 + if format_output: + print(format_output) + return 1 + if cpplint._cpplint_state.error_count != 0: + print('cpplint errors found: %d\n' % cpplint._cpplint_state.error_count) + return 1 - print('lint and format checks succeeded') - return 0 + print('lint and format checks succeeded') + return 0 if __name__ == '__main__': diff --git a/build/mac/build_delta_installer.py b/build/mac/build_delta_installer.py index 3d5d3c7a6cf..c9e107708d1 100644 --- a/build/mac/build_delta_installer.py +++ b/build/mac/build_delta_installer.py @@ -12,66 +12,66 @@ import sys def unmount(volume_path): - print('-> unmount ' + volume_path) - command = ['hdiutil', 'detach', volume_path] - try: - subprocess.check_call(command) - except subprocess.CalledProcessError as e: - print(e.output) - raise e + print('-> unmount ' + volume_path) + command = ['hdiutil', 'detach', volume_path] + try: + subprocess.check_call(command) + except subprocess.CalledProcessError as e: + print(e.output) + raise e def mount_dmg(dmg_path, mount_point): - print('-> mounting ' + dmg_path + ' to ' + mount_point) + print('-> mounting ' + dmg_path + ' to ' + mount_point) - command = ['hdiutil', 'attach', dmg_path, '-mountpoint', mount_point] - try: - output = subprocess.check_output(command) - except subprocess.CalledProcessError as e: - print(e.output) - raise e + command = ['hdiutil', 'attach', dmg_path, '-mountpoint', mount_point] + try: + output = subprocess.check_output(command) + except subprocess.CalledProcessError as e: + print(e.output) + raise e def Main(argv): - parser = optparse.OptionParser('%prog [options]') - parser.add_option('--binary-delta', dest='binary_delta_path', action='store', - type='string', default=None, help='The path of BinaryDelta binary.') - parser.add_option('--root-out-dir', dest='root_out_dir_path', action='store', - type='string', default=None, help='The path of root output dir.') - parser.add_option('--old-dmg', dest='old_dmg_path', action='store', - type='string', default=None, help='The path of old dmg.') - parser.add_option('--new-dmg', dest='new_dmg_path', action='store', - type='string', default=None, help='The path of new dmg.') - parser.add_option('--delta-output', dest='delta_output_path', action='store', - type='string', default=None, help='The path of generated delta file.') - (options, args) = parser.parse_args(argv) + parser = optparse.OptionParser('%prog [options]') + parser.add_option('--binary-delta', dest='binary_delta_path', action='store', + type='string', default=None, help='The path of BinaryDelta binary.') + parser.add_option('--root-out-dir', dest='root_out_dir_path', action='store', + type='string', default=None, help='The path of root output dir.') + parser.add_option('--old-dmg', dest='old_dmg_path', action='store', + type='string', default=None, help='The path of old dmg.') + parser.add_option('--new-dmg', dest='new_dmg_path', action='store', + type='string', default=None, help='The path of new dmg.') + parser.add_option('--delta-output', dest='delta_output_path', action='store', + type='string', default=None, help='The path of generated delta file.') + (options, args) = parser.parse_args(argv) - if len(args) > 0: - print >> sys.stderr, parser.get_usage() - return 1 + if len(args) > 0: + print >> sys.stderr, parser.get_usage() + return 1 - old_dmg_mount_point = os.path.join(options.root_out_dir_path, 'old_dmg_mount_for_delta') - mount_dmg(options.old_dmg_path, old_dmg_mount_point) - old_app_path = os.path.join(old_dmg_mount_point, os.path.splitext(os.path.basename(options.old_dmg_path))[0] + '.app') + old_dmg_mount_point = os.path.join(options.root_out_dir_path, 'old_dmg_mount_for_delta') + mount_dmg(options.old_dmg_path, old_dmg_mount_point) + old_app_path = os.path.join(old_dmg_mount_point, os.path.splitext(os.path.basename(options.old_dmg_path))[0] + '.app') - new_dmg_mount_point = os.path.join(options.root_out_dir_path, 'new_dmg_mount_for_delta') - mount_dmg(options.new_dmg_path, new_dmg_mount_point) - new_app_path = os.path.join(new_dmg_mount_point, os.path.splitext(os.path.basename(options.new_dmg_path))[0] + '.app') + new_dmg_mount_point = os.path.join(options.root_out_dir_path, 'new_dmg_mount_for_delta') + mount_dmg(options.new_dmg_path, new_dmg_mount_point) + new_app_path = os.path.join(new_dmg_mount_point, os.path.splitext(os.path.basename(options.new_dmg_path))[0] + '.app') - # generate delta file - print('-> generate delta file from ' + old_app_path + ' and ' + new_app_path) - command = [options.binary_delta_path, 'create', old_app_path, new_app_path, options.delta_output_path] - try: - subprocess.check_call(command) - except subprocess.CalledProcessError as e: - print(e.output) - raise e + # generate delta file + print('-> generate delta file from ' + old_app_path + ' and ' + new_app_path) + command = [options.binary_delta_path, 'create', old_app_path, new_app_path, options.delta_output_path] + try: + subprocess.check_call(command) + except subprocess.CalledProcessError as e: + print(e.output) + raise e - unmount(old_dmg_mount_point) - unmount(new_dmg_mount_point) + unmount(old_dmg_mount_point) + unmount(new_dmg_mount_point) - return 0 + return 0 if __name__ == '__main__': - sys.exit(Main(sys.argv[1:])) + sys.exit(Main(sys.argv[1:])) diff --git a/build/mac/notarize_dmg_pkg.py b/build/mac/notarize_dmg_pkg.py index 050b318af4b..dcf293e86c7 100755 --- a/build/mac/notarize_dmg_pkg.py +++ b/build/mac/notarize_dmg_pkg.py @@ -10,6 +10,10 @@ import os import subprocess import sys +# Import the entire module to avoid circular dependencies in the functions +from signing import chromium_config, commands, model, notarize # noqa: E402 +from signing_helper import GetBraveSigningConfig + """ The sign_chrome.py script does notarization of the app, but we don't use it to create our .dmg and .pkg files. Instead we do our own signing and packaging, @@ -23,10 +27,6 @@ performs notarizing and stapling of those files. packaging_signing_path = os.path.realpath(os.path.dirname(os.path.realpath(__file__))) sys.path.append(packaging_signing_path) -# Import the entire module to avoid circular dependencies in the functions -from signing import chromium_config, config, commands, model, notarize, pipeline, signing # noqa: E402 -from signing_helper import GetBraveSigningConfig - def run_command(args, **kwargs): print('Running command: {}'.format(args)) subprocess.check_call(args, **kwargs) @@ -147,5 +147,4 @@ def parse_args(): if __name__ == '__main__': - import sys sys.exit(main()) diff --git a/installer/mac/copy_x64_binary.py b/installer/mac/copy_x64_binary.py index bbd283c7e1f..2055c6155f8 100644 --- a/installer/mac/copy_x64_binary.py +++ b/installer/mac/copy_x64_binary.py @@ -10,7 +10,6 @@ import argparse import os import shutil import sys -import re def copy_x64(x64_src_path, x64_dest_path): @@ -19,12 +18,12 @@ def copy_x64(x64_src_path, x64_dest_path): if not os.path.exists(x64_src_path): raise Exception('Could not find x64 app (%s)' % x64_src_path) - if (os.path.exists(x64_dest_path)): + if os.path.exists(x64_dest_path): shutil.rmtree(x64_dest_path) # TODO check why brave_resources.pak differs between x64 and arm64 shutil.copytree(x64_src_path, x64_dest_path, symlinks=True, - ignore=shutil.ignore_patterns('Sparkle.framework', '*.pak')) + ignore=shutil.ignore_patterns('Sparkle.framework', '*.pak')) # remove conflicting files os.remove(os.path.join(x64_dest_path, 'Contents', 'Info.plist')) @@ -33,9 +32,9 @@ def main(args): parser = argparse.ArgumentParser( description='Copy macos x64 binary to arm64 for universalize') parser.add_argument('x64_src_path', - help='Root output dir for arm64 build') + help='Root output dir for arm64 build') parser.add_argument('x64_dest_path', - help='The location to copy the x64 binary to in root_out_dir.') + help='The location to copy the x64 binary to in root_out_dir.') parsed = parser.parse_args(args) copy_x64(parsed.x64_src_path, parsed.x64_dest_path) diff --git a/script/apply-patches.py b/script/apply-patches.py index 62cb572177f..35c87c86377 100755 --- a/script/apply-patches.py +++ b/script/apply-patches.py @@ -24,7 +24,7 @@ def main(): def apply_patches_for_dir(directory): - for root, dirs, files in os.walk(directory): + for root, files in os.walk(directory): prefix = os.path.relpath(root, directory) target = os.path.join(SRC_DIR, prefix) args = [sys.executable, PATCH_PY, '--directory', target, '--quiet'] @@ -37,7 +37,7 @@ def apply_patches_for_dir(directory): def revert_changes_for_dir(directory): - for root, dirs, files in reversed(list(os.walk(directory))): + for root, files in reversed(list(os.walk(directory))): prefix = os.path.relpath(root, directory) target = os.path.join(SRC_DIR, prefix) args = [sys.executable, PATCH_PY, '--directory', target, '--quiet', diff --git a/script/build-bisect.py b/script/build-bisect.py index ea483875729..9317a32f237 100755 --- a/script/build-bisect.py +++ b/script/build-bisect.py @@ -4,17 +4,11 @@ # You can obtain one at http://mozilla.org/MPL/2.0/. import argparse -import errno -import hashlib import math import os import platform -import requests import re -import shutil -import subprocess import sys -import json from io import StringIO from lib.config import get_env_var