Page MenuHomePhabricator

Add support to marking commits as UNREACHABLE for Mercurial
ClosedPublic

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

Details

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
  1. I set up a hosted Mercurial repository.
  2. I removed the head commit from the on-disk repository state.
  3. I attempted to load the repository page and saw an exception due to a missing commit.
  4. I went to /manage for the repository and scheduled an update of the repository.
  5. 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.
  6. I checked the phd logs and verified there were no exceptions related to the repository.
  7. I ran the ./bin/repository mark-reachable command on the Mercurial repository and it reported that it marked the commit as unreachable.
  8. 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
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

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.

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

cspeckmim edited the summary of this revision. (Show Details)
cspeckmim edited the test plan for 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.

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

This revision is now accepted and ready to land.Sat, Sep 4, 7:18 PM