(stable) Promote 2021 Week 4

src/__phutil_library_map__.php

@@ -1430,12 +1430,16 @@ phutil_register_library_map(array(
     'HarbormasterBuildStep' => 'applications/harbormaster/storage/configuration/HarbormasterBuildStep.php',
     'HarbormasterBuildStepCoreCustomField' => 'applications/harbormaster/customfield/HarbormasterBuildStepCoreCustomField.php',
     'HarbormasterBuildStepCustomField' => 'applications/harbormaster/customfield/HarbormasterBuildStepCustomField.php',
+    'HarbormasterBuildStepEditAPIMethod' => 'applications/harbormaster/conduit/HarbormasterBuildStepEditAPIMethod.php',
+    'HarbormasterBuildStepEditEngine' => 'applications/harbormaster/editor/HarbormasterBuildStepEditEngine.php',
     'HarbormasterBuildStepEditor' => 'applications/harbormaster/editor/HarbormasterBuildStepEditor.php',
     'HarbormasterBuildStepGroup' => 'applications/harbormaster/stepgroup/HarbormasterBuildStepGroup.php',
     'HarbormasterBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterBuildStepImplementation.php',
     'HarbormasterBuildStepImplementationTestCase' => 'applications/harbormaster/step/__tests__/HarbormasterBuildStepImplementationTestCase.php',
     'HarbormasterBuildStepPHIDType' => 'applications/harbormaster/phid/HarbormasterBuildStepPHIDType.php',
     'HarbormasterBuildStepQuery' => 'applications/harbormaster/query/HarbormasterBuildStepQuery.php',
+    'HarbormasterBuildStepSearchAPIMethod' => 'applications/harbormaster/conduit/HarbormasterBuildStepSearchAPIMethod.php',
+    'HarbormasterBuildStepSearchEngine' => 'applications/harbormaster/query/HarbormasterBuildStepSearchEngine.php',
     'HarbormasterBuildStepTransaction' => 'applications/harbormaster/storage/configuration/HarbormasterBuildStepTransaction.php',
     'HarbormasterBuildStepTransactionQuery' => 'applications/harbormaster/query/HarbormasterBuildStepTransactionQuery.php',
     'HarbormasterBuildTarget' => 'applications/harbormaster/storage/build/HarbormasterBuildTarget.php',
@@ -7627,18 +7631,23 @@ phutil_register_library_map(array(
       'PhabricatorApplicationTransactionInterface',
       'PhabricatorPolicyInterface',
       'PhabricatorCustomFieldInterface',
+      'PhabricatorConduitResultInterface',
     ),
     'HarbormasterBuildStepCoreCustomField' => array(
       'HarbormasterBuildStepCustomField',
       'PhabricatorStandardCustomFieldInterface',
     ),
     'HarbormasterBuildStepCustomField' => 'PhabricatorCustomField',
+    'HarbormasterBuildStepEditAPIMethod' => 'PhabricatorEditEngineAPIMethod',
+    'HarbormasterBuildStepEditEngine' => 'PhabricatorEditEngine',
     'HarbormasterBuildStepEditor' => 'PhabricatorApplicationTransactionEditor',
     'HarbormasterBuildStepGroup' => 'Phobject',
     'HarbormasterBuildStepImplementation' => 'Phobject',
     'HarbormasterBuildStepImplementationTestCase' => 'PhabricatorTestCase',
     'HarbormasterBuildStepPHIDType' => 'PhabricatorPHIDType',
     'HarbormasterBuildStepQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
+    'HarbormasterBuildStepSearchAPIMethod' => 'PhabricatorSearchEngineAPIMethod',
+    'HarbormasterBuildStepSearchEngine' => 'PhabricatorApplicationSearchEngine',
     'HarbormasterBuildStepTransaction' => 'PhabricatorApplicationTransaction',
     'HarbormasterBuildStepTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
     'HarbormasterBuildTarget' => array(

src/aphront/configuration/AphrontApplicationConfiguration.php

@@ -176,7 +176,7 @@ final class AphrontApplicationConfiguration
     }
 
     $host = AphrontRequest::getHTTPHeader('Host');
-    $path = $_REQUEST['__path__'];
+    $path = PhabricatorStartup::getRequestPath();
 
     $application = new self();
 
@@ -759,7 +759,7 @@ final class AphrontApplicationConfiguration
   }
 
   private static function newSelfCheckResponse() {
-    $path = idx($_REQUEST, '__path__', '');
+    $path = PhabricatorStartup::getRequestPath();
     $query = idx($_SERVER, 'QUERY_STRING', '');
 
     $pairs = id(new PhutilQueryStringParser())

src/applications/audit/query/PhabricatorCommitSearchEngine.php

@@ -54,8 +54,8 @@ final class PhabricatorCommitSearchEngine
       $query->withUnreachable($map['unreachable']);
     }
 
-    if ($map['unpublished'] !== null) {
+    if ($map['permanent'] !== null) {
-      $query->withUnpublished($map['unpublished']);
+      $query->withPermanent($map['permanent']);
     }
 
     if ($map['ancestorsOf']) {
@@ -132,15 +132,15 @@ final class PhabricatorCommitSearchEngine
             'Find or exclude unreachable commits which are not ancestors of '.
             'any branch, tag, or ref.')),
       id(new PhabricatorSearchThreeStateField())
-        ->setLabel(pht('Unpublished'))
+        ->setLabel(pht('Permanent'))
-        ->setKey('unpublished')
+        ->setKey('permanent')
         ->setOptions(
           pht('(Show All)'),
-          pht('Show Only Unpublished Commits'),
+          pht('Show Only Permanent Commits'),
-          pht('Hide Unpublished Commits'))
+          pht('Hide Permanent Commits'))
         ->setDescription(
           pht(
-            'Find or exclude unpublished commits which are not ancestors of '.
+            'Find or exclude permanent commits which are ancestors of '.
             'any permanent branch, tag, or ref.')),
       id(new PhabricatorSearchStringListField())
         ->setLabel(pht('Ancestors Of'))

src/applications/auth/engine/PhabricatorAuthPasswordEngine.php

@@ -181,6 +181,12 @@ final class PhabricatorAuthPasswordEngine
     $normal_password = phutil_utf8_strtolower($raw_password);
     if (strlen($normal_password) >= $minimum_similarity) {
       foreach ($normal_map as $term => $source) {
+
+        // See T2312. This may be required if the term list includes numeric
+        // strings like "12345", which will be cast to integers when used as
+        // array keys.
+        $term = phutil_string_cast($term);
+
         if (strpos($term, $normal_password) === false &&
             strpos($normal_password, $term) === false) {
           continue;

src/applications/config/option/PhabricatorCoreConfigOptions.php

@@ -169,7 +169,21 @@ EOREMARKUP
             'Maniphest. If you\'d prefer more traditional UI strings like '.
             '"Add Comment", you can set this flag to disable most of the '.
             'extra flavor.')),
-      $this->newOption('remarkup.ignored-object-names', 'string', '/^(Q|V)\d$/')
+      $this->newOption(
+        'remarkup.ignored-object-names',
+        'string',
+
+        // Q1, Q2, etc., are common abbreviations for "Quarter".
+        // V1, V2, etc., are common abbreviations for "Version".
+        // P1, P2, etc., are common abbreviations for "Priority".
+
+        // M1 is a computer chip manufactured by Apple.
+        // M2 (commonly spelled "M.2") is an expansion slot on motherboards.
+        // M4 is a carbine.
+        // M8 is a phonetic spelling of "mate", used in culturally significant
+        // copypasta about navy seals.
+
+        '/^(Q|V|M|P)\d$/')
         ->setSummary(
           pht('Text values that match this regex and are also object names '.
           'will not be linked.'))

src/applications/diffusion/conduit/DiffusionBrowseQueryConduitAPIMethod.php

@@ -35,22 +35,26 @@ final class DiffusionBrowseQueryConduitAPIMethod
   protected function getGitResult(ConduitAPIRequest $request) {
     $drequest = $this->getDiffusionRequest();
     $repository = $drequest->getRepository();
+
     $path = $request->getValue('path');
+    if (!strlen($path)) {
+      $path = null;
+    }
+
     $commit = $request->getValue('commit');
     $offset = (int)$request->getValue('offset');
     $limit = (int)$request->getValue('limit');
     $result = $this->getEmptyResultSet();
 
-    if ($path == '') {
+    if ($path === null) {
       // Fast path to improve the performance of the repository view; we know
       // the root is always a tree at any commit and always exists.
       $stdout = 'tree';
     } else {
       try {
         list($stdout) = $repository->execxLocalCommand(
-          'cat-file -t %s:%s',
+          'cat-file -t -- %s',
-          $commit,
+          sprintf('%s:%s', $commit, $path));
-          $path);
       } catch (CommandException $e) {
         // The "cat-file" command may fail if the path legitimately does not
         // exist, but it may also fail if the path is a submodule. This can
@@ -62,7 +66,7 @@ final class DiffusionBrowseQueryConduitAPIMethod
 
         list($sub_err, $sub_stdout) = $repository->execLocalCommand(
           'ls-tree %s -- %s',
-          $commit,
+          gitsprintf('%s', $commit),
           $path);
         if (!$sub_err) {
           // If the path failed "cat-file" but "ls-tree" worked, we assume it
@@ -86,8 +90,9 @@ final class DiffusionBrowseQueryConduitAPIMethod
         if (preg_match('/^fatal: Not a valid object name/', $stderr)) {
           // Grab two logs, since the first one is when the object was deleted.
           list($stdout) = $repository->execxLocalCommand(
-            'log -n2 --format="%%H" %s -- %s',
+            'log -n2 %s %s -- %s',
-            $commit,
+            '--format=%H',
+            gitsprintf('%s', $commit),
             $path);
           $stdout = trim($stdout);
           if ($stdout) {
@@ -120,14 +125,20 @@ final class DiffusionBrowseQueryConduitAPIMethod
       return $result;
     }
 
-    list($stdout) = $repository->execxLocalCommand(
+    if ($path === null) {
-      'ls-tree -z -l %s:%s',
+      list($stdout) = $repository->execxLocalCommand(
-      $commit,
+        'ls-tree -z -l %s --',
-      $path);
+        gitsprintf('%s', $commit));
+    } else {
+      list($stdout) = $repository->execxLocalCommand(
+        'ls-tree -z -l %s -- %s',
+        gitsprintf('%s', $commit),
+        $path);
+    }
 
     $submodules = array();
 
-    if (strlen($path)) {
+    if ($path !== null) {
       $prefix = rtrim($path, '/').'/';
     } else {
       $prefix = '';
@@ -207,7 +218,7 @@ final class DiffusionBrowseQueryConduitAPIMethod
       // the wild.
 
       list($err, $contents) = $repository->execLocalCommand(
-        'cat-file blob %s:.gitmodules',
+        'cat-file blob -- %s:.gitmodules',
         $commit);
 
       if (!$err) {
@@ -225,8 +236,8 @@ final class DiffusionBrowseQueryConduitAPIMethod
           $dict[$key] = $value;
         }
 
-        foreach ($submodules as $path) {
+        foreach ($submodules as $submodule_path) {
-          $full_path = $path->getFullPath();
+          $full_path = $submodule_path->getFullPath();
           $key = 'submodule.'.$full_path.'.url';
           if (isset($dict[$key])) {
             $path->setExternalURI($dict[$key]);

src/applications/diffusion/conduit/DiffusionExistsQueryConduitAPIMethod.php

@@ -25,7 +25,7 @@ final class DiffusionExistsQueryConduitAPIMethod
     $repository = $this->getDiffusionRequest()->getRepository();
     $commit = $request->getValue('commit');
     list($err, $merge_base) = $repository->execLocalCommand(
-      'cat-file -t %s',
+      'cat-file -t -- %s',
       $commit);
     return !$err;
   }

src/applications/diffusion/conduit/DiffusionHistoryQueryConduitAPIMethod.php

@@ -45,7 +45,12 @@ final class DiffusionHistoryQueryConduitAPIMethod
     $repository = $drequest->getRepository();
     $commit_hash = $request->getValue('commit');
     $against_hash = $request->getValue('against');
+
     $path = $request->getValue('path');
+    if (!strlen($path)) {
+      $path = null;
+    }
+
     $offset = $request->getValue('offset');
     $limit = $request->getValue('limit');
 
@@ -55,18 +60,27 @@ final class DiffusionHistoryQueryConduitAPIMethod
       $commit_range = $commit_hash;
     }
 
+    $argv = array();
+
+    $argv[] = '--skip';
+    $argv[] = $offset;
+
+    $argv[] = '--max-count';
+    $argv[] = $limit;
+
+    $argv[] = '--format=%H:%P';
+
+    $argv[] = gitsprintf('%s', $commit_range);
+
+    $argv[] = '--';
+
+    if ($path !== null) {
+      $argv[] = $path;
+    }
+
     list($stdout) = $repository->execxLocalCommand(
-      'log '.
+      'log %Ls',
-        '--skip=%d '.
+      $argv);
-        '-n %d '.
-        '--pretty=format:%s '.
-        '%s -- %C',
-      $offset,
-      $limit,
-      '%H:%P',
-      $commit_range,
-      // Git omits merge commits if the path is provided, even if it is empty.
-      (strlen($path) ? csprintf('%s', $path) : ''));
 
     $lines = explode("\n", trim($stdout));
     $lines = array_filter($lines);

src/applications/diffusion/conduit/DiffusionInternalGitRawDiffQueryConduitAPIMethod.php

@@ -43,9 +43,9 @@ final class DiffusionInternalGitRawDiffQueryConduitAPIMethod
     // it requires the commit to have a parent that we can diff against. The
     // first commit doesn't, so "commit^" is not a valid ref.
     list($parents) = $repository->execxLocalCommand(
-      'log -n1 --format=%s %s',
+      'log -n1 %s %s --',
-      '%P',
+      '--format=%P',
-      $commit);
+      gitsprintf('%s', $commit));
     $use_log = !strlen(trim($parents));
 
     // First, get a fast raw diff without "--find-copies-harder". This flag
@@ -96,18 +96,18 @@ final class DiffusionInternalGitRawDiffQueryConduitAPIMethod
       // NOTE: "--pretty=format: " is to disable diff output, we only want the
       // part we get from "--raw".
       $future = $repository->getLocalCommandFuture(
-        'log %Ls --pretty=format: %s',
+        'log %Ls --pretty=format: %s --',
         $flags,
-        $commit);
+        gitsprintf('%s', $commit));
     } else {
       // Otherwise, we can use "diff", which will give us output for merges.
       // We diff against the first parent, as this is generally the expectation
       // and results in sensible behavior.
       $future = $repository->getLocalCommandFuture(
-        'diff %Ls %s^1 %s',
+        'diff %Ls %s %s --',
         $flags,
-        $commit,
+        gitsprintf('%s^1', $commit),
-        $commit);
+        gitsprintf('%s', $commit));
     }
 
     // Don't spend more than 30 seconds generating the slower output.

src/applications/diffusion/conduit/DiffusionLastModifiedQueryConduitAPIMethod.php

@@ -33,8 +33,9 @@ final class DiffusionLastModifiedQueryConduitAPIMethod
         continue;
       }
       list($hash) = $repository->execxLocalCommand(
-        'log -n1 --format=%%H %s -- %s',
+        'log -n1 %s %s -- %s',
-        $commit,
+        '--format=%H',
+        gitsprintf('%s', $commit),
         $path);
       $results[$path] = trim($hash);
     }

src/applications/diffusion/conduit/DiffusionMergedCommitsQueryConduitAPIMethod.php

@@ -35,9 +35,9 @@ final class DiffusionMergedCommitsQueryConduitAPIMethod
     $limit = $this->getLimit($request);
 
     list($parents) = $repository->execxLocalCommand(
-      'log -n 1 --format=%s %s',
+      'log -n 1 %s %s --',
-      '%P',
+      '--format=%P',
-      $commit);
+      gitsprintf('%s', $commit));
 
     $parents = preg_split('/\s+/', trim($parents));
     if (count($parents) < 2) {
@@ -50,12 +50,12 @@ final class DiffusionMergedCommitsQueryConduitAPIMethod
 
     $first_parent = head($parents);
     list($logs) = $repository->execxLocalCommand(
-      'log -n %d --format=%s %s %s --',
+      'log -n %d %s %s %s --',
       // NOTE: "+ 1" accounts for the merge commit itself.
       $limit + 1,
-      '%H',
+      '--format=%H',
-      $commit,
+      gitsprintf('%s', $commit),
-      '^'.$first_parent);
+      gitsprintf('%s', '^'.$first_parent));
 
     $hashes = explode("\n", trim($logs));
 

src/applications/diffusion/conduit/DiffusionQueryPathsConduitAPIMethod.php

@@ -45,7 +45,7 @@ final class DiffusionQueryPathsConduitAPIMethod
 
     $future = $repository->getLocalCommandFuture(
       'ls-tree --name-only -r -z %s -- %s',
-      $commit,
+      gitsprintf('%s', $commit),
       $path);
 
     $lines = id(new LinesOfALargeExecFuture($future))->setDelimiter("\0");

src/applications/diffusion/conduit/DiffusionRefsQueryConduitAPIMethod.php

@@ -28,9 +28,9 @@ final class DiffusionRefsQueryConduitAPIMethod
     $commit = $request->getValue('commit');
 
     list($stdout) = $repository->execxLocalCommand(
-      'log --format=%s -n 1 %s --',
+      'log -n 1 %s %s --',
-      '%d',
+      '--format=%d',
-      $commit);
+      gitsprintf('%s', $commit));
 
     // %d, gives a weird output format
     // similar to (remote/one, remote/two, remote/three)

src/applications/diffusion/conduit/DiffusionSearchQueryConduitAPIMethod.php

@@ -64,7 +64,7 @@ final class DiffusionSearchQueryConduitAPIMethod
     $future = $repository->getLocalCommandFuture(
       // NOTE: --perl-regexp is available only with libpcre compiled in.
       'grep --extended-regexp --null -n --no-color -f - %s -- %s',
-      $drequest->getStableCommit(),
+      gitsprintf('%s', $drequest->getStableCommit()),
       $path);
 
     // NOTE: We're writing the pattern on stdin to avoid issues with UTF8

src/applications/diffusion/controller/DiffusionServeController.php

@@ -922,18 +922,26 @@ final class DiffusionServeController extends DiffusionController {
     // This is a pretty funky fix: it would be nice to more precisely detect
     // that a request is a `--depth N` clone request, but we don't have any code
     // to decode protocol frames yet. Instead, look for reasonable evidence
-    // in the error and output that we're looking at a `--depth` clone.
+    // in the output that we're looking at a `--depth` clone.
 
-    // For evidence this isn't completely crazy, see:
+    // A valid x-git-upload-pack-result response during packfile negotiation
-    // https://github.com/schacon/grack/pull/7
+    // should end with a flush packet ("0000"). As long as that packet
+    // terminates the response body in the response, we'll assume the response
+    // is correct and complete.
+
+    // See https://git-scm.com/docs/pack-protocol#_packfile_negotiation
 
     $stdout_regexp = '(^Content-Type: application/x-git-upload-pack-result)m';
-    $stderr_regexp = '(The remote end hung up unexpectedly)';
 
     $has_pack = preg_match($stdout_regexp, $stdout);
-    $is_hangup = preg_match($stderr_regexp, $stderr);
 
-    return $has_pack && $is_hangup;
+    if (strlen($stdout) >= 4) {
+      $has_flush_packet = (substr($stdout, -4) === "0000");
+    } else {
+      $has_flush_packet = false;
+    }
+
+    return ($has_pack && $has_flush_packet);
   }
 
   private function getCommonEnvironment(PhabricatorUser $viewer) {

src/applications/diffusion/engine/DiffusionCommitHookEngine.php

@@ -608,9 +608,9 @@ final class DiffusionCommitHookEngine extends Phobject {
       // repository. Particularly, this will cover the cases of a new branch, a
       // completely moved tag, etc.
       $futures[$key] = $this->getRepository()->getLocalCommandFuture(
-        'log --format=%s %s --not --all',
+        'log %s %s --not --all --',
-        '%H',
+        '--format=%H',
-        $ref_update->getRefNew());
+        gitsprintf('%s', $ref_update->getRefNew()));
     }
 
     $content_updates = array();

src/applications/diffusion/query/DiffusionCommitQuery.php

@@ -15,7 +15,7 @@ final class DiffusionCommitQuery
   private $statuses;
   private $packagePHIDs;
   private $unreachable;
-  private $unpublished;
+  private $permanent;
 
   private $needAuditRequests;
   private $needAuditAuthority;
@@ -154,8 +154,8 @@ final class DiffusionCommitQuery
     return $this;
   }
 
