diff --git a/rbtools/clients/svn.py b/rbtools/clients/svn.py index c132db0b07bb13a49a8dda0cf3339c47761e2376..cbd85e3271bebe0e6109d5bc1d79d082722dd3f6 100644 --- a/rbtools/clients/svn.py +++ b/rbtools/clients/svn.py @@ -42,7 +42,11 @@ class SVNClient(SCMClient): # Match the diff control lines generated by 'svn diff'. DIFF_ORIG_FILE_LINE_RE = re.compile(br'^---\s+.*\s+\(.*\)') DIFF_NEW_FILE_LINE_RE = re.compile(br'^\+\+\+\s+.*\s+\(.*\)') + DIFF_URL_ORIG_FILE_LINE_RE = re.compile(br'^---\s+(.*)\s+\(\.\.\.(.*)\)\s+\((.*)\)') + DIFF_URL_NEW_FILE_LINE_RE = re.compile(br'^\+\+\+\s+(.*)\s+\(\.\.\.(.*)\)\s+\((.*)\)') DIFF_COMPLETE_REMOVAL_RE = re.compile(br'^@@ -1,\d+ \+0,0 @@$') + DIFF_CHUNK_RE = re.compile(br'^@@ ') + DIFF_BINARY_RE = re.compile(br'^Cannot display: file marked as a binary type.') ADDED_FILES_RE = re.compile(br'^Index:\s+(\S+)\t\(added\)$', re.M) DELETED_FILES_RE = re.compile(br'^Index:\s+(\S+)\t\(deleted\)$', re.M) @@ -338,8 +342,6 @@ class SVNClient(SCMClient): empty_files_revisions['base'] = '(revision %s)' % base empty_files_revisions['tip'] = '(revision %s)' % tip - diff_cmd.extend(include_files) - # Check for and validate --svn-show-copies-as-adds option, or evaluate # working copy to determine if scheduled commit will contain # addition-with-history commit. When this case occurs then @@ -365,19 +367,69 @@ class SVNClient(SCMClient): if svn_show_copies_as_adds in 'Yy': diff_cmd.append("--show-copies-as-adds") - diff = self._run_svn(diff_cmd, split_lines=True, results_unicode=False, - log_output_on_error=False) + diff = self._run_svn(diff_cmd + include_files, split_lines=True, + results_unicode=False, log_output_on_error=False) diff = self.handle_renames(diff) if self.supports_empty_files(): - diff = self._handle_empty_files(diff, diff_cmd, + diff = self._handle_empty_files(diff, diff_cmd + include_files, empty_files_revisions) diff = self.convert_to_absolute_paths(diff, repository_info) + if getattr(self.options, 'svn_diff_origin', None): + # Sort by string length - this makes more specific patterns to + # be processed after the less specific ones. + origins = sorted(self.options.svn_diff_origin, + lambda x, y: len(x[0]) - len(y[0])) + for path, origin in origins: + logging.debug("Replacing diff origin for [%s] from [%s]" % + (path, origin)) + info = self.svn_info(path) + # Filter out previously generated diff content; it will be + # replaced with a diff against a different origin. + if info["Node Kind"] == "directory": + if path != ".": + exclude = [ posixpath.join(path, "*") ] + else: + exclude = [ "*" ] + else: + exclude = [ path ] + exclude = normalize_patterns(exclude, + '/', + repository_info.base_path) + diff = filter_diff(diff, self.INDEX_FILE_RE, exclude) + if origin == "": + # Revert to normal behavior; re-diff path + diff2_cmd = diff_cmd + [ path ] + else: + # Generate the patch as requested. --notice-ancestry here + # causes SVN to erroneously diff 'added with history' + # files ("A +") as if they were deleted instead. + diff2_cmd = [ 'diff', '--diff-cmd=diff', origin, path ] + diff2 = self._run_svn(diff2_cmd, + split_lines=True, + results_unicode=False, + log_output_on_error=False) + # No need to handle renames when diffing against a repository + # path - it already has proper source indicated. But, it uses + # a different format for the from/to diff lines. + if origin == "": + diff2 = self.handle_renames(diff2) + if self.supports_empty_files(): + diff2 = self._handle_empty_files(diff2, + diff2_cmd, + empty_files_revisions) + else: + diff2 = self.handle_url_diff(diff2, path) + diff2 = self.convert_to_absolute_paths(diff2, repository_info) + diff = list(diff) + list(diff2) + if exclude_patterns: diff = filter_diff(diff, self.INDEX_FILE_RE, exclude_patterns) + diff = self.filter_nocontent(diff) + return { 'diff': b''.join(diff), } @@ -501,6 +553,8 @@ class SVNClient(SCMClient): result.append(from_line) result.append(to_line) else: + # Note that we are only interested in the file name - the + # revision information is already correct in the header. to_file, _ = self.parse_filename_header(to_line[4:]) copied_from = self.find_copyfrom(to_file) if copied_from is not None: @@ -516,6 +570,85 @@ class SVNClient(SCMClient): return result + def handle_url_diff(self, diff_content, path): + """ + When diffing a working copy against a URL, the format of the from/to + lines is different. Convert them to the regular format that RB server + expects. + + The format produced by 'svn diff' in this case is (path = 'foo'): + --- foo/bar/x.txt (.../vendor/foo/1.0) (revision 1000) + +++ foo/bar/x.txt (.../trunk/foo) (working copy) + """ + result = [] + + from_path = None + to_path = None + for line in diff_content: + # The regexps have three groups, for path, base dir and revision info + m = self.DIFF_URL_ORIG_FILE_LINE_RE.match(line) + if m: + from_path, from_base, from_rev = m.groups() + if path != ".": + from_path = from_path[len(path) + 1:] + continue + + m = self.DIFF_URL_NEW_FILE_LINE_RE.match(line) + if m: + to_path, to_base, to_rev = m.groups() + if path != ".": + to_path = to_path[len(path) + 1:] + continue + + # This is where we decide how mangle the previous '--- ' and '+++ ' + if from_path and to_path: + result.append("--- %s\t(%s)\n" % + (posixpath.join(from_base, from_path), from_rev)) + result.append("+++ %s\t(%s)\n" % + (posixpath.join(to_base, to_path), to_rev)) + from_path = to_path = None + result.append(line) + + return result + + def filter_nocontent(self, diff_content): + """Removes files without any meaningful diff. + + With SVN-generated diff, it is easy for the patch to exceed any given + patch size limitation set by RB or by the backing SQL server. Thus, + clean up the patch a bit. + + First, the property diffs are not interesting to the ReviewBoard. Newer + SVN can turn them off using --ignore-properties switch to the diff, but + older releases (1.7.x) cannot. + + Second, SVN 1.9.x started to generate empty chunks ("Index: ...") + followed by the separator line for every file, even if there is no + diff generated. + """ + result = [] + hold = [] + for line in diff_content: + if len(hold): + # Holding up until we see a diff chunk + if self.INDEX_FILE_RE.match(line): + # Previously held content did not yield anything valuable + hold = [ line ] + else: + hold.append(line) + if self.DIFF_CHUNK_RE.match(line) or \ + self.DIFF_BINARY_RE.match(line): + result.extend(hold) + hold = [] + else: + # Passing through until the next Index: + if self.INDEX_FILE_RE.match(line): + hold = [ line ] + else: + result.append(line) + return result + + def _handle_empty_files(self, diff_content, diff_cmd, revisions): """Handles added and deleted 0-length files in the diff output. diff --git a/rbtools/commands/__init__.py b/rbtools/commands/__init__.py index e25367929844651eae2b444c0efce6e1c26ca849..6e734c0e5333d42d4a4b8ad7615563804185504a 100644 --- a/rbtools/commands/__init__.py +++ b/rbtools/commands/__init__.py @@ -506,6 +506,19 @@ class Command(object): help='Generates the diff for review based on a ' 'local changelist.', deprecated_in='0.6'), + Option('--svn-diff-origin', + dest='svn_diff_origin', + default=None, + action='append', + nargs=2, + metavar=('PATH','SOURCE-URL'), + help='Diffs DIR against the SOURCE-URL instead of ' + 'its normal origin; useful when posting diffs of ' + 'merges from a vendor branch. This can be specified ' + 'multiple times to specify multiple paths; for any ' + 'given changed file, most specific origin will be ' + 'used. Specifying empty string as SOURCE-URL ' + 'will diff PATH against the default origin.'), ] )