Improve Diffusion error messages and UI for partially imported repositories

Summary:
  - When you have an un-cloned repository, we currently throw random-looking Git/Hg exception. Instead, throw a useful error.
  - When you have a cloned but undiscovered repository, we show no commits. This is crazy confusing. Instead, show commits as "importing...".
  - Fix some warnings and errors for empty path table cases, etc.

Test Plan:
  - Wiped database.
  - Added Mercurial repo without running daemons. Viewed in Diffusion, got a good exception.
  - Pulled Mercurial repo without discovering it. Got "Importing...".
  - Discovered Mercurial repo without parsing it. Got "Importing..." plus date information.
  - Parsed Mercurial repo, got everything working properly.
  - Added Git repo without running daemons, did all the stuff above, same results.
  - This doesn't improve SVN much but that's a trickier case since we don't actually make SVN calls and rely only on the parse state.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T776

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

src/__phutil_library_map__.php

@@ -373,6 +373,7 @@ phutil_register_library_map(array(
     'DiffusionRepositoryPath' => 'applications/diffusion/data/repositorypath',
     'DiffusionRepositoryTag' => 'applications/diffusion/tag',
     'DiffusionRequest' => 'applications/diffusion/request/base',
+    'DiffusionSetupException' => 'applications/diffusion/exception/setup',
     'DiffusionSvnBrowseQuery' => 'applications/diffusion/query/browse/svn',
     'DiffusionSvnCommitParentsQuery' => 'applications/diffusion/query/parents/svn',
     'DiffusionSvnCommitTagsQuery' => 'applications/diffusion/query/committags/svn',

src/applications/diffusion/exception/setup/DiffusionSetupException.php

@@ -0,0 +1,25 @@
+<?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.
+ */
+
+final class DiffusionSetupException extends AphrontUsageException {
+
+  public function __construct($message) {
+    parent::__construct('Diffusion Setup Exception', $message);
+  }
+
+}

src/applications/diffusion/exception/setup/__init__.php

@@ -0,0 +1,12 @@
+<?php
+/**
+ * This file is automatically generated. Lint this module to rebuild it.
+ * @generated
+ */
+
+
+
+phutil_require_module('phabricator', 'aphront/exception/usage');
+
+
+phutil_require_source('DiffusionSetupException.php');

src/applications/diffusion/query/base/DiffusionQuery.php

@@ -75,20 +75,37 @@ abstract class DiffusionQuery {
       $identifiers);
     $commits = mpull($commits, null, 'getCommitIdentifier');
 
-    // Reorder the commits in identifier order so we preserve nth-parent
+    // Build empty commit objects for every commit, so we can show unparsed
-    // relationships when the identifiers are the parents of a merge commit.
+    // commits in history views as "unparsed" instead of not showing them. This
-    $commits = array_select_keys($commits, $identifiers);
+    // makes the process of importing and parsing commits much clearer to the
+    // user.
 
-    if (!$commits) {
+    $commit_list = array();
-      return array();
+    foreach ($identifiers as $identifier) {
+      $commit_obj = idx($commits, $identifier);
+      if (!$commit_obj) {
+        $commit_obj = new PhabricatorRepositoryCommit();
+        $commit_obj->setRepositoryID($repository->getID());
+        $commit_obj->setCommitIdentifier($identifier);
+        $commit_obj->setIsUnparsed(true);
+        $commit_obj->makeEphemeral();
+      }
+      $commit_list[$identifier] = $commit_obj;
+    }
+    $commits = $commit_list;
+
+    $commit_ids = array_filter(mpull($commits, 'getID'));
+    if ($commit_ids) {
+      $commit_data = id(new PhabricatorRepositoryCommitData())->loadAllWhere(
+        'commitID in (%Ld)',
+        $commit_ids);
+      $commit_data = mpull($commit_data, null, 'getCommitID');
     }
 
-    $commit_data = id(new PhabricatorRepositoryCommitData())->loadAllWhere(
-      'commitID in (%Ld)',
-      mpull($commits, 'getID'));
-    $commit_data = mpull($commit_data, null, 'getCommitID');
-
     foreach ($commits as $commit) {
+      if (!$commit->getID()) {
+        continue;
+      }
       if (idx($commit_data, $commit->getID())) {
         $commit->attachCommitData($commit_data[$commit->getID()]);
       }
@@ -123,13 +140,18 @@ abstract class DiffusionQuery {
     $paths = ipull($paths, 'id', 'path');
     $path_id = idx($paths, $path_normal);
 
-    $path_changes = queryfx_all(
+    $commit_ids = array_filter(mpull($commits, 'getID'));
-      $conn_r,
+
-      'SELECT * FROM %T WHERE commitID IN (%Ld) AND pathID = %d',
+    $path_changes = array();
-      PhabricatorRepository::TABLE_PATHCHANGE,
+    if ($path_id && $commit_ids) {
-      mpull($commits, 'getID'),
+      $path_changes = queryfx_all(
-      $path_id);
+        $conn_r,
-    $path_changes = ipull($path_changes, null, 'commitID');
+        'SELECT * FROM %T WHERE commitID IN (%Ld) AND pathID = %d',
+        PhabricatorRepository::TABLE_PATHCHANGE,
+        $commit_ids,
+        $path_id);
+      $path_changes = ipull($path_changes, null, 'commitID');
+    }
 
     $history = array();
     foreach ($identifiers as $identifier) {

src/applications/diffusion/request/base/DiffusionRequest.php

@@ -510,4 +510,16 @@ abstract class DiffusionRequest {
     return $result;
   }
 
+  protected function raiseCloneException() {
+    $host = php_uname('n');
+    $callsign = $this->getRepository()->getCallsign();
+    throw new DiffusionSetupException(
+      "The working copy for this repository ('{$callsign}') hasn't been ".
+      "cloned yet on this machine ('{$host}'). Make sure you've started the ".
+      "Phabricator daemons. If this problem persists for longer than a clone ".
+      "should take, check the daemon logs (in the Daemon Console) to see if ".
+      "there were errors cloning the repository. Consult the 'Diffusion User ".
+      "Guide' in the documentation for help setting up repositories.");
+  }
+
 }

src/applications/diffusion/request/base/__init__.php

@@ -6,6 +6,7 @@
 
 
 
+phutil_require_module('phabricator', 'applications/diffusion/exception/setup');
 phutil_require_module('phabricator', 'applications/repository/constants/repositorytype');
 phutil_require_module('phabricator', 'applications/repository/storage/commit');
 phutil_require_module('phabricator', 'applications/repository/storage/commitdata');

src/applications/diffusion/request/git/DiffusionGitRequest.php

@@ -26,13 +26,19 @@ final class DiffusionGitRequest extends DiffusionRequest {
   }
 
   protected function didInitialize() {
+    $repository = $this->getRepository();
+
+    if (!Filesystem::pathExists($repository->getLocalPath())) {
+      $this->raiseCloneException();
+    }
+
     if (!$this->commit) {
       return;
     }
 
     // Expand short commit names and verify
 
-    $future = $this->getRepository()->getLocalCommandFuture(
+    $future = $repository->getLocalCommandFuture(
       'cat-file --batch');
     $future->write($this->commit);
     list($stdout) = $future->resolvex();

src/applications/diffusion/request/git/__init__.php

@@ -10,5 +10,7 @@ phutil_require_module('phabricator', 'aphront/exception/usage');
 phutil_require_module('phabricator', 'applications/diffusion/data/branch');
 phutil_require_module('phabricator', 'applications/diffusion/request/base');
 
+phutil_require_module('phutil', 'filesystem');
+
 
 phutil_require_source('DiffusionGitRequest.php');

src/applications/diffusion/request/mercurial/DiffusionMercurialRequest.php

@@ -26,6 +26,12 @@ final class DiffusionMercurialRequest extends DiffusionRequest {
   }
 
   protected function didInitialize() {
+    $repository = $this->getRepository();
+
+    if (!Filesystem::pathExists($repository->getLocalPath())) {
+      $this->raiseCloneException();
+    }
+
     return;
   }
 
@@ -33,9 +39,11 @@ final class DiffusionMercurialRequest extends DiffusionRequest {
     if ($this->branch) {
       return $this->branch;
     }
+
     if ($this->repository) {
       return $this->repository->getDetail('default-branch', 'default');
     }
+
     throw new Exception("Unable to determine branch!");
   }
 

src/applications/diffusion/request/mercurial/__init__.php

@@ -8,5 +8,7 @@
 
 phutil_require_module('phabricator', 'applications/diffusion/request/base');
 
+phutil_require_module('phutil', 'filesystem');
+
 
 phutil_require_source('DiffusionMercurialRequest.php');

src/applications/diffusion/view/historytable/DiffusionHistoryTableView.php

@@ -93,6 +93,17 @@ final class DiffusionHistoryTableView extends DiffusionView {
         $author = phutil_escape_html($history->getAuthorName());
       }
 
+      $commit = $history->getCommit();
+      if ($commit && !$commit->getIsUnparsed() && $data) {
+        $change = $this->linkChange(
+          $history->getChangeType(),
+          $history->getFileType(),
+          $path = null,
+          $history->getCommitIdentifier());
+      } else {
+        $change = "<em>Importing\xE2\x80\xA6</em>";
+      }
+
       $rows[] = array(
         $this->linkBrowse(
           $drequest->getPath(),
@@ -103,11 +114,7 @@ final class DiffusionHistoryTableView extends DiffusionView {
         self::linkCommit(
           $drequest->getRepository(),
           $history->getCommitIdentifier()),
-        $this->linkChange(
+        $change,
-          $history->getChangeType(),
-          $history->getFileType(),
-          null,
-          $history->getCommitIdentifier()),
         $date,
         $time,
         $author,

src/applications/repository/storage/commit/PhabricatorRepositoryCommit.php

@@ -27,6 +27,16 @@ final class PhabricatorRepositoryCommit extends PhabricatorRepositoryDAO {
   protected $auditStatus = PhabricatorAuditCommitStatusConstants::NONE;
 
   private $commitData;
+  private $isUnparsed;
+
+  public function setIsUnparsed($is_unparsed) {
+    $this->isUnparsed = $is_unparsed;
+    return $this;
+  }
+
+  public function getIsUnparsed() {
+    return $this->isUnparsed;
+  }
 
   public function getConfiguration() {
     return array(