Strip symlink mode information from files on Git-style Mercurial diffs.

Review Request #12061 — Created Feb. 17, 2022 and submitted

Information

Review Board
release-4.0.x

Reviewers

Git-style Mercurial diffs, like normal Git diffs, contain symlink mode
information. We've hit issues in the past with the server applying
patches containing symlink modes.

Since a Git-style diff otherwise represents a symlink change as a
standard file change, with the content being the old/new target names,
we addressed this problem by stripping out the symlink mode and treating
it as a normal file when patching for display in the diff viewer.

This is by no means a perfect solution (that is in development, with
initial steps having been done in Review Board 4.0.5), but it addresses
the issue in Git.

This change brings that fix over to Mercurial when working with
Git-style diffs. The code in Git has been split out into a common
function that both can call, simplifying maintenance and allowing both
to benefit from any future bug fixes.

Unit tests pass.

Tested with a customer who hit this problem. Verified that they were able to
review symlinks in Mercurial.

Summary ID
Strip symlink mode information from files on Git-style Mercurial diffs.
Git-style Mercurial diffs, like normal Git diffs, contain symlink mode information. We've hit issues in the past with the server applying patches containing symlink modes. Since a Git-style diff otherwise represents a symlink change as a standard file change, with the content being the old/new target names, we addressed this problem by stripping out the symlink mode and treating it as a normal file when patching for display in the diff viewer. This is by no means a perfect solution (that is in development, with initial steps having been done in Review Board 4.0.5), but it addresses the issue in Git. This change brings that fix over to Mercurial when working with Git-style diffs. The code in Git has been split out into a common function that both can call, simplifying maintenance and allowing both to benefit from any future bug fixes.
b0f0b5324db02a8d20e78b2394f02b213be68794
Description From Last Updated

F401 'stat' imported but unused

reviewbotreviewbot

How does the performance of this particular usage compare to just using b'diff -r' in data?

daviddavid
Checks run (1 failed, 1 succeeded)
flake8 failed.
JSHint passed.

flake8

chipx86
david
  1. 
      
  2. reviewboard/scmtools/hg.py (Diff revision 2)
     
     
     
     
     
     
    Show all issues

    How does the performance of this particular usage compare to just using b'diff -r' in data?

    1. The important bit is the ordering. diff --git must be before a diff -r for it to be a git-style diff, and not after (it might be testing data within a diff).

      And that's the bigger problem. It doesn't account for whether things are at the beginning of a line, or data within a diff.

      This logic is not new, though, just moved. I don't feel comfortable redoing it for this change.

  3. 
      
david
  1. Ship It!
  2. 
      
chipx86
Review request changed
Status:
Completed
Change Summary:
Pushed to release-4.0.x (c567608)