-  public function withUnpublished($unpublished) {
+  public function withPermanent($permanent) {
-    $this->unpublished = $unpublished;
+    $this->permanent = $permanent;
     return $this;
   }
 
@@ -859,18 +859,18 @@ final class DiffusionCommitQuery
       }
     }
 
-    if ($this->unpublished !== null) {
+    if ($this->permanent !== null) {
-      if ($this->unpublished) {
+      if ($this->permanent) {
-        $where[] = qsprintf(
-          $conn,
-          '(commit.importStatus & %d) = 0',
-          PhabricatorRepositoryCommit::IMPORTED_CLOSEABLE);
-      } else {
         $where[] = qsprintf(
           $conn,
           '(commit.importStatus & %d) = %d',
-          PhabricatorRepositoryCommit::IMPORTED_CLOSEABLE,
+          PhabricatorRepositoryCommit::IMPORTED_PERMANENT,
-          PhabricatorRepositoryCommit::IMPORTED_CLOSEABLE);
+          PhabricatorRepositoryCommit::IMPORTED_PERMANENT);
+      } else {
+        $where[] = qsprintf(
+          $conn,
+          '(commit.importStatus & %d) = 0',
+          PhabricatorRepositoryCommit::IMPORTED_PERMANENT);
       }
     }
 

src/applications/diffusion/query/blame/DiffusionGitBlameQuery.php

@@ -13,7 +13,7 @@ final class DiffusionGitBlameQuery extends DiffusionBlameQuery {
 
     return $repository->getLocalCommandFuture(
       '--no-pager blame --root -s -l %s -- %s',
-      $commit,
+      gitsprintf('%s', $commit),
       $path);
   }
 

src/applications/diffusion/query/filecontent/DiffusionGitFileContentQuery.php

@@ -10,9 +10,8 @@ final class DiffusionGitFileContentQuery extends DiffusionFileContentQuery {
     $commit = $drequest->getCommit();
 
     return $repository->getLocalCommandFuture(
-      'cat-file blob %s:%s',
+      'cat-file blob -- %s',
-      $commit,
+      sprintf('%s:%s', $commit, $path));
-      $path);
   }
 
 }

