diff --git a/reviewboard/diffviewer/diffutils.py b/reviewboard/diffviewer/diffutils.py index f6836c6c397e336d69c695bebdce4ab86cec7472..c172bef620f1bfc64bd7916a1c3d59b6cd2f2616 100644 --- a/reviewboard/diffviewer/diffutils.py +++ b/reviewboard/diffviewer/diffutils.py @@ -657,6 +657,7 @@ def get_diff_files(diffset, filediff=None, interdiffset=None, 'copied': filediff.copied, 'moved_or_copied': filediff.moved or filediff.copied, 'newfile': newfile, + 'is_symlink': filediff.extra_data.get('is_symlink', False), 'index': len(files), 'chunks_loaded': False, 'is_new_file': (newfile and not interfilediff and diff --git a/reviewboard/diffviewer/managers.py b/reviewboard/diffviewer/managers.py index 134eb4cdbf6eda72d528b035592b3a1bf6afafee..4e4ac7f15c9342898f3a2647e5106ac2b78f55a7 100644 --- a/reviewboard/diffviewer/managers.py +++ b/reviewboard/diffviewer/managers.py @@ -699,11 +699,15 @@ class DiffSetManager(models.Manager): binary=f.binary, status=status) + filediff.extra_data = { + 'is_symlink': f.is_symlink, + } + if (parent_file and (parent_file.moved or parent_file.copied) and parent_file.insert_count == 0 and parent_file.delete_count == 0): - filediff.extra_data = {'parent_moved': True} + filediff.extra_data['parent_moved'] = True if not validate_only: # This state all requires making modifications to the database. diff --git a/reviewboard/diffviewer/parser.py b/reviewboard/diffviewer/parser.py index ea587e8b88212b694a6d46267a6fd06b87018333..fbe0234e339b18a74c418171fc30fa76cf255c0c 100644 --- a/reviewboard/diffviewer/parser.py +++ b/reviewboard/diffviewer/parser.py @@ -34,6 +34,7 @@ class ParsedDiffFile(object): self.deleted = False self.moved = False self.copied = False + self.is_symlink = False self.insert_count = 0 self.delete_count = 0 @@ -193,27 +194,18 @@ class DiffParser(object): return linenum, None parsed_file = ParsedDiffFile() - parsed_file.binary = info.get('binary', False) - parsed_file.deleted = info.get('deleted', False) - parsed_file.moved = info.get('moved', False) - parsed_file.copied = info.get('copied', False) - parsed_file.origFile = info.get('origFile') - parsed_file.newFile = info.get('newFile') - parsed_file.origInfo = info.get('origInfo') - parsed_file.newInfo = info.get('newInfo') parsed_file.origChangesetId = info.get('origChangesetId') - if isinstance(parsed_file.origFile, six.binary_type): - parsed_file.origFile = parsed_file.origFile.decode('utf-8') + for attr in ('binary', 'deleted', 'moved', 'copied', 'is_symlink'): + setattr(parsed_file, attr, info.get(attr, False)) - if isinstance(parsed_file.newFile, six.binary_type): - parsed_file.newFile = parsed_file.newFile.decode('utf-8') + for attr in ('origFile', 'newFile', 'origInfo', 'newInfo'): + attr_value = info.get(attr) - if isinstance(parsed_file.origInfo, six.binary_type): - parsed_file.origInfo = parsed_file.origInfo.decode('utf-8') + if isinstance(attr_value, six.binary_type): + attr_value = attr_value.decode('utf-8') - if isinstance(parsed_file.newInfo, six.binary_type): - parsed_file.newInfo = parsed_file.newInfo.decode('utf-8') + setattr(parsed_file, attr, attr_value) # The header is part of the diff, so make sure it gets in the # diff content. diff --git a/reviewboard/scmtools/git.py b/reviewboard/scmtools/git.py index f5df5c28f9e06dc5d87e29c8c46ba18169d71a7b..51eb50f8d9d8612b3f7eebfc056da5626006ca1a 100644 --- a/reviewboard/scmtools/git.py +++ b/reviewboard/scmtools/git.py @@ -2,8 +2,9 @@ from __future__ import unicode_literals import logging import os -import re import platform +import re +import stat from django.utils import six from django.utils.six.moves import cStringIO as StringIO @@ -103,6 +104,41 @@ class GitTool(SCMTool): except (FileNotFoundError, InvalidRevisionFormatError): return False + def normalize_patch(self, patch, filename, revision): + """Normalize the provided patch file. + + This will make new, changed, and deleted symlinks look like + regular files. + + Otherwise patch fails to apply the diff, complaining about the + file not being a symlink. + + Args: + patch (bytes): + The diff/patch file to normalize. + + filename (unicode): + The name of the file being changed in the diff. + + revision (unicode): + The revision of the file being changed in the diff. + + Returns: + bytes: + The resulting diff/patch file. + """ + m = GitDiffParser.FILE_MODE_RE.search(patch) + + if m: + mode = int(m.group('mode'), 8) + + if stat.S_ISLNK(mode): + mode = stat.S_IFREG | stat.S_IMODE(mode) + patch = b'%s%o%s' % (patch[:m.start('mode')], mode, + patch[m.end('mode'):]) + + return patch + def parse_diff_revision(self, file_str, revision_str, moved=False, copied=False, *args, **kwargs): revision = revision_str @@ -152,6 +188,10 @@ class GitDiffParser(DiffParser): """ pre_creation_regexp = re.compile(b"^0+$") + FILE_MODE_RE = re.compile( + b'^(?:(?:new|deleted) file mode|index \w+\.\.\w+) (?P\d+)$', + re.M) + DIFF_GIT_LINE_RES = [ # Match with a/ and b/ prefixes. Common case. re.compile( @@ -280,6 +320,8 @@ class GitDiffParser(DiffParser): # a deleted file with no content # then skip + start_linenum = linenum + # Now we have a diff we are going to use so get the filenames + commits diff_git_line = self.lines[linenum] @@ -301,6 +343,16 @@ class GitDiffParser(DiffParser): headers, linenum = self._parse_extended_headers(linenum) + for line in self.lines[start_linenum:linenum]: + m = GitDiffParser.FILE_MODE_RE.search(line) + + if m: + mode = int(m.group('mode'), 8) + + if stat.S_ISLNK(mode): + file_info.is_symlink = True + break + if self._is_new_file(headers): file_info.append_data(headers[b'new file mode'][1]) file_info.origInfo = PRE_CREATION diff --git a/reviewboard/scmtools/tests/test_git.py b/reviewboard/scmtools/tests/test_git.py index 9e154fd1c00ee1c5f89b1cf2cc2891350328913f..b8b384c71aece8d78b66f26b3bcae4b0b0e4e0f7 100644 --- a/reviewboard/scmtools/tests/test_git.py +++ b/reviewboard/scmtools/tests/test_git.py @@ -77,6 +77,7 @@ class GitTests(SpyAgency, SCMTestCase): self.assertEqual(file.newInfo, 'bcae657') self.assertFalse(file.binary) self.assertFalse(file.deleted) + self.assertFalse(file.is_symlink) self.assertEqual(file.data.splitlines()[0], "diff --git a/testing b/testing") self.assertEqual(file.data.splitlines()[-1], "+ADD") @@ -94,6 +95,7 @@ class GitTests(SpyAgency, SCMTestCase): self.assertEqual(file.newInfo, 'bcae657') self.assertFalse(file.binary) self.assertFalse(file.deleted) + self.assertFalse(file.is_symlink) self.assertEqual(file.data.splitlines()[0], "diff --git a/testing b/testing") self.assertEqual(file.data.splitlines()[-1], "+ADD") @@ -122,6 +124,7 @@ class GitTests(SpyAgency, SCMTestCase): self.assertEqual(file.newInfo, '5e70b73') self.assertFalse(file.binary) self.assertFalse(file.deleted) + self.assertFalse(file.is_symlink) self.assertEqual(len(file.data), 249) self.assertEqual(file.data.splitlines()[0], "diff --git a/cfg/testcase.ini b/cfg/testcase.ini") @@ -152,6 +155,7 @@ class GitTests(SpyAgency, SCMTestCase): self.assertEqual(file.newInfo, '5e70b73') self.assertFalse(file.binary) self.assertFalse(file.deleted) + self.assertFalse(file.is_symlink) self.assertEqual(file.data.splitlines()[0].decode('utf-8'), 'diff --git a/cfg/téstcase.ini b/cfg/téstcase.ini') self.assertEqual(file.data.splitlines()[-1].decode('utf-8'), @@ -195,6 +199,7 @@ class GitTests(SpyAgency, SCMTestCase): self.assertEqual(file.newInfo, 'e69de29') self.assertFalse(file.binary) self.assertFalse(file.deleted) + self.assertFalse(file.is_symlink) self.assertEqual(len(file.data), 123) self.assertEqual(file.data.splitlines()[0], 'diff --git a/IAMNEW b/IAMNEW') @@ -215,6 +220,7 @@ class GitTests(SpyAgency, SCMTestCase): self.assertEqual(file.newInfo, 'e69de29') self.assertFalse(file.binary) self.assertFalse(file.deleted) + self.assertFalse(file.is_symlink) lines = file.data.splitlines() self.assertEqual(len(lines), 3) self.assertEqual(lines[0], 'diff --git a/newfile b/newfile') @@ -233,6 +239,7 @@ class GitTests(SpyAgency, SCMTestCase): self.assertEqual(files[0].newInfo, 'e69de29') self.assertFalse(files[0].binary) self.assertFalse(files[0].deleted) + self.assertFalse(files[0].is_symlink) lines = files[0].data.splitlines() self.assertEqual(len(lines), 3) self.assertEqual(lines[0], 'diff --git a/newfile b/newfile') @@ -262,6 +269,7 @@ class GitTests(SpyAgency, SCMTestCase): self.assertEqual(file.newInfo, '0000000') self.assertFalse(file.binary) self.assertTrue(file.deleted) + self.assertFalse(file.is_symlink) self.assertEqual(len(file.data), 132) self.assertEqual(file.data.splitlines()[0], 'diff --git a/OLDFILE b/OLDFILE') @@ -286,6 +294,7 @@ class GitTests(SpyAgency, SCMTestCase): '0000000000000000000000000000000000000000') self.assertFalse(files[0].binary) self.assertTrue(files[0].deleted) + self.assertFalse(files[0].is_symlink) self.assertEqual(len(files[0].data), 141) self.assertEqual(files[0].data.splitlines()[0], 'diff --git a/empty b/empty') @@ -319,6 +328,7 @@ class GitTests(SpyAgency, SCMTestCase): '0000000000000000000000000000000000000000') self.assertFalse(files[0].binary) self.assertTrue(files[0].deleted) + self.assertFalse(files[0].is_symlink) self.assertEqual(len(files[0].data), 141) self.assertEqual(files[0].data.splitlines()[0], 'diff --git a/empty b/empty') @@ -333,6 +343,7 @@ class GitTests(SpyAgency, SCMTestCase): '0ae4095ddfe7387d405bd53bd59bbb5d861114c5') self.assertFalse(files[1].binary) self.assertFalse(files[1].deleted) + self.assertFalse(files[1].is_symlink) lines = files[1].data.splitlines() self.assertEqual(len(lines), 7) self.assertEqual(lines[0], 'diff --git a/foo/bar b/foo/bar') @@ -351,6 +362,7 @@ class GitTests(SpyAgency, SCMTestCase): self.assertEqual(file.newInfo, '86b520c') self.assertTrue(file.binary) self.assertFalse(file.deleted) + self.assertFalse(file.is_symlink) lines = file.data.splitlines() self.assertEqual(len(lines), 4) self.assertEqual( @@ -371,6 +383,7 @@ class GitTests(SpyAgency, SCMTestCase): self.assertEqual(files[0].newInfo, 'e254ef4') self.assertFalse(files[0].binary) self.assertFalse(files[0].deleted) + self.assertFalse(files[0].is_symlink) self.assertEqual(files[0].insert_count, 2) self.assertEqual(files[0].delete_count, 1) self.assertEqual(len(files[0].data), 549) @@ -385,6 +398,7 @@ class GitTests(SpyAgency, SCMTestCase): self.assertEqual(files[1].newInfo, 'e69de29') self.assertFalse(files[1].binary) self.assertFalse(files[1].deleted) + self.assertFalse(files[1].is_symlink) self.assertEqual(files[1].insert_count, 0) self.assertEqual(files[1].delete_count, 0) lines = files[1].data.splitlines() @@ -398,6 +412,7 @@ class GitTests(SpyAgency, SCMTestCase): self.assertEqual(files[2].newInfo, 'e279a06') self.assertFalse(files[2].binary) self.assertFalse(files[2].deleted) + self.assertFalse(files[2].is_symlink) self.assertEqual(files[2].insert_count, 2) self.assertEqual(files[2].delete_count, 0) lines = files[2].data.splitlines() @@ -413,6 +428,7 @@ class GitTests(SpyAgency, SCMTestCase): self.assertEqual(files[3].newInfo, '86b520c') self.assertTrue(files[3].binary) self.assertFalse(files[3].deleted) + self.assertFalse(files[3].is_symlink) self.assertEqual(files[3].insert_count, 0) self.assertEqual(files[3].delete_count, 0) lines = files[3].data.splitlines() @@ -429,6 +445,7 @@ class GitTests(SpyAgency, SCMTestCase): self.assertEqual(files[4].newInfo, 'e254ef4') self.assertFalse(files[4].binary) self.assertFalse(files[4].deleted) + self.assertFalse(files[4].is_symlink) self.assertEqual(files[4].insert_count, 1) self.assertEqual(files[4].delete_count, 1) lines = files[4].data.splitlines() @@ -442,6 +459,7 @@ class GitTests(SpyAgency, SCMTestCase): self.assertEqual(files[5].newInfo, '0000000') self.assertFalse(files[5].binary) self.assertTrue(files[5].deleted) + self.assertFalse(files[5].is_symlink) self.assertEqual(files[5].insert_count, 0) self.assertEqual(files[5].delete_count, 1) lines = files[5].data.splitlines() @@ -455,6 +473,7 @@ class GitTests(SpyAgency, SCMTestCase): self.assertEqual(files[6].newInfo, 'e248ef4') self.assertFalse(files[6].binary) self.assertFalse(files[6].deleted) + self.assertFalse(files[6].is_symlink) self.assertEqual(files[6].insert_count, 1) self.assertEqual(files[6].delete_count, 1) lines = files[6].data.splitlines() @@ -500,12 +519,14 @@ class GitTests(SpyAgency, SCMTestCase): self.assertEqual(files[0].newFile, 'foo.bin') self.assertEqual(files[0].binary, True) self.assertEqual(files[0].deleted, True) + self.assertFalse(files[0].is_symlink) self.assertEqual(files[0].insert_count, 0) self.assertEqual(files[0].delete_count, 0) self.assertEqual(files[1].origFile, 'bar.bin') self.assertEqual(files[1].newFile, 'bar.bin') self.assertEqual(files[1].binary, True) self.assertEqual(files[1].deleted, True) + self.assertFalse(files[1].is_symlink) self.assertEqual(files[1].insert_count, 0) self.assertEqual(files[1].delete_count, 0) @@ -611,6 +632,7 @@ class GitTests(SpyAgency, SCMTestCase): self.assertEqual(f.delete_count, 0) self.assertFalse(f.moved) self.assertTrue(f.copied) + self.assertFalse(f.is_symlink) f = files[1] self.assertEqual(f.origFile, 'foo/bar') @@ -623,6 +645,7 @@ class GitTests(SpyAgency, SCMTestCase): self.assertEqual(f.delete_count, 1) self.assertTrue(f.moved) self.assertFalse(f.copied) + self.assertFalse(f.is_symlink) def test_parse_diff_with_mode_change_and_rename(self): """Testing Git diff parsing with mode change and rename""" @@ -653,6 +676,7 @@ class GitTests(SpyAgency, SCMTestCase): self.assertEqual(f.delete_count, 1) self.assertTrue(f.moved) self.assertFalse(f.copied) + self.assertFalse(f.is_symlink) def test_diff_git_line_without_a_b(self): """Testing parsing Git diff with deleted file without a/ and @@ -670,6 +694,7 @@ class GitTests(SpyAgency, SCMTestCase): self.assertEqual(f.origFile, 'foo') self.assertEqual(f.newFile, 'foo') self.assertTrue(f.deleted) + self.assertFalse(f.is_symlink) def test_diff_git_line_without_a_b_quotes(self): """Testing parsing Git diff with deleted file without a/ and @@ -687,6 +712,7 @@ class GitTests(SpyAgency, SCMTestCase): self.assertEqual(f.origFile, 'foo') self.assertEqual(f.newFile, 'foo') self.assertTrue(f.deleted) + self.assertFalse(f.is_symlink) def test_diff_git_line_without_a_b_and_spaces(self): """Testing parsing Git diff with deleted file without a/ and @@ -704,6 +730,7 @@ class GitTests(SpyAgency, SCMTestCase): self.assertEqual(f.origFile, 'foo bar1') self.assertEqual(f.newFile, 'foo bar1') self.assertTrue(f.deleted) + self.assertFalse(f.is_symlink) def test_diff_git_line_without_a_b_and_spaces_quotes(self): """Testing parsing Git diff with deleted file without a/ and @@ -761,6 +788,7 @@ class GitTests(SpyAgency, SCMTestCase): self.assertEqual(f.origFile, 'foo bar1') self.assertEqual(f.newFile, 'foo bar2') self.assertTrue(f.deleted) + self.assertFalse(f.is_symlink) f = files[1] self.assertEqual(f.origFile, 'foo bar1') @@ -770,6 +798,61 @@ class GitTests(SpyAgency, SCMTestCase): self.assertEqual(f.origFile, 'foo') self.assertEqual(f.newFile, 'foo bar1') + def test_diff_git_symlink_added(self): + """Testing parsing Git diff with symlink added""" + diff = (b'diff --git a/link b/link\n' + b'new file mode 120000\n' + b'index 0000000..100b938\n' + b'--- /dev/null\n' + b'+++ b/link\n' + b'@@ -0,0 +1 @@\n' + b'+README\n' + b'\\ No newline at end of file\n') + files = self.tool.get_parser(diff).parse() + self.assertEqual(len(files), 1) + + f = files[0] + self.assertEqual(f.origInfo, PRE_CREATION) + self.assertEqual(f.newFile, 'link') + self.assertTrue(f.is_symlink) + + def test_diff_git_symlink_changed(self): + """Testing parsing Git diff with symlink changed""" + diff = (b'diff --git a/link b/link\n' + b'index 100b937..100b938 120000\n' + b'--- a/link\n' + b'+++ b/link\n' + b'@@ -1 +1 @@\n' + b'-README\n' + b'\\ No newline at end of file\n' + b'+README.md\n' + b'\\ No newline at end of file\n') + files = self.tool.get_parser(diff).parse() + self.assertEqual(len(files), 1) + + f = files[0] + self.assertEqual(f.newFile, 'link') + self.assertEqual(f.origFile, 'link') + self.assertTrue(f.is_symlink) + + def test_diff_git_symlink_removed(self): + """Testing parsing Git diff with symlink removed""" + diff = (b'diff --git a/link b/link\n' + b'deleted file mode 120000\n' + b'index 100b938..0000000\n' + b'--- a/link\n' + b'+++ /dev/null\n' + b'@@ -1 +0,0 @@\n' + b'-README.txt\n' + b'\\ No newline at end of file\n') + files = self.tool.get_parser(diff).parse() + self.assertEqual(len(files), 1) + + f = files[0] + self.assertEqual(f.origFile, 'link') + self.assertTrue(f.deleted) + self.assertTrue(f.is_symlink) + def test_file_exists(self): """Testing GitTool.file_exists""" self.assertTrue(self.tool.file_exists('readme', 'e965047')) diff --git a/reviewboard/templates/diffviewer/diff_file_fragment.html b/reviewboard/templates/diffviewer/diff_file_fragment.html index 4ea38b1ca95b085e97964e844887f173fe4f09df..1b8f922eb6f86564910bb25f8aadab11ce82962d 100644 --- a/reviewboard/templates/diffviewer/diff_file_fragment.html +++ b/reviewboard/templates/diffviewer/diff_file_fragment.html @@ -72,6 +72,7 @@ {% endif %} {% endif %} {{file.depot_filename}} + {% if file.is_symlink %}{% trans " (symlink)" %}{% endif %} {% else %} {% if not file.is_new_file %} @@ -81,7 +82,7 @@ {% if file.is_new_file %} {% endif %} - {{file.dest_filename}}{% if file.moved %}{% trans " (moved)" %}{% elif file.copied %}{% trans " (copied)" %}{% endif %} + {{file.dest_filename}}{% if file.moved %}{% trans " (moved)" %}{% elif file.copied %}{% trans " (copied)" %}{% endif %}{% if file.is_symlink %}{% trans " (symlink)" %}{% endif %} {% endif %}{# file.dest_filename == file.depot_filename #}