Phabricator messes up my reviews by considering the first commit to be the closing one

Question

we are using Phabricator to performs pre-commit reviews & audits on a mercurial repo. Phabricator does not host the repo, it only observes it. The project was configured with auto-close, so until recently, pushing all commits of a validated review on the repository automatically closed the related revision. So far so good.

Today, our admin found out that a php lib was missing and installed it. It seems the deamon never work 'correctly' until now. However, from this moment, all existing reviews have been updated in a way that the very first commit of the review is the only one that can be seen. Any new review is fine until it is closed, where Phabricator writes:

Closed by commit R1:a9a9e1153022: doc: update changelog (authored by vsiles). · Explain WhyThu, Sep 7, 5:09 PM
This revision was automatically updated to reflect the committed changes.

(were a9a9e1153022 is the revision of the very first commit, but that's was always the case iirc). Now the whole review only shows the first commit, not all of them.

If we go to the 'History' tab in the review, we can see that Phabricator added an additional 'Diff' with the revision & timestamp of the first commit, after all the diff we pushed. This seems to be the problem. If we modify the history to show all commits but this one, we find back all our review content.

For the moment, my only work-around is to remove the 'auto-close' feature, and close reviews by hand using arc close-revision DXX. There is no more automatic step by Phabricator that messes things up, and the additional diff is not generated by Phabricator.

Can someone explains why Phabricator considers that the review is closed by the first commit, which seems to be the issue here ?

Answer 1

The recommended workflow for Phabricator is to use arc land to push and close the revision. Pushing using mercurial directly may not have the desired outcome because mercurial has no knowledge of Differential revisions, and Phabricator daemons are left to try to reconstruct what they can of the links between revisions and commits after the fact.