src/applications/diffusion/query/lowlevel/DiffusionLowLevelCommitQuery.php

@@ -55,12 +55,15 @@ final class DiffusionLowLevelCommitQuery
       $split_body = true;
     }
 
-    // Even though we pass --encoding here, git doesn't always succeed, so
+    $argv = array();
-    // we try a little harder, since git *does* tell us what the actual encoding
+
-    // is correctly (unless it doesn't; encoding is sometimes empty).
+    $argv[] = '-n';
-    list($info) = $repository->execxLocalCommand(
+    $argv[] = '1';
-      'log -n 1 --encoding=%s --format=%s %s --',
+
-      'UTF-8',
+    $argv[] = '--encoding=UTF-8';
+
+    $argv[] = sprintf(
+      '--format=%s',
       implode(
         '%x00',
         array(
@@ -78,8 +81,15 @@ final class DiffusionLowLevelCommitQuery
           // so include an explicit terminator: this makes sure the exact
           // body text is surrounded by "\0" characters.
           '~',
-        )),
+        )));
-      $this->identifier);
+
+    // Even though we pass --encoding here, git doesn't always succeed, so
+    // we try a little harder, since git *does* tell us what the actual encoding
+    // is correctly (unless it doesn't; encoding is sometimes empty).
+    list($info) = $repository->execxLocalCommand(
+      'log -n 1 %Ls %s --',
+      $argv,
+      gitsprintf('%s', $this->identifier));
 
     $parts = explode("\0", $info);
     $encoding = array_shift($parts);

src/applications/diffusion/query/lowlevel/DiffusionLowLevelFilesizeQuery.php

@@ -33,7 +33,7 @@ final class DiffusionLowLevelFilesizeQuery
 
     $paths_future = $repository->getLocalCommandFuture(
       'diff-tree -z -r --no-commit-id %s --',
-      $identifier);
+      gitsprintf('%s', $identifier));
 
     // With "-z" we get "<fields>\0<filename>\0" for each line. Process the
     // delimited text as "<fields>, <filename>" pairs.

src/applications/diffusion/query/lowlevel/DiffusionLowLevelParentsQuery.php

@@ -37,9 +37,9 @@ final class DiffusionLowLevelParentsQuery
     $repository = $this->getRepository();
 
     list($stdout) = $repository->execxLocalCommand(
-      'log -n 1 --format=%s %s',
+      'log -n 1 %s %s --',
-      '%P',
+      '--format=%P',
-      $this->identifier);
+      gitsprintf('%s', $this->identifier));
 
     return preg_split('/\s+/', trim($stdout));
   }

src/applications/diffusion/query/rawdiff/DiffusionGitRawDiffQuery.php

