Make SVN revision parsing a little more complete and reliable.

Review Request #13580 — Created Feb. 27, 2024 and submitted — Latest diff uploaded

Information

Review Board
release-7.x

Reviewers

The way we deal with parsing SVN revisions had two major problems.

First, SVN diffs traditionally didn't have any revision information for
binary files. I'm making a change in RBTools to add the --force
argument to svn diff, which will end up including that in the "Binary
files ... differ" line, but that needed to be added to the parser.

The more general problem is that we didn't have any parsed revision
information for the modified version of the file. This meant that
ParsedDiffFile.modified_file_details / FileDiff.dest_detail would
just be whatever the raw string was in the diff, whether it was a real
revision, "(nonexistent)", or "(working copy)".

Our current design of the way diffs parse is that the SCMTool's
DiffParser is responsible for most of the actual parsing. The
create_filediffs method would then call the tool's
parse_diff_revision method to do further parsing on the filename and
revision, but only for the original file/revision, not the modified.

This change moves things around so that all parsing happens within the
SVNDiffParser class. We do the same revision parsing as before, but
it's now applied to both the original and the modified versions. We can
also then make some better determinations about how to handle things
like IntelliJ's awkward revisionless revision labels, since we can
process that when we have the full context of the file header instead of
acting on a filename/revision in isolation without knowing whether it's
for the original or modified file.

My thinking is that this should become the general pattern. We should
have a goal of basically deprecating SCMTool.parse_diff_revision
entirely, and move all revision parsing into the diff parsers.

  • Ran unit tests.
  • Posted a variety of changes, including some with binary files in both
    the working copy and in revision ranges. Saw everything get parsed and
    displayed correctly.

Commits

Files

    Loading...