Revert of Refactor //tools/gn/bin/gyp_flag_compare.py to be usable in interactive Python. (patchset #1 id:1 of https://codereview.chromium.org/2108683002/ )
Reason for revert:
A significant number of bots are broken after this landed. This is a speculative revert. Please re-land in case this was not the error.
Original issue's description:
> Refactor //tools/gn/bin/gyp_flag_compare.py to be usable in interactive Python.
>
> When comparing large targets like //chrome, it is nearly impossible to deal with
> the mountainous 46,000 lines of output the executable script produces. With an
> interactive environment, it is much easier to compare differences and drill-down
> into specific issues.
>
> BUG=622889
> R=scottmg@chromium.org
>
> Committed: https://chromium.googlesource.com/chromium/src/+/9f1375ea1bfb9f35e48b69f4c10c8ffa3233ca2a
TBR=scottmg@chromium.org,rsesek@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=622889
Review-Url: https://codereview.chromium.org/2104083002
Cr-Original-Commit-Position: refs/heads/master@{#402514}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 74af4d66235496370bd173c6139314e8adfecb2e
diff --git a/tools/gn/bin/gyp_flag_compare.py b/tools/gn/bin/gyp_flag_compare.py
index 7bece4d..ddd260a 100755
--- a/tools/gn/bin/gyp_flag_compare.py
+++ b/tools/gn/bin/gyp_flag_compare.py
@@ -5,33 +5,7 @@
# found in the LICENSE file.
"""Given the output of -t commands from a ninja build for a gyp and GN generated
-build, report on differences between the command lines.
-
-This script is best used interactively. Here's a recommended setup:
- $ PYTHONPATH=tools/gn/bin python
- >>> import sys
- >>> import pprint
- >>> sys.displayhook = pprint.pprint
- >>> import gyp_flag_compare as fc
- >>> fc.ConfigureBuild(['gyp_define=1', 'define=2'], ['gn_arg=1', 'arg=2'])
- >>> chrome = fc.Comparison('chrome')
-
-The above starts interactive Python, sets up the output to be pretty-printed
-(useful for making lists, dicts, and sets readable), configures the build with
-GN arguments and GYP defines, and then generates a comparison for that build
-configuration for the "chrome" target.
-
-After that, the |chrome| object can be used to investigate differences in the
-build.
-
-To configure an official build, use this configuration. Disabling NaCl produces
-a more meaningful comparison, as certain files need to get compiled twice
-for the IRT build, which uses different flags:
- >>> fc.ConfigureBuild(
- ['disable_nacl=1', 'buildtype=Official', 'branding=Chrome'],
- ['enable_nacl=false', 'is_official_build=true',
- 'is_chrome_branded=true'])
-"""
+build, report on differences between the command lines."""
import os
@@ -39,286 +13,15 @@
import subprocess
import sys
+
# Must be in src/.
os.chdir(os.path.join(os.path.dirname(__file__), '..', '..', '..'))
-_GN_OUT_DIR = 'out/gn_flags'
-_GYP_OUT_DIR = 'out_gyp_flags/Release'
+
+g_total_differences = 0
-def ConfigureBuild(gyp_args=[], gn_args=[]):
- print >> sys.stderr, 'Regenerating in %s...' % _GN_OUT_DIR
- # Currently only Release, non-component.
- Run('gn gen %s --args="is_debug=false is_component_build=false %s"' % \
- (_GN_OUT_DIR, ' '.join(gn_args)))
-
- os.environ.pop('GYP_DEFINES', None)
- # Remove environment variables required by gn but conflicting with GYP.
- # Relevant if Windows toolchain isn't provided by depot_tools.
- os.environ.pop('GYP_MSVS_OVERRIDE_PATH', None)
- os.environ.pop('WINDOWSSDKDIR', None)
-
- gyp_defines = ''
- if len(gyp_args) > 0:
- gyp_defines = '-D' + ' -D'.join(gyp_args)
-
- print >> sys.stderr, 'Regenerating in %s...' % _GYP_OUT_DIR
- Run('python build/gyp_chromium -Goutput_dir=out_gyp_flags ' + \
- '-Gconfig=Release %s' % gyp_defines)
-
-
-def Counts(dict_of_list):
- """Given a dictionary whose value are lists, returns a dictionary whose values
- are the length of the list. This can be used to summarize a dictionary.
- """
- return {k: len(v) for k, v in dict_of_list.iteritems()}
-
-
-def CountsByDirname(dict_of_list):
- """Given a list of files, returns a dict of dirname to counts in that dir."""
- r = {}
- for path in dict_of_list:
- dirname = os.path.dirname(path)
- r.setdefault(dirname, 0)
- r[dirname] += 1
- return r
-
-
-class Comparison(object):
- """A comparison of the currently-configured build for a target."""
-
- def __init__(self, gyp_target, gn_target=None):
- """Creates a comparison of a GN and GYP target. If the target names differ
- between the two build systems, then two names may be passed.
- """
- if gn_target is None:
- gn_target = gyp_target
- self._gyp_target = gyp_target
- self._gn_target = gn_target
-
- self._skipped = []
-
- self._total_diffs = 0
-
- self._missing_gyp_flags = {}
- self._missing_gn_flags = {}
-
- self._missing_gyp_files = {}
- self._missing_gn_files = {}
-
- self._CompareFiles()
-
- @property
- def gyp_files(self):
- """Returns the set of files that are in the GYP target."""
- return set(self._gyp_flags.keys())
-
- @property
- def gn_files(self):
- """Returns the set of files that are in the GN target."""
- return set(self._gn_flags.keys())
-
- @property
- def skipped(self):
- """Returns the list of compiler commands that were not processed during the
- comparison.
- """
- return self._skipped
-
- @property
- def total_differences(self):
- """Returns the total number of differences detected."""
- return self._total_diffs
-
- @property
- def missing_in_gyp(self):
- """Differences that are only in GYP build but not in GN, indexed by the
- difference."""
- return self._missing_gyp_flags
-
- @property
- def missing_in_gn(self):
- """Differences that are only in the GN build but not in GYP, indexed by
- the difference."""
- return self._missing_gn_flags
-
- @property
- def missing_in_gyp_by_file(self):
- """Differences that are only in the GYP build but not in GN, indexed by
- file.
- """
- return self._missing_gyp_files
-
- @property
- def missing_in_gn_by_file(self):
- """Differences that are only in the GYP build but not in GN, indexed by
- file.
- """
- return self._missing_gn_files
-
- def _CompareFiles(self):
- """Performs the actual target comparison."""
- if sys.platform == 'win32':
- # On Windows flags are stored in .rsp files which are created by building.
- print >> sys.stderr, 'Building in %s...' % _GN_OUT_DIR
- Run('ninja -C %s -d keeprsp %s' % (_GN_OUT_DIR, self._gn_target))
- print >> sys.stderr, 'Building in %s...' % _GYP_OUT_DIR
- Run('ninja -C %s -d keeprsp %s' % (_GYP_OUT_DIR, self._gn_target))
-
- gn = Run('ninja -C %s -t commands %s' % (_GN_OUT_DIR, self._gn_target))
- gyp = Run('ninja -C %s -t commands %s' % (_GYP_OUT_DIR, self._gyp_target))
-
- self._gn_flags = self._GetFlags(gn.splitlines(),
- os.path.join(os.getcwd(), _GN_OUT_DIR))
- self._gyp_flags = self._GetFlags(gyp.splitlines(),
- os.path.join(os.getcwd(), _GYP_OUT_DIR))
-
- all_files = sorted(self.gn_files & self.gyp_files)
- for filename in all_files:
- gyp_flags = self._gyp_flags[filename]
- gn_flags = self._gn_flags[filename]
- self._CompareLists(filename, gyp_flags, gn_flags, 'dash_f')
- self._CompareLists(filename, gyp_flags, gn_flags, 'defines')
- self._CompareLists(filename, gyp_flags, gn_flags, 'include_dirs')
- self._CompareLists(filename, gyp_flags, gn_flags, 'warnings',
- # More conservative warnings in GN we consider to be OK.
- dont_care_gyp=[
- '/wd4091', # 'keyword' : ignored on left of 'type' when no variable
- # is declared.
- '/wd4456', # Declaration hides previous local declaration.
- '/wd4457', # Declaration hides function parameter.
- '/wd4458', # Declaration hides class member.
- '/wd4459', # Declaration hides global declaration.
- '/wd4702', # Unreachable code.
- '/wd4800', # Forcing value to bool 'true' or 'false'.
- '/wd4838', # Conversion from 'type' to 'type' requires a narrowing
- # conversion.
- ] if sys.platform == 'win32' else None,
- dont_care_gn=[
- '-Wendif-labels',
- '-Wextra',
- '-Wsign-compare',
- ] if not sys.platform == 'win32' else None)
- self._CompareLists(filename, gyp_flags, gn_flags, 'other')
-
- def _CompareLists(self, filename, gyp, gn, name,
- dont_care_gyp=None, dont_care_gn=None):
- """Return a report of any differences between gyp and gn lists, ignoring
- anything in |dont_care_{gyp|gn}| respectively."""
- if gyp[name] == gn[name]:
- return
- if not dont_care_gyp:
- dont_care_gyp = []
- if not dont_care_gn:
- dont_care_gn = []
- gyp_set = set(gyp[name])
- gn_set = set(gn[name])
- missing_in_gyp = gyp_set - gn_set
- missing_in_gn = gn_set - gyp_set
- missing_in_gyp -= set(dont_care_gyp)
- missing_in_gn -= set(dont_care_gn)
-
- for m in missing_in_gyp:
- self._missing_gyp_flags.setdefault(name, {}) \
- .setdefault(m, []).append(filename)
- self._total_diffs += 1
- self._missing_gyp_files.setdefault(filename, {}) \
- .setdefault(name, set()).update(missing_in_gyp)
-
- for m in missing_in_gn:
- self._missing_gn_flags.setdefault(name, {}) \
- .setdefault(m, []).append(filename)
- self._total_diffs += 1
- self._missing_gn_files.setdefault(filename, {}) \
- .setdefault(name, set()).update(missing_in_gn)
-
- def _GetFlags(self, lines, build_dir):
- """Turn a list of command lines into a semi-structured dict."""
- is_win = sys.platform == 'win32'
- flags_by_output = {}
- for line in lines:
- command_line = shlex.split(line.strip(), posix=not is_win)[1:]
-
- output_name = _FindAndRemoveArgWithValue(command_line, '-o')
- dep_name = _FindAndRemoveArgWithValue(command_line, '-MF')
-
- command_line = _MergeSpacedArgs(command_line, '-Xclang')
-
- cc_file = [x for x in command_line if x.endswith('.cc') or
- x.endswith('.c') or
- x.endswith('.cpp') or
- x.endswith('.mm') or
- x.endswith('.m')]
- if len(cc_file) != 1:
- self._skipped.append(command_line)
- continue
- assert len(cc_file) == 1
-
- if is_win:
- rsp_file = [x for x in command_line if x.endswith('.rsp')]
- assert len(rsp_file) <= 1
- if rsp_file:
- rsp_file = os.path.join(build_dir, rsp_file[0][1:])
- with open(rsp_file, "r") as open_rsp_file:
- command_line = shlex.split(open_rsp_file, posix=False)
-
- defines = [x for x in command_line if x.startswith('-D')]
- include_dirs = [x for x in command_line if x.startswith('-I')]
- dash_f = [x for x in command_line if x.startswith('-f')]
- warnings = \
- [x for x in command_line if x.startswith('/wd' if is_win else '-W')]
- others = [x for x in command_line if x not in defines and \
- x not in include_dirs and \
- x not in dash_f and \
- x not in warnings and \
- x not in cc_file]
-
- for index, value in enumerate(include_dirs):
- if value == '-Igen':
- continue
- path = value[2:]
- if not os.path.isabs(path):
- path = os.path.join(build_dir, path)
- include_dirs[index] = '-I' + os.path.normpath(path)
-
- # GYP supports paths above the source root like <(DEPTH)/../foo while such
- # paths are unsupported by gn. But gn allows to use system-absolute paths
- # instead (paths that start with single '/'). Normalize all paths.
- cc_file = [os.path.normpath(os.path.join(build_dir, cc_file[0]))]
-
- # Filter for libFindBadConstructs.so having a relative path in one and
- # absolute path in the other.
- others_filtered = []
- for x in others:
- if x.startswith('-Xclang ') and \
- (x.endswith('libFindBadConstructs.so') or \
- x.endswith('libFindBadConstructs.dylib')):
- others_filtered.append(
- '-Xclang ' +
- os.path.join(os.getcwd(), os.path.normpath(
- os.path.join('out/gn_flags', x.split(' ', 1)[1]))))
- elif x.startswith('-B'):
- others_filtered.append(
- '-B' +
- os.path.join(os.getcwd(), os.path.normpath(
- os.path.join('out/gn_flags', x[2:]))))
- else:
- others_filtered.append(x)
- others = others_filtered
-
- flags_by_output[cc_file[0]] = {
- 'output': output_name,
- 'depname': dep_name,
- 'defines': sorted(defines),
- 'include_dirs': sorted(include_dirs), # TODO(scottmg): This is wrong.
- 'dash_f': sorted(dash_f),
- 'warnings': sorted(warnings),
- 'other': sorted(others),
- }
- return flags_by_output
-
-
-def _FindAndRemoveArgWithValue(command_line, argname):
+def FindAndRemoveArgWithValue(command_line, argname):
"""Given a command line as a list, remove and return the value of an option
that takes a value as a separate entry.
@@ -332,7 +35,7 @@
return value
-def _MergeSpacedArgs(command_line, argname):
+def MergeSpacedArgs(command_line, argname):
"""Combine all arguments |argname| with their values, separated by a space."""
i = 0
result = []
@@ -347,9 +50,141 @@
return result
+def NormalizeSymbolArguments(command_line):
+ """Normalize -g arguments.
+
+ If there's no -g args, it's equivalent to -g0. -g2 is equivalent to -g.
+ Modifies |command_line| in place.
+ """
+ # Strip -g0 if there's no symbols.
+ have_some_symbols = False
+ for x in command_line:
+ if x.startswith('-g') and x != '-g0':
+ have_some_symbols = True
+ if not have_some_symbols and '-g0' in command_line:
+ command_line.remove('-g0')
+
+ # Rename -g2 to -g.
+ if '-g2' in command_line:
+ command_line[command_line.index('-g2')] = '-g'
+
+
+def GetFlags(lines, build_dir):
+ """Turn a list of command lines into a semi-structured dict."""
+ is_win = sys.platform == 'win32'
+ flags_by_output = {}
+ for line in lines:
+ command_line = shlex.split(line.strip(), posix=not is_win)[1:]
+
+ output_name = FindAndRemoveArgWithValue(command_line, '-o')
+ dep_name = FindAndRemoveArgWithValue(command_line, '-MF')
+
+ NormalizeSymbolArguments(command_line)
+
+ command_line = MergeSpacedArgs(command_line, '-Xclang')
+
+ cc_file = [x for x in command_line if x.endswith('.cc') or
+ x.endswith('.c') or
+ x.endswith('.cpp')]
+ if len(cc_file) != 1:
+ print 'Skipping %s' % command_line
+ continue
+ assert len(cc_file) == 1
+
+ if is_win:
+ rsp_file = [x for x in command_line if x.endswith('.rsp')]
+ assert len(rsp_file) <= 1
+ if rsp_file:
+ rsp_file = os.path.join(build_dir, rsp_file[0][1:])
+ with open(rsp_file, "r") as open_rsp_file:
+ command_line = shlex.split(open_rsp_file, posix=False)
+
+ defines = [x for x in command_line if x.startswith('-D')]
+ include_dirs = [x for x in command_line if x.startswith('-I')]
+ dash_f = [x for x in command_line if x.startswith('-f')]
+ warnings = \
+ [x for x in command_line if x.startswith('/wd' if is_win else '-W')]
+ others = [x for x in command_line if x not in defines and \
+ x not in include_dirs and \
+ x not in dash_f and \
+ x not in warnings and \
+ x not in cc_file]
+
+ for index, value in enumerate(include_dirs):
+ if value == '-Igen':
+ continue
+ path = value[2:]
+ if not os.path.isabs(path):
+ path = os.path.join(build_dir, path)
+ include_dirs[index] = '-I' + os.path.normpath(path)
+
+ # GYP supports paths above the source root like <(DEPTH)/../foo while such
+ # paths are unsupported by gn. But gn allows to use system-absolute paths
+ # instead (paths that start with single '/'). Normalize all paths.
+ cc_file = [os.path.normpath(os.path.join(build_dir, cc_file[0]))]
+
+ # Filter for libFindBadConstructs.so having a relative path in one and
+ # absolute path in the other.
+ others_filtered = []
+ for x in others:
+ if x.startswith('-Xclang ') and x.endswith('libFindBadConstructs.so'):
+ others_filtered.append(
+ '-Xclang ' +
+ os.path.join(os.getcwd(),
+ os.path.normpath(
+ os.path.join('out/gn_flags', x.split(' ', 1)[1]))))
+ elif x.startswith('-B'):
+ others_filtered.append(
+ '-B' +
+ os.path.join(os.getcwd(),
+ os.path.normpath(os.path.join('out/gn_flags', x[2:]))))
+ else:
+ others_filtered.append(x)
+ others = others_filtered
+
+ flags_by_output[cc_file[0]] = {
+ 'output': output_name,
+ 'depname': dep_name,
+ 'defines': sorted(defines),
+ 'include_dirs': sorted(include_dirs), # TODO(scottmg): This is wrong.
+ 'dash_f': sorted(dash_f),
+ 'warnings': sorted(warnings),
+ 'other': sorted(others),
+ }
+ return flags_by_output
+
+
+def CompareLists(gyp, gn, name, dont_care_gyp=None, dont_care_gn=None):
+ """Return a report of any differences between gyp and gn lists, ignoring
+ anything in |dont_care_{gyp|gn}| respectively."""
+ global g_total_differences
+ if not dont_care_gyp:
+ dont_care_gyp = []
+ if not dont_care_gn:
+ dont_care_gn = []
+ output = ''
+ if gyp[name] != gn[name]:
+ gyp_set = set(gyp[name])
+ gn_set = set(gn[name])
+ missing_in_gyp = gyp_set - gn_set
+ missing_in_gn = gn_set - gyp_set
+ missing_in_gyp -= set(dont_care_gyp)
+ missing_in_gn -= set(dont_care_gn)
+ if missing_in_gyp or missing_in_gn:
+ output += ' %s differ:\n' % name
+ if missing_in_gyp:
+ output += ' In gyp, but not in GN:\n %s' % '\n '.join(
+ sorted(missing_in_gyp)) + '\n'
+ g_total_differences += len(missing_in_gyp)
+ if missing_in_gn:
+ output += ' In GN, but not in gyp:\n %s' % '\n '.join(
+ sorted(missing_in_gn)) + '\n\n'
+ g_total_differences += len(missing_in_gn)
+ return output
+
+
def Run(command_line):
"""Run |command_line| as a subprocess and return stdout. Raises on error."""
- print >> sys.stderr, command_line
return subprocess.check_output(command_line, shell=True)
@@ -359,18 +194,41 @@
print ' or: %s target' % __file__
return 1
- ConfigureBuild()
-
- gyp_target = sys.argv[1]
if len(sys.argv) == 2:
- gn_target = gyp_target
- else:
- gn_target = sys.argv[2]
- comparison = Comparison(gyp_target, gn_target)
+ sys.argv.append(sys.argv[1])
- gyp_files = comparison.gyp_files
- gn_files = comparison.gn_files
- different_source_list = comparison.gyp_files != comparison.gn_files
+ gn_out_dir = 'out/gn_flags'
+ print >> sys.stderr, 'Regenerating in %s...' % gn_out_dir
+ # Currently only Release, non-component.
+ Run('gn gen %s --args="is_debug=false is_component_build=false"' % gn_out_dir)
+ gn = Run('ninja -C %s -t commands %s' % (gn_out_dir, sys.argv[2]))
+ if sys.platform == 'win32':
+ # On Windows flags are stored in .rsp files which are created during build.
+ print >> sys.stderr, 'Building in %s...' % gn_out_dir
+ Run('ninja -C %s -d keeprsp %s' % (gn_out_dir, sys.argv[2]))
+
+ os.environ.pop('GYP_DEFINES', None)
+ # Remove environment variables required by gn but conflicting with GYP.
+ # Relevant if Windows toolchain isn't provided by depot_tools.
+ os.environ.pop('GYP_MSVS_OVERRIDE_PATH', None)
+ os.environ.pop('WINDOWSSDKDIR', None)
+
+ gyp_out_dir = 'out_gyp_flags/Release'
+ print >> sys.stderr, 'Regenerating in %s...' % gyp_out_dir
+ Run('python build/gyp_chromium -Goutput_dir=out_gyp_flags -Gconfig=Release')
+ gyp = Run('ninja -C %s -t commands %s' % (gyp_out_dir, sys.argv[1]))
+ if sys.platform == 'win32':
+ # On Windows flags are stored in .rsp files which are created during build.
+ print >> sys.stderr, 'Building in %s...' % gyp_out_dir
+ Run('ninja -C %s -d keeprsp %s' % (gyp_out_dir, sys.argv[2]))
+
+ all_gyp_flags = GetFlags(gyp.splitlines(),
+ os.path.join(os.getcwd(), gyp_out_dir))
+ all_gn_flags = GetFlags(gn.splitlines(),
+ os.path.join(os.getcwd(), gn_out_dir))
+ gyp_files = set(all_gyp_flags.keys())
+ gn_files = set(all_gn_flags.keys())
+ different_source_list = gyp_files != gn_files
if different_source_list:
print 'Different set of sources files:'
print ' In gyp, not in GN:\n %s' % '\n '.join(
@@ -378,31 +236,42 @@
print ' In GN, not in gyp:\n %s' % '\n '.join(
sorted(gn_files - gyp_files))
print '\nNote that flags will only be compared for files in both sets.\n'
-
- differing_files = set(comparison.missing_in_gn_by_file.keys()) & \
- set(comparison.missing_in_gyp_by_file.keys())
+ file_list = gyp_files & gn_files
files_with_given_differences = {}
- for filename in differing_files:
- output = ''
- missing_in_gyp = comparison.missing_in_gyp_by_file.get(filename, {})
- missing_in_gn = comparison.missing_in_gn_by_file.get(filename, {})
- difference_types = sorted(set(missing_in_gyp.keys() + missing_in_gn.keys()))
- for difference_type in difference_types:
- output += ' %s differ:\n' % difference_type
- if difference_type in missing_in_gyp:
- output += ' In gyp, but not in GN:\n %s' % '\n '.join(
- sorted(missing_in_gyp[difference_type])) + '\n'
- if difference_type in missing_in_gn:
- output += ' In GN, but not in gyp:\n %s' % '\n '.join(
- sorted(missing_in_gn[difference_type])) + '\n'
- if output:
- files_with_given_differences.setdefault(output, []).append(filename)
+ for filename in sorted(file_list):
+ gyp_flags = all_gyp_flags[filename]
+ gn_flags = all_gn_flags[filename]
+ differences = CompareLists(gyp_flags, gn_flags, 'dash_f')
+ differences += CompareLists(gyp_flags, gn_flags, 'defines')
+ differences += CompareLists(gyp_flags, gn_flags, 'include_dirs')
+ differences += CompareLists(gyp_flags, gn_flags, 'warnings',
+ # More conservative warnings in GN we consider to be OK.
+ dont_care_gyp=[
+ '/wd4091', # 'keyword' : ignored on left of 'type' when no variable
+ # is declared.
+ '/wd4456', # Declaration hides previous local declaration.
+ '/wd4457', # Declaration hides function parameter.
+ '/wd4458', # Declaration hides class member.
+ '/wd4459', # Declaration hides global declaration.
+ '/wd4702', # Unreachable code.
+ '/wd4800', # Forcing value to bool 'true' or 'false'.
+ '/wd4838', # Conversion from 'type' to 'type' requires a narrowing
+ # conversion.
+ ] if sys.platform == 'win32' else None,
+ dont_care_gn=[
+ '-Wendif-labels',
+ '-Wextra',
+ '-Wsign-compare',
+ ] if not sys.platform == 'win32' else None)
+ differences += CompareLists(gyp_flags, gn_flags, 'other')
+ if differences:
+ files_with_given_differences.setdefault(differences, []).append(filename)
for diff, files in files_with_given_differences.iteritems():
print '\n'.join(sorted(files))
print diff
- print 'Total differences:', comparison.total_differences
+ print 'Total differences:', g_total_differences
# TODO(scottmg): Return failure on difference once we're closer to identical.
return 0