@@ -23,9 +23,9 @@ final class DiffusionGitRawDiffQuery extends DiffusionRawDiffQuery {
       // Check if this is the root commit by seeing if it has parents, since
       // `git diff X^ X` does not work if "X" is the initial commit.
       list($parents) = $repository->execxLocalCommand(
-        'log -n 1 --format=%s %s --',
+        'log -n 1 %s %s --',
-        '%P',
+        '--format=%P',
-        $commit);
+        gitsprintf('%s', $commit));
 
       if (strlen(trim($parents))) {
         $against = $commit.'^';
@@ -42,8 +42,8 @@ final class DiffusionGitRawDiffQuery extends DiffusionRawDiffQuery {
     return $repository->getLocalCommandFuture(
       'diff %Ls %s %s -- %s',
       $options,
-      $against,
+      gitsprintf('%s', $against),
-      $commit,
+      gitsprintf('%s', $commit),
       $path);
   }
 

src/applications/harbormaster/conduit/HarbormasterBuildStepEditAPIMethod.php

@@ -0,0 +1,20 @@
+<?php
+
+final class HarbormasterBuildStepEditAPIMethod
+  extends PhabricatorEditEngineAPIMethod {
+
+  public function getAPIMethodName() {
+    return 'harbormaster.step.edit';
+  }
+
+  public function newEditEngine() {
+    return new HarbormasterBuildStepEditEngine();
+  }
+
+  public function getMethodSummary() {
+    return pht(
+      'Apply transactions to create a new build step or edit an existing '.
+      'one.');
+  }
+
+}

src/applications/harbormaster/conduit/HarbormasterBuildStepSearchAPIMethod.php

@@ -0,0 +1,18 @@
+<?php
+
+final class HarbormasterBuildStepSearchAPIMethod
+  extends PhabricatorSearchEngineAPIMethod {
+
+  public function getAPIMethodName() {
+    return 'harbormaster.step.search';
+  }
+
+  public function newSearchEngine() {
+    return new HarbormasterBuildStepSearchEngine();
+  }
+
+  public function getMethodSummary() {
+    return pht('Retrieve information about Harbormaster build steps.');
+  }
+
+}

src/applications/harbormaster/editor/HarbormasterBuildStepEditEngine.php

@@ -0,0 +1,107 @@
+<?php
+
+final class HarbormasterBuildStepEditEngine
+  extends PhabricatorEditEngine {
+
+  const ENGINECONST = 'harbormaster.buildstep';
+
+  private $buildPlan;
+
+  public function setBuildPlan(HarbormasterBuildPlan $build_plan) {
+    $this->buildPlan = $build_plan;
+    return $this;
+  }
+
+  public function getBuildPlan() {
+    if ($this->buildPlan === null) {
+      throw new PhutilInvalidStateException('setBuildPlan');
+    }
+
+    return $this->buildPlan;
+  }
+
+  public function isEngineConfigurable() {
+    return false;
+  }
+
+  public function getEngineName() {
+    return pht('Harbormaster Build Steps');
+  }
+
+  public function getSummaryHeader() {
+    return pht('Edit Harbormaster Build Step Configurations');
+  }
+
+  public function getSummaryText() {
+    return pht('This engine is used to edit Harbormaster build steps.');
+  }
+
+  public function getEngineApplicationClass() {
+    return 'PhabricatorHarbormasterApplication';
+  }
+
+  protected function newEditableObject() {
+    $viewer = $this->getViewer();
+
+
+    $plan = HarbormasterBuildPlan::initializeNewBuildPlan($viewer);
+    $this->setBuildPlan($plan);
+
+    $plan = $this->getBuildPlan();
+
+    $step = HarbormasterBuildStep::initializeNewStep($viewer);
+
+    $step->setBuildPlanPHID($plan->getPHID());
+    $step->attachBuildPlan($plan);
+
+    return $step;
+  }
+
+  protected function newObjectQuery() {
+    return new HarbormasterBuildStepQuery();
+  }
+
+  protected function getObjectCreateTitleText($object) {
+    return pht('Create Build Step');
+  }
+
+  protected function getObjectCreateButtonText($object) {
+    return pht('Create Build Step');
+  }
+
+  protected function getObjectEditTitleText($object) {
+    return pht('Edit Build Step: %s', $object->getName());
+  }
+
+  protected function getObjectEditShortText($object) {
+    return pht('Edit Build Step');
+  }
+
+  protected function getObjectCreateShortText() {
+    return pht('Create Build Step');
+  }
+
+  protected function getObjectName() {
+    return pht('Build Step');
+  }
+
+  protected function getEditorURI() {
+    return '/harbormaster/step/edit/';
+  }
+
+  protected function getObjectCreateCancelURI($object) {
+    return '/harbormaster/step/';
+  }
+
+  protected function getObjectViewURI($object) {
+    $id = $object->getID();
+    return "/harbormaster/step/{$id}/";
+  }
+
+  protected function buildCustomEditFields($object) {
+    $fields = array();
+
+    return $fields;
+  }
+
+}

src/applications/harbormaster/query/HarbormasterBuildStepSearchEngine.php

@@ -0,0 +1,58 @@
+<?php
+
+final class HarbormasterBuildStepSearchEngine
+  extends PhabricatorApplicationSearchEngine {
+
+  public function getResultTypeDescription() {
+    return pht('Harbormaster Build Steps');
+  }
+
+  public function getApplicationClassName() {
+    return 'PhabricatorHarbormasterApplication';
+  }
+
+  public function newQuery() {
+    return new HarbormasterBuildStepQuery();
+  }
+
+  protected function buildCustomSearchFields() {
+    return array();
+  }
+
+  protected function buildQueryFromParameters(array $map) {
+    $query = $this->newQuery();
+
+    return $query;
+  }
+
+  protected function getURI($path) {
+    return '/harbormaster/step/'.$path;
+  }
+
+  protected function getBuiltinQueryNames() {
+    return array(
+      'all' => pht('All Steps'),
+    );
+  }
+
+  public function buildSavedQueryFromBuiltin($query_key) {
+    $query = $this->newSavedQuery();
+    $query->setQueryKey($query_key);
+
+    switch ($query_key) {
+      case 'all':
+        return $query;
+    }
+
+    return parent::buildSavedQueryFromBuiltin($query_key);
+  }
+
+  protected function renderResultList(
+    array $plans,
+    PhabricatorSavedQuery $query,
+    array $handles) {
+    assert_instances_of($plans, 'HarbormasterBuildStep');
+    return null;
+  }
+
+}

src/applications/harbormaster/storage/configuration/HarbormasterBuildStep.php

@@ -4,7 +4,8 @@ final class HarbormasterBuildStep extends HarbormasterDAO
   implements
     PhabricatorApplicationTransactionInterface,
     PhabricatorPolicyInterface,
-    PhabricatorCustomFieldInterface {
+    PhabricatorCustomFieldInterface,
+    PhabricatorConduitResultInterface {
 
   protected $name;
   protected $description;
@@ -169,5 +170,45 @@ final class HarbormasterBuildStep extends HarbormasterDAO
     return $this;
   }
 
+/* -(  PhabricatorConduitResultInterface  )---------------------------------- */
+
+
+  public function getFieldSpecificationsForConduit() {
+    return array(
+      id(new PhabricatorConduitSearchFieldSpecification())
+        ->setKey('name')
+        ->setType('string')
+        ->setDescription(pht('The name of the build step.')),
+      id(new PhabricatorConduitSearchFieldSpecification())
+        ->setKey('description')
+        ->setType('remarkup')
+        ->setDescription(pht('The build step description.')),
+      id(new PhabricatorConduitSearchFieldSpecification())
+        ->setKey('buildPlanPHID')
+        ->setType('phid')
+        ->setDescription(
+          pht(
+            'The PHID of the build plan this build step belongs to.')),
+    );
+  }
+
+  public function getFieldValuesForConduit() {
+    // T6203: This can be removed once the field becomes non-nullable.
+    $name = $this->getName();
+    $name = phutil_string_cast($name);
+
+    return array(
+      'name' => $name,
+      'description' => array(
+        'raw' => $this->getDescription(),
+      ),
+      'buildPlanPHID' => $this->getBuildPlanPHID(),
+    );
+  }
+
+  public function getConduitSearchAttachments() {
+    return array();
+  }
+
 
 }

src/applications/metamta/query/PhabricatorMetaMTAMailQuery.php

@@ -64,24 +64,6 @@ final class PhabricatorMetaMTAMailQuery
         $this->actorPHIDs);
     }
 
