Commit 2cd3c14e authored by Matt Giuca's avatar Matt Giuca Committed by Commit Bot

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/472648Reviewed-by: 's avatarNico Weber <thakis@chromium.org>
Reviewed-by: 's avatarJochen Eisinger <jochen@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
parent 28d840eb
...@@ -312,9 +312,9 @@ def hyper_blame(ignored, filename, revision='HEAD', out=sys.stdout, ...@@ -312,9 +312,9 @@ def hyper_blame(ignored, filename, revision='HEAD', out=sys.stdout,
newline = parent_blame[lineno_previous - 1] newline = parent_blame[lineno_previous - 1]
# Replace the commit and lineno_then, but not the lineno_now or context. # 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, newline.lineno_then,
line = BlameLine(newline.commit, line.context, lineno_previous,
line.lineno_now, True) 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 # If any line has a different filename to the file's current name, turn on
# filename display for the entire blame output. # filename display for the entire blame output.
......
...@@ -498,6 +498,52 @@ class GitHyperBlameLineMotionTest(GitHyperBlameTestBase): ...@@ -498,6 +498,52 @@ class GitHyperBlameLineMotionTest(GitHyperBlameTestBase):
self.assertEqual(expected_output, output) 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__': if __name__ == '__main__':
sys.exit(coverage_utils.covered_main( sys.exit(coverage_utils.covered_main(
os.path.join(DEPOT_TOOLS_ROOT, 'git_hyper_blame.py'))) os.path.join(DEPOT_TOOLS_ROOT, 'git_hyper_blame.py')))
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment