(stable) Promote 2018 Week 25

src/applications/config/controller/PhabricatorConfigEditController.php

@@ -231,7 +231,6 @@ final class PhabricatorConfigEditController
     $box_header[] = $key;
 
     $crumbs = $this->buildApplicationCrumbs();
-    $crumbs->addTextCrumb(pht('Config'), $this->getApplicationURI());
     if ($group) {
       $crumbs->addTextCrumb($group->getName(), $group_uri);
     }

src/applications/config/controller/PhabricatorConfigGroupController.php

@@ -30,7 +30,7 @@ final class PhabricatorConfigGroupController
     $view = $this->buildConfigBoxView($box_header, $list);
 
     $crumbs = $this->buildApplicationCrumbs()
-      ->addTextCrumb($group_name, $this->getApplicationURI($group_uri))
+      ->addTextCrumb($group_name, $group_uri)
       ->addTextCrumb($options->getName())
       ->setBorder(true);
 

src/applications/diffusion/conduit/DiffusionBranchQueryConduitAPIMethod.php

@@ -30,18 +30,31 @@ final class DiffusionBranchQueryConduitAPIMethod
 
     $contains = $request->getValue('contains');
     if (strlen($contains)) {
+
+      // See PHI720. If the standard "branch" field is provided, use it
+      // as the "pattern" argument to "git branch ..." to let callers test
+      // for reachability from a particular branch head.
+      $pattern = $request->getValue('branch');
+      if (strlen($pattern)) {
+        $pattern_argv = array($pattern);
+      } else {
+        $pattern_argv = array();
+      }
+
       // NOTE: We can't use DiffusionLowLevelGitRefQuery here because
       // `git for-each-ref` does not support `--contains`.
       if ($repository->isWorkingCopyBare()) {
         list($stdout) = $repository->execxLocalCommand(
-          'branch --verbose --no-abbrev --contains %s --',
-          $contains);
+          'branch --verbose --no-abbrev --contains %s -- %Ls',
+          $contains,
+          $pattern_argv);
         $ref_map = DiffusionGitBranch::parseLocalBranchOutput(
           $stdout);
       } else {
         list($stdout) = $repository->execxLocalCommand(
-          'branch -r --verbose --no-abbrev --contains %s --',
-          $contains);
+          'branch -r --verbose --no-abbrev --contains %s -- %Ls',
+          $contains,
+          $pattern_argv);
         $ref_map = DiffusionGitBranch::parseRemoteBranchOutput(
           $stdout,
           DiffusionGitBranch::DEFAULT_GIT_REMOTE);

src/applications/diffusion/controller/DiffusionCommitBranchesController.php

@@ -22,6 +22,7 @@ final class DiffusionCommitBranchesController extends DiffusionController {
         array(
           'contains' => $drequest->getCommit(),
           'limit' => $branch_limit + 1,
+          'branch' => null,
         )));
 
     $has_more_branches = (count($branches) > $branch_limit);

src/applications/diffusion/engine/DiffusionCommitHookEngine.php

@@ -1212,7 +1212,7 @@ final class DiffusionCommitHookEngine extends Phobject {
 
     if (!strlen($raw_diff)) {
       // If the commit is actually empty, just return no changesets.
-      return array();
+      return array(array(), 0);
     }
 
     $parser = new ArcanistDiffParser();

src/applications/diffusion/query/DiffusionCommitQuery.php

@@ -26,6 +26,7 @@ final class DiffusionCommitQuery
 
   private $needCommitData;
   private $needDrafts;
+  private $needIdentities;
 
   private $mustFilterRefs = false;
   private $refRepository;
@@ -110,6 +111,11 @@ final class DiffusionCommitQuery
     return $this;
   }
 
+  public function needIdentities($need) {
+    $this->needIdentities = $need;
+    return $this;
+  }
+
   public function needAuditRequests($need) {
     $this->needAuditRequests = $need;
     return $this;
@@ -393,6 +399,24 @@ final class DiffusionCommitQuery
       }
     }
 
+    if ($this->needIdentities) {
+      $identity_phids = array_merge(
+        mpull($commits, 'getAuthorIdentityPHID'),
+        mpull($commits, 'getCommitterIdentityPHID'));
+
+      $data = id(new PhabricatorRepositoryIdentityQuery())
+        ->withPHIDs($identity_phids)
+        ->setViewer($this->getViewer())
+        ->execute();
+      $data = mpull($data, null, 'getPHID');
+
+      foreach ($commits as $commit) {
+        $author_identity = idx($data, $commit->getAuthorIdentityPHID());
+        $committer_identity = idx($data, $commit->getCommitterIdentityPHID());
+        $commit->attachIdentities($author_identity, $committer_identity);
+      }
+    }
+
     if ($this->needDrafts) {
       PhabricatorDraftEngine::attachDrafts(
         $viewer,

src/applications/repository/phid/PhabricatorRepositoryIdentityPHIDType.php

@@ -28,6 +28,18 @@ final class PhabricatorRepositoryIdentityPHIDType
   public function loadHandles(
     PhabricatorHandleQuery $query,
     array $handles,
-    array $objects) {}
+    array $objects) {
+
+    foreach ($handles as $phid => $handle) {
+      $identity = $objects[$phid];
+
+      $id = $identity->getID();
+      $name = $identity->getIdentityNameRaw();
+
+      $handle->setObjectName(pht('Identity %d', $id));
+      $handle->setName($name);
+      $handle->setURI($identity->getURI());
+    }
+  }
 
 }

src/applications/repository/query/PhabricatorRepositoryIdentityQuery.php

@@ -124,6 +124,29 @@ final class PhabricatorRepositoryIdentityQuery
     return $where;
   }
 
