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:]))
