Page MenuHomePhabricator

arc diff errors when a milestone is a group reviewer
Closed, ResolvedPublic


Reproduction Steps

  1. Create a new project ("Get Your Life Together")
  2. Add a milestone to the project ("Learn How Money Works")
  3. Create a revision on any repo using arc diff
  4. Through the web UI, add the milestone as a Reviewer:

image.png (78ร—444 px, 10 KB)

  1. Save the revision
  2. Run arc diff again. You'll be greeted with the following error:
Error parsing field "Reviewers": The objects you have listed include objects which do not exist (Get, Your, Life, Together, (Learn, How, Money, Works)).
(Run with `--trace` for a full exception trace.)

At this point the only way to recover and be able to run arc diff again is to remove the milestone reviewer via the web UI.
Running with --trace shows that the error is thrown on a call to /api/differential.parsecommitmessage

Version Information

phabricator d2baa88171fcc3f327b157277afb905430b5a918 (Thu, Apr 27) 
arcanist 27b51e619237116d99e03fc1b3e8cd6399087214 (Fri, Apr 28) 
phutil a900d7b63e954e221efe140f0f33d3d701524aae (Sun, Apr 23)

Event Timeline

See also vaguely related T12505.

I think the issue here is that the project has no hashtag (by default, milestones do not have a hashtag). Elsewhere, in Owners, we fall back to O123: fulll name which we ignore and just parse the monogram. Projects don't have a similar monogram, and allow commas in their names. In this case we should probably just list the raw PHID, like we do in cases where you don't have permission to see a reviewer, since this will at least make stuff work as expected. Users can go give the milestone a hashtag if they want to improve this, or we could eventually refine it (I think the idea of #parent/25 to indicate "25th milestone of #parent" was tossed around originally).

This is probably the fix, if you want to test it and diff it before I do:

diff --git a/src/applications/project/phid/PhabricatorProjectProjectPHIDType.php b/src/applications/project/phid/PhabricatorProjectProjectPHIDType.php
index 7a04103a7c..c3ea3b599b 100644
--- a/src/applications/project/phid/PhabricatorProjectProjectPHIDType.php
+++ b/src/applications/project/phid/PhabricatorProjectProjectPHIDType.php
@@ -47,6 +47,8 @@ final class PhabricatorProjectProjectPHIDType extends PhabricatorPHIDType {
       } else {
+        // Blah blah blah, see T12659.
+        $handle->setCommandLineObjectName($project->getPHID());