Page MenuHomePhabricator

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

Authored by epriestley on Jan 29 2019, 5:15 PM.



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

rP Phabricator
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

epriestley created this revision.Jan 29 2019, 5:15 PM
epriestley requested review of this revision.Jan 29 2019, 5:16 PM
amckinley accepted this revision.Jan 31 2019, 2:22 AM
This revision is now accepted and ready to land.Jan 31 2019, 2:22 AM
This revision was automatically updated to reflect the committed changes.