+  protected function didFilterPage(array $identities) {
+    $user_ids = array_filter(
+      mpull($identities, 'getCurrentEffectiveUserPHID', 'getID'));
+    if (!$user_ids) {
+      return $identities;
+    }
+
+    $users = id(new PhabricatorPeopleQuery())
+      ->withPHIDs($user_ids)
+      ->setViewer($this->getViewer())
+      ->execute();
+    $users = mpull($users, null, 'getPHID');
+
+    foreach ($identities as $identity) {
+      if ($identity->hasEffectiveUser()) {
+        $user = idx($users, $identity->getCurrentEffectiveUserPHID());
+        $identity->attachEffectiveUser($user);
+      }
+    }
+
+    return $identities;
+  }
+
   public function getQueryApplicationClass() {
     return 'PhabricatorDiffusionApplication';
   }

src/applications/repository/storage/PhabricatorRepositoryCommit.php

@@ -44,6 +44,9 @@ final class PhabricatorRepositoryCommit
   private $audits = self::ATTACHABLE;
   private $repository = self::ATTACHABLE;
   private $customFields = self::ATTACHABLE;
+  private $authorIdentity = self::ATTACHABLE;
+  private $committerIdentity = self::ATTACHABLE;
+
   private $drafts = array();
   private $auditAuthorityPHIDs = array();
 
@@ -191,6 +194,22 @@ final class PhabricatorRepositoryCommit
     return ($this->audits !== self::ATTACHABLE);
   }
 
+  public function attachIdentities(
+    PhabricatorRepositoryIdentity $author = null,
+    PhabricatorRepositoryIdentity $committer = null) {
+
+    $this->authorIdentity = $author;
+    $this->committerIdentity = $committer;
+  }
+
+  public function getAuthorIdentity() {
+    return $this->assertAttached($this->authorIdentity);
+  }
+
+  public function getCommiterIdentity() {
+    return $this->assertAttached($this->committerIdentity);
+  }
+
   public function loadAndAttachAuditAuthority(
     PhabricatorUser $viewer,
     $actor_phid = null) {

src/applications/repository/storage/PhabricatorRepositoryIdentity.php

@@ -14,6 +14,17 @@ final class PhabricatorRepositoryIdentity
   protected $manuallySetUserPHID;
   protected $currentEffectiveUserPHID;
 
+  private $effectiveUser = self::ATTACHABLE;
+
+  public function attachEffectiveUser(PhabricatorUser $user) {
+    $this->effectiveUser = $user;
+    return $this;
+  }
+
+  public function getEffectiveUser() {
+    return $this->assertAttached($this->effectiveUser);
+  }
+
   protected function getConfiguration() {
     return array(
       self::CONFIG_AUX_PHID => true,
@@ -63,6 +74,10 @@ final class PhabricatorRepositoryIdentity
     return '/diffusion/identity/view/'.$this->getID().'/';
   }
 
+  public function hasEffectiveUser() {
+    return ($this->currentEffectiveUserPHID != null);
+  }
+
   public function save() {
     if ($this->manuallySetUserPHID) {
       $this->currentEffectiveUserPHID = $this->manuallySetUserPHID;

src/applications/search/worker/PhabricatorSearchWorker.php

@@ -15,6 +15,7 @@ final class PhabricatorSearchWorker extends PhabricatorWorker {
       ),
       array(
         'priority' => parent::PRIORITY_IMPORT,
+        'objectPHID' => $phid,
       ));
   }
 
@@ -37,7 +38,15 @@ final class PhabricatorSearchWorker extends PhabricatorWorker {
     $key = "index.{$object_phid}";
     $lock = PhabricatorGlobalLock::newLock($key);
 
+    try {
       $lock->lock(1);
+    } catch (PhutilLockException $ex) {
+      // If we fail to acquire the lock, just yield. It's expected that we may
+      // contend on this lock occasionally if a large object receives many
+      // updates in a short period of time, and it's appropriate to just retry
+      // rebuilding the index later.
+      throw new PhabricatorWorkerYieldException(15);
+    }
 
     try {
       // Reload the object now that we have a lock, to make sure we have the

src/applications/transactions/conduit/TransactionSearchConduitAPIMethod.php

@@ -205,6 +205,9 @@ final class TransactionSearchConduitAPIMethod
           case PhabricatorTransactions::TYPE_COMMENT:
             $type = 'comment';
             break;
+          case PhabricatorTransactions::TYPE_CREATE:
+            $type = 'create';
+            break;
         }
       }
 

src/applications/transactions/engineextension/PhabricatorTransactionsFulltextEngineExtension.php

@@ -22,12 +22,28 @@ final class PhabricatorTransactionsFulltextEngineExtension
       return;
     }
 
-    $xactions = $query
+    $query
       ->setViewer($this->getViewer())
       ->withObjectPHIDs(array($object->getPHID()))
       ->withComments(true)
-      ->needComments(true)
-      ->execute();
+      ->needComments(true);
+
+    // See PHI719. Users occasionally create objects with huge numbers of
+    // comments, which can be slow to index. We handle this with reasonable
+    // grace: at time of writing, we can index a task with 100K comments in
+    // about 30 seconds. However, we do need to hold all the comments in
+    // memory in the AbstractDocument, so there's some practical limit to what
+    // we can realistically index.
+
+    // Since objects with more than 1,000 comments are not likely to be
+    // legitimate objects with actual discussion, index only the first
+    // thousand comments.
+
+    $query
+      ->setOrderVector(array('-id'))
+      ->setLimit(1000);
+
+    $xactions = $query->execute();
 
     foreach ($xactions as $xaction) {
       if (!$xaction->hasComment()) {