-    if ($this->recipientPHIDs !== null) {
-      $where[] = qsprintf(
-        $conn,
-        'recipient.dst IN (%Ls)',
-        $this->recipientPHIDs);
-    }
-
-    if ($this->actorPHIDs === null && $this->recipientPHIDs === null) {
-      $viewer = $this->getViewer();
-      if (!$viewer->isOmnipotent()) {
-        $where[] = qsprintf(
-          $conn,
-          'edge.dst = %s OR actorPHID = %s',
-          $viewer->getPHID(),
-          $viewer->getPHID());
-      }
-    }
-
     if ($this->createdMin !== null) {
       $where[] = qsprintf(
         $conn,
@@ -102,26 +84,29 @@ final class PhabricatorMetaMTAMailQuery
   protected function buildJoinClauseParts(AphrontDatabaseConnection $conn) {
     $joins = parent::buildJoinClauseParts($conn);
 
-    if ($this->actorPHIDs === null && $this->recipientPHIDs === null) {
+    if ($this->shouldJoinRecipients()) {
       $joins[] = qsprintf(
         $conn,
-        'LEFT JOIN %T edge ON mail.phid = edge.src AND edge.type = %d',
+        'JOIN %T recipient
+          ON mail.phid = recipient.src
+            AND recipient.type = %d
+            AND recipient.dst IN (%Ls)',
         PhabricatorEdgeConfig::TABLE_NAME_EDGE,
-        PhabricatorMetaMTAMailHasRecipientEdgeType::EDGECONST);
+        PhabricatorMetaMTAMailHasRecipientEdgeType::EDGECONST,
-    }
+        $this->recipientPHIDs);
-
-    if ($this->recipientPHIDs !== null) {
-      $joins[] = qsprintf(
-        $conn,
-        'LEFT JOIN %T recipient '.
-        'ON mail.phid = recipient.src AND recipient.type = %d',
-        PhabricatorEdgeConfig::TABLE_NAME_EDGE,
-        PhabricatorMetaMTAMailHasRecipientEdgeType::EDGECONST);
     }
 
     return $joins;
   }
 
+  private function shouldJoinRecipients() {
+    if ($this->recipientPHIDs === null) {
+      return false;
+    }
+
+    return true;
+  }
+
   protected function getPrimaryTableAlias() {
     return 'mail';
   }
@@ -134,4 +119,14 @@ final class PhabricatorMetaMTAMailQuery
     return 'PhabricatorMetaMTAApplication';
   }
 
+  protected function shouldGroupQueryResultRows() {
+    if ($this->shouldJoinRecipients()) {
+      if (count($this->recipientPHIDs) > 1) {
+        return true;
+      }
+    }
+
+    return parent::shouldGroupQueryResultRows();
+  }
+
 }

src/applications/packages/application/PhabricatorPackagesApplication.php

@@ -15,7 +15,7 @@ final class PhabricatorPackagesApplication extends PhabricatorApplication {
   }
 
   public function getBaseURI() {
-    return '/packages/';
+    return '/packages/package/';
   }
 
   public function getIcon() {

src/applications/repository/daemon/PhabricatorGitGraphStream.php

@@ -19,13 +19,13 @@ final class PhabricatorGitGraphStream
 
     if ($start_commit !== null) {
       $future = $repository->getLocalCommandFuture(
-        'log --format=%s %s --',
+        'log %s %s --',
-        '%H%x01%P%x01%ct',
+        '--format=%H%x01%P%x01%ct',
-        $start_commit);
+        gitsprintf('%s', $start_commit));
     } else {
       $future = $repository->getLocalCommandFuture(
-        'log --format=%s --all --',
+        'log %s --all --',
-        '%H%x01%P%x01%ct');
+        '--format=%H%x01%P%x01%ct');
     }
 
     $this->iterator = new LinesOfALargeExecFuture($future);

src/applications/repository/engine/PhabricatorRepositoryCommitRef.php

