From 2cd3c14ef72a306bfacbb0dc961d61c238a85244 Mon Sep 17 00:00:00 2001 From: Matt Giuca Date: Mon, 10 Apr 2017 17:31:44 +1000 Subject: [PATCH] git_hyper_blame: Fix wrong line number on lines changed many times. This could, in extreme cases, result in a crash due to the wrong line number being out of bounds of the file line count. BUG=709831 Change-Id: I08ec75362d49c4a72e7ee9fd489d5f9baa6d31bd Reviewed-on: https://chromium-review.googlesource.com/472648 Reviewed-by: Nico Weber Reviewed-by: Jochen Eisinger Commit-Queue: Nico Weber --- git_hyper_blame.py | 4 +-- tests/git_hyper_blame_test.py | 46 +++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/git_hyper_blame.py b/git_hyper_blame.py index 8045683590..246ccb21d4 100755 --- a/git_hyper_blame.py +++ b/git_hyper_blame.py @@ -312,9 +312,9 @@ def hyper_blame(ignored, filename, revision='HEAD', out=sys.stdout, newline = parent_blame[lineno_previous - 1] # Replace the commit and lineno_then, but not the lineno_now or context. - logging.debug(' replacing with %r', newline) - line = BlameLine(newline.commit, line.context, lineno_previous, + line = BlameLine(newline.commit, line.context, newline.lineno_then, line.lineno_now, True) + logging.debug(' replacing with %r', line) # If any line has a different filename to the file's current name, turn on # filename display for the entire blame output. diff --git a/tests/git_hyper_blame_test.py b/tests/git_hyper_blame_test.py index f690e567a3..e646edcbb3 100755 --- a/tests/git_hyper_blame_test.py +++ b/tests/git_hyper_blame_test.py @@ -498,6 +498,52 @@ class GitHyperBlameLineMotionTest(GitHyperBlameTestBase): self.assertEqual(expected_output, output) +class GitHyperBlameLineNumberTest(GitHyperBlameTestBase): + REPO_SCHEMA = """ + A B C D + """ + + COMMIT_A = { + 'file': {'data': 'red\nblue\n'}, + } + + # Change "blue" to "green". + COMMIT_B = { + 'file': {'data': 'red\ngreen\n'}, + } + + # Insert 2 lines at the top, + COMMIT_C = { + 'file': {'data': '\n\nred\ngreen\n'}, + } + + # Change "green" to "yellow". + COMMIT_D = { + 'file': {'data': '\n\nred\nyellow\n'}, + } + + def testTwoChangesWithAddedLines(self): + """Regression test for https://crbug.com/709831. + + Tests a line with multiple ignored edits, and a line number change in + between (such that the line number in the current revision is bigger than + the file's line count at the older ignored revision). + """ + expected_output = [self.blame_line('C', ' 1) '), + self.blame_line('C', ' 2) '), + self.blame_line('A', ' 3) red'), + self.blame_line('A', '4*) yellow'), + ] + # Due to https://crbug.com/709831, ignoring both B and D would crash, + # because of C (in between those revisions) which moves Line 2 to Line 4. + # The algorithm would incorrectly think that Line 4 was still on Line 4 in + # Commit B, even though it was Line 2 at that time. Its index is out of + # range in the number of lines in Commit B. + retval, output = self.run_hyperblame(['B', 'D'], 'file', 'tag_D') + self.assertEqual(0, retval) + self.assertEqual(expected_output, output) + + if __name__ == '__main__': sys.exit(coverage_utils.covered_main( os.path.join(DEPOT_TOOLS_ROOT, 'git_hyper_blame.py')))