Revert "Fix checkout.py issues when p.patchlevel > 1."

This reverts commit 5908f9906d.

Reason for revert:
Introduces bugs when deleting files.
The reason is that 
  patchlevel = patchlevel or self.patchlevel
will evaluate to self.patchlevel also when patchlevel is 0, which is wrong.

Original change's description:
> Fix checkout.py issues when p.patchlevel > 1.
> 
> When p.patchlevel > 1, p.filename does not correspond to the files that
> git-apply would modify.
> 
> See bug for details
> 
> Bug: 764294
> Change-Id: Icdb803056e306edb25238b2d9cdabd3ff175d8ed
> Reviewed-on: https://chromium-review.googlesource.com/663357
> Reviewed-by: Aaron Gable <agable@chromium.org>
> Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>

TBR=kjellander@chromium.org,agable@chromium.org,ehmaldonado@chromium.org

Change-Id: Ifa1f94602a023228cb32e5fe3fa07586b466981a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 764294
Reviewed-on: https://chromium-review.googlesource.com/663266
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
changes/66/663266/2
Edward Lesmes 8 years ago committed by Commit Bot
parent 5908f9906d
commit f8792079d2

@ -252,8 +252,7 @@ class GitCheckout(CheckoutBase):
for index, p in enumerate(patches):
stdout = []
try:
filepath = os.path.join(self.project_path,
p.filename_after_patchlevel())
filepath = os.path.join(self.project_path, p.filename)
if p.is_delete:
if (not os.path.exists(filepath) and
any(p1.source_filename == p.filename for p1 in patches[0:index])):
@ -261,12 +260,11 @@ class GitCheckout(CheckoutBase):
# was already processed because 'git apply' did it for us.
pass
else:
stdout.append(self._check_output_git(
['rm', p.filename_after_patchlevel()]))
stdout.append(self._check_output_git(['rm', p.filename]))
assert(not os.path.exists(filepath))
stdout.append('Deleted.')
else:
dirname = os.path.dirname(p.filename_after_patchlevel())
dirname = os.path.dirname(p.filename)
full_dir = os.path.join(self.project_path, dirname)
if dirname and not os.path.isdir(full_dir):
os.makedirs(full_dir)
@ -276,7 +274,7 @@ class GitCheckout(CheckoutBase):
with open(filepath, 'wb') as f:
f.write(content)
stdout.append('Added binary file %d bytes' % len(content))
cmd = ['add', p.filename_after_patchlevel()]
cmd = ['add', p.filename]
if verbose:
cmd.append('--verbose')
stdout.append(self._check_output_git(cmd))

