diff options
| -rwxr-xr-x | tools/git/mass-rename.py | 3 | ||||
| -rwxr-xr-x | tools/git/move_source_file.py | 34 | ||||
| -rwxr-xr-x | tools/sort-headers.py | 43 |
3 files changed, 64 insertions, 16 deletions
diff --git a/tools/git/mass-rename.py b/tools/git/mass-rename.py index 89171e5..8efec09 100755 --- a/tools/git/mass-rename.py +++ b/tools/git/mass-rename.py @@ -34,7 +34,8 @@ def main(): subprocess.check_call([ sys.executable, os.path.join(BASE_DIR, 'move_source_file.py'), - '--already-moved', + '--already_moved', + '--no_error_for_non_source_file', fro, to]) else: print "Skipping: %s -- not a rename?" % fro diff --git a/tools/git/move_source_file.py b/tools/git/move_source_file.py index eacdccd..1a953f3 100755 --- a/tools/git/move_source_file.py +++ b/tools/git/move_source_file.py @@ -19,6 +19,7 @@ find files that reference the moved file. """ +import optparse import os import re import subprocess @@ -36,6 +37,9 @@ sort_headers = __import__('sort-headers') HANDLED_EXTENSIONS = ['.cc', '.mm', '.h', '.hh'] +def IsHandledFile(path): + return os.path.splitext(path)[1] in HANDLED_EXTENSIONS + def MakeDestinationPath(from_path, to_path): """Given the from and to paths, return a correct destination path. @@ -43,7 +47,7 @@ def MakeDestinationPath(from_path, to_path): in which case the path must end with /. Also does basic sanity checks. """ - if os.path.splitext(from_path)[1] not in HANDLED_EXTENSIONS: + if not IsHandledFile(from_path): raise Exception('Only intended to move individual source files. (%s)' % from_path) dest_extension = os.path.splitext(to_path)[1] @@ -84,7 +88,7 @@ def UpdatePostMove(from_path, to_path): # Reorder headers in files that changed. for changed_file in files_with_changed_includes: def AlwaysConfirm(a, b): return True - sort_headers.FixFileWithConfirmFunction(changed_file, AlwaysConfirm) + sort_headers.FixFileWithConfirmFunction(changed_file, AlwaysConfirm, True) # Update comments; only supports // comments, which are primarily # used in our code. @@ -149,25 +153,35 @@ def main(): if not os.path.isdir('.git'): print 'Fatal: You must run from the root of a git checkout.' return 1 - args = sys.argv[1:] - already_moved = False - if len(args) > 0 and args[0] == '--already-moved': - args = args[1:] - already_moved = True + parser = optparse.OptionParser(usage='%prog FROM_PATH... TO_PATH') + parser.add_option('--already_moved', action='store_true', + dest='already_moved', + help='Causes the script to skip moving the file.') + parser.add_option('--no_error_for_non_source_file', action='store_false', + default='True', + dest='error_for_non_source_file', + help='Causes the script to simply print a warning on ' + 'encountering a non-source file rather than raising an ' + 'error.') + opts, args = parser.parse_args() if len(args) < 2: - print ('Usage: move_source_file.py [--already-moved] FROM_PATH... TO_PATH' - '\n\n%s' % __doc__) + parser.print_help() return 1 if len(args) > 2 and not args[-1].endswith('/'): print 'Target %s is not a directory.' % args[-1] + print + parser.print_help() return 1 for from_path in args[:len(args)-1]: + if not opts.error_for_non_source_file and not IsHandledFile(from_path): + print '%s does not appear to be a source file, skipping' % (from_path) + continue to_path = MakeDestinationPath(from_path, args[-1]) - if not already_moved: + if not opts.already_moved: MoveFile(from_path, to_path) UpdatePostMove(from_path, to_path) return 0 diff --git a/tools/sort-headers.py b/tools/sort-headers.py index fb70678..42a8be8 100755 --- a/tools/sort-headers.py +++ b/tools/sort-headers.py @@ -77,23 +77,43 @@ def SortHeader(infile, outfile): if IsInclude(line): headerblock = [] while IsInclude(line): + infile_ended_on_include_line = False headerblock.append(line) - line = infile.next() + # Ensure we don't die due to trying to read beyond the end of the file. + try: + line = infile.next() + except StopIteration: + infile_ended_on_include_line = True + break for header in sorted(headerblock, key=IncludeCompareKey): outfile.write(header) + if infile_ended_on_include_line: + # We already wrote the last line above; exit to ensure it isn't written + # again. + return # Intentionally fall through, to write the line that caused # the above while loop to exit. outfile.write(line) -def FixFileWithConfirmFunction(filename, confirm_function): +def FixFileWithConfirmFunction(filename, confirm_function, + perform_safety_checks): """Creates a fixed version of the file, invokes |confirm_function| to decide whether to use the new file, and cleans up. |confirm_function| takes two parameters, the original filename and the fixed-up filename, and returns True to use the fixed-up file, false to not use it. + + If |perform_safety_checks| is True, then the function checks whether it is + unsafe to reorder headers in this file and skips the reorder with a warning + message in that case. """ + if perform_safety_checks and IsUnsafeToReorderHeaders(filename): + print ('Not reordering headers in %s as the script thinks that the ' + 'order of headers in this file is semantically significant.' + % (filename)) + return fixfilename = filename + '.new' infile = open(filename, 'rb') outfile = open(fixfilename, 'wb') @@ -114,7 +134,7 @@ def FixFileWithConfirmFunction(filename, confirm_function): pass -def DiffAndConfirm(filename, should_confirm): +def DiffAndConfirm(filename, should_confirm, perform_safety_checks): """Shows a diff of what the tool would change the file named filename to. Shows a confirmation prompt if should_confirm is true. Saves the resulting file if should_confirm is false or the user @@ -130,14 +150,27 @@ def DiffAndConfirm(filename, should_confirm): return (not should_confirm or YesNo('Use new file (y/N)?')) - FixFileWithConfirmFunction(filename, ConfirmFunction) + FixFileWithConfirmFunction(filename, ConfirmFunction, perform_safety_checks) +def IsUnsafeToReorderHeaders(filename): + # *_message_generator.cc is almost certainly a file that generates IPC + # definitions. Changes in include order in these files can result in them not + # building correctly. + if filename.find("message_generator.cc") != -1: + return True + return False def main(): parser = optparse.OptionParser(usage='%prog filename1 filename2 ...') parser.add_option('-f', '--force', action='store_false', default=True, dest='should_confirm', help='Turn off confirmation prompt.') + parser.add_option('--no_safety_checks', + action='store_false', default=True, + dest='perform_safety_checks', + help='Do not perform the safety checks via which this ' + 'script refuses to operate on files for which it thinks ' + 'the include ordering is semantically significant.') opts, filenames = parser.parse_args() if len(filenames) < 1: @@ -145,7 +178,7 @@ def main(): return 1 for filename in filenames: - DiffAndConfirm(filename, opts.should_confirm) + DiffAndConfirm(filename, opts.should_confirm, opts.perform_safety_checks) if __name__ == '__main__': |