Page MenuHomePhabricator

Allow diff change detection to complete for Mercurial changes which remove a binary file
ClosedPublic

Authored by epriestley on Jan 29 2019, 5:15 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Apr 11, 10:29 AM
Unknown Object (File)
Fri, Apr 5, 10:19 AM
Unknown Object (File)
Tue, Apr 2, 1:17 AM
Unknown Object (File)
Mon, Apr 1, 12:35 PM
Unknown Object (File)
Mon, Apr 1, 12:33 AM
Unknown Object (File)
Wed, Mar 27, 5:21 PM
Unknown Object (File)
Wed, Mar 27, 5:21 PM
Unknown Object (File)
Wed, Mar 27, 5:21 PM
Subscribers
None

Details

Summary

See PHI1044. When you push a commit, we try to detect if the diff is the same as the last diff in Differential. This is a soft/heuristic check and only used to provide a hint in email ("CHANGED SINCE LAST DIFF").

For some changes, we can end up on this pathway with a binary changeset for a deleted file in the commit, and try to fetch the file data. This will fail since the file has been deleted. I'm not entirely sure why this hasn't cropped up before and didn't try to truly build an end-to-end test case, but it's a valid state that we shouldn't fatal in.

When we get here in this state, just detect a change. This is safe, even if it isn't always correct.

(This will be revisited in the future so I'm favoring fixing the broken behavior for now.)

Test Plan

This required some rigging, but I modified bin/differential extract to run the isDiffChangedBeforeCommit() code, then rigged a commit/diff to hit this case and reproduced the reported error. After the change, the comparison worked cleanly.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable