build/gen.py: Fix regen command arguments generation. The 'regen' rule embedded in the build.ninja generated by the script was missing critical arguments like --use-lto, which caused incremental rebuilds to alter the build configuration, leading to surprises (especially when measuring performance!). This fixes the issue by ensuring that any command-line argument is properly re-created in the regen's command, with proper shell quoting. + Upgrade from optparse to argparse Python module. + Remove obsolete imports Bug: None Change-Id: I8e3a995c0a9c6c46ceeb7878cd1db04674ef02cd Reviewed-on: https://gn-review.googlesource.com/c/gn/+/12640 Reviewed-by: Brett Wilson <brettw@chromium.org> Commit-Queue: David Turner <digit@google.com>
diff --git a/build/gen.py b/build/gen.py index 772d090..ae9c40d 100755 --- a/build/gen.py +++ b/build/gen.py
@@ -5,15 +5,16 @@ """Generates build.ninja that will build GN.""" -import contextlib -import errno -import optparse +import argparse import os import platform import re +import shlex import subprocess import sys -import tempfile + +# IMPORTANT: This script is also executed as python2 on +# GN's CI builders. try: # py3 from shlex import quote as shell_quote @@ -97,33 +98,82 @@ def is_zos(self): return self._platform == 'zos' +class ArgumentsList: + """Helper class to accumulate ArgumentParser argument definitions + and be able to regenerate a corresponding command-line to be + written in the generated Ninja file for the 'regen' rule. + """ + def __init__(self): + self._arguments = [] + + def add(self, *args, **kwargs): + """Add an argument definition, use as argparse.ArgumentParser.add_argument().""" + self._arguments.append((args, kwargs)) + + def add_to_parser(self, parser): + """Add all known arguments to parser.""" + for args, kwargs in self._arguments: + parser.add_argument(*args, **kwargs) + + def gen_command_line_args(self, parser_arguments): + """Generate a gen.py argument list to be embedded in a Ninja file.""" + result = [] + for args, kwargs in self._arguments: + if len(args) == 2: + long_option = args[1] + else: + long_option = args[0] + dest = kwargs.get('dest', None) + if dest is None: + assert long_option.startswith('--') + dest = long_option[2:].replace('-', '_') + + if getattr(parser_arguments, dest, None) is None: + # This was not set on the command-line so skip it. + continue + + action = kwargs.get('action', None) + if action == 'store_true': + if getattr(parser_arguments, dest): + result.append(long_option) + elif action == 'store' or action is None: + result.append('%s=%s' % (long_option, getattr(parser_arguments, dest))) + elif action == 'append': + for item in getattr(parser_arguments, dest): + result.append('%s=%s' % (long_option, item)) + else: + assert action is None, "Unsupported action " + action + return ' '.join(shell_quote(item) for item in result) + def main(argv): - parser = optparse.OptionParser(description=sys.modules[__name__].__doc__) - parser.add_option('-d', '--debug', action='store_true', + parser = argparse.ArgumentParser(description=sys.modules[__name__].__doc__) + args_list = ArgumentsList() + + args_list.add('-d', '--debug', action='store_true', help='Do a debug build. Defaults to release build.') - parser.add_option('--platform', + args_list.add('--platform', help='target platform (' + '/'.join(Platform.known_platforms()) + ')', choices=Platform.known_platforms()) - parser.add_option('--host', + args_list.add('--host', help='host platform (' + '/'.join(Platform.known_platforms()) + ')', choices=Platform.known_platforms()) - parser.add_option('--use-lto', action='store_true', + args_list.add('--use-lto', action='store_true', help='Enable the use of LTO') - parser.add_option('--use-icf', action='store_true', + args_list.add('--use-icf', action='store_true', help='Enable the use of Identical Code Folding') - parser.add_option('--no-last-commit-position', action='store_true', + args_list.add('--no-last-commit-position', action='store_true', help='Do not generate last_commit_position.h.') - parser.add_option('--out-path', + args_list.add('--out-path', help='The path to generate the build files in.') - parser.add_option('--no-strip', action='store_true', + args_list.add('--no-strip', action='store_true', help='Don\'t strip release build. Useful for profiling.') - parser.add_option('--no-static-libstdc++', action='store_true', + args_list.add('--no-static-libstdc++', action='store_true', default=False, dest='no_static_libstdcpp', help='Don\'t link libstdc++ statically') - parser.add_option('--link-lib', + args_list.add('--link-lib', action='append', metavar='LINK_LIB', default=[], @@ -134,7 +184,7 @@ 'used multiple times. Useful to link custom malloc ' + 'or cpu profiling libraries.')) if sys.platform == 'zos': - parser.add_option('--zoslib-dir', + args_list.add('--zoslib-dir', action='store', default='../third_party/zoslib', dest='zoslib_dir', @@ -143,10 +193,8 @@ 'add -I<ZOSLIB_DIR>/install/include to the compile ' + 'commands. See README.md for details.')) - options, args = parser.parse_args(argv) - - if args: - parser.error('Unrecognized command line arguments: %s.' % ', '.join(args)) + args_list.add_to_parser(parser) + options = parser.parse_args(argv) platform = Platform(options.platform) if options.host: @@ -160,7 +208,7 @@ if not options.no_last_commit_position: GenerateLastCommitPosition(host, os.path.join(out_dir, 'last_commit_position.h')) - WriteGNNinja(os.path.join(out_dir, 'build.ninja'), platform, host, options) + WriteGNNinja(os.path.join(out_dir, 'build.ninja'), platform, host, options, args_list) return 0 @@ -198,11 +246,11 @@ def WriteGenericNinja(path, static_libraries, executables, cxx, ar, ld, platform, host, options, - cflags=[], ldflags=[], libflags=[], - include_dirs=[], solibs=[]): - args = ' -d' if options.debug else '' - for link_lib in options.link_libs: - args += ' --link-lib=' + shell_quote(link_lib) + args_list, cflags=[], ldflags=[], + libflags=[], include_dirs=[], solibs=[]): + args = args_list.gen_command_line_args(options) + if args: + args = " " + args ninja_header_lines = [ 'cxx = ' + cxx, @@ -308,7 +356,7 @@ os.path.relpath(template_filename, os.path.dirname(path)) + '\n') -def WriteGNNinja(path, platform, host, options): +def WriteGNNinja(path, platform, host, options, args_list): if platform.is_msvc(): cxx = os.environ.get('CXX', 'cl.exe') ld = os.environ.get('LD', 'link.exe') @@ -830,9 +878,8 @@ executables['gn_unittests']['libs'].extend(static_libraries.keys()) WriteGenericNinja(path, static_libraries, executables, cxx, ar, ld, - platform, host, options, cflags, ldflags, - libflags, include_dirs, libs) - + platform, host, options, args_list, + cflags, ldflags, libflags, include_dirs, libs) if __name__ == '__main__': sys.exit(main(sys.argv[1:]))