@@ -5,7 +5,7 @@ final class PhabricatorRepositoryCommitRef extends Phobject {
   private $identifier;
   private $epoch;
   private $branch;
-  private $canCloseImmediately;
+  private $isPermanent;
   private $parents = array();
 
   public function setIdentifier($identifier) {
@@ -35,13 +35,13 @@ final class PhabricatorRepositoryCommitRef extends Phobject {
     return $this->branch;
   }
 
-  public function setCanCloseImmediately($can_close_immediately) {
+  public function setIsPermanent($is_permanent) {
-    $this->canCloseImmediately = $can_close_immediately;
+    $this->isPermanent = $is_permanent;
     return $this;
   }
 
-  public function getCanCloseImmediately() {
+  public function getIsPermanent() {
-    return $this->canCloseImmediately;
+    return $this->isPermanent;
   }
 
   public function setParents(array $parents) {

src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php

@@ -101,7 +101,7 @@ final class PhabricatorRepositoryDiscoveryEngine
         $repository,
         $ref->getIdentifier(),
         $ref->getEpoch(),
-        $ref->getCanCloseImmediately(),
+        $ref->getIsPermanent(),
         $ref->getParents(),
         $task_priority);
 
@@ -189,7 +189,7 @@ final class PhabricatorRepositoryDiscoveryEngine
       $head_refs = $this->discoverStreamAncestry(
         new PhabricatorGitGraphStream($repository, $commit),
         $commit,
-        $publisher->shouldPublishRef($ref));
+        $publisher->isPermanentRef($ref));
 
       $this->didDiscoverRefs($head_refs);
 
@@ -250,7 +250,7 @@ final class PhabricatorRepositoryDiscoveryEngine
         $refs[$identifier] = id(new PhabricatorRepositoryCommitRef())
           ->setIdentifier($identifier)
           ->setEpoch($epoch)
-          ->setCanCloseImmediately(true);
+          ->setIsPermanent(true);
 
         if ($upper_bound === null) {
           $upper_bound = $identifier;
@@ -354,7 +354,7 @@ final class PhabricatorRepositoryDiscoveryEngine
       $branch_refs = $this->discoverStreamAncestry(
         new PhabricatorMercurialGraphStream($repository, $commit),
         $commit,
-        $close_immediately = true);
+        $is_permanent = true);
 
       $this->didDiscoverRefs($branch_refs);
 
@@ -371,7 +371,7 @@ final class PhabricatorRepositoryDiscoveryEngine
   private function discoverStreamAncestry(
     PhabricatorRepositoryGraphStream $stream,
     $commit,
-    $close_immediately) {
+    $is_permanent) {
 
     $discover = array($commit);
     $graph = array();
@@ -424,7 +424,7 @@ final class PhabricatorRepositoryDiscoveryEngine
       $refs[] = id(new PhabricatorRepositoryCommitRef())
         ->setIdentifier($commit)
         ->setEpoch($epoch)
-        ->setCanCloseImmediately($close_immediately)
+        ->setIsPermanent($is_permanent)
         ->setParents($stream->getParents($commit));
     }
 
@@ -459,13 +459,6 @@ final class PhabricatorRepositoryDiscoveryEngine
       return true;
     }
 
-    if ($this->repairMode) {
-      // In repair mode, rediscover the entire repository, ignoring the
-      // database state. We can hit the local cache above, but if we miss it
-      // stop the script from going to the database cache.
-      return false;
-    }
-
     $this->fillCommitCache(array($identifier));
 
     return isset($this->commitCache[$identifier]);
@@ -476,6 +469,13 @@ final class PhabricatorRepositoryDiscoveryEngine
       return;
     }
 
+    if ($this->repairMode) {
+      // In repair mode, rediscover the entire repository, ignoring the
+      // database state. The engine still maintains a local cache (the
+      // "Working Set") but we just give up before looking in the database.
+      return;
+    }
+
     $max_size = self::MAX_COMMIT_CACHE_SIZE;
 
     // If we're filling more identifiers than would fit in the cache, ignore
@@ -507,9 +507,9 @@ final class PhabricatorRepositoryDiscoveryEngine
   }
 
   /**
-   * Sort branches so we process closeable branches first. This makes the
+   * Sort refs so we process permanent refs first. This makes the whole import
-   * whole import process a little cheaper, since we can close these commits
+   * process a little cheaper, since we can publish these commits the first
-   * the first time through rather than catching them in the refs step.
+   * time through rather than catching them in the refs step.
    *
    * @task internal
    *
@@ -523,7 +523,7 @@ final class PhabricatorRepositoryDiscoveryEngine
     $head_refs = array();
     $tail_refs = array();
     foreach ($refs as $ref) {
-      if ($publisher->shouldPublishRef($ref)) {
+      if ($publisher->isPermanentRef($ref)) {
         $head_refs[] = $ref;
       } else {
         $tail_refs[] = $ref;
@@ -538,7 +538,7 @@ final class PhabricatorRepositoryDiscoveryEngine
     PhabricatorRepository $repository,
     $commit_identifier,
     $epoch,
-    $close_immediately,
+    $is_permanent,
     array $parents,
     $task_priority) {
 
@@ -570,8 +570,8 @@ final class PhabricatorRepositoryDiscoveryEngine
     $commit->setRepositoryID($repository->getID());
     $commit->setCommitIdentifier($commit_identifier);
     $commit->setEpoch($epoch);
-    if ($close_immediately) {
+    if ($is_permanent) {
-      $commit->setImportStatus(PhabricatorRepositoryCommit::IMPORTED_CLOSEABLE);
+      $commit->setImportStatus(PhabricatorRepositoryCommit::IMPORTED_PERMANENT);
     }
 
     $data = new PhabricatorRepositoryCommitData();
@@ -655,7 +655,12 @@ final class PhabricatorRepositoryDiscoveryEngine
     $epoch,
     $task_priority) {
 
-    $this->insertTask($repository, $commit, $task_priority);
+    $this->queueCommitImportTask(
+      $repository,
+      $commit->getID(),
+      $commit->getPHID(),
+      $task_priority,
+      $via = 'discovery');
 
     // Update the repository summary table.
     queryfx(
@@ -679,36 +684,6 @@ final class PhabricatorRepositoryDiscoveryEngine
     }
   }
 
-  private function insertTask(
-    PhabricatorRepository $repository,
-    PhabricatorRepositoryCommit $commit,
-    $task_priority,
-    $data = array()) {
-
-    $vcs = $repository->getVersionControlSystem();
-    switch ($vcs) {
-      case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
-        $class = 'PhabricatorRepositoryGitCommitMessageParserWorker';
-        break;
-      case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
-        $class = 'PhabricatorRepositorySvnCommitMessageParserWorker';
-        break;
-      case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL:
-        $class = 'PhabricatorRepositoryMercurialCommitMessageParserWorker';
-        break;
-      default:
-        throw new Exception(pht("Unknown repository type '%s'!", $vcs));
-    }
-
-    $data['commitID'] = $commit->getID();
-
-    $options = array(
-      'priority' => $task_priority,
-    );
-
-    PhabricatorWorker::scheduleTask($class, $data, $options);
-  }
-
   private function isInitialImport(array $refs) {
     $commit_count = count($refs);
 
@@ -926,71 +901,4 @@ final class PhabricatorRepositoryDiscoveryEngine
       $data['epoch']);
   }
 
-  private function getImportTaskPriority(
-    PhabricatorRepository $repository,
-    array $refs) {
-
-    // If the repository is importing for the first time, we schedule tasks
-    // at IMPORT priority, which is very low. Making progress on importing a
-    // new repository for the first time is less important than any other
-    // daemon task.
-
-    // If the repository has finished importing and we're just catching up
-    // on recent commits, we usually schedule discovery at COMMIT priority,
-    // which is slightly below the default priority.
-
-    // Note that followup tasks and triggered tasks (like those generated by
-    // Herald or Harbormaster) will queue at DEFAULT priority, so that each
-    // commit tends to fully import before we start the next one. This tends
-    // to give imports fairly predictable progress. See T11677 for some
-    // discussion.
-
-    if ($repository->isImporting()) {
-      $this->log(
-        pht(
-          'Importing %s commit(s) at low priority ("PRIORITY_IMPORT") '.
-          'because this repository is still importing.',
-          phutil_count($refs)));
-
-      return PhabricatorWorker::PRIORITY_IMPORT;
-    }
-
-    // See T13369. If we've discovered a lot of commits at once, import them
-    // at lower priority.
-
-    // This is mostly aimed at reducing the impact that synchronizing thousands
-    // of commits from a remote upstream has on other repositories. The queue
-    // is "mostly FIFO", so queueing a thousand commit imports can stall other
-    // repositories.
-
-    // In a perfect world we'd probably give repositories round-robin queue
-    // priority, but we don't currently have the primitives for this and there
-    // isn't a strong case for building them.
-
-    // Use "a whole lot of commits showed up at once" as a heuristic for
-    // detecting "someone synchronized an upstream", and import them at a lower
-    // priority to more closely approximate fair scheduling.
-
-    if (count($refs) >= PhabricatorRepository::LOWPRI_THRESHOLD) {
-      $this->log(
-        pht(
-          'Importing %s commit(s) at low priority ("PRIORITY_IMPORT") '.
-          'because many commits were discovered at once.',
-          phutil_count($refs)));
-
-      return PhabricatorWorker::PRIORITY_IMPORT;
-    }
-
-    // Otherwise, import at normal priority.
-
-    if ($refs) {
-      $this->log(
-        pht(
-          'Importing %s commit(s) at normal priority ("PRIORITY_COMMIT").',
-          phutil_count($refs)));
-    }
-
-    return PhabricatorWorker::PRIORITY_COMMIT;
-  }
-
 }

src/applications/repository/engine/PhabricatorRepositoryEngine.php

@@ -83,4 +83,114 @@ abstract class PhabricatorRepositoryEngine extends Phobject {
     return $this;
   }
 
+  final protected function queueCommitImportTask(
+    PhabricatorRepository $repository,
+    $commit_id,
+    $commit_phid,
+    $task_priority,
+    $via = null) {
+
+    $vcs = $repository->getVersionControlSystem();
+    switch ($vcs) {
+      case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
+        $class = 'PhabricatorRepositoryGitCommitMessageParserWorker';
+        break;
+      case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
+        $class = 'PhabricatorRepositorySvnCommitMessageParserWorker';
+        break;
+      case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL:
+        $class = 'PhabricatorRepositoryMercurialCommitMessageParserWorker';
+        break;
+      default:
+        throw new Exception(
+          pht(
+            'Unknown repository type "%s"!',
+            $vcs));
+    }
+
+    $data = array(
+      'commitID' => $commit_id,
+    );
+
+    if ($via !== null) {
+      $data['via'] = $via;
+    }
+
+    $options = array(
+      'priority' => $task_priority,
+      'objectPHID' => $commit_phid,
+    );
+
+    PhabricatorWorker::scheduleTask($class, $data, $options);
+  }
+
+  final protected function getImportTaskPriority(
+    PhabricatorRepository $repository,
+    array $refs) {
+    assert_instances_of($refs, 'PhabricatorRepositoryCommitRef');
+
+    // If the repository is importing for the first time, we schedule tasks
+    // at IMPORT priority, which is very low. Making progress on importing a
+    // new repository for the first time is less important than any other
+    // daemon task.
+
+    // If the repository has finished importing and we're just catching up
+    // on recent commits, we usually schedule discovery at COMMIT priority,
+    // which is slightly below the default priority.
+
+    // Note that followup tasks and triggered tasks (like those generated by
+    // Herald or Harbormaster) will queue at DEFAULT priority, so that each
+    // commit tends to fully import before we start the next one. This tends
+    // to give imports fairly predictable progress. See T11677 for some
+    // discussion.
+
+    if ($repository->isImporting()) {
+      $this->log(
+        pht(
+          'Importing %s commit(s) at low priority ("PRIORITY_IMPORT") '.
+          'because this repository is still importing.',
+          phutil_count($refs)));
+
+      return PhabricatorWorker::PRIORITY_IMPORT;
+    }
+
+    // See T13369. If we've discovered a lot of commits at once, import them
+    // at lower priority.
+
+    // This is mostly aimed at reducing the impact that synchronizing thousands
+    // of commits from a remote upstream has on other repositories. The queue
+    // is "mostly FIFO", so queueing a thousand commit imports can stall other
+    // repositories.
+
+    // In a perfect world we'd probably give repositories round-robin queue
+    // priority, but we don't currently have the primitives for this and there
+    // isn't a strong case for building them.
+
+    // Use "a whole lot of commits showed up at once" as a heuristic for
+    // detecting "someone synchronized an upstream", and import them at a lower
+    // priority to more closely approximate fair scheduling.
+
+    if (count($refs) >= PhabricatorRepository::LOWPRI_THRESHOLD) {
+      $this->log(
+        pht(
+          'Importing %s commit(s) at low priority ("PRIORITY_IMPORT") '.
+          'because many commits were discovered at once.',
+          phutil_count($refs)));
+
+      return PhabricatorWorker::PRIORITY_IMPORT;
+    }
+
+    // Otherwise, import at normal priority.
+
+    if ($refs) {
+      $this->log(
+        pht(
+          'Importing %s commit(s) at normal priority ("PRIORITY_COMMIT").',
+          phutil_count($refs)));
+    }
+
+    return PhabricatorWorker::PRIORITY_COMMIT;
+  }
+
+
 }

src/applications/repository/engine/PhabricatorRepositoryRefEngine.php

@@ -9,7 +9,7 @@ final class PhabricatorRepositoryRefEngine
 
   private $newPositions = array();
   private $deadPositions = array();
-  private $closeCommits = array();
+  private $permanentCommits = array();
   private $rebuild;
 
   public function setRebuild($rebuild) {
@@ -24,7 +24,7 @@ final class PhabricatorRepositoryRefEngine
   public function updateRefs() {
     $this->newPositions = array();
     $this->deadPositions = array();
-    $this->closeCommits = array();
+    $this->permanentCommits = array();
 
     $repository = $this->getRepository();
     $viewer = $this->getViewer();
@@ -96,11 +96,11 @@ final class PhabricatorRepositoryRefEngine
       $this->updateCursors($cursor_group, $refs, $type, $all_closing_heads);
     }
 
-    if ($this->closeCommits) {
+    if ($this->permanentCommits) {
-      $this->setCloseFlagOnCommits($this->closeCommits);
+      $this->setPermanentFlagOnCommits($this->permanentCommits);
     }
 
-    $save_cursors = $this->getCursorsForUpdate($all_cursors);
+    $save_cursors = $this->getCursorsForUpdate($repository, $all_cursors);
 
     if ($this->newPositions || $this->deadPositions || $save_cursors) {
       $repository->openTransaction();
@@ -121,17 +121,19 @@ final class PhabricatorRepositoryRefEngine
     }
   }
 
-  private function getCursorsForUpdate(array $cursors) {
+  private function getCursorsForUpdate(
+    PhabricatorRepository $repository,
+    array $cursors) {
     assert_instances_of($cursors, 'PhabricatorRepositoryRefCursor');
 
+    $publisher = $repository->newPublisher();
+
     $results = array();
 
     foreach ($cursors as $cursor) {
-      $ref_type = $cursor->getRefType();
+      $diffusion_ref = $cursor->newDiffusionRepositoryRef();
-      $ref_name = $cursor->getRefName();
-
-      $is_permanent = $this->isPermanentRef($ref_type, $ref_name);
 
+      $is_permanent = $publisher->isPermanentRef($diffusion_ref);
       if ($is_permanent == $cursor->getIsPermanent()) {
         continue;
       }
@@ -217,9 +219,9 @@ final class PhabricatorRepositoryRefEngine
     return $this;
   }
 
-  private function markCloseCommits(array $identifiers) {
+  private function markPermanentCommits(array $identifiers) {
     foreach ($identifiers as $identifier) {
-      $this->closeCommits[$identifier] = $identifier;
+      $this->permanentCommits[$identifier] = $identifier;
     }
     return $this;
   }
@@ -259,6 +261,7 @@ final class PhabricatorRepositoryRefEngine
     $ref_type,
     array $all_closing_heads) {
     $repository = $this->getRepository();
+    $publisher = $repository->newPublisher();
 
     // NOTE: Mercurial branches may have multiple branch heads; this logic
     // is complex primarily to account for that.
@@ -341,7 +344,7 @@ final class PhabricatorRepositoryRefEngine
         $this->markPositionNew($new_position);
       }
 
-      if ($this->isPermanentRef($ref_type, $name)) {
+      if ($publisher->isPermanentRef(head($refs))) {
 
         // See T13284. If this cursor was already marked as permanent, we
         // only need to publish the newly created ref positions. However, if
@@ -377,7 +380,7 @@ final class PhabricatorRepositoryRefEngine
             $identifier,
             $exclude);
 
-          $this->markCloseCommits($new_identifiers);
+          $this->markPermanentCommits($new_identifiers);
         }
       }
     }
@@ -404,14 +407,6 @@ final class PhabricatorRepositoryRefEngine
     }
   }
 
-  private function isPermanentRef($ref_type, $ref_name) {
-    if ($ref_type !== PhabricatorRepositoryRefCursor::TYPE_BRANCH) {
-      return false;
-    }
-
-    return $this->getRepository()->isBranchPermanentRef($ref_name);
-  }
-
   /**
    * Find all ancestors of a new closing branch head which are not ancestors
    * of any old closing branch head.
@@ -483,17 +478,17 @@ final class PhabricatorRepositoryRefEngine
           $ref_list = implode("\n", $ref_list)."\n";
 
           $future = $this->getRepository()->getLocalCommandFuture(
-            'log --format=%s --stdin',
+            'log %s --stdin --',
-            '%H');
+            '--format=%H');
 
           list($stdout) = $future
             ->write($ref_list)
             ->resolvex();
         } else {
           list($stdout) = $this->getRepository()->execxLocalCommand(
-            'log --format=%s %s',
+            'log %s %s --',
-            '%H',
+            '--format=%H',
-            $new_head);
+            gitsprintf('%s', $new_head));
         }
 
         $stdout = trim($stdout);
@@ -507,10 +502,10 @@ final class PhabricatorRepositoryRefEngine
   }
 
   /**
-   * Mark a list of commits as closeable, and queue workers for those commits
+   * Mark a list of commits as permanent, and queue workers for those commits
    * which don't already have the flag.
    */
