Add support to marking commits as UNREACHABLE for Mercurial
ClosedPublic
Actions

Authored by cspeckmim on Sep 3 2021, 2:55 AM.

Details

Reviewers

epriestley

Group Reviewers

Blessed Reviewers

Maniphest Tasks

T13634: Support marking commits as UNREACHABLE in Mercurial

Commits

rP09c3c7d87931: Add support to marking commits as UNREACHABLE for Mercurial

Summary

When previously known commits have been destroyed in a Mercurial repository, Phabricator does not end up marking the commits as unreachable. This results in daemon tasks which continuously fail and retry.

This updates PhabricatorRepositoryDiscoveryEngine and PhabricatorManagementRepositoryMarkReachableWorkflow to include support of marking commits as unreachable for Mercurial repositories.

The PhabricatorMercurialGraphStream also needed updated to support a stream with no starting commit.

Refs T13634

Test Plan

I set up a hosted Mercurial repository.
I removed the head commit from the on-disk repository state.
I attempted to load the repository page and saw an exception due to a missing commit.
I went to /manage for the repository and scheduled an update of the repository.
After an updated performed, I went to the repository main page and saw there was no exception and the history view properly did not have the commit I had removed.
I checked the phd logs and verified there were no exceptions related to the repository.
I ran the ./bin/repository mark-reachable command on the Mercurial repository and it reported that it marked the commit as unreachable.
I pushed the same commit back upstream and verified that the commit was found and displayed in the history view of the repository page and mark-unreachable did not identify it as being unreachable.

Diff Detail

Repository

rP Phabricator

Branch

unreachgable

Lint

Lint Passed

Unit

Tests Skipped

Build Status

Buildable 25537
Build 35311: Run Core Tests
Build 35310: arc lint + arc unit

Event Timeline

cspeckmim created this revision.Sep 3 2021, 2:55 AM

cspeckmim held this revision as a draft.

Herald added a reviewer: Blessed Reviewers. · View Herald TranscriptSep 3 2021, 2:55 AM

Herald added a subscriber: epriestley. · View Herald Transcript

Harbormaster completed remote builds in B25535: Diff 51737.Sep 3 2021, 2:56 AM

I think this is a bit more involved in Mercurial, I'll follow up in T13634 with particulars.

src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php
782–783	You could just remove this comment now; it's no longer accurate, and this operation isn't really sensible in SVN so it's now moot.

I initially thought the "window" in T13634 was a bit wider in Mercurial than in Git (that is, it was more likely that real users could get into an inconsistent state by doing plausible sorts of things), but after completing the writeup, I think I've convinced myself that it's of a similar size or perhaps smaller.

So I think this change is fine as-is and I'm happy to accept it once you're comfortable with it, although I'd recommend also making a similar change to PhabricatorRepositoryManagementMarkReachableWorkflow so that bin/repository mark-reachable can run on Mercurial repositories.

epriestley mentioned this in T13634: Support marking commits as UNREACHABLE in Mercurial.Sep 3 2021, 3:39 AM

So I think this change is fine as-is and I'm happy to accept it once you're comfortable with it, although I'd recommend also making a similar change to PhabricatorRepositoryManagementMarkReachableWorkflow so that bin/repository mark-reachable can run on Mercurial repositories.

Ah yea I’ll look at updating that too. I haven’t yet tested this change; I don’t yet have a development environment set up for Phab after my last hardware update a few years back - all the small server-side changes I’ve been verifying on our shared test instance.

src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php
782–783	🤦‍♂️

Updating the mark-reachable workflow to support Mercurial repositories as well

Harbormaster completed remote builds in B25536: Diff 51738.Sep 3 2021, 2:32 PM

Possibly better phrasing

Harbormaster completed remote builds in B25537: Diff 51739.Sep 3 2021, 2:54 PM

cspeckmim published this revision for review.Sep 3 2021, 9:44 PM

cspeckmim edited the summary of this revision. (Show Details)

cspeckmim edited the test plan for this revision. (Show Details)

I attempted to load the repository page and saw an exception due to a missing commit.

I don't think this is expected behavior but I'll make a new task for it since it's outside the scope of this change.

In D21715#276590, @cspeckmim wrote:

I attempted to load the repository page and saw an exception due to a missing commit.

I don't think this is expected behavior but I'll make a new task for it since it's outside the scope of this change.

Created T13666