Page MenuHomePhabricator

Drop the "update revision with commit diff" transaction if the revision is already closed
ClosedPublic

Authored by epriestley on May 22 2019, 9:31 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Nov 19, 9:46 PM
Unknown Object (File)
Sat, Nov 9, 4:03 PM
Unknown Object (File)
Tue, Nov 5, 6:23 PM
Unknown Object (File)
Mon, Nov 4, 10:43 AM
Unknown Object (File)
Mon, Nov 4, 10:43 AM
Unknown Object (File)
Mon, Nov 4, 10:43 AM
Unknown Object (File)
Sun, Nov 3, 1:58 AM
Unknown Object (File)
Oct 26 2024, 3:28 AM
Subscribers
None

Details

Summary

Ref T13290. Prior to recent changes, if we parsed some commit C which was associated with a revision R, but R was already closed, we'd skip the whole set of updates because the "close the revision" transaction would fail and we'd throw because we did not setContinueOnNoEffect().

We now continue on no effect so we can get the edge ("commit has revision" / "revision has commit"), since we want it in all cases, but this means we may also apply an extra "Updated revision to reflect committed changes" transaction and new diff. This can happen even if we're careful about not trying to apply this transaction to closed revisions, since two workers may race. (Today, we aren't too careful about this.)

To fix this, just make this transaction no-op itself if the revision is already closed by the time it tries to apply.

This happened on D20451 because a merge commit with the same hash as the last diff was pushed, but it's easiest to reproduce by just running bin/repository reparse --message <commit>, which updates related revisions with a new diff every time.

Test Plan
  • Ran bin/repository reparse --messsage <commit> several times, on a commit with an associated revision.
  • Before: each run attached a new diff and created a new "updated to reflect committed changes" transaction.
  • After: repeated runs had no effects.

Diff Detail

Repository
rP Phabricator
Branch
updatediff1
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 22884
Build 31392: Run Core Tests
Build 31391: arc lint + arc unit