-  private function setCloseFlagOnCommits(array $identifiers) {
+  private function setPermanentFlagOnCommits(array $identifiers) {
     $repository = $this->getRepository();
     $commit_table = new PhabricatorRepositoryCommit();
     $conn = $commit_table->establishConnection('w');
@@ -552,7 +547,23 @@ final class PhabricatorRepositoryRefEngine
       }
     }
 
-    $closeable_flag = PhabricatorRepositoryCommit::IMPORTED_CLOSEABLE;
+    $commit_refs = array();
+    foreach ($identifiers as $identifier) {
+
+      // See T13591. This construction is a bit ad-hoc, but the priority
+      // function currently only cares about the number of refs we have
+      // discovered, so we'll get the right result even without filling
+      // these records out in detail.
+
+      $commit_refs[] = id(new PhabricatorRepositoryCommitRef())
+        ->setIdentifier($identifier);
+    }
+
+    $task_priority = $this->getImportTaskPriority(
+      $repository,
+      $commit_refs);
+
+    $permanent_flag = PhabricatorRepositoryCommit::IMPORTED_PERMANENT;
     $published_flag = PhabricatorRepositoryCommit::IMPORTED_PUBLISH;
 
     $all_commits = ipull($all_commits, null, 'commitIdentifier');
@@ -568,9 +579,9 @@ final class PhabricatorRepositoryRefEngine
       }
 
       $import_status = $row['importStatus'];
-      if (!($import_status & $closeable_flag)) {
+      if (!($import_status & $permanent_flag)) {
-        // Set the "closeable" flag.
+        // Set the "permanent" flag.
-        $import_status = ($import_status | $closeable_flag);
+        $import_status = ($import_status | $permanent_flag);
 
         // See T13580. Clear the "published" flag, so publishing executes
         // again. We may have previously performed a no-op "publish" on the
@@ -584,17 +595,12 @@ final class PhabricatorRepositoryRefEngine
           $import_status,
           $row['id']);
 
-        $data = array(
+        $this->queueCommitImportTask(
-          'commitID' => $row['id'],
+          $repository,
-        );
+          $row['id'],
-
+          $row['phid'],
-        PhabricatorWorker::scheduleTask(
+          $task_priority,
-          $class,
+          $via = 'ref');
-          $data,
-          array(
-            'priority' => PhabricatorWorker::PRIORITY_COMMIT,
-            'objectPHID' => $row['phid'],
-          ));
       }
     }
 
