Skip to content

C
Chromium_Depot_tools
Commit a19047cd authored by maruel@chromium.org's avatar maruel@chromium.org
Browse files
Options

Add support forcopied and renamed files. 

R=dpranke@chromium.org
BUG=
TEST=


Review URL: http://codereview.chromium.org/6874006

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@100138 0039d316-1c4b-4281-b951-d872f2087c98
parent 63c2c1d2
Hide whitespace changes
Inline Side-by-side
Showing with 105 additions and 44 deletions
patch.py
View file @ a19047cd
...@@ -33,6 +33,8 @@ class FilePatchBase(object): ...@@ -33,6 +33,8 @@ class FilePatchBase(object):
def __init__(self, filename): def __init__(self, filename):
self.filename = self._process_filename(filename) self.filename = self._process_filename(filename)
# Set when the file is copied or moved.
self.source_filename = None
@staticmethod @staticmethod
def _process_filename(filename): def _process_filename(filename):
...@@ -59,6 +61,9 @@ class FilePatchBase(object): ...@@ -59,6 +61,9 @@ class FilePatchBase(object):
self._fail('Relative path starts with %s' % relpath[0]) self._fail('Relative path starts with %s' % relpath[0])
self.filename = self._process_filename( self.filename = self._process_filename(
posixpath.join(relpath, self.filename)) posixpath.join(relpath, self.filename))
if self.source_filename:
self.source_filename = self._process_filename(
posixpath.join(relpath, self.source_filename))
def _fail(self, msg): def _fail(self, msg):
"""Shortcut function to raise UnsupportedPatchFormat.""" """Shortcut function to raise UnsupportedPatchFormat."""
...@@ -112,9 +117,21 @@ class FilePatchDiff(FilePatchBase): ...@@ -112,9 +117,21 @@ class FilePatchDiff(FilePatchBase):
def set_relpath(self, relpath): def set_relpath(self, relpath):
old_filename = self.filename old_filename = self.filename
old_source_filename = self.source_filename or self.filename
super(FilePatchDiff, self).set_relpath(relpath) super(FilePatchDiff, self).set_relpath(relpath)
# Update the header too. # Update the header too.
self.diff_header = self.diff_header.replace(old_filename, self.filename) source_filename = self.source_filename or self.filename
lines = self.diff_header.splitlines(True)
for i, line in enumerate(lines):
if line.startswith('diff --git'):
lines[i] = line.replace(
'a/' + old_source_filename, source_filename).replace(
'b/' + old_filename, self.filename)
elif re.match(r'^\w+ from .+$', line) or line.startswith('---'):
lines[i] = line.replace(old_source_filename, source_filename)
elif re.match(r'^\w+ to .+$', line) or line.startswith('+++'):
lines[i] = line.replace(old_filename, self.filename)
self.diff_header = ''.join(lines)
def _split_header(self, diff): def _split_header(self, diff):
"""Splits a diff in two: the header and the hunks.""" """Splits a diff in two: the header and the hunks."""
...@@ -186,12 +203,11 @@ class FilePatchDiff(FilePatchBase): ...@@ -186,12 +203,11 @@ class FilePatchDiff(FilePatchBase):
match = re.match(r'^diff \-\-git (.*?) (.*)$', lines.pop(0)) match = re.match(r'^diff \-\-git (.*?) (.*)$', lines.pop(0))
if not match: if not match:
continue continue
old = match.group(1).replace('\\', '/') if match.group(1).startswith('a/') and match.group(2).startswith('b/'):
new = match.group(2).replace('\\', '/')
if old.startswith('a/') and new.startswith('b/'):
self.patchlevel = 1 self.patchlevel = 1
old = old[2:] old = self.mangle(match.group(1))
new = new[2:] new = self.mangle(match.group(2))
# The rename is about the new file so the old file can be anything. # The rename is about the new file so the old file can be anything.
if new not in (self.filename, 'dev/null'): if new not in (self.filename, 'dev/null'):
self._fail('Unexpected git diff output name %s.' % new) self._fail('Unexpected git diff output name %s.' % new)
...@@ -202,13 +218,15 @@ class FilePatchDiff(FilePatchBase): ...@@ -202,13 +218,15 @@ class FilePatchDiff(FilePatchBase):
if not old or not new: if not old or not new:
self._fail('Unexpected git diff; couldn\'t find git header.') self._fail('Unexpected git diff; couldn\'t find git header.')
if old not in (self.filename, 'dev/null'):
# Copy or rename.
self.source_filename = old
last_line = '' last_line = ''
while lines: while lines:
line = lines.pop(0) line = lines.pop(0)
# TODO(maruel): old should be replace with self.source_file self._verify_git_header_process_line(lines, line, last_line)
# TODO(maruel): new == self.filename and remove new
self._verify_git_header_process_line(lines, line, last_line, old, new)
last_line = line last_line = line
# Cheap check to make sure the file name is at least mentioned in the # Cheap check to make sure the file name is at least mentioned in the
...@@ -216,7 +234,7 @@ class FilePatchDiff(FilePatchBase): ...@@ -216,7 +234,7 @@ class FilePatchDiff(FilePatchBase):
if not self.filename in self.diff_header: if not self.filename in self.diff_header:
self._fail('Diff seems corrupted.') self._fail('Diff seems corrupted.')
def _verify_git_header_process_line(self, lines, line, last_line, old, new): def _verify_git_header_process_line(self, lines, line, last_line):
"""Processes a single line of the header. """Processes a single line of the header.
Returns True if it should continue looping. Returns True if it should continue looping.
...@@ -225,6 +243,7 @@ class FilePatchDiff(FilePatchBase): ...@@ -225,6 +243,7 @@ class FilePatchDiff(FilePatchBase):
http://www.kernel.org/pub/software/scm/git/docs/git-diff.html http://www.kernel.org/pub/software/scm/git/docs/git-diff.html
""" """
match = re.match(r'^(rename|copy) from (.+)$', line) match = re.match(r'^(rename|copy) from (.+)$', line)
old = self.source_filename or self.filename
if match: if match:
if old != match.group(2): if old != match.group(2):
self._fail('Unexpected git diff input name for line %s.' % line) self._fail('Unexpected git diff input name for line %s.' % line)
...@@ -236,7 +255,7 @@ class FilePatchDiff(FilePatchBase): ...@@ -236,7 +255,7 @@ class FilePatchDiff(FilePatchBase):
match = re.match(r'^(rename|copy) to (.+)$', line) match = re.match(r'^(rename|copy) to (.+)$', line)
if match: if match:
if new != match.group(2): if self.filename != match.group(2):
self._fail('Unexpected git diff output name for line %s.' % line) self._fail('Unexpected git diff output name for line %s.' % line)
if not last_line.startswith('%s from ' % match.group(1)): if not last_line.startswith('%s from ' % match.group(1)):
self._fail( self._fail(
...@@ -258,7 +277,7 @@ class FilePatchDiff(FilePatchBase): ...@@ -258,7 +277,7 @@ class FilePatchDiff(FilePatchBase):
self._fail('--- and +++ are reversed') self._fail('--- and +++ are reversed')
self.is_new = match.group(1) == '/dev/null' self.is_new = match.group(1) == '/dev/null'
# TODO(maruel): Use self.source_file. # TODO(maruel): Use self.source_file.
if old != self.mangle(match.group(1)) and match.group(1) != '/dev/null': if self.mangle(match.group(1)) not in (old, 'dev/null'):
self._fail('Unexpected git diff: %s != %s.' % (old, match.group(1))) self._fail('Unexpected git diff: %s != %s.' % (old, match.group(1)))
if not lines or not lines[0].startswith('+++'): if not lines or not lines[0].startswith('+++'):
self._fail('Missing git diff output name.') self._fail('Missing git diff output name.')
...@@ -271,8 +290,9 @@ class FilePatchDiff(FilePatchBase): ...@@ -271,8 +290,9 @@ class FilePatchDiff(FilePatchBase):
# TODO(maruel): new == self.filename. # TODO(maruel): new == self.filename.
if '/dev/null' == match.group(1): if '/dev/null' == match.group(1):
self.is_delete = True self.is_delete = True
elif new != self.mangle(match.group(1)): elif self.filename != self.mangle(match.group(1)):
self._fail('Unexpected git diff: %s != %s.' % (new, match.group(1))) self._fail(
'Unexpected git diff: %s != %s.' % (self.filename, match.group(1)))
if lines: if lines:
self._fail('Crap after +++') self._fail('Crap after +++')
# We're done. # We're done.
...@@ -308,13 +328,9 @@ class FilePatchDiff(FilePatchBase): ...@@ -308,13 +328,9 @@ class FilePatchDiff(FilePatchBase):
if last_line[:3] in ('---', '+++'): if last_line[:3] in ('---', '+++'):
self._fail('--- and +++ are reversed') self._fail('--- and +++ are reversed')
self.is_new = match.group(1) == '/dev/null' self.is_new = match.group(1) == '/dev/null'
# For copy and renames, it's possible that the -- line doesn't match if (self.mangle(match.group(1)) != self.filename and
# +++, so don't check match.group(1) to match self.filename or match.group(1) != '/dev/null'):
# '/dev/null', it can be anything else. self.source_filename = match.group(1)
# TODO(maruel): Handle rename/copy explicitly.
# if (self.mangle(match.group(1)) != self.filename and
# match.group(1) != '/dev/null'):
# self.source_file = match.group(1)
if not lines or not lines[0].startswith('+++'): if not lines or not lines[0].startswith('+++'):
self._fail('Nothing after header.') self._fail('Nothing after header.')
return return
......
rietveld.py
View file @ a19047cd
...@@ -155,10 +155,14 @@ class Rietveld(object): ...@@ -155,10 +155,14 @@ class Rietveld(object):
# Support this use case if it ever happen. # Support this use case if it ever happen.
raise patch.UnsupportedPatchFormat( raise patch.UnsupportedPatchFormat(
filename, 'Empty diff is not supported yet.\n') filename, 'Empty diff is not supported yet.\n')
out.append(patch.FilePatchDiff(filename, diff, svn_props)) p = patch.FilePatchDiff(filename, diff, svn_props)
out.append(p)
if status[0] == 'A': if status[0] == 'A':
# It won't be set for empty file. # It won't be set for empty file.
out[-1].is_new = True p.is_new = True
if status[1] == '+' and not (p.source_filename or p.svn_properties):
raise patch.UnsupportedPatchFormat(
filename, 'Failed to process the svn properties')
else: else:
raise patch.UnsupportedPatchFormat( raise patch.UnsupportedPatchFormat(
filename, 'Change with status \'%s\' is not supported.' % status) filename, 'Change with status \'%s\' is not supported.' % status)
......
tests/patch_test.py
View file @ a19047cd
...@@ -74,16 +74,16 @@ GIT_DELETE = ( ...@@ -74,16 +74,16 @@ GIT_DELETE = (
# http://codereview.chromium.org/download/issue6250123_3013_6010.diff # http://codereview.chromium.org/download/issue6250123_3013_6010.diff
GIT_RENAME_PARTIAL = ( GIT_RENAME_PARTIAL = (
'Index: chrome/browser/chromeos/views/webui_menu_widget.h\n' 'Index: chromeos/views/webui_menu_widget.h\n'
'diff --git a/chrome/browser/chromeos/views/domui_menu_widget.h ' 'diff --git a/chromeos/views/DOMui_menu_widget.h '
'b/chrome/browser/chromeos/views/webui_menu_widget.h\n' 'b/chromeos/views/webui_menu_widget.h\n'
'similarity index 79%\n' 'similarity index 79%\n'
'rename from chrome/browser/chromeos/views/domui_menu_widget.h\n' 'rename from chromeos/views/DOMui_menu_widget.h\n'
'rename to chrome/browser/chromeos/views/webui_menu_widget.h\n' 'rename to chromeos/views/webui_menu_widget.h\n'
'index 095d4c474fd9718f5aebfa41a1ccb2d951356d41..' 'index 095d4c474fd9718f5aebfa41a1ccb2d951356d41..'
'157925075434b590e8acaaf605a64f24978ba08b 100644\n' '157925075434b590e8acaaf605a64f24978ba08b 100644\n'
'--- a/chrome/browser/chromeos/views/domui_menu_widget.h\n' '--- a/chromeos/views/DOMui_menu_widget.h\n'
'+++ b/chrome/browser/chromeos/views/webui_menu_widget.h\n' '+++ b/chromeos/views/webui_menu_widget.h\n'
'@@ -1,9 +1,9 @@\n' '@@ -1,9 +1,9 @@\n'
'-// Copyright (c) 2010 The Chromium Authors. All rights reserved.\n' '-// Copyright (c) 2010 The Chromium Authors. All rights reserved.\n'
'+// Copyright (c) 2011 The Chromium Authors. All rights reserved.\n' '+// Copyright (c) 2011 The Chromium Authors. All rights reserved.\n'
...@@ -103,9 +103,9 @@ GIT_RENAME_PARTIAL = ( ...@@ -103,9 +103,9 @@ GIT_RENAME_PARTIAL = (
# http://codereview.chromium.org/download/issue6287022_3001_4010.diff # http://codereview.chromium.org/download/issue6287022_3001_4010.diff
GIT_RENAME = ( GIT_RENAME = (
'Index: tools/run_local_server.sh\n' 'Index: tools/run_local_server.sh\n'
'diff --git a/tools/run_local_server.py b/tools/run_local_server.sh\n' 'diff --git a/tools/run_local_server.PY b/tools/run_local_server.sh\n'
'similarity index 100%\n' 'similarity index 100%\n'
'rename from tools/run_local_server.py\n' 'rename from tools/run_local_server.PY\n'
'rename to tools/run_local_server.sh\n') 'rename to tools/run_local_server.sh\n')
...@@ -145,6 +145,7 @@ class PatchTest(unittest.TestCase): ...@@ -145,6 +145,7 @@ class PatchTest(unittest.TestCase):
p, p,
filename, filename,
diff, diff,
source_filename=None,
is_binary=False, is_binary=False,
is_delete=False, is_delete=False,
is_git_diff=False, is_git_diff=False,
...@@ -153,6 +154,7 @@ class PatchTest(unittest.TestCase): ...@@ -153,6 +154,7 @@ class PatchTest(unittest.TestCase):
svn_properties=None): svn_properties=None):
svn_properties = svn_properties or [] svn_properties = svn_properties or []
self.assertEquals(p.filename, filename) self.assertEquals(p.filename, filename)
self.assertEquals(p.source_filename, source_filename)
self.assertEquals(p.is_binary, is_binary) self.assertEquals(p.is_binary, is_binary)
self.assertEquals(p.is_delete, is_delete) self.assertEquals(p.is_delete, is_delete)
if hasattr(p, 'is_git_diff'): if hasattr(p, 'is_git_diff'):
...@@ -378,8 +380,7 @@ class PatchTest(unittest.TestCase): ...@@ -378,8 +380,7 @@ class PatchTest(unittest.TestCase):
'tools\\clang_check/README.chromium', GIT_DELETE, []), 'tools\\clang_check/README.chromium', GIT_DELETE, []),
patch.FilePatchDiff('tools/run_local_server.sh', GIT_RENAME, []), patch.FilePatchDiff('tools/run_local_server.sh', GIT_RENAME, []),
patch.FilePatchDiff( patch.FilePatchDiff(
'chrome\\browser/chromeos/views/webui_menu_widget.h', 'chromeos\\views/webui_menu_widget.h', GIT_RENAME_PARTIAL, []),
GIT_RENAME_PARTIAL, []),
patch.FilePatchDiff('pp', GIT_COPY, []), patch.FilePatchDiff('pp', GIT_COPY, []),
patch.FilePatchDiff('foo', GIT_NEW, []), patch.FilePatchDiff('foo', GIT_NEW, []),
patch.FilePatchDelete('other/place/foo', True), patch.FilePatchDelete('other/place/foo', True),
...@@ -388,20 +389,24 @@ class PatchTest(unittest.TestCase): ...@@ -388,20 +389,24 @@ class PatchTest(unittest.TestCase):
expected = [ expected = [
'chrome/file.cc', 'tools/clang_check/README.chromium', 'chrome/file.cc', 'tools/clang_check/README.chromium',
'tools/run_local_server.sh', 'tools/run_local_server.sh',
'chrome/browser/chromeos/views/webui_menu_widget.h', 'pp', 'foo', 'chromeos/views/webui_menu_widget.h', 'pp', 'foo',
'other/place/foo', 'bar'] 'other/place/foo', 'bar']
self.assertEquals(expected, patches.filenames) self.assertEquals(expected, patches.filenames)
orig_name = patches.patches[0].filename orig_name = patches.patches[0].filename
orig_source_name = patches.patches[0].source_filename or orig_name
patches.set_relpath(os.path.join('a', 'bb')) patches.set_relpath(os.path.join('a', 'bb'))
expected = [os.path.join('a', 'bb', x) for x in expected] expected = [os.path.join('a', 'bb', x) for x in expected]
self.assertEquals(expected, patches.filenames) self.assertEquals(expected, patches.filenames)
# Make sure each header is updated accordingly. # Make sure each header is updated accordingly.
header = [] header = []
new_name = os.path.join('a', 'bb', orig_name) new_name = os.path.join('a', 'bb', orig_name)
new_source_name = os.path.join('a', 'bb', orig_source_name)
for line in SVN_PATCH.splitlines(True): for line in SVN_PATCH.splitlines(True):
if line.startswith('@@'): if line.startswith('@@'):
break break
if line[:3] in ('---', '+++', 'Ind'): if line[:3] == '---':
line = line.replace(orig_source_name, new_source_name)
if line[:3] == '+++':
line = line.replace(orig_name, new_name) line = line.replace(orig_name, new_name)
header.append(line) header.append(line)
header = ''.join(header) header = ''.join(header)
...@@ -427,6 +432,7 @@ class PatchTest(unittest.TestCase): ...@@ -427,6 +432,7 @@ class PatchTest(unittest.TestCase):
self.assertEquals( self.assertEquals(
['chrome/file.cc', 'other/place/foo'], ['chrome/file.cc', 'other/place/foo'],
[f.filename for f in patches]) [f.filename for f in patches])
self.assertEquals([None, None], [f.source_filename for f in patches])
def testBackSlash(self): def testBackSlash(self):
mangled_patch = SVN_PATCH.replace('chrome/', 'chrome\\') mangled_patch = SVN_PATCH.replace('chrome/', 'chrome\\')
...@@ -452,19 +458,21 @@ class PatchTest(unittest.TestCase): ...@@ -452,19 +458,21 @@ class PatchTest(unittest.TestCase):
def testGitRename(self): def testGitRename(self):
p = patch.FilePatchDiff('tools/run_local_server.sh', GIT_RENAME, []) p = patch.FilePatchDiff('tools/run_local_server.sh', GIT_RENAME, [])
self._check_patch(p, 'tools/run_local_server.sh', GIT_RENAME, self._check_patch(p, 'tools/run_local_server.sh', GIT_RENAME,
is_git_diff=True, patchlevel=1) is_git_diff=True, patchlevel=1,
source_filename='tools/run_local_server.PY')
def testGitRenamePartial(self): def testGitRenamePartial(self):
p = patch.FilePatchDiff( p = patch.FilePatchDiff(
'chrome/browser/chromeos/views/webui_menu_widget.h', 'chromeos/views/webui_menu_widget.h', GIT_RENAME_PARTIAL, [])
GIT_RENAME_PARTIAL, [])
self._check_patch( self._check_patch(
p, 'chrome/browser/chromeos/views/webui_menu_widget.h', p, 'chromeos/views/webui_menu_widget.h', GIT_RENAME_PARTIAL,
GIT_RENAME_PARTIAL, is_git_diff=True, patchlevel=1) source_filename='chromeos/views/DOMui_menu_widget.h', is_git_diff=True,
patchlevel=1)
def testGitCopy(self): def testGitCopy(self):
p = patch.FilePatchDiff('pp', GIT_COPY, []) p = patch.FilePatchDiff('pp', GIT_COPY, [])
self._check_patch(p, 'pp', GIT_COPY, is_git_diff=True, patchlevel=1) self._check_patch(p, 'pp', GIT_COPY, is_git_diff=True, patchlevel=1,
source_filename='PRESUBMIT.py')
def testOnlyHeader(self): def testOnlyHeader(self):
diff = '--- file_a\n+++ file_a\n' diff = '--- file_a\n+++ file_a\n'
...@@ -495,7 +503,7 @@ class PatchTest(unittest.TestCase): ...@@ -495,7 +503,7 @@ class PatchTest(unittest.TestCase):
diff = '--- file_a\n+++ file_b\n' diff = '--- file_a\n+++ file_b\n'
p = patch.FilePatchDiff('file_b', diff, []) p = patch.FilePatchDiff('file_b', diff, [])
# Should it be marked as new? # Should it be marked as new?
self._check_patch(p, 'file_b', diff) self._check_patch(p, 'file_b', diff, source_filename='file_a')
def testGitCopyPartial(self): def testGitCopyPartial(self):
diff = ( diff = (
...@@ -514,7 +522,8 @@ class PatchTest(unittest.TestCase): ...@@ -514,7 +522,8 @@ class PatchTest(unittest.TestCase):
' # found in the LICENSE file.\n') ' # found in the LICENSE file.\n')
p = patch.FilePatchDiff('wtf2', diff, []) p = patch.FilePatchDiff('wtf2', diff, [])
# Should it be marked as new? # Should it be marked as new?
self._check_patch(p, 'wtf2', diff, is_git_diff=True, patchlevel=1) self._check_patch(
p, 'wtf2', diff, source_filename='wtf', is_git_diff=True, patchlevel=1)
def testGitExe(self): def testGitExe(self):
diff = ( diff = (
......
tests/rietveld_test.py
View file @ a19047cd
...@@ -288,6 +288,38 @@ class RietveldTest(unittest.TestCase): ...@@ -288,6 +288,38 @@ class RietveldTest(unittest.TestCase):
# TODO(maruel): Change with no diff, only svn property change: # TODO(maruel): Change with no diff, only svn property change:
# http://codereview.chromium.org/6462019/ # http://codereview.chromium.org/6462019/
def test_get_patch_moved(self):
output = (
'{\n'
' "files":\n'
' {\n'
' "file_a":\n'
' {\n'
' "status": "A+",\n'
' "is_binary": false,\n'
' "num_chunks": 1,\n'
' "id": 789\n'
' }\n'
' }\n'
'}\n')
diff = (
'--- /dev/null\n'
'+++ file_a\n'
'@@ -0,0 +1 @@\n'
'+foo\n')
r = rietveld.Rietveld('url', 'email', 'password')
def _send(args, **kwargs):
if args == '/api/123/456':
return output
elif args == '/download/issue123_456_789.diff':
return diff
else:
self.fail()
r._send = _send
patches = r.get_patch(123, 456)
self.assertEquals(diff, patches.patches[0].get())
self.assertEquals([], patches.patches[0].svn_properties)
if __name__ == '__main__': if __name__ == '__main__':
logging.basicConfig(level=logging.ERROR) logging.basicConfig(level=logging.ERROR)
......
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