From 17fa4beec4a1170e645409d79235299dbe342ab4 Mon Sep 17 00:00:00 2001 From: "maruel@chromium.org" Date: Wed, 29 Aug 2012 17:18:12 +0000 Subject: [PATCH] Fix hunk handling for 'default hunk header values'. Add new test cases to verify it properly handles "-1 +1, 2" and "-1 +1" for short files. R=petermayo@chromium.org BUG= Review URL: https://chromiumcodereview.appspot.com/10894036 git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@153906 0039d316-1c4b-4281-b951-d872f2087c98 --- patch.py | 29 +++++++++++++++++++++-------- testing_support/patches_data.py | 10 ++++++++++ tests/patch_test.py | 26 ++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 8 deletions(-) diff --git a/patch.py b/patch.py index 90886e9a0..b8325da48 100644 --- a/patch.py +++ b/patch.py @@ -135,6 +135,11 @@ class Hunk(object): self.variation = self.lines_dst - self.lines_src self.text = [] + def __repr__(self): + return '%s<(%d, %d) to (%d, %d)>' % ( + self.__class__.__name__, + self.start_src, self.lines_src, self.start_dst, self.lines_dst) + class FilePatchDiff(FilePatchBase): """Patch for a single file.""" @@ -235,18 +240,26 @@ class FilePatchDiff(FilePatchBase): # "-1,N +0,0" where N is the number of lines deleted. That's from diff # and svn diff. git diff doesn't exhibit this behavior. # svn diff for a single line file rewrite "@@ -1 +1 @@". Fun. + # "@@ -1 +1,N @@" is also valid where N is the length of the new file. if not match: self._fail('Hunk header is unparsable') - if ',' in match.group(1): + count = match.group(1).count(',') + if not count: + start_src = int(match.group(1)) + lines_src = 1 + elif count == 1: start_src, lines_src = map(int, match.group(1).split(',', 1)) else: - start_src = int(match.group(1)) - lines_src = 0 - if ',' in match.group(2): + self._fail('Hunk header is malformed') + + count = match.group(2).count(',') + if not count: + start_dst = int(match.group(2)) + lines_dst = 1 + elif count == 1: start_dst, lines_dst = map(int, match.group(2).split(',', 1)) else: - start_dst = int(match.group(2)) - lines_dst = 0 + self._fail('Hunk header is malformed') new_hunk = Hunk(start_src, lines_src, start_dst, lines_dst) if hunks: if new_hunk.start_src <= hunks[-1].start_src: @@ -273,8 +286,8 @@ class FilePatchDiff(FilePatchBase): len([1 for i in hunk.text if i.startswith('-')])) if variation != hunk.variation: self._fail( - 'Hunk header is incorrect: %d vs %d' % ( - variation, hunk.variation)) + 'Hunk header is incorrect: %d vs %d; %r' % ( + variation, hunk.variation, hunk)) if not hunk.start_src: self._fail( 'Hunk header start line is incorrect: %d' % hunk.start_src) diff --git a/testing_support/patches_data.py b/testing_support/patches_data.py index ce94170de..3134a98a2 100644 --- a/testing_support/patches_data.py +++ b/testing_support/patches_data.py @@ -152,6 +152,16 @@ class GIT(object): ' ggg\n' ' hh\n') + # http://codereview.chromium.org/download/issue10868039_12001_10003.diff + PATCH_SHORT_HUNK_HEADER = ( + 'Index: chrome/browser/api/OWNERS\n' + 'diff --git a/chrome/browser/api/OWNERS b/chrome/browser/api/OWNERS\n' + '--- a/chrome/browser/api/OWNERS\n' + '+++ b/chrome/browser/api/OWNERS\n' + '@@ -1 +1,2 @@\n' + '+erikwright@chromium.org\n' + ' joi@chromium.org\n') + # http://codereview.chromium.org/download/issue6368055_22_29.diff DELETE = ( 'Index: tools/clang_check/README.chromium\n' diff --git a/tests/patch_test.py b/tests/patch_test.py index 41ef0cef1..6aa7cd17b 100755 --- a/tests/patch_test.py +++ b/tests/patch_test.py @@ -360,6 +360,19 @@ class PatchTest(unittest.TestCase): patchset = patch.PatchSet(patches) self.assertEquals(expected, patchset.filenames) + def testGitPatch(self): + p = patch.FilePatchDiff('chrome/file.cc', GIT.PATCH, []) + self._check_patch( + p, 'chrome/file.cc', GIT.PATCH, is_git_diff=True, patchlevel=1, + nb_hunks=1) + + def testGitPatchShortHunkHeader(self): + p = patch.FilePatchDiff( + 'chrome/browser/api/OWNERS', GIT.PATCH_SHORT_HUNK_HEADER, []) + self._check_patch( + p, 'chrome/browser/api/OWNERS', GIT.PATCH_SHORT_HUNK_HEADER, + is_git_diff=True, patchlevel=1, nb_hunks=1) + class PatchTestFail(unittest.TestCase): # All patches that should throw. @@ -513,6 +526,19 @@ class PatchTestFail(unittest.TestCase): except patch.UnsupportedPatchFormat: pass + def testBadHunkCommas(self): + try: + patch.FilePatchDiff( + 'file_a', + '--- file_a\n' + '+++ file_a\n' + '@@ -0,,0 +1 @@\n' + '+foo\n', + []) + self.fail() + except patch.UnsupportedPatchFormat: + pass + if __name__ == '__main__': logging.basicConfig(level=