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