(stable) Promote 2019 Week 33

src/applications/config/management/PhabricatorConfigManagementSetWorkflow.php

@@ -30,11 +30,10 @@ final class PhabricatorConfigManagementSetWorkflow
   }
 
   public function execute(PhutilArgumentParser $args) {
-    $console = PhutilConsole::getConsole();
     $argv = $args->getArg('args');
-    if (count($argv) == 0) {
+    if (!$argv) {
       throw new PhutilArgumentUsageException(
-        pht('Specify a configuration key and a value to set it to.'));
+        pht('Specify the configuration key you want to set.'));
     }
 
     $is_stdin = $args->getArg('stdin');
@@ -45,7 +44,8 @@ final class PhabricatorConfigManagementSetWorkflow
       if (count($argv) > 1) {
         throw new PhutilArgumentUsageException(
           pht(
-            'Too many arguments: expected only a key when using "--stdin".'));
+            'Too many arguments: expected only a configuration key when '.
+            'using "--stdin".'));
       }
 
       fprintf(STDERR, tsprintf("%s\n", pht('Reading value from stdin...')));
@@ -54,7 +54,8 @@ final class PhabricatorConfigManagementSetWorkflow
       if (count($argv) == 1) {
         throw new PhutilArgumentUsageException(
           pht(
-            "Specify a value to set the key '%s' to.",
+            'Specify a value to set the configuration key "%s" to, or '.
+            'use "--stdin" to read a value from stdin.',
             $key));
       }
 
@@ -67,14 +68,13 @@ final class PhabricatorConfigManagementSetWorkflow
       $value = $argv[1];
     }
 
-
     $options = PhabricatorApplicationConfigOptions::loadAllOptions();
     if (empty($options[$key])) {
       throw new PhutilArgumentUsageException(
         pht(
-          "No such configuration key '%s'! Use `%s` to list all keys.",
-          $key,
-          'config list'));
+          'Configuration key "%s" is unknown. Use "bin/config list" to list '.
+          'all known keys.',
+          $key));
     }
 
     $option = $options[$key];
@@ -99,7 +99,7 @@ final class PhabricatorConfigManagementSetWorkflow
             switch ($type) {
               default:
                 $message = pht(
-                  'Config key "%s" is of type "%s". Specify it in JSON.',
+                  'Configuration key "%s" is of type "%s". Specify it in JSON.',
                   $key,
                   $type);
                 break;
@@ -128,7 +128,6 @@ final class PhabricatorConfigManagementSetWorkflow
     }
 
     if ($use_database) {
-      $config_type = 'database';
       $config_entry = PhabricatorConfigEntry::loadConfigEntry($key);
       $config_entry->setValue($value);
 
@@ -136,15 +135,28 @@ final class PhabricatorConfigManagementSetWorkflow
       $config_entry->setIsDeleted(0);
 
       $config_entry->save();
+
+      $write_message = pht(
+        'Wrote configuration key "%s" to database storage.',
+        $key);
     } else {
-      $config_type = 'local';
-      id(new PhabricatorConfigLocalSource())
+      $config_source = id(new PhabricatorConfigLocalSource())
         ->setKeys(array($key => $value));
+
+      $local_path = $config_source->getReadablePath();
+
+      $write_message = pht(
+        'Wrote configuration key "%s" to local storage (in file "%s").',
+        $key,
+        $local_path);
     }
 
-    $console->writeOut(
-      "%s\n",
-      pht("Set '%s' in %s configuration.", $key, $config_type));
+    echo tsprintf(
+      "<bg:green>** %s **</bg> %s\n",
+      pht('DONE'),
+      $write_message);
+
+    return 0;
   }
 
 }

src/applications/daemon/controller/PhabricatorDaemonConsoleController.php

@@ -85,6 +85,7 @@ final class PhabricatorDaemonConsoleController
         phutil_tag('em', array(), pht('Temporary Failures')),
         count($failed),
         null,
+        null,
       );
     }
 

src/applications/project/query/PhabricatorProjectQuery.php

@@ -464,28 +464,28 @@ final class PhabricatorProjectQuery
       }
       $where[] = qsprintf(
         $conn,
-        'status IN (%Ld)',
+        'project.status IN (%Ld)',
         $filter);
     }
 
     if ($this->statuses !== null) {
       $where[] = qsprintf(
         $conn,
-        'status IN (%Ls)',
+        'project.status IN (%Ls)',
         $this->statuses);
     }
 
     if ($this->ids !== null) {
       $where[] = qsprintf(
         $conn,
-        'id IN (%Ld)',
+        'project.id IN (%Ld)',
         $this->ids);
     }
 
     if ($this->phids !== null) {
       $where[] = qsprintf(
         $conn,
-        'phid IN (%Ls)',
+        'project.phid IN (%Ls)',
         $this->phids);
     }
 
