Use one daemon to discover commits in all repositories, not one per repository

Summary:
See D2418. This merges the commit discovery daemon into the same single daemon, and applies all the same rules to it.

There are relatively few implementation changes, but a few things did change:

  - I simplified/improved Mercurial importing, by finding full branch tip hashes with "--debug branches" and using "parents --template {node}" so we don't need to do separate "--debug id" calls.
  - Added a new "--not" flag to exclude repositories, since I switched to real arg parsing anyway.
  - I removed a web UI notification that you need to restart the daemons, this is no longer true.
  - I added a web UI notification that no pull daemon is running on the machine.

NOTE: @makinde, this doesn't change anything from your perspective, but it something breaks this is the likely cause.

This implicitly resolves T792, because discovery no longer runs before pulling.

Test Plan:

  - Swapped databases to a fresh install.
  - Ran "pulllocal" in debug mode. Verified it correctly does nothing (fixed a minor issue with min() on empty array).
  - Added an SVN repository. Verified it cloned and discovered correctly.
  - Added a Mercurial repository. Verified it cloned and discovered correctly.
  - Added a Git repository. Verified it cloned and discovered correctly.
  - Ran with arguments to verify behaviors: "--not MTEST --not STEST", "P --no-discovery", "P".

Reviewers: btrahan, csilvers, Makinde

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T792

Differential Revision: https://secure.phabricator.com/D2430

Author: epriestley

scripts/daemon/phabricator_daemon_launcher.php

@@ -52,69 +52,45 @@ function must_have_extension($ext) {
     $need_launch = phd_load_tracked_repositories();
     if (!$need_launch) {
       echo "There are no repositories with tracking enabled.\n";
-      exit(0);
+      exit(1);
     }
 
     will_launch($control);
+
+    echo "Launching PullLocal daemon in readonly mode...\n";
+
     $control->launchDaemon(
       'PhabricatorRepositoryPullLocalDaemon',
-      array());
+      array(
+        '--no-discovery',
+      ));
+
+    echo "Done.\n";
     break;
 
   case 'repository-launch-master':
     $need_launch = phd_load_tracked_repositories();
     if (!$need_launch) {
       echo "There are no repositories with tracking enabled.\n";
       exit(1);
-    } else {
-      will_launch($control);
-
-      $control->launchDaemon(
-        'PhabricatorRepositoryPullLocalDaemon',
-        array());
-
-      foreach ($need_launch as $repository) {
-        $name = $repository->getName();
-        $callsign = $repository->getCallsign();
-        $desc = "'{$name}' ({$callsign})";
-        $phid = $repository->getPHID();
-
-        switch ($repository->getVersionControlSystem()) {
-          case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
-            echo "Launching discovery daemon on the {$desc} repository...\n";
-            $control->launchDaemon(
-              'PhabricatorRepositoryGitCommitDiscoveryDaemon',
-              array(
-                $phid,
-              ));
-            break;
-          case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
-            echo "Launching discovery daemon on the {$desc} repository...\n";
-            $control->launchDaemon(
-              'PhabricatorRepositorySvnCommitDiscoveryDaemon',
-              array(
-                $phid,
-              ));
-            break;
-          case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL:
-            echo "Launching discovery daemon on the {$desc} repository...\n";
-            $control->launchDaemon(
-              'PhabricatorRepositoryMercurialCommitDiscoveryDaemon',
-              array(
-                $phid,
-              ));
-            break;
+    }
 
-        }
-      }
+    will_launch($control);
 
-      echo "Launching CommitTask daemon...\n";
-      $control->launchDaemon(
-        'PhabricatorRepositoryCommitTaskDaemon',
-        array());
+    echo "Launching PullLocal daemon in master mode...\n";
+    $control->launchDaemon(
+      'PhabricatorRepositoryPullLocalDaemon',
+      array());
 
-      echo "Done.\n";
-    }
+    echo "Launching CommitTask daemon...\n";
+    $control->launchDaemon(
+      'PhabricatorRepositoryCommitTaskDaemon',
+      array());
+
+    echo "NOTE: Make sure you run some taskmaster daemons too, e.g. ".
+         "with 'phd launch 4 taskmaster'.\n";
+
+    echo "Done.\n";
     break;
 
   case 'launch':

