From 53937ba767a36b6bc271d5ab2afba5a5817af4b2 Mon Sep 17 00:00:00 2001 From: "iannucci@chromium.org" Date: Tue, 2 Oct 2012 18:20:43 +0000 Subject: [PATCH] Add option to specify similarity level for git diff operations on commandline BUG=125983 Review URL: https://chromiumcodereview.appspot.com/10945002 git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@159732 0039d316-1c4b-4281-b951-d872f2087c98 --- git_cl.py | 48 +++++++++++++++++++++++++++++++++---- tests/git_cl_test.py | 57 ++++++++++++++++++++++++++++++++++---------- 2 files changed, 88 insertions(+), 17 deletions(-) diff --git a/git_cl.py b/git_cl.py index 6c7afe48f..acd04a57a 100755 --- a/git_cl.py +++ b/git_cl.py @@ -92,6 +92,41 @@ def ask_for_data(prompt): sys.exit(1) +def add_git_similarity(parser): + parser.add_option( + '--similarity', metavar='SIM', type='int', action='store', default=None, + help='Sets the percentage that a pair of files need to match in order to' + ' be considered copies (default 50)') + + old_parser_args = parser.parse_args + def Parse(args): + options, args = old_parser_args(args) + + branch = Changelist().GetBranch() + key = 'branch.%s.git-cl-similarity' % branch + if options.similarity is None: + if branch: + (_, stdout) = RunGitWithCode(['config', '--int', '--get', key]) + try: + options.similarity = int(stdout.strip()) + except ValueError: + pass + options.similarity = options.similarity or 50 + else: + if branch: + print('Note: Saving similarity of %d%% in git config.' + % options.similarity) + RunGit(['config', '--int', key, str(options.similarity)]) + + options.similarity = max(1, min(options.similarity, 100)) + + print('Using %d%% similarity for rename/copy detection. ' + 'Override with --similarity.' % options.similarity) + + return options, args + parser.parse_args = Parse + + def MatchSvnGlob(url, base_url, glob_spec, allow_wildcards): """Return the corresponding git ref if |base_url| together with |glob_spec| matches the full |url|. @@ -134,7 +169,7 @@ def MatchSvnGlob(url, base_url, glob_spec, allow_wildcards): return None -def print_stats(args): +def print_stats(similarity, args): """Prints statistics about the change to the user.""" # --no-ext-diff is broken in some versions of Git, so try to work around # this by overriding the environment (but there is still a problem if the @@ -144,7 +179,7 @@ def print_stats(args): del env['GIT_EXTERNAL_DIFF'] return subprocess2.call( ['git', 'diff', '--no-ext-diff', '--stat', '--find-copies-harder', - '-l100000'] + args, env=env) + '-C%s' % similarity, '-l100000'] + args, env=env) class Settings(object): @@ -1037,6 +1072,8 @@ def RietveldUpload(options, args, cl): if cc: upload_args.extend(['--cc', cc]) + upload_args.extend(['--git_similarity', str(options.similarity)]) + # Include the upstream repo's URL in the change -- this is useful for # projects that have their source spread across multiple repos. remote_url = cl.GetGitBaseUrlFromConfig() @@ -1103,6 +1140,7 @@ def CMDupload(parser, args): if settings.GetIsGerrit(): parser.add_option('--target_branch', dest='target_branch', default='master', help='target branch to upload') + add_git_similarity(parser) (options, args) = parser.parse_args(args) # Print warning if the user used the -m/--message argument. This will soon @@ -1138,7 +1176,7 @@ def CMDupload(parser, args): if not options.reviewers and hook_results.reviewers: options.reviewers = hook_results.reviewers - print_stats(args) + print_stats(options.similarity, args) if settings.GetIsGerrit(): return GerritUpload(options, args, cl) return RietveldUpload(options, args, cl) @@ -1170,6 +1208,7 @@ def SendUpstream(parser, args, cmd): help="external contributor for patch (appended to " + "description and used as author for git). Should be " + "formatted as 'First Last '") + add_git_similarity(parser) (options, args) = parser.parse_args(args) cl = Changelist() @@ -1271,7 +1310,7 @@ def SendUpstream(parser, args, cmd): branches = [base_branch, cl.GetBranchRef()] if not options.force: - print_stats(branches) + print_stats(options.similarity, branches) ask_for_data('About to commit; enter to confirm.') # We want to squash all this branch's commits into one commit with the proper @@ -1320,6 +1359,7 @@ def SendUpstream(parser, args, cmd): else: # dcommit the merge branch. retcode, output = RunGitWithCode(['svn', 'dcommit', + '-C%s' % options.similarity, '--no-rebase', '--rmdir']) finally: # And then swap back to the original branch and clean up. diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index f02465db9..3ca0f2b33 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -93,16 +93,25 @@ class TestGitCl(TestCase): return result @classmethod - def _upload_calls(cls): - return cls._git_base_calls() + cls._git_upload_calls() + def _upload_calls(cls, similarity): + return cls._git_base_calls(similarity) + cls._git_upload_calls() @staticmethod - def _git_base_calls(): + def _git_base_calls(similarity): + if similarity is None: + similarity = '50' + similarity_call = ((['git', 'config', '--int', '--get', + 'branch.master.git-cl-similarity'],), '') + else: + similarity_call = ((['git', 'config', '--int', + 'branch.master.git-cl-similarity', similarity],), '') return [ ((['git', 'config', 'gerrit.host'],), ''), + ((['git', 'config', 'rietveld.server'],), 'codereview.example.com'), + ((['git', 'symbolic-ref', 'HEAD'],), 'master'), + similarity_call, ((['git', 'update-index', '--refresh', '-q'],), ''), ((['git', 'diff-index', 'HEAD'],), ''), - ((['git', 'config', 'rietveld.server'],), 'codereview.example.com'), ((['git', 'symbolic-ref', 'HEAD'],), 'master'), ((['git', 'config', 'branch.master.merge'],), 'master'), ((['git', 'config', 'branch.master.remote'],), 'origin'), @@ -115,7 +124,7 @@ class TestGitCl(TestCase): ((['git', 'log', '--pretty=format:%s%n%n%b', 'master...'],), 'foo'), ((['git', 'config', 'user.email'],), 'me@example.com'), ((['git', 'diff', '--no-ext-diff', '--stat', '--find-copies-harder', - '-l100000', 'master...'],), + '-C'+similarity, '-l100000', 'master...'],), '+dat'), ((['git', 'log', '--pretty=format:%s\n\n%b', 'master..'],), 'desc\n'), ] @@ -145,6 +154,9 @@ class TestGitCl(TestCase): 0)), ((['git', 'config', 'rietveld.server'],), 'codereview.example.com'), ((['git', 'symbolic-ref', 'HEAD'],), 'refs/heads/working'), + ((['git', 'config', '--int', '--get', + 'branch.working.git-cl-similarity'],), ''), + ((['git', 'symbolic-ref', 'HEAD'],), 'refs/heads/working'), ((['git', 'config', 'branch.working.merge'],), 'refs/heads/master'), ((['git', 'config', 'branch.working.remote'],), 'origin'), ((['git', 'rev-list', '--merges', @@ -193,7 +205,8 @@ class TestGitCl(TestCase): def _dcommit_calls_3(cls): return [ ((['git', 'diff', '--no-ext-diff', '--stat', '--find-copies-harder', - '-l100000', 'refs/remotes/origin/master', 'refs/heads/working'],), + '-C50', '-l100000', 'refs/remotes/origin/master', + 'refs/heads/working'],), (' PRESUBMIT.py | 2 +-\n' ' 1 files changed, 1 insertions(+), 1 deletions(-)\n')), (('About to commit; enter to confirm.',), None), @@ -209,13 +222,14 @@ class TestGitCl(TestCase): ((['git', 'commit', '-m', 'Issue: 12345\n\nReview URL: https://codereview.example.com/12345'],), ''), - ((['git', 'svn', 'dcommit', '--no-rebase', '--rmdir'],), (('', None), 0)), + ((['git', 'svn', 'dcommit', '-C50', '--no-rebase', '--rmdir'],), + (('', None), 0)), ((['git', 'checkout', '-q', 'working'],), ''), ((['git', 'branch', '-D', 'git-cl-commit'],), ''), ] @staticmethod - def _cmd_line(description, args): + def _cmd_line(description, args, similarity): """Returns the upload command line passed to upload.RealMain().""" return [ 'upload', '--assume_yes', '--server', @@ -223,6 +237,7 @@ class TestGitCl(TestCase): '--message', description ] + args + [ '--cc', 'joe@example.com', + '--git_similarity', similarity or '50', 'master...' ] @@ -234,7 +249,11 @@ class TestGitCl(TestCase): final_description, reviewers): """Generic reviewer test framework.""" - self.calls = self._upload_calls() + try: + similarity = upload_args[upload_args.index('--similarity')+1] + except ValueError: + similarity = None + self.calls = self._upload_calls(similarity) def RunEditor(desc, _): self.assertEquals( '# Enter a description of the change.\n' @@ -245,7 +264,8 @@ class TestGitCl(TestCase): return returned_description self.mock(git_cl.gclient_utils, 'RunEditor', RunEditor) def check_upload(args): - self.assertEquals(self._cmd_line(final_description, reviewers), args) + cmd_line = self._cmd_line(final_description, reviewers, similarity) + self.assertEquals(cmd_line, args) return 1, 2 self.mock(git_cl.upload, 'RealMain', check_upload) git_cl.main(['upload'] + upload_args) @@ -258,6 +278,14 @@ class TestGitCl(TestCase): 'desc\n\nBUG=\n', []) + def test_keep_similarity(self): + self._run_reviewer_test( + ['--similarity', '70'], + 'desc\n\nBUG=\n', + '# Blah blah comment.\ndesc\n\nBUG=\n', + 'desc\n\nBUG=\n', + []) + def test_reviewers_cmd_line(self): # Reviewer is passed as-is description = 'desc\n\nR=foo@example.com\nBUG=\n' @@ -309,7 +337,7 @@ class TestGitCl(TestCase): mock = FileMock() try: - self.calls = self._git_base_calls() + self.calls = self._git_base_calls(None) def RunEditor(desc, _): return desc self.mock(git_cl.gclient_utils, 'RunEditor', RunEditor) @@ -339,9 +367,12 @@ class TestGitCl(TestCase): def _gerrit_base_calls(): return [ ((['git', 'config', 'gerrit.host'],), 'gerrit.example.com'), + ((['git', 'config', 'rietveld.server'],), 'codereview.example.com'), + ((['git', 'symbolic-ref', 'HEAD'],), 'master'), + ((['git', 'config', '--int', '--get', + 'branch.master.git-cl-similarity'],), ''), ((['git', 'update-index', '--refresh', '-q'],), ''), ((['git', 'diff-index', 'HEAD'],), ''), - ((['git', 'config', 'rietveld.server'],), 'codereview.example.com'), ((['git', 'symbolic-ref', 'HEAD'],), 'master'), ((['git', 'config', 'branch.master.merge'],), 'master'), ((['git', 'config', 'branch.master.remote'],), 'origin'), @@ -354,7 +385,7 @@ class TestGitCl(TestCase): ((['git', 'log', '--pretty=format:%s%n%n%b', 'master...'],), 'foo'), ((['git', 'config', 'user.email'],), 'me@example.com'), ((['git', 'diff', '--no-ext-diff', '--stat', '--find-copies-harder', - '-l100000', 'master...'],), + '-C50', '-l100000', 'master...'],), '+dat'), ]