recaptime.dev / phorge
@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

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

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.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D20056

epriestley 6bcfc212 1767b806

+15 -8
+15 -8
src/applications/differential/engine/DifferentialDiffExtractionEngine.php
··· 177 177 'repository' => $repository, 178 178 )); 179 179 180 - $response = DiffusionQuery::callConduitWithDiffusionRequest( 181 - $viewer, 182 - $drequest, 183 - 'diffusion.filecontentquery', 184 - array( 185 - 'commit' => $identifier, 186 - 'path' => $path, 187 - )); 180 + try { 181 + $response = DiffusionQuery::callConduitWithDiffusionRequest( 182 + $viewer, 183 + $drequest, 184 + 'diffusion.filecontentquery', 185 + array( 186 + 'commit' => $identifier, 187 + 'path' => $path, 188 + )); 189 + } catch (Exception $ex) { 190 + // TODO: See PHI1044. This call may fail if the diff deleted the 191 + // file. If the call fails, just detect a change for now. This should 192 + // generally be made cleaner in the future. 193 + return true; 194 + } 188 195 189 196 $new_file_phid = $response['filePHID']; 190 197 if (!$new_file_phid) {