@@ -513,7 +513,7 @@ final class PhabricatorProjectQuery
     if ($this->names !== null) {
       $where[] = qsprintf(
         $conn,
-        'name IN (%Ls)',
+        'project.name IN (%Ls)',
         $this->names);
     }
 
@@ -522,7 +522,7 @@ final class PhabricatorProjectQuery
       foreach ($this->namePrefixes as $name_prefix) {
         $parts[] = qsprintf(
           $conn,
-          'name LIKE %>',
+          'project.name LIKE %>',
           $name_prefix);
       }
       $where[] = qsprintf($conn, '%LO', $parts);
@@ -531,21 +531,21 @@ final class PhabricatorProjectQuery
     if ($this->icons !== null) {
       $where[] = qsprintf(
         $conn,
-        'icon IN (%Ls)',
+        'project.icon IN (%Ls)',
         $this->icons);
     }
 
     if ($this->colors !== null) {
       $where[] = qsprintf(
         $conn,
-        'color IN (%Ls)',
+        'project.color IN (%Ls)',
         $this->colors);
     }
 
     if ($this->parentPHIDs !== null) {
       $where[] = qsprintf(
         $conn,
-        'parentProjectPHID IN (%Ls)',
+        'project.parentProjectPHID IN (%Ls)',
         $this->parentPHIDs);
     }
 
@@ -563,7 +563,7 @@ final class PhabricatorProjectQuery
       foreach ($ancestor_paths as $ancestor_path) {
         $sql[] = qsprintf(
           $conn,
-          '(projectPath LIKE %> AND projectDepth > %d)',
+          '(project.projectPath LIKE %> AND project.projectDepth > %d)',
           $ancestor_path['projectPath'],
           $ancestor_path['projectDepth']);
       }
@@ -572,18 +572,18 @@ final class PhabricatorProjectQuery
 
       $where[] = qsprintf(
         $conn,
-        'parentProjectPHID IS NOT NULL');
+        'project.parentProjectPHID IS NOT NULL');
     }
 
     if ($this->isMilestone !== null) {
       if ($this->isMilestone) {
         $where[] = qsprintf(
           $conn,
-          'milestoneNumber IS NOT NULL');
+          'project.milestoneNumber IS NOT NULL');
       } else {
         $where[] = qsprintf(
           $conn,
-          'milestoneNumber IS NULL');
+          'project.milestoneNumber IS NULL');
       }
     }
 
@@ -591,42 +591,42 @@ final class PhabricatorProjectQuery
     if ($this->hasSubprojects !== null) {
       $where[] = qsprintf(
         $conn,
-        'hasSubprojects = %d',
+        'project.hasSubprojects = %d',
         (int)$this->hasSubprojects);
     }
 
     if ($this->minDepth !== null) {
       $where[] = qsprintf(
         $conn,
-        'projectDepth >= %d',
+        'project.projectDepth >= %d',
         $this->minDepth);
     }
 
     if ($this->maxDepth !== null) {
       $where[] = qsprintf(
         $conn,
-        'projectDepth <= %d',
+        'project.projectDepth <= %d',
         $this->maxDepth);
     }
 
     if ($this->minMilestoneNumber !== null) {
       $where[] = qsprintf(
         $conn,
-        'milestoneNumber >= %d',
+        'project.milestoneNumber >= %d',
         $this->minMilestoneNumber);
     }
 
     if ($this->maxMilestoneNumber !== null) {
       $where[] = qsprintf(
         $conn,
-        'milestoneNumber <= %d',
+        'project.milestoneNumber <= %d',
         $this->maxMilestoneNumber);
     }
 
     if ($this->subtypes !== null) {
       $where[] = qsprintf(
         $conn,
-        'subtype IN (%Ls)',
+        'project.subtype IN (%Ls)',
         $this->subtypes);
     }
 
@@ -646,7 +646,7 @@ final class PhabricatorProjectQuery
     if ($this->memberPHIDs !== null) {
       $joins[] = qsprintf(
         $conn,
-        'JOIN %T e ON e.src = p.phid AND e.type = %d',
+        'JOIN %T e ON e.src = project.phid AND e.type = %d',
         PhabricatorEdgeConfig::TABLE_NAME_EDGE,
         PhabricatorProjectMaterializedMemberEdgeType::EDGECONST);
     }
@@ -654,7 +654,7 @@ final class PhabricatorProjectQuery
     if ($this->watcherPHIDs !== null) {
       $joins[] = qsprintf(
         $conn,
-        'JOIN %T w ON w.src = p.phid AND w.type = %d',
+        'JOIN %T w ON w.src = project.phid AND w.type = %d',
         PhabricatorEdgeConfig::TABLE_NAME_EDGE,
         PhabricatorObjectHasWatcherEdgeType::EDGECONST);
     }
