aiden
/
depot_tools
mirror of https://chromium.googlesource.com/chromium/tools/depot_tools
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Add support to put a patchset into a subdirectory.

Browse Source 
Add better code to handle diff header, especially add more patch verification
code.

Add a lot of tests.

Add mangling of \ to /.

R=dpranke@chromium.org
BUG=78561
TEST=

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@81017 0039d316-1c4b-4281-b951-d872f2087c98
experimental/szager/collated-output
maruel@chromium.org 15 years ago
parent 899e1c1a61
commit cd6194027f
2 changed files with 400 additions and 111 deletions
Show Stats Download Patch File Download Diff File

272
patch.py
Unescape Escape View File

@ -4,6 +4,8 @@
# found in the LICENSE file.
"""Utility functions to handle patches."""
import posixpath
import os
import re
@ -21,21 +23,48 @@ class UnsupportedPatchFormat(Exception):
class FilePatchBase(object):
"""Defines a single file being modified."""
"""Defines a single file being modified.
'/' is always used instead of os.sep for consistency.
"""
is_delete = False
is_binary = False
def __init__(self, filename):
self.filename = None
self._set_filename(filename)
def _set_filename(self, filename):
self.filename = filename.replace('\\', '/')
# Blacklist a few characters for simplicity.
for i in ('%', '$', '..', '\'', '"'):
if i in self.filename:
self._fail('Can\'t use \'%s\' in filename.' % i)
for i in ('/', 'CON', 'COM'):
if self.filename.startswith(i):
self._fail('Filename can\'t start with \'%s\'.' % i)
def get(self):
raise NotImplementedError('Nothing to grab')
def set_relpath(self, relpath):
if not relpath:
return
relpath = relpath.replace('\\', '/')
if relpath[0] == '/':
self._fail('Relative path starts with %s' % relpath[0])
self._set_filename(posixpath.join(relpath, self.filename))
def _fail(self, msg):
raise UnsupportedPatchFormat(self.filename, msg)
class FilePatchDelete(FilePatchBase):
"""Deletes a file."""
is_delete = True
def __init__(self, filename, is_binary):
super(FilePatchDelete, self).__init__()
self.filename = filename
super(FilePatchDelete, self).__init__(filename)
self.is_binary = is_binary
def get(self):
@ -47,8 +76,7 @@ class FilePatchBinary(FilePatchBase):
is_binary = True
def __init__(self, filename, data, svn_properties):
super(FilePatchBinary, self).__init__()
self.filename = filename
super(FilePatchBinary, self).__init__(filename)
self.data = data
self.svn_properties = svn_properties or []
@ -60,113 +88,176 @@ class FilePatchDiff(FilePatchBase):
"""Patch for a single file."""
def __init__(self, filename, diff, svn_properties):
super(FilePatchDiff, self).__init__()
self.filename = filename
self.diff = diff
super(FilePatchDiff, self).__init__(filename)
self.diff_header, self.diff_hunks = self._split_header(diff)
self.svn_properties = svn_properties or []
self.is_git_diff = self._is_git_diff(diff)
self.is_git_diff = self._is_git_diff_header(self.diff_header)
self.patchlevel = 0
if self.is_git_diff:
self.patchlevel = 1
self._verify_git_patch(filename, diff)
self._verify_git_header()
assert not svn_properties
else:
self.patchlevel = 0
self._verify_svn_patch(filename, diff)
self._verify_svn_header()
def get(self):
return self.diff
return self.diff_header + self.diff_hunks
def set_relpath(self, relpath):
old_filename = self.filename
super(FilePatchDiff, self).set_relpath(relpath)
# Update the header too.
self.diff_header = self.diff_header.replace(old_filename, self.filename)
def _split_header(self, diff):
"""Splits a diff in two: the header and the hunks."""
header = []
hunks = diff.splitlines(True)
while hunks:
header.append(hunks.pop(0))
if header[-1].startswith('--- '):
break
else:
# Some diff may not have a ---/+++ set like a git rename with no change or
# a svn diff with only property change.
pass
if hunks:
if not hunks[0].startswith('+++ '):
self._fail('Inconsistent header')
header.append(hunks.pop(0))
if hunks:
if not hunks[0].startswith('@@ '):
self._fail('Inconsistent hunk header')
# Mangle any \\ in the header to /.
header_lines = ('Index:', 'diff', 'copy', 'rename', '+++', '---')
basename = os.path.basename(self.filename)
for i in xrange(len(header)):
if (header[i].split(' ', 1)[0] in header_lines or
header[i].endswith(basename)):
header[i] = header[i].replace('\\', '/')
return ''.join(header), ''.join(hunks)
@staticmethod
def _is_git_diff(diff):
"""Returns True if the diff for a single files was generated with gt.
def _is_git_diff_header(diff_header):
"""Returns True if the diff for a single files was generated with git."""
# Delete: http://codereview.chromium.org/download/issue6368055_22_29.diff
# Rename partial change:
# http://codereview.chromium.org/download/issue6250123_3013_6010.diff
# Rename no change:
# http://codereview.chromium.org/download/issue6287022_3001_4010.diff
return any(l.startswith('diff --git') for l in diff_header.splitlines())
def mangle(self, string):
"""Mangle a file path."""
return '/'.join(string.replace('\\', '/').split('/')[self.patchlevel:])
def _verify_git_header(self):
"""Sanity checks the header.
Expects the following format:
Index: <filename>
diff --git a/<filename> b/<filename>
<garbagge>
diff --git (|a/)<filename> (|b/)<filename>
<similarity>
<filemode changes>
<index>
<copy|rename from>
<copy|rename to>
--- <filename>
+++ <filename>
<hunks>
Index: is a rietveld specific line.
Everything is optional except the diff --git line.
"""
# Delete: http://codereview.chromium.org/download/issue6368055_22_29.diff
# Rename partial change:
# http://codereview.chromium.org/download/issue6250123_3013_6010.diff
# Rename no change:
# http://codereview.chromium.org/download/issue6287022_3001_4010.diff
return any(l.startswith('diff --git') for l in diff.splitlines()[:3])
lines = self.diff_header.splitlines()
@staticmethod
def _verify_git_patch(filename, diff):
lines = diff.splitlines()
# First fine the git diff header:
# Verify the diff --git line.
old = None
new = None
while lines:
line = lines.pop(0)
match = re.match(r'^diff \-\-git a\/(.*?) b\/(.*)$', line)
if match:
a = match.group(1)
b = match.group(2)
if a != filename and a != 'dev/null':
raise UnsupportedPatchFormat(
filename, 'Unexpected git diff input name.')
if b != filename and b != 'dev/null':
raise UnsupportedPatchFormat(
filename, 'Unexpected git diff output name.')
if a == 'dev/null' and b == 'dev/null':
raise UnsupportedPatchFormat(
filename, 'Unexpected /dev/null git diff.')
break
else:
raise UnsupportedPatchFormat(
filename, 'Unexpected git diff; couldn\'t find git header.')
match = re.match(r'^diff \-\-git (.*?) (.*)$', lines.pop(0))
if not match:
continue
old = match.group(1).replace('\\', '/')
new = match.group(2).replace('\\', '/')
if old.startswith('a/') and new.startswith('b/'):
self.patchlevel = 1
old = old[2:]
new = new[2:]
# The rename is about the new file so the old file can be anything.
if new not in (self.filename, 'dev/null'):
self._fail('Unexpected git diff output name %s.' % new)
if old == 'dev/null' and new == 'dev/null':
self._fail('Unexpected /dev/null git diff.')
break
if not old or not new:
self._fail('Unexpected git diff; couldn\'t find git header.')
# Handle these:
# rename from <>
# rename to <>
# copy from <>
# copy to <>
while lines:
line = lines.pop(0)
match = re.match(r'^--- a/(.*)$', line)
if match:
if a != match.group(1):
raise UnsupportedPatchFormat(
filename, 'Unexpected git diff: %s != %s.' % (a, match.group(1)))
match = re.match(r'^\+\+\+ b/(.*)$', lines.pop(0))
if not match:
raise UnsupportedPatchFormat(
filename, 'Unexpected git diff: --- not following +++.')
if b != match.group(1):
raise UnsupportedPatchFormat(
filename, 'Unexpected git diff: %s != %s.' % (b, match.group(1)))
if lines[0].startswith('--- '):
break
# Don't fail if the patch header is not found, the diff could be a
# file-mode-change-only diff.
match = re.match(r'^(rename|copy) from (.+)$', lines.pop(0))
if not match:
continue
if old != match.group(2):
self._fail('Unexpected git diff input name for %s.' % match.group(1))
if not lines:
self._fail('Missing git diff output name for %s.' % match.group(1))
match = re.match(r'^(rename|copy) to (.+)$', lines.pop(0))
if not match:
self._fail('Missing git diff output name for %s.' % match.group(1))
if new != match.group(2):
self._fail('Unexpected git diff output name for %s.' % match.group(1))
@staticmethod
def _verify_svn_patch(filename, diff):
lines = diff.splitlines()
# Handle ---/+++
while lines:
line = lines.pop(0)
match = re.match(r'^--- ([^\t]+).*$', line)
if match:
if match.group(1) not in (filename, '/dev/null'):
raise UnsupportedPatchFormat(
filename,
'Unexpected diff: %s != %s.' % (filename, match.group(1)))
# Grab next line.
line2 = lines.pop(0)
match = re.match(r'^\+\+\+ ([^\t]+).*$', line2)
if not match:
raise UnsupportedPatchFormat(
filename,
'Unexpected diff: --- not following +++.:\n%s\n%s' % (
line, line2))
if match.group(1) not in (filename, '/dev/null'):
raise UnsupportedPatchFormat(
filename,
'Unexpected diff: %s != %s.' % (filename, match.group(1)))
break
# Don't fail if the patch header is not found, the diff could be a
# svn-property-change-only diff.
match = re.match(r'^--- (.*)$', lines.pop(0))
if not match:
continue
if old != self.mangle(match.group(1)) and match.group(1) != '/dev/null':
self._fail('Unexpected git diff: %s != %s.' % (old, match.group(1)))
if not lines:
self._fail('Missing git diff output name.')
match = re.match(r'^\+\+\+ (.*)$', lines.pop(0))
if not match:
self._fail('Unexpected git diff: --- not following +++.')
if new != self.mangle(match.group(1)) and '/dev/null' != match.group(1):
self._fail('Unexpected git diff: %s != %s.' % (new, match.group(1)))
assert not lines, '_split_header() is broken'
break
def _verify_svn_header(self):
"""Sanity checks the header.
A svn diff can contain only property changes, in that case there will be no
proper header. To make things worse, this property change header is
localized.
"""
lines = self.diff_header.splitlines()
while lines:
match = re.match(r'^--- ([^\t]+).*$', lines.pop(0))
if not match:
continue
if match.group(1) not in (self.filename, '/dev/null'):
self._fail('Unexpected diff: %s.' % match.group(1))
match = re.match(r'^\+\+\+ ([^\t]+).*$', lines.pop(0))
if not match:
self._fail('Unexpected diff: --- not following +++.')
if match.group(1) not in (self.filename, '/dev/null'):
self._fail('Unexpected diff: %s.' % match.group(1))
assert not lines, '_split_header() is broken'
break
else:
# Cheap check to make sure the file name is at least mentioned in the
# 'diff' header. That the only remaining invariant.
if not self.filename in self.diff_header:
self._fail('Diff seems corrupted.')
class PatchSet(object):
@ -175,6 +266,11 @@ class PatchSet(object):
def __init__(self, patches):
self.patches = patches
def set_relpath(self, relpath):
"""Used to offset the patch into a subdirectory."""
for patch in self.patches:
patch.set_relpath(relpath)
def __iter__(self):
for patch in self.patches:
yield patch

239
tests/patch_test.py
Unescape Escape View File

@ -15,6 +15,116 @@ sys.path.insert(0, os.path.join(ROOT_DIR, '..'))
import patch
SVN_PATCH = (
'Index: chrome/file.cc\n'
'===================================================================\n'
'--- chrome/file.cc\t(revision 74690)\n'
'+++ chrome/file.cc\t(working copy)\n'
'@@ -80,10 +80,13 @@\n'
' // Foo\n'
' // Bar\n'
' void foo() {\n'
'- return bar;\n'
'+ return foo;\n'
' }\n'
' \n'
' \n')
GIT_PATCH = (
'diff --git a/chrome/file.cc b/chrome/file.cc\n'
'index 0e4de76..8320059 100644\n'
'--- a/chrome/file.cc\n'
'+++ b/chrome/file.cc\n'
'@@ -3,6 +3,7 @@ bb\n'
' ccc\n'
' dd\n'
' e\n'
'+FOO!\n'
' ff\n'
' ggg\n'
' hh\n')
# http://codereview.chromium.org/download/issue6368055_22_29.diff
GIT_DELETE = (
'Index: tools/clang_check/README.chromium\n'
'diff --git a/tools/clang_check/README.chromium '
'b/tools/clang_check/README.chromium\n'
'deleted file mode 100644\n'
'index fcaa7e0e94bb604a026c4f478fecb1c5796f5413..'
'0000000000000000000000000000000000000000\n'
'--- a/tools/clang_check/README.chromium\n'
'+++ /dev/null\n'
'@@ -1,9 +0,0 @@\n'
'-These are terrible, terrible hacks.\n'
'-\n'
'-They are meant to be temporary. clang currently doesn\'t allow running a '
'plugin\n'
'-AND doing the normal codegen process. We want our syntax complaining '
'plugins to\n'
'-run during normal compile, but for now, you can user run_plugin.sh to '
'hack the\n'
'-build system to do a syntax check.\n'
'-\n'
'-Also see http://code.google.com/p/chromium/wiki/WritingClangPlugins\n'
'-\n')
# http://codereview.chromium.org/download/issue6250123_3013_6010.diff
GIT_RENAME_PARTIAL = (
'Index: chrome/browser/chromeos/views/webui_menu_widget.h\n'
'diff --git a/chrome/browser/chromeos/views/domui_menu_widget.h '
'b/chrome/browser/chromeos/views/webui_menu_widget.h\n'
'similarity index 79%\n'
'rename from chrome/browser/chromeos/views/domui_menu_widget.h\n'
'rename to chrome/browser/chromeos/views/webui_menu_widget.h\n'
'index 095d4c474fd9718f5aebfa41a1ccb2d951356d41..'
'157925075434b590e8acaaf605a64f24978ba08b 100644\n'
'--- a/chrome/browser/chromeos/views/domui_menu_widget.h\n'
'+++ b/chrome/browser/chromeos/views/webui_menu_widget.h\n'
'@@ -1,9 +1,9 @@\n'
'-// Copyright (c) 2010 The Chromium Authors. All rights reserved.\n'
'+// Copyright (c) 2011 The Chromium Authors. All rights reserved.\n'
' // Use of this source code is governed by a BSD-style license that can be'
'\n'
' // found in the LICENSE file.\n'
' \n'
'-#ifndef CHROME_BROWSER_CHROMEOS_VIEWS_DOMUI_MENU_WIDGET_H_\n'
'-#define CHROME_BROWSER_CHROMEOS_VIEWS_DOMUI_MENU_WIDGET_H_\n'
'+#ifndef CHROME_BROWSER_CHROMEOS_VIEWS_WEBUI_MENU_WIDGET_H_\n'
'+#define CHROME_BROWSER_CHROMEOS_VIEWS_WEBUI_MENU_WIDGET_H_\n'
' #pragma once\n'
' \n'
' #include <string>\n')
# http://codereview.chromium.org/download/issue6287022_3001_4010.diff
GIT_RENAME = (
'Index: 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'
'rename from tools/run_local_server.py\n'
'rename to tools/run_local_server.sh\n')
GIT_COPY = (
'diff --git a/PRESUBMIT.py b/pp\n'
'similarity index 100%\n'
'copy from PRESUBMIT.py\n'
'copy to pp\n')
GIT_NEW = (
'diff --git a/foo b/foo\n'
'new file mode 100644\n'
'index 0000000..5716ca5\n'
'--- /dev/null\n'
'+++ b/foo\n'
'@@ -0,0 +1 @@\n'
'+bar\n')
class PatchTest(unittest.TestCase):
def testFilePatchDelete(self):
c = patch.FilePatchDelete('foo', False)
@ -44,13 +154,13 @@ class PatchTest(unittest.TestCase):
self.assertEquals(c.get(), 'data')
def testFilePatchDiff(self):
c = patch.FilePatchDiff('foo', 'data', [])
c = patch.FilePatchDiff('chrome/file.cc', SVN_PATCH, [])
self.assertEquals(c.is_delete, False)
self.assertEquals(c.is_binary, False)
self.assertEquals(c.filename, 'foo')
self.assertEquals(c.filename, 'chrome/file.cc')
self.assertEquals(c.is_git_diff, False)
self.assertEquals(c.patchlevel, 0)
self.assertEquals(c.get(), 'data')
self.assertEquals(c.get(), SVN_PATCH)
diff = (
'diff --git a/git_cl/git-cl b/git_cl/git-cl\n'
'old mode 100644\n'
@ -75,6 +185,20 @@ class PatchTest(unittest.TestCase):
self.assertEquals(c.patchlevel, 1)
self.assertEquals(c.get(), diff)
def testFilePatchBadDiff(self):
try:
patch.FilePatchDiff('foo', 'data', [])
self.fail()
except patch.UnsupportedPatchFormat:
pass
def testFilePatchBadDiffName(self):
try:
patch.FilePatchDiff('foo', SVN_PATCH, [])
self.fail()
except patch.UnsupportedPatchFormat:
pass
def testInvalidFilePatchDiffGit(self):
try:
patch.FilePatchDiff('svn_utils_test.txt', (
@ -137,26 +261,95 @@ class PatchTest(unittest.TestCase):
# pylint: disable=R0201
# Method could be a function
# Should not throw.
patch.FilePatchDiff('chrome/file.cc', (
'Index: chrome/file.cc\n'
'===================================================================\n'
'--- chrome/file.cc\t(revision 74690)\n'
'+++ chrome/file.cc\t(working copy)\n'
'@@ -80,10 +80,13 @@\n'
' // Foo\n'
' // Bar\n'
' void foo() {\n'
'- return bar;\n'
'+ return foo;\n'
' }\n'
' \n'
' \n'), [])
patch.FilePatchDiff('chrome/file.cc', (
'--- /dev/null\t2\n'
'+++ chrome/file.cc\tfoo\n'), [])
patch.FilePatchDiff('chrome/file.cc', (
'--- chrome/file.cc\tbar\n'
'+++ /dev/null\tfoo\n'), [])
p = patch.FilePatchDiff('chrome/file.cc', SVN_PATCH, [])
lines = SVN_PATCH.splitlines(True)
header = ''.join(lines[:4])
hunks = ''.join(lines[4:])
self.assertEquals(header, p.diff_header)
self.assertEquals(hunks, p.diff_hunks)
self.assertEquals(SVN_PATCH, p.get())
def testValidSvnNew(self):
text = '--- /dev/null\t2\n+++ chrome/file.cc\tfoo\n'
p = patch.FilePatchDiff('chrome/file.cc', text, [])
self.assertEquals(text, p.diff_header)
self.assertEquals('', p.diff_hunks)
self.assertEquals(text, p.get())
def testValidSvnDelete(self):
text = '--- chrome/file.cc\tbar\n+++ /dev/null\tfoo\n'
p = patch.FilePatchDiff('chrome/file.cc', text, [])
self.assertEquals(text, p.diff_header)
self.assertEquals('', p.diff_hunks)
self.assertEquals(text, p.get())
def testRelPath(self):
patches = patch.PatchSet([
patch.FilePatchDiff('chrome/file.cc', SVN_PATCH, []),
patch.FilePatchDiff(
'tools\\clang_check/README.chromium', GIT_DELETE, []),
patch.FilePatchDiff('tools/run_local_server.sh', GIT_RENAME, []),
patch.FilePatchDiff(
'chrome\\browser/chromeos/views/webui_menu_widget.h',
GIT_RENAME_PARTIAL, []),
patch.FilePatchDiff('pp', GIT_COPY, []),
patch.FilePatchDiff('foo', GIT_NEW, []),
patch.FilePatchDelete('other/place/foo', True),
patch.FilePatchBinary('bar', 'data', []),
])
expected = [
'chrome/file.cc', 'tools/clang_check/README.chromium',
'tools/run_local_server.sh',
'chrome/browser/chromeos/views/webui_menu_widget.h', 'pp', 'foo',
'other/place/foo', 'bar']
self.assertEquals(expected, patches.filenames)
orig_name = patches.patches[0].filename
patches.set_relpath(os.path.join('a', 'bb'))
expected = [os.path.join('a', 'bb', x) for x in expected]
self.assertEquals(expected, patches.filenames)
# Make sure each header is updated accordingly.
header = []
new_name = os.path.join('a', 'bb', orig_name)
for line in SVN_PATCH.splitlines(True):
if line.startswith('@@'):
break
if line[:3] in ('---', '+++', 'Ind'):
line = line.replace(orig_name, new_name)
header.append(line)
header = ''.join(header)
self.assertEquals(header, patches.patches[0].diff_header)
def testRelPathBad(self):
patches = patch.PatchSet([
patch.FilePatchDiff('chrome\\file.cc', SVN_PATCH, []),
patch.FilePatchDelete('other\\place\\foo', True),
])
try:
patches.set_relpath('..')
self.fail()
except patch.UnsupportedPatchFormat:
pass
def testBackSlash(self):
mangled_patch = SVN_PATCH.replace('chrome/', 'chrome\\')
patches = patch.PatchSet([
patch.FilePatchDiff('chrome\\file.cc', mangled_patch, []),
patch.FilePatchDelete('other\\place\\foo', True),
])
expected = ['chrome/file.cc', 'other/place/foo']
self.assertEquals(expected, patches.filenames)
self.assertEquals(SVN_PATCH, patches.patches[0].get())
def testGitPatches(self):
# Shouldn't throw.
patch.FilePatchDiff('tools/clang_check/README.chromium', GIT_DELETE, [])
patch.FilePatchDiff('tools/run_local_server.sh', GIT_RENAME, [])
patch.FilePatchDiff(
'chrome/browser/chromeos/views/webui_menu_widget.h',
GIT_RENAME_PARTIAL, [])
patch.FilePatchDiff('pp', GIT_COPY, [])
patch.FilePatchDiff('foo', GIT_NEW, [])
self.assertTrue(True)
if __name__ == '__main__':

Write Preview
Loading…
Cancel
Save