(stable) Promote 2018 Week 39

resources/celerity/map.php

@@ -146,7 +146,7 @@ return array(
     'rsrc/css/phui/phui-comment-panel.css' => 'f50152ad',
     'rsrc/css/phui/phui-crumbs-view.css' => '10728aaa',
     'rsrc/css/phui/phui-curtain-view.css' => '2bdaf026',
-    'rsrc/css/phui/phui-document-pro.css' => 'd033e8d5',
+    'rsrc/css/phui/phui-document-pro.css' => 'dd79b5df',
     'rsrc/css/phui/phui-document-summary.css' => '9ca48bdf',
     'rsrc/css/phui/phui-document.css' => 'c4ac41f9',
     'rsrc/css/phui/phui-feed-story.css' => '44a9c8e9',
@@ -814,7 +814,7 @@ return array(
     'phui-curtain-view-css' => '2bdaf026',
     'phui-document-summary-view-css' => '9ca48bdf',
     'phui-document-view-css' => 'c4ac41f9',
-    'phui-document-view-pro-css' => 'd033e8d5',
+    'phui-document-view-pro-css' => 'dd79b5df',
     'phui-feed-story-css' => '44a9c8e9',
     'phui-font-icon-base-css' => '870a7360',
     'phui-fontkit-css' => '1320ed01',

src/applications/auth/factor/PhabricatorTOTPAuthFactor.php

@@ -154,6 +154,7 @@ final class PhabricatorTOTPAuthFactor extends PhabricatorAuthFactor {
       id(new PHUIFormNumberControl())
         ->setName($this->getParameterName($config, 'totpcode'))
         ->setLabel(pht('App Code'))
+        ->setDisableAutocomplete(true)
         ->setCaption(pht('Factor Name: %s', $config->getFactorName()))
         ->setValue(idx($validation_result, 'value'))
         ->setError(idx($validation_result, 'error', true)));

src/applications/differential/mail/DifferentialInlineCommentMailView.php

@@ -345,7 +345,13 @@ final class DifferentialInlineCommentMailView
       $offset_mode = 'old';
     }
 
+    // See PHI894. Use the parse cache since we can end up with a large
+    // rendering cost otherwise when users or bots leave hundreds of inline
+    // comments on diffs with long recipient lists.
+    $cache_key = $changeset->getID();
+
     $parser = id(new DifferentialChangesetParser())
+      ->setRenderCacheKey($cache_key)
       ->setUser($viewer)
       ->setChangeset($changeset)
       ->setOffsetMode($offset_mode)

src/applications/diffusion/controller/DiffusionBlameController.php

@@ -118,10 +118,6 @@ final class DiffusionBlameController extends DiffusionController {
         $author_phid = $commit->getAuthorDisplayPHID();
       }
 
-      if (!$author_phid && $revision) {
-        $author_phid = $revision->getAuthorPHID();
-      }
-
       if (!$author_phid) {
         // This means we couldn't identify an author for the commit or the
         // revision. We just render a blank for alignment.
@@ -137,6 +133,7 @@ final class DiffusionBlameController extends DiffusionController {
         $author_meta = array(
           'tip' => $handles[$author_phid]->getName(),
           'align' => 'E',
+          'size'  => 'auto',
         );
       }
 

src/applications/diffusion/protocol/DiffusionRepositoryClusterEngine.php

@@ -170,12 +170,13 @@ final class DiffusionRepositoryClusterEngine extends Phobject {
           pht(
             'Acquired read lock immediately.'));
       }
-    } catch (Exception $ex) {
+    } catch (PhutilLockException $ex) {
       throw new PhutilProxyException(
         pht(
           'Failed to acquire read lock after waiting %s second(s). You '.
-          'may be able to retry later.',
-          new PhutilNumber($lock_wait)),
+          'may be able to retry later. (%s)',
+          new PhutilNumber($lock_wait),
+          $ex->getHint()),
         $ex);
     }
 
@@ -207,7 +208,7 @@ final class DiffusionRepositoryClusterEngine extends Phobject {
 
           $this->synchronizeWorkingCopyFromDevices($fetchable);
         } else {
-          $this->synchornizeWorkingCopyFromRemote();
+          $this->synchronizeWorkingCopyFromRemote();
         }
 
         PhabricatorRepositoryWorkingCopyVersion::updateVersion(
@@ -349,12 +350,13 @@ final class DiffusionRepositoryClusterEngine extends Phobject {
           pht(
             'Acquired write lock immediately.'));
       }
-    } catch (Exception $ex) {
+    } catch (PhutilLockException $ex) {
       throw new PhutilProxyException(
         pht(
           'Failed to acquire write lock after waiting %s second(s). You '.
-          'may be able to retry later.',
-          new PhutilNumber($lock_wait)),
+          'may be able to retry later. (%s)',
+          new PhutilNumber($lock_wait),
+          $ex->getHint()),
         $ex);
     }
 