@@ -606,13 +612,17 @@ final class PhabricatorRepositoryRefEngine
     $ref_type,
     $ref_name) {
 
-    $is_permanent = $this->isPermanentRef($ref_type, $ref_name);
-
     $cursor = id(new PhabricatorRepositoryRefCursor())
       ->setRepositoryPHID($repository->getPHID())
       ->setRefType($ref_type)
-      ->setRefName($ref_name)
+      ->setRefName($ref_name);
-      ->setIsPermanent((int)$is_permanent);
+
+    $publisher = $repository->newPublisher();
+
+    $diffusion_ref = $cursor->newDiffusionRepositoryRef();
+    $is_permanent = $publisher->isPermanentRef($diffusion_ref);
+
+    $cursor->setIsPermanent((int)$is_permanent);
 
     try {
       return $cursor->save();

src/applications/repository/engine/__tests__/PhabricatorWorkingCopyDiscoveryTestCase.php

@@ -4,6 +4,8 @@ final class PhabricatorWorkingCopyDiscoveryTestCase
   extends PhabricatorWorkingCopyTestCase {
 
   public function testSubversionCommitDiscovery() {
+    $this->requireBinaryForTest('svn');
+
     $refs = $this->discoverRefs('ST');
     $this->assertEqual(
       array(

src/applications/repository/management/PhabricatorRepositoryManagementDiscoverWorkflow.php

@@ -7,21 +7,22 @@ final class PhabricatorRepositoryManagementDiscoverWorkflow
     $this
       ->setName('discover')
       ->setExamples('**discover** [__options__] __repository__ ...')
-      ->setSynopsis(pht('Discover __repository__.'))
+      ->setSynopsis(pht('Discover commits in __repository__.'))
       ->setArguments(
         array(
           array(
-            'name'        => 'verbose',
+            'name' => 'verbose',
-            'help'        => pht('Show additional debugging information.'),
+            'help' => pht('Show additional debugging information.'),
           ),
           array(
-            'name'        => 'repair',
+            'name' => 'repair',
-            'help'        => pht(
+            'help' => pht(
-              'Repair a repository with gaps in commit history.'),
+              'Discover all commits, even if they are ancestors of known '.
+              'commits. This can repair gaps in repository history.'),
           ),
           array(
-            'name'        => 'repos',
+            'name' => 'repos',
-            'wildcard'    => true,
+            'wildcard' => true,
           ),
         ));
   }

src/applications/repository/query/PhabricatorRepositoryPublisher.php

@@ -38,6 +38,10 @@ final class PhabricatorRepositoryPublisher
     return !$this->getCommitHoldReasons($commit);
   }
 
+  public function isPermanentRef(DiffusionRepositoryRef $ref) {
+    return !$this->getRefImpermanentReasons($ref);
+  }
+
 /* -(  Hold Reasons  )------------------------------------------------------- */
 
   public function getRepositoryHoldReasons() {
@@ -59,18 +63,8 @@ final class PhabricatorRepositoryPublisher
     $repository = $this->getRepository();
     $reasons = $this->getRepositoryHoldReasons();
 
-    if (!$ref->isBranch()) {
+    foreach ($this->getRefImpermanentReasons($ref) as $reason) {
-      $reasons[] = self::HOLD_REF_NOT_BRANCH;
+      $reasons[] = $reason;
-    } else {
-      $branch_name = $ref->getShortName();
-
-      if (!$repository->shouldTrackBranch($branch_name)) {
-        $reasons[] = self::HOLD_UNTRACKED;
-      }
-
-      if (!$repository->isBranchPermanentRef($branch_name)) {
-        $reasons[] = self::HOLD_NOT_PERMANENT_REF;
-      }
     }
 
     return $reasons;
@@ -89,4 +83,25 @@ final class PhabricatorRepositoryPublisher
     return $reasons;
   }
 
+  public function getRefImpermanentReasons(DiffusionRepositoryRef $ref) {
+    $repository = $this->getRepository();
+    $reasons = array();
+
+    if (!$ref->isBranch()) {
+      $reasons[] = self::HOLD_REF_NOT_BRANCH;
+    } else {
+      $branch_name = $ref->getShortName();
+
+      if (!$repository->shouldTrackBranch($branch_name)) {
+        $reasons[] = self::HOLD_UNTRACKED;
+      }
+
+      if (!$repository->isBranchPermanentRef($branch_name)) {
+        $reasons[] = self::HOLD_NOT_PERMANENT_REF;
+      }
+    }
+
+    return $reasons;
+  }
+
 }

src/applications/repository/storage/PhabricatorRepositoryCommit.php

@@ -36,7 +36,7 @@ final class PhabricatorRepositoryCommit
   const IMPORTED_PUBLISH = 8;
   const IMPORTED_ALL = 11;
 
-  const IMPORTED_CLOSEABLE = 1024;
+  const IMPORTED_PERMANENT = 1024;
   const IMPORTED_UNREACHABLE = 2048;
 
   private $commitData = self::ATTACHABLE;
@@ -467,7 +467,7 @@ final class PhabricatorRepositoryCommit
   }
 
   public function isPermanentCommit() {
-    return (bool)$this->isPartiallyImported(self::IMPORTED_CLOSEABLE);
+    return (bool)$this->isPartiallyImported(self::IMPORTED_PERMANENT);
   }
 
   public function newCommitAuthorView(PhabricatorUser $viewer) {

src/applications/repository/storage/PhabricatorRepositoryRefCursor.php

@@ -88,6 +88,12 @@ final class PhabricatorRepositoryRefCursor
     return mpull($this->getPositions(), 'getCommitIdentifier');
   }
 
+  public function newDiffusionRepositoryRef() {
+    return id(new DiffusionRepositoryRef())
+      ->setRefType($this->getRefType())
+      ->setShortName($this->getRefName());
+  }
+
 
 /* -(  PhabricatorPolicyInterface  )----------------------------------------- */
 

src/infrastructure/daemon/workers/management/PhabricatorWorkerManagementWorkflow.php

@@ -21,6 +21,10 @@ abstract class PhabricatorWorkerManagementWorkflow
         'param' => 'int',
         'help' => pht('Limit to tasks with at least this many failures.'),
       ),
+      array(
+        'name' => 'active',
+        'help' => pht('Select all active tasks.'),
+      ),
     );
   }
 
@@ -28,10 +32,13 @@ abstract class PhabricatorWorkerManagementWorkflow
     $ids = $args->getArg('id');
     $class = $args->getArg('class');
     $min_failures = $args->getArg('min-failure-count');
+    $active = $args->getArg('active');
 
-    if (!$ids && !$class && !$min_failures) {
+    if (!$ids && !$class && !$min_failures && !$active) {
       throw new PhutilArgumentUsageException(
-        pht('Use --id, --class, or --min-failure-count to select tasks.'));
+        pht(
+          'Use "--id", "--class", "--active", and/or "--min-failure-count" '.
+          'to select tasks.'));
     }
 
     $active_query = new PhabricatorWorkerActiveTaskQuery();
@@ -56,7 +63,13 @@ abstract class PhabricatorWorkerManagementWorkflow
     }
 
     $active_tasks = $active_query->execute();
-    $archive_tasks = $archive_query->execute();
+
+    if ($active) {
+      $archive_tasks = array();
+    } else {
+      $archive_tasks = $archive_query->execute();
+    }
+
     $tasks =
       mpull($active_tasks, null, 'getID') +
       mpull($archive_tasks, null, 'getID');

src/infrastructure/markup/rule/PhabricatorKeyboardRemarkupRule.php

@@ -61,6 +61,22 @@ final class PhabricatorKeyboardRemarkupRule extends PhutilRemarkupRule {
           'escape',
         ),
       ),
+      array(
+        'name' => pht('Enter'),
+        'symbol' => "\xE2\x8F\x8E",
+        'aliases' => array(
+          'enter',
+          'return',
+        ),
+      ),
+      array(
+        'name' => pht('Control'),
+        'symbol' => "\xE2\x8C\x83",
+        'aliases' => array(
+          'ctrl',
+          'control',
+        ),
+      ),
       array(
         'name' => pht('Up'),
         'symbol' => "\xE2\x86\x91",

src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php

@@ -234,7 +234,7 @@ abstract class PhabricatorPolicyAwareQuery extends PhabricatorOffsetPagedQuery {
     // T11773 for some discussion.
     $this->isOverheated = false;
 
-    // See T13386. If we on an old offset-based paging workflow, we need
+    // See T13386. If we are on an old offset-based paging workflow, we need
     // to base the overheating limit on both the offset and limit.
     $overheat_limit = $need * 10;
     $total_seen = 0;

support/startup/PhabricatorStartup.php

@@ -35,6 +35,7 @@
  * @task validation   Validation
  * @task ratelimit    Rate Limiting
  * @task phases       Startup Phase Timers
+ * @task request-path Request Path
  */
 final class PhabricatorStartup {
 
@@ -47,6 +48,7 @@ final class PhabricatorStartup {
   private static $phases;
 
   private static $limits = array();
+  private static $requestPath;
 
 
 /* -(  Accessing Request Information  )-------------------------------------- */
@@ -119,6 +121,7 @@ final class PhabricatorStartup {
     self::$phases = array();
 
     self::$accessLog = null;
+    self::$requestPath = null;
 
     static $registered;
     if (!$registered) {
@@ -140,7 +143,7 @@ final class PhabricatorStartup {
 
     self::normalizeInput();
 
-    self::verifyRewriteRules();
+    self::readRequestPath();
 
     self::beginOutputCapture();
   }
@@ -552,17 +555,29 @@ final class PhabricatorStartup {
 
 
   /**
-   * @task validation
+   * @task request-path
    */
-  private static function verifyRewriteRules() {
+  private static function readRequestPath() {
+
+    // See T13575. The request path may be provided in:
+    //
+    //  - the "$_GET" parameter "__path__" (normal for Apache and nginx); or
+    //  - the "$_SERVER" parameter "REQUEST_URI" (normal for the PHP builtin
+    //    webserver).
+    //
+    // Locate it wherever it is, and store it for later use. Note that writing
+    // to "$_REQUEST" here won't always work, because later code may rebuild
+    // "$_REQUEST" from other sources.
+
     if (isset($_REQUEST['__path__']) && strlen($_REQUEST['__path__'])) {
+      self::setRequestPath($_REQUEST['__path__']);
       return;
     }
 
+    // Compatibility with PHP 5.4+ built-in web server.
     if (php_sapi_name() == 'cli-server') {
-      // Compatibility with PHP 5.4+ built-in web server.
+      $path = parse_url($_SERVER['REQUEST_URI']);
-      $url = parse_url($_SERVER['REQUEST_URI']);
+      self::setRequestPath($path['path']);
-      $_REQUEST['__path__'] = $url['path'];
       return;
     }
 
@@ -580,6 +595,30 @@ final class PhabricatorStartup {
     }
   }
 
+  /**
+   * @task request-path
+   */
+  public static function getRequestPath() {
+    $path = self::$requestPath;
+
+    if ($path === null) {
+      self::didFatal(
+        'Request attempted to access request path, but no request path is '.
+        'available for this request. You may be calling web request code '.
+        'from a non-request context, or your webserver may not be passing '.
+        'a request path to Phabricator in a format that it understands.');
+    }
+
+    return $path;
+  }
+
+  /**
+   * @task request-path
+   */
+  public static function setRequestPath($path) {
+    self::$requestPath = $path;
+  }
+
 
 /* -(  Rate Limiting  )------------------------------------------------------ */