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
F18853274: D20545.id.diff
Fri, Oct 31, 3:31 PM
F18848094: D20545.diff
Thu, Oct 30, 3:21 AM
F18838306: D20545.id.diff
Mon, Oct 27, 10:11 AM
F18762972: D20545.id49008.diff
Mon, Oct 6, 9:54 PM
F18758565: D20545.id49018.diff
Sun, Oct 5, 10:45 PM
F18710862: D20545.diff
Sep 29 2025, 3:01 AM
F18632243: D20545.id49018.diff
Sep 16 2025, 5:29 PM
F18631476: D20545.id49018.diff
Sep 16 2025, 3:28 PM
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
Lint
Lint Not Applicable
Unit
Tests Not Applicable