@@ -607,7 +609,7 @@ final class DiffusionRepositoryClusterEngine extends Phobject {
   /**
    * @task internal
    */
-  private function synchornizeWorkingCopyFromRemote() {
+  private function synchronizeWorkingCopyFromRemote() {
     $repository = $this->getRepository();
     $device = AlmanacKeys::getLiveDevice();
 

src/applications/repository/phid/PhabricatorRepositoryIdentityPHIDType.php

@@ -30,6 +30,7 @@ final class PhabricatorRepositoryIdentityPHIDType
     array $handles,
     array $objects) {
 
+    $avatar_uri = celerity_get_resource_uri('/rsrc/image/avatar.png');
     foreach ($handles as $phid => $handle) {
       $identity = $objects[$phid];
 
@@ -40,6 +41,7 @@ final class PhabricatorRepositoryIdentityPHIDType
       $handle->setName($name);
       $handle->setURI($identity->getURI());
       $handle->setIcon('fa-user');
+      $handle->setImageURI($avatar_uri);
     }
   }
 

src/applications/repository/storage/PhabricatorRepositoryWorkingCopyVersion.php

@@ -76,18 +76,20 @@ final class PhabricatorRepositoryWorkingCopyVersion
 
 
   public static function getReadLock($repository_phid, $device_phid) {
-    $repository_hash = PhabricatorHash::digestForIndex($repository_phid);
-    $device_hash = PhabricatorHash::digestForIndex($device_phid);
-    $lock_key = "repo.read({$repository_hash}, {$device_hash})";
+    $parameters = array(
+      'repositoryPHID' => $repository_phid,
+      'devicePHID' => $device_phid,
+    );
 
-    return PhabricatorGlobalLock::newLock($lock_key);
+    return PhabricatorGlobalLock::newLock('repo.read', $parameters);
   }
 
   public static function getWriteLock($repository_phid) {
-    $repository_hash = PhabricatorHash::digestForIndex($repository_phid);
-    $lock_key = "repo.write({$repository_hash})";
+    $parameters = array(
+      'repositoryPHID' => $repository_phid,
+    );
 
-    return PhabricatorGlobalLock::newLock($lock_key);
+    return PhabricatorGlobalLock::newLock('repo.write', $parameters);
   }
 
 

src/infrastructure/util/PhabricatorGlobalLock.php

@@ -144,7 +144,8 @@ final class PhabricatorGlobalLock extends PhutilLock {
 
     $ok = head($result);
     if (!$ok) {
-      throw new PhutilLockException($lock_name);
+      throw id(new PhutilLockException($lock_name))
+        ->setHint($this->newHint($lock_name, $wait));
     }
 
     $conn->rememberLock($lock_name);
@@ -152,13 +153,7 @@ final class PhabricatorGlobalLock extends PhutilLock {
     $this->conn = $conn;
 
     if ($this->shouldLogLock()) {
-      global $argv;
-
-      $lock_context = array(
-        'pid' => getmypid(),
-        'host' => php_uname('n'),
-        'argv' => $argv,
-      );
+      $lock_context = $this->newLockContext();
 
       $log = id(new PhabricatorDaemonLockLog())
         ->setLockName($lock_name)
@@ -228,4 +223,153 @@ final class PhabricatorGlobalLock extends PhutilLock {
     return true;
   }
 
+  private function newLockContext() {
+    $context = array(
+      'pid' => getmypid(),
+      'host' => php_uname('n'),
+      'sapi' => php_sapi_name(),
+    );
+
+    global $argv;
+    if ($argv) {
+      $context['argv'] = $argv;
+    }
+
+    $access_log = null;
+
+    // TODO: There's currently no cohesive way to get the parameterized access
+    // log for the current request across different request types. Web requests
+    // have an "AccessLog", SSH requests have an "SSHLog", and other processes
+    // (like scripts) have no log. But there's no method to say "give me any
+    // log you've got". For now, just test if we have a web request and use the
+    // "AccessLog" if we do, since that's the only one we actually read any
+    // parameters from.
+
+    // NOTE: "PhabricatorStartup" is only available from web requests, not
+    // from CLI scripts.
+    if (class_exists('PhabricatorStartup', false)) {
+      $access_log = PhabricatorAccessLog::getLog();
+    }
+
+    if ($access_log) {
+      $controller = $access_log->getData('C');
+      if ($controller) {
+        $context['controller'] = $controller;
+      }
+
+      $method = $access_log->getData('m');
+      if ($method) {
+        $context['method'] = $method;
+      }
+    }
+
+    return $context;
+  }
+
+  private function newHint($lock_name, $wait) {
+    if (!$this->shouldLogLock()) {
+      return pht(
+        'Enable the lock log for more detailed information about '.
+        'which process is holding this lock.');
+    }
+
+    $now = PhabricatorTime::getNow();
+
+    // First, look for recent logs. If other processes have been acquiring and
+    // releasing this lock while we've been waiting, this is more likely to be
+    // a contention/throughput issue than an issue with something hung while
+    // holding the lock.
+    $limit = 100;
+    $logs = id(new PhabricatorDaemonLockLog())->loadAllWhere(
+      'lockName = %s AND dateCreated >= %d ORDER BY id ASC LIMIT %d',
+      $lock_name,
+      ($now - $wait),
+      $limit);
+
+    if ($logs) {
+      if (count($logs) === $limit) {
+        return pht(
+          'During the last %s second(s) spent waiting for the lock, more '.
+          'than %s other process(es) acquired it, so this is likely a '.
+          'bottleneck. Use "bin/lock log --name %s" to review log activity.',
+          new PhutilNumber($wait),
+          new PhutilNumber($limit),
+          $lock_name);
+      } else {
+        return pht(
+          'During the last %s second(s) spent waiting for the lock, %s '.
+          'other process(es) acquired it, so this is likely a '.
+          'bottleneck. Use "bin/lock log --name %s" to review log activity.',
+          new PhutilNumber($wait),
+          phutil_count($logs),
+          $lock_name);
+      }
+    }
+
+    $last_log = id(new PhabricatorDaemonLockLog())->loadOneWhere(
+      'lockName = %s ORDER BY id DESC LIMIT 1',
+      $lock_name);
+
+    if ($last_log) {
+      $info = array();
+
+      $acquired = $last_log->getDateCreated();
+      $context = $last_log->getLockContext();
+
+      $process_info = array();
+
+      $pid = idx($context, 'pid');
+      if ($pid) {
+        $process_info[] = 'pid='.$pid;
+      }
+
+      $host = idx($context, 'host');
+      if ($host) {
+        $process_info[] = 'host='.$host;
+      }
+
+      $sapi = idx($context, 'sapi');
+      if ($sapi) {
+        $process_info[] = 'sapi='.$sapi;
+      }
+
+      $argv = idx($context, 'argv');
+      if ($argv) {
+        $process_info[] = 'argv='.(string)csprintf('%LR', $argv);
+      }
+
+      $controller = idx($context, 'controller');
+      if ($controller) {
+        $process_info[] = 'controller='.$controller;
+      }
+
+      $method = idx($context, 'method');
+      if ($method) {
+        $process_info[] = 'method='.$method;
+      }
+
+      $process_info = implode(', ', $process_info);
+
+      $info[] = pht(
+        'This lock was most recently acquired by a process (%s) '.
+        '%s second(s) ago.',
+        $process_info,
+        new PhutilNumber($now - $acquired));
+
+      $released = $last_log->getLockReleased();
+      if ($released) {
+        $info[] = pht(
+          'This lock was released %s second(s) ago.',
+          new PhutilNumber($now - $released));
+      } else {
+        $info[] = pht('There is no record of this lock being released.');
+      }
+
+      return implode(' ', $info);
+    }
+
+    return pht(
+      'Found no records of processes acquiring or releasing this lock.');
+  }
+
 }

src/view/form/control/PHUIFormNumberControl.php

@@ -2,11 +2,28 @@
 
 final class PHUIFormNumberControl extends AphrontFormControl {
 
+  private $disableAutocomplete;
+
+  public function setDisableAutocomplete($disable_autocomplete) {
+    $this->disableAutocomplete = $disable_autocomplete;
+    return $this;
+  }
+
+  public function getDisableAutocomplete() {
+    return $this->disableAutocomplete;
+  }
+
   protected function getCustomControlClass() {
     return 'phui-form-number';
   }
 
   protected function renderInput() {
+    if ($this->getDisableAutocomplete()) {
+      $autocomplete = 'off';
+    } else {
+      $autocomplete = null;
+    }
+
     return javelin_tag(
       'input',
       array(
@@ -15,6 +32,7 @@ final class PHUIFormNumberControl extends AphrontFormControl {
         'name' => $this->getName(),
         'value' => $this->getValue(),
         'disabled' => $this->getDisabled() ? 'disabled' : null,
+        'autocomplete' => $autocomplete,
         'id' => $this->getID(),
       ));
   }

webroot/rsrc/css/phui/phui-document-pro.css

@@ -34,6 +34,12 @@
   layout: fixed;
 }
 
+/* Force very wide content, like tables with many columns, to scroll inside
+   the frame. See T13202. */
+.phui-document-content-view {
+  max-width: 800px;
+}
+
 .device-desktop .phui-document-content-inner {
   display: table-row;
 }