1827632 - Lando uplifts don't handle copies correctly

Status: RESOLVED FIXED

(Conduit :: Lando, defect)

Description

[Connor Sheehan [:sheehan]]

Lando uplifts are incorrectly marking file copies as file renames.

• https://phabricator.services.mozilla.com/D174915 is the original patch.
• https://phabricator.services.mozilla.com/D175240 is the uplift patch.
• You can see from the files view that the uplift patch and the original patch differ in that test_videocontrols_scrubber_position.html is a copy in the original but is moved in the uplift.

I jumped into an ipython REPL to try and reproduce the problem. I pulled the diff of the original patch down from Phabricator using the differential.getrawdiff API endpoint. Then I passed it into rs_parsepatch.get_diffs(diff, hunks=True) in the same way Lando does. I noticed the returned diffs from rs_parsepatch have the filename: 'toolkit/content/tests/widgets/test_videocontrols_scrubber_position_nopreload.html' entry having 'copied_from': None and 'renamed_from': 'toolkit/content/tests/widgets/test_videocontrols_scrubber_position.html'.

I believe this issue originates in rs_parsepatch's handling of file copies.

Comment 1

[Calixte Denizet (:calixte)]

I think the problem is because of:
https://github.com/mozilla/rust-parsepatch/blob/b3dbb48ecc73de3fecea20c7e8848a3fc2239772/src/parser.rs#L441
where the passed operation is FileOp::Renamed but in considering the if before:
https://github.com/mozilla/rust-parsepatch/blob/b3dbb48ecc73de3fecea20c7e8848a3fc2239772/src/parser.rs#L417
we should likely pass op instead.

Comment 2

[Calixte Denizet (:calixte)]

Attachment: GitHub Pull Request

Comment 3

[BMO Github Automation]

Attachment: [mozilla-conduit/lando-api] requirements: update `rs-parsepatch` to 0.3.9 (Bug 1827632) (#303)

Comment 4

[Connor Sheehan [:sheehan]]

This will be deployed tomorrow.