@ -34,7 +34,6 @@ class FilePatchBase(object):
def __init__(self, filename):
assert self.__class__ is not FilePatchBase
self.filename = self._process_filename(filename)
self.patchlevel = 0
# Set when the file is copied or moved.
self.source_filename = None
@ -66,25 +65,6 @@ class FilePatchBase(object):
filename, 'Filename can\'t be \'%s\'.' % filename)
return filename
def filename_after_patchlevel(self):
"""Applies patchlevel to self.filename.
Applies patchlevel to self.filename so the resulting filename is the same as
the one git-apply would have used.
"""
# We use self.patchlevel-1 since git-apply considers the "a/" in the diff
# as part of the file path.
return self._apply_patchlevel(self.filename, self.patchlevel-1)
def _apply_patchlevel(self, string, patchlevel=None):
"""Apply patchlevel to a file path.
This function replaces backslashes with slashes and removes the first
patchlevel elements of string. patchlevel is self.patchlevel by default.
"""
patchlevel = patchlevel or self.patchlevel
return '/'.join(string.replace('\\', '/').split('/')[patchlevel:])
def set_relpath(self, relpath):
if not relpath:
return
@ -183,6 +163,7 @@ class FilePatchDiff(FilePatchBase):
self.diff_header, self.diff_hunks = self._split_header(diff)
self.svn_properties = svn_properties or []
self.is_git_diff = self._is_git_diff_header(self.diff_header)
self.patchlevel = 0
if self.is_git_diff:
self._verify_git_header()
else:
@ -333,6 +314,10 @@ class FilePatchDiff(FilePatchBase):
hunks[0].start_src -= 1
return hunks
def mangle(self, string):
"""Mangle a file path."""
return '/'.join(string.replace('\\', '/').split('/')[self.patchlevel:])
def _verify_git_header(self):
"""Sanity checks the header.
@ -361,8 +346,8 @@ class FilePatchDiff(FilePatchBase):
continue
if match.group(1).startswith('a/') and match.group(2).startswith('b/'):
self.patchlevel = 1
old = self._apply_patchlevel(match.group(1))
new = self._apply_patchlevel(match.group(2))
old = self.mangle(match.group(1))
new = self.mangle(match.group(2))
# The rename is about the new file so the old file can be anything.
if new not in (self.filename_utf8, 'dev/null'):
@ -445,7 +430,7 @@ class FilePatchDiff(FilePatchBase):
self._fail('--- and +++ are reversed')
if match.group(1) == '/dev/null':
self.is_new = True
elif self._apply_patchlevel(match.group(1)) != old:
elif self.mangle(match.group(1)) != old:
# git patches are always well formatted, do not allow random filenames.
self._fail('Unexpected git diff: %s != %s.' % (old, match.group(1)))
if not lines or not lines[0].startswith('+++'):
@ -458,7 +443,7 @@ class FilePatchDiff(FilePatchBase):
self._fail('Unexpected git diff: --- not following +++.')
if '/dev/null' == match.group(1):
self.is_delete = True
elif self.filename_utf8 != self._apply_patchlevel(match.group(1)):
elif self.filename_utf8 != self.mangle(match.group(1)):
self._fail(
'Unexpected git diff: %s != %s.' % (self.filename, match.group(1)))
if lines:
@ -497,7 +482,7 @@ class FilePatchDiff(FilePatchBase):
self._fail('--- and +++ are reversed')
if match.group(1) == '/dev/null':
self.is_new = True
elif self._apply_patchlevel(match.group(1)) != self.filename_utf8:
elif self.mangle(match.group(1)) != self.filename_utf8:
# guess the source filename.
self.source_filename = match.group(1).decode('utf-8')
self.is_new = True
@ -511,7 +496,7 @@ class FilePatchDiff(FilePatchBase):
self._fail('Unexpected diff: --- not following +++.')
if match.group(1) == '/dev/null':
self.is_delete = True
elif self._apply_patchlevel(match.group(1)) != self.filename_utf8:
elif self.mangle(match.group(1)) != self.filename_utf8:
self._fail('Unexpected diff: %s.' % match.group(1))
if lines:
self._fail('Crap after +++')

@ -180,23 +180,6 @@ class BaseTest(fake_repos.FakeReposTestBase):
#print fake_repos.read_tree(root)
self.assertTree(tree, root)
def _check_delete_patchlevel(self, co):
"""Makes sure file moves are handled correctly."""
co.prepare(None)
patchset = patch.PatchSet([
patch.FilePatchDelete(
'something/chromeos/views/DOMui_menu_widget.h', False),
])
for p in patchset:
p.patchlevel = 2
co.apply_patch(patchset)
# Make sure chromeos/views/DOMui_menu_widget.h is deleted and
# chromeos/views/webui_menu_widget.h is correctly created.
root = os.path.join(self.root_dir, self.name)
tree = self.get_trunk(False)
del tree['chromeos/views/DOMui_menu_widget.h']
self.assertTree(tree, root)
class GitBaseTest(BaseTest):
def setUp(self):
@ -338,19 +321,6 @@ class GitCheckout(GitBaseTest):
])
self.assertEquals(expected, out)
def testDeletePatchlevel(self):
co = self._get_co(None)
self._check_delete_patchlevel(co)
out = subprocess2.check_output(
['git', 'diff', '--staged', '--name-status', '--no-renames'],
cwd=co.project_path)
out = sorted(out.splitlines())
expected = sorted(
[
'D\tchromeos/views/DOMui_menu_widget.h',
])
self.assertEquals(expected, out)
if __name__ == '__main__':
if '-v' in sys.argv:

Loading…
Cancel
Save