@@ -662,7 +662,7 @@ final class PhabricatorProjectQuery
     if ($this->slugs !== null) {
       $joins[] = qsprintf(
         $conn,
-        'JOIN %T slug on slug.projectPHID = p.phid',
+        'JOIN %T slug on slug.projectPHID = project.phid',
         id(new PhabricatorProjectSlug())->getTableName());
     }
 
@@ -672,7 +672,7 @@ final class PhabricatorProjectQuery
         $token_table = 'token_'.$key;
         $joins[] = qsprintf(
           $conn,
-          'JOIN %T %T ON %T.projectID = p.id AND %T.token LIKE %>',
+          'JOIN %T %T ON %T.projectID = project.id AND %T.token LIKE %>',
           PhabricatorProject::TABLE_DATASOURCE_TOKEN,
           $token_table,
           $token_table,
@@ -689,7 +689,7 @@ final class PhabricatorProjectQuery
   }
 
   protected function getPrimaryTableAlias() {
-    return 'p';
+    return 'project';
   }
 
   private function linkProjectGraph(array $projects, array $ancestors) {

src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php

@@ -93,6 +93,8 @@ final class PhabricatorRepositoryDiscoveryEngine
     // Clear the working set cache.
     $this->workingSet = array();
 
+    $task_priority = $this->getImportTaskPriority($repository, $refs);
+
     // Record discovered commits and mark them in the cache.
     foreach ($refs as $ref) {
       $this->recordCommit(
@@ -100,7 +102,8 @@ final class PhabricatorRepositoryDiscoveryEngine
         $ref->getIdentifier(),
         $ref->getEpoch(),
         $ref->getCanCloseImmediately(),
-        $ref->getParents());
+        $ref->getParents(),
+        $task_priority);
 
       $this->commitCache[$ref->getIdentifier()] = true;
     }
@@ -536,7 +539,8 @@ final class PhabricatorRepositoryDiscoveryEngine
     $commit_identifier,
     $epoch,
     $close_immediately,
-    array $parents) {
+    array $parents,
+    $task_priority) {
 
     $commit = new PhabricatorRepositoryCommit();
     $conn_w = $repository->establishConnection('w');
@@ -559,7 +563,7 @@ final class PhabricatorRepositoryDiscoveryEngine
         $commit_identifier);
 
       // After reviving a commit, schedule new daemons for it.
-      $this->didDiscoverCommit($repository, $commit, $epoch);
+      $this->didDiscoverCommit($repository, $commit, $epoch, $task_priority);
       return;
     }
 
@@ -620,7 +624,7 @@ final class PhabricatorRepositoryDiscoveryEngine
         }
       $commit->saveTransaction();
 
-      $this->didDiscoverCommit($repository, $commit, $epoch);
+      $this->didDiscoverCommit($repository, $commit, $epoch, $task_priority);
 
       if ($this->repairMode) {
         // Normally, the query should throw a duplicate key exception. If we
@@ -648,9 +652,10 @@ final class PhabricatorRepositoryDiscoveryEngine
   private function didDiscoverCommit(
     PhabricatorRepository $repository,
     PhabricatorRepositoryCommit $commit,
-    $epoch) {
+    $epoch,
+    $task_priority) {
 
-    $this->insertTask($repository, $commit);
+    $this->insertTask($repository, $commit, $task_priority);
 
     // Update the repository summary table.
     queryfx(
@@ -677,6 +682,7 @@ final class PhabricatorRepositoryDiscoveryEngine
   private function insertTask(
     PhabricatorRepository $repository,
     PhabricatorRepositoryCommit $commit,
+    $task_priority,
     $data = array()) {
 
     $vcs = $repository->getVersionControlSystem();
@@ -696,27 +702,6 @@ final class PhabricatorRepositoryDiscoveryEngine
 
     $data['commitID'] = $commit->getID();
 
-    // 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 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()) {
-      $task_priority = PhabricatorWorker::PRIORITY_IMPORT;
-    } else {
-      $task_priority = PhabricatorWorker::PRIORITY_COMMIT;
-    }
-
     $options = array(
       'priority' => $task_priority,
     );
@@ -934,4 +919,71 @@ 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/storage/PhabricatorRepository.php

@@ -34,6 +34,8 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
    */
   const IMPORT_THRESHOLD = 7;
 
+  const LOWPRI_THRESHOLD = 64;
+
   const TABLE_PATH = 'repository_path';
   const TABLE_PATHCHANGE = 'repository_pathchange';
   const TABLE_FILESYSTEM = 'repository_filesystem';

src/infrastructure/env/PhabricatorConfigLocalSource.php

@@ -65,4 +65,9 @@ final class PhabricatorConfigLocalSource extends PhabricatorConfigProxySource {
     return $path;
   }
 
+  public function getReadablePath() {
+    $path = $this->getConfigPath();
+    return Filesystem::readablePath($path);
+  }
+
 }