diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 720683c371..b6817f97eb 100644 --- a/resources/celerity/map.php +++ b/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', diff --git a/src/applications/auth/factor/PhabricatorTOTPAuthFactor.php b/src/applications/auth/factor/PhabricatorTOTPAuthFactor.php index 10c44aaec0..373adfbb54 100644 --- a/src/applications/auth/factor/PhabricatorTOTPAuthFactor.php +++ b/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))); diff --git a/src/applications/differential/mail/DifferentialInlineCommentMailView.php b/src/applications/differential/mail/DifferentialInlineCommentMailView.php index 57958202b1..d88effa46c 100644 --- a/src/applications/differential/mail/DifferentialInlineCommentMailView.php +++ b/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) diff --git a/src/applications/diffusion/controller/DiffusionBlameController.php b/src/applications/diffusion/controller/DiffusionBlameController.php index 42cabbc0d9..95cc039217 100644 --- a/src/applications/diffusion/controller/DiffusionBlameController.php +++ b/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', ); } diff --git a/src/applications/diffusion/protocol/DiffusionRepositoryClusterEngine.php b/src/applications/diffusion/protocol/DiffusionRepositoryClusterEngine.php index d5ba74a30e..f85c7862fe 100644 --- a/src/applications/diffusion/protocol/DiffusionRepositoryClusterEngine.php +++ b/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(); diff --git a/src/applications/repository/phid/PhabricatorRepositoryIdentityPHIDType.php b/src/applications/repository/phid/PhabricatorRepositoryIdentityPHIDType.php index 5d9b06e0a3..873cfbbae6 100644 --- a/src/applications/repository/phid/PhabricatorRepositoryIdentityPHIDType.php +++ b/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); } } diff --git a/src/applications/repository/storage/PhabricatorRepositoryWorkingCopyVersion.php b/src/applications/repository/storage/PhabricatorRepositoryWorkingCopyVersion.php index da5d54b57d..e297dfbf0e 100644 --- a/src/applications/repository/storage/PhabricatorRepositoryWorkingCopyVersion.php +++ b/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); } diff --git a/src/infrastructure/util/PhabricatorGlobalLock.php b/src/infrastructure/util/PhabricatorGlobalLock.php index 827d7e0e3d..221a75b37c 100644 --- a/src/infrastructure/util/PhabricatorGlobalLock.php +++ b/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.'); + } + } diff --git a/src/view/form/control/PHUIFormNumberControl.php b/src/view/form/control/PHUIFormNumberControl.php index d65e590746..26e7e03955 100644 --- a/src/view/form/control/PHUIFormNumberControl.php +++ b/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(), )); } diff --git a/webroot/rsrc/css/phui/phui-document-pro.css b/webroot/rsrc/css/phui/phui-document-pro.css index 520ae597cb..84dc11601f 100644 --- a/webroot/rsrc/css/phui/phui-document-pro.css +++ b/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; }