scripts/repository/discover.php

@@ -0,0 +1,51 @@
+#!/usr/bin/env php
+<?php
+
+/*
+ * Copyright 2012 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+$root = dirname(dirname(dirname(__FILE__)));
+require_once $root.'/scripts/__init_script__.php';
+
+$args = new PhutilArgumentParser($argv);
+$args->setTagline('manually discover working copies');
+$args->setSynopsis(<<<EOHELP
+**discover.php** [__options__] __repository-callsign-or-phid ...__
+    Manually discover commits in working copies for the named repositories.
+EOHELP
+);
+$args->parseStandardArguments();
+$args->parse(
+  array(
+    array(
+      'name'      => 'repositories',
+      'wildcard'  => true,
+    ),
+  ));
+
+$repo_names = $args->getArg('repositories');
+if (!$repo_names) {
+  echo "Specify one or more repositories to pull, by callsign or PHID.\n";
+  exit(1);
+}
+
+$repos = PhabricatorRepository::loadAllByPHIDOrCallsign($repo_names);
+foreach ($repos as $repo) {
+  $callsign = $repo->getCallsign();
+  echo "Discovering '{$callsign}'...\n";
+  PhabricatorRepositoryPullLocalDaemon::discoverRepository($repo);
+}
+echo "Done.\n";

src/__phutil_library_map__.php

@@ -840,7 +840,6 @@
     'PhabricatorRepositoryCommit' => 'applications/repository/storage/commit',
     'PhabricatorRepositoryCommitChangeParserWorker' => 'applications/repository/worker/commitchangeparser/base',
     'PhabricatorRepositoryCommitData' => 'applications/repository/storage/commitdata',
-    'PhabricatorRepositoryCommitDiscoveryDaemon' => 'applications/repository/daemon/commitdiscovery/base',
     'PhabricatorRepositoryCommitHeraldWorker' => 'applications/repository/worker/herald',
     'PhabricatorRepositoryCommitMessageDetailParser' => 'applications/repository/parser/base',
     'PhabricatorRepositoryCommitMessageParserWorker' => 'applications/repository/worker/commitmessageparser/base',
@@ -850,22 +849,18 @@
     'PhabricatorRepositoryController' => 'applications/repository/controller/base',
     'PhabricatorRepositoryCreateController' => 'applications/repository/controller/create',
     'PhabricatorRepositoryDAO' => 'applications/repository/storage/base',
-    'PhabricatorRepositoryDaemon' => 'applications/repository/daemon/base',
     'PhabricatorRepositoryDefaultCommitMessageDetailParser' => 'applications/repository/parser/default',
     'PhabricatorRepositoryDeleteController' => 'applications/repository/controller/delete',
     'PhabricatorRepositoryEditController' => 'applications/repository/controller/edit',
     'PhabricatorRepositoryGitCommitChangeParserWorker' => 'applications/repository/worker/commitchangeparser/git',
-    'PhabricatorRepositoryGitCommitDiscoveryDaemon' => 'applications/repository/daemon/commitdiscovery/git',
-    'PhabricatorRepositoryGitCommitDiscoveryDaemonTestCase' => 'applications/repository/daemon/commitdiscovery/git/__tests__',
     'PhabricatorRepositoryGitCommitMessageParserWorker' => 'applications/repository/worker/commitmessageparser/git',
     'PhabricatorRepositoryListController' => 'applications/repository/controller/list',
     'PhabricatorRepositoryMercurialCommitChangeParserWorker' => 'applications/repository/worker/commitchangeparser/mercurial',
-    'PhabricatorRepositoryMercurialCommitDiscoveryDaemon' => 'applications/repository/daemon/commitdiscovery/mercurial',
     'PhabricatorRepositoryMercurialCommitMessageParserWorker' => 'applications/repository/worker/commitmessageparser/mercurial',
     'PhabricatorRepositoryPullLocalDaemon' => 'applications/repository/daemon/pulllocal',
+    'PhabricatorRepositoryPullLocalDaemonTestCase' => 'applications/repository/daemon/pulllocal/__tests__',
     'PhabricatorRepositoryShortcut' => 'applications/repository/storage/shortcut',
     'PhabricatorRepositorySvnCommitChangeParserWorker' => 'applications/repository/worker/commitchangeparser/svn',
-    'PhabricatorRepositorySvnCommitDiscoveryDaemon' => 'applications/repository/daemon/commitdiscovery/svn',
     'PhabricatorRepositorySvnCommitMessageParserWorker' => 'applications/repository/worker/commitmessageparser/svn',
     'PhabricatorRepositorySymbol' => 'applications/repository/storage/symbol',
     'PhabricatorRepositoryTestCase' => 'applications/repository/storage/repository/__tests__',
@@ -1735,31 +1730,26 @@
     'PhabricatorRepositoryCommit' => 'PhabricatorRepositoryDAO',
     'PhabricatorRepositoryCommitChangeParserWorker' => 'PhabricatorRepositoryCommitParserWorker',
     'PhabricatorRepositoryCommitData' => 'PhabricatorRepositoryDAO',
-    'PhabricatorRepositoryCommitDiscoveryDaemon' => 'PhabricatorRepositoryDaemon',
     'PhabricatorRepositoryCommitHeraldWorker' => 'PhabricatorRepositoryCommitParserWorker',
     'PhabricatorRepositoryCommitMessageParserWorker' => 'PhabricatorRepositoryCommitParserWorker',
     'PhabricatorRepositoryCommitOwnersWorker' => 'PhabricatorRepositoryCommitParserWorker',
     'PhabricatorRepositoryCommitParserWorker' => 'PhabricatorWorker',
-    'PhabricatorRepositoryCommitTaskDaemon' => 'PhabricatorRepositoryDaemon',
+    'PhabricatorRepositoryCommitTaskDaemon' => 'PhabricatorDaemon',
     'PhabricatorRepositoryController' => 'PhabricatorController',
     'PhabricatorRepositoryCreateController' => 'PhabricatorRepositoryController',
     'PhabricatorRepositoryDAO' => 'PhabricatorLiskDAO',
-    'PhabricatorRepositoryDaemon' => 'PhabricatorDaemon',
     'PhabricatorRepositoryDefaultCommitMessageDetailParser' => 'PhabricatorRepositoryCommitMessageDetailParser',
     'PhabricatorRepositoryDeleteController' => 'PhabricatorRepositoryController',
     'PhabricatorRepositoryEditController' => 'PhabricatorRepositoryController',
     'PhabricatorRepositoryGitCommitChangeParserWorker' => 'PhabricatorRepositoryCommitChangeParserWorker',
-    'PhabricatorRepositoryGitCommitDiscoveryDaemon' => 'PhabricatorRepositoryCommitDiscoveryDaemon',
-    'PhabricatorRepositoryGitCommitDiscoveryDaemonTestCase' => 'PhabricatorTestCase',
     'PhabricatorRepositoryGitCommitMessageParserWorker' => 'PhabricatorRepositoryCommitMessageParserWorker',
     'PhabricatorRepositoryListController' => 'PhabricatorRepositoryController',
     'PhabricatorRepositoryMercurialCommitChangeParserWorker' => 'PhabricatorRepositoryCommitChangeParserWorker',
-    'PhabricatorRepositoryMercurialCommitDiscoveryDaemon' => 'PhabricatorRepositoryCommitDiscoveryDaemon',
     'PhabricatorRepositoryMercurialCommitMessageParserWorker' => 'PhabricatorRepositoryCommitMessageParserWorker',
     'PhabricatorRepositoryPullLocalDaemon' => 'PhabricatorDaemon',
+    'PhabricatorRepositoryPullLocalDaemonTestCase' => 'PhabricatorTestCase',
     'PhabricatorRepositoryShortcut' => 'PhabricatorRepositoryDAO',
     'PhabricatorRepositorySvnCommitChangeParserWorker' => 'PhabricatorRepositoryCommitChangeParserWorker',
-    'PhabricatorRepositorySvnCommitDiscoveryDaemon' => 'PhabricatorRepositoryCommitDiscoveryDaemon',
     'PhabricatorRepositorySvnCommitMessageParserWorker' => 'PhabricatorRepositoryCommitMessageParserWorker',
     'PhabricatorRepositorySymbol' => 'PhabricatorRepositoryDAO',
     'PhabricatorRepositoryTestCase' => 'PhabricatorTestCase',

src/applications/diffusion/query/history/svn/DiffusionSvnHistoryQuery.php

@@ -1,7 +1,7 @@
 <?php
 
 /*
- * Copyright 2011 Facebook, Inc.
+ * Copyright 2012 Facebook, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -33,7 +33,11 @@ protected function executeQuery() {
       PhabricatorRepository::TABLE_PATH,
       array(md5('/'.trim($path, '/'))));
     $paths = ipull($paths, 'id', 'path');
-    $path_id = $paths['/'.trim($path, '/')];
+    $path_id = idx($paths, '/'.trim($path, '/'));
+
+    if (!$path_id) {
+      return array();
+    }
 
     $filter_query = '';
     if ($this->needDirectChanges) {

src/applications/repository/controller/base/PhabricatorRepositoryController.php

@@ -1,7 +1,7 @@
 <?php
 
 /*
- * Copyright 2011 Facebook, Inc.
+ * Copyright 2012 Facebook, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -32,8 +32,43 @@ public function buildStandardPageResponse($view, array $data) {
     $page->setGlyph("rX");
     $page->appendChild($view);
 
+
     $response = new AphrontWebpageResponse();
     return $response->setContent($page->render());
   }
 
+  private function isPullDaemonRunningOnThisMachine() {
+
+    // This is sort of hacky, but should probably work.
+
+    list($stdout) = execx('ps auxwww');
+    return preg_match('/PhabricatorRepositoryPullLocalDaemon/', $stdout);
+  }
+
+  protected function renderDaemonNotice() {
+    $daemon_running = $this->isPullDaemonRunningOnThisMachine();
+    if ($daemon_running) {
+      return null;
+    }
+
+    $documentation = phutil_render_tag(
+      'a',
+      array(
+        'href' => PhabricatorEnv::getDoclink(
+          'article/Diffusion_User_Guide.html'),
+      ),
+      'Diffusion User Guide');
+
+    $view = new AphrontErrorView();
+    $view->setSeverity(AphrontErrorView::SEVERITY_WARNING);
+    $view->setTitle('Repository Daemon Not Running');
+    $view->appendChild(
+      "<p>The repository daemon is not running on this machine. Without this ".
+      "daemon, Phabricator will not be able to import or update repositories. ".
+      "For instructions on starting the daemon, see ".
+      "<strong>{$documentation}</strong>.</p>");
+
+    return $view;
+  }
+
 }

src/applications/repository/controller/base/__init__.php

@@ -8,7 +8,11 @@
 
 phutil_require_module('phabricator', 'aphront/response/webpage');
 phutil_require_module('phabricator', 'applications/base/controller/base');
+phutil_require_module('phabricator', 'infrastructure/env');
+phutil_require_module('phabricator', 'view/form/error');
 
+phutil_require_module('phutil', 'future/exec');
+phutil_require_module('phutil', 'markup');
 phutil_require_module('phutil', 'utils');
 
 

src/applications/repository/controller/edit/PhabricatorRepositoryEditController.php

@@ -63,6 +63,8 @@ public function processRequest() {
           phutil_escape_html($name)));
     }
 
+    $nav->appendChild($this->renderDaemonNotice());
+
     $this->sideNav = $nav;
 
     switch ($this->view) {
@@ -345,9 +347,7 @@ private function processTrackingRequest() {
       $error_view = new AphrontErrorView();
       $error_view->setSeverity(AphrontErrorView::SEVERITY_NOTICE);
       $error_view->setTitle('Changes Saved');
-      $error_view->appendChild(
-        'Tracking changes were saved. You may need to restart the daemon '.
-        'before changes will take effect.');
+      $error_view->appendChild('Tracking changes were saved.');
     } else if (!$repository->isTracked()) {
       $error_view = new AphrontErrorView();
       $error_view->setSeverity(AphrontErrorView::SEVERITY_WARNING);

src/applications/repository/controller/list/PhabricatorRepositoryListController.php

@@ -157,6 +157,7 @@ public function processRequest() {
 
     return $this->buildStandardPageResponse(
       array(
+        $this->renderDaemonNotice(),
         $panel,
         $project_panel,
       ),