From 4cf1270ecdd8570f904d48533795aa66b7e39d30 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 5 Oct 2015 05:58:53 -0700
Subject: [PATCH 01/11] In Harbormaster, make sure artifacts are destroyed even
 if a build is aborted

Summary:
Ref T9252. Currently, Harbormaster and Drydock work like this in some cases:

  # Queue a lease for activation.
  # Then, a little later, save the lease PHID somewhere.
  # When the target/resource is destroyed, destroy the lease.

However, something can happen between (1) and (2). In Drydock this window is very short and the "something" would have to be a lighting strike or something similar, but in Harbormaster we wait until the resource activates to do (2) so the window can be many minutes long. In particular, a user can use "Abort Build" during those many minutes.

If they do, the target is destroyed but it doesn't yet have a record of the artifact, so the artifact isn't cleaned up.

Make these things work like this instead:

  # Create a new lease and pre-generate a PHID for it.
  # Save that PHID as something that needs to be cleaned up.
  # Queue the lease for activation.
  # When the target/resource is destroyed, destroy the lease if it exists.

This makes sure there's no step in the process where we might lose track of a lease/resource.

Also, clean up and standardize some other stuff I hit.

Test Plan:
  - Stopped daemons.
  - Restarted a build in Harbormaster.
  - Stepped through the build one stage at a time using `bin/worker execute ...`.
  - After the lease was queued, but before it activated, aborted the build.
  - Processed the Harbormaster side of things only.
  - Saw the lease get destroyed properly.

Reviewers: chad, hach-que

Reviewed By: hach-que

Maniphest Tasks: T9252

Differential Revision: https://secure.phabricator.com/D14234
---
 ...anacServiceHostBlueprintImplementation.php |  3 +-
 .../DrydockBlueprintImplementation.php        |  2 +-
 ...dockWorkingCopyBlueprintImplementation.php | 18 +++++--
 ...DrydockLeaseWaitingForResourcesLogType.php |  2 +-
 .../drydock/storage/DrydockLease.php          | 47 ++++++++++++------
 .../drydock/storage/DrydockResource.php       | 49 ++++++++++---------
 .../drydock/storage/DrydockSlotLock.php       |  1 +
 .../HarbormasterDrydockLeaseArtifact.php      | 14 +++++-
 ...easeWorkingCopyBuildStepImplementation.php | 22 +++++----
 ...ricatorWorkerManagementExecuteWorkflow.php |  3 +-
 .../PhabricatorUSEnglishTranslation.php       |  5 ++
 11 files changed, 111 insertions(+), 55 deletions(-)

diff --git a/src/applications/drydock/blueprint/DrydockAlmanacServiceHostBlueprintImplementation.php b/src/applications/drydock/blueprint/DrydockAlmanacServiceHostBlueprintImplementation.php
index e458020600..e62c7b7558 100644
--- a/src/applications/drydock/blueprint/DrydockAlmanacServiceHostBlueprintImplementation.php
+++ b/src/applications/drydock/blueprint/DrydockAlmanacServiceHostBlueprintImplementation.php
@@ -262,13 +262,14 @@ final class DrydockAlmanacServiceHostBlueprintImplementation
           array(
             DrydockResourceStatus::STATUS_PENDING,
             DrydockResourceStatus::STATUS_ACTIVE,
+            DrydockResourceStatus::STATUS_BROKEN,
             DrydockResourceStatus::STATUS_RELEASED,
           ))
         ->execute();
 
       $allocated_phids = array();
       foreach ($pool as $resource) {
-        $allocated_phids[] = $resource->getAttribute('almanacDevicePHID');
+        $allocated_phids[] = $resource->getAttribute('almanacBindingPHID');
       }
       $allocated_phids = array_fuse($allocated_phids);
 
diff --git a/src/applications/drydock/blueprint/DrydockBlueprintImplementation.php b/src/applications/drydock/blueprint/DrydockBlueprintImplementation.php
index 513d7b37f9..e3cdff34c5 100644
--- a/src/applications/drydock/blueprint/DrydockBlueprintImplementation.php
+++ b/src/applications/drydock/blueprint/DrydockBlueprintImplementation.php
@@ -288,7 +288,7 @@ abstract class DrydockBlueprintImplementation extends Phobject {
   }
 
   protected function newLease(DrydockBlueprint $blueprint) {
-    return id(new DrydockLease());
+    return DrydockLease::initializeNewLease();
   }
 
   protected function requireActiveLease(DrydockLease $lease) {
diff --git a/src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php b/src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php
index 7a34e2d3bc..e086294c4d 100644
--- a/src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php
+++ b/src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php
@@ -104,8 +104,13 @@ final class DrydockWorkingCopyBlueprintImplementation
     $host_lease = $this->newLease($blueprint)
       ->setResourceType('host')
       ->setOwnerPHID($resource_phid)
-      ->setAttribute('workingcopy.resourcePHID', $resource_phid)
-      ->queueForActivation();
+      ->setAttribute('workingcopy.resourcePHID', $resource_phid);
+
+    $resource
+      ->setAttribute('host.leasePHID', $host_lease->getPHID())
+      ->save();
+
+    $host_lease->queueForActivation();
 
     // TODO: Add some limits to the number of working copies we can have at
     // once?
@@ -121,7 +126,6 @@ final class DrydockWorkingCopyBlueprintImplementation
 
     return $resource
       ->setAttribute('repositories.map', $map)
-      ->setAttribute('host.leasePHID', $host_lease->getPHID())
       ->allocateResource();
   }
 
@@ -165,7 +169,13 @@ final class DrydockWorkingCopyBlueprintImplementation
     DrydockBlueprint $blueprint,
     DrydockResource $resource) {
 
-    $lease = $this->loadHostLease($resource);
+    try {
+      $lease = $this->loadHostLease($resource);
+    } catch (Exception $ex) {
+      // If we can't load the lease, assume we don't need to take any actions
+      // to destroy it.
+      return;
+    }
 
     // Destroy the lease on the host.
     $lease->releaseOnDestruction();
diff --git a/src/applications/drydock/logtype/DrydockLeaseWaitingForResourcesLogType.php b/src/applications/drydock/logtype/DrydockLeaseWaitingForResourcesLogType.php
index 1faf762ae8..46ab965b36 100644
--- a/src/applications/drydock/logtype/DrydockLeaseWaitingForResourcesLogType.php
+++ b/src/applications/drydock/logtype/DrydockLeaseWaitingForResourcesLogType.php
@@ -19,7 +19,7 @@ final class DrydockLeaseWaitingForResourcesLogType extends DrydockLogType {
 
     return pht(
       'Waiting for available resources from: %s.',
-      $viewer->renderHandleList($blueprint_phids));
+      $viewer->renderHandleList($blueprint_phids)->render());
   }
 
 }
diff --git a/src/applications/drydock/storage/DrydockLease.php b/src/applications/drydock/storage/DrydockLease.php
index aa2ea753f0..01ccb0a5c4 100644
--- a/src/applications/drydock/storage/DrydockLease.php
+++ b/src/applications/drydock/storage/DrydockLease.php
@@ -19,6 +19,16 @@ final class DrydockLease extends DrydockDAO
   private $activateWhenAcquired = false;
   private $slotLocks = array();
 
+  public static function initializeNewLease() {
+    $lease = new DrydockLease();
+
+    // Pregenerate a PHID so that the caller can set something up to release
+    // this lease before queueing it for activation.
+    $lease->setPHID($lease->generatePHID());
+
+    return $lease;
+  }
+
   /**
    * Flag this lease to be released when its destructor is called. This is
    * mostly useful if you have a script which acquires, uses, and then releases
@@ -232,6 +242,7 @@ final class DrydockLease extends DrydockDAO
     }
 
     $this->openTransaction();
+
     try {
       DrydockSlotLock::acquireLocks($this->getPHID(), $this->slotLocks);
       $this->slotLocks = array();
@@ -247,16 +258,12 @@ final class DrydockLease extends DrydockDAO
       throw $ex;
     }
 
-    try {
-      $this
-        ->setResourcePHID($resource->getPHID())
-        ->attachResource($resource)
-        ->setStatus($new_status)
-        ->save();
-    } catch (Exception $ex) {
-      $this->killTransaction();
-      throw $ex;
-    }
+    $this
+      ->setResourcePHID($resource->getPHID())
+      ->attachResource($resource)
+      ->setStatus($new_status)
+      ->save();
+
     $this->saveTransaction();
 
     $this->isAcquired = true;
@@ -295,12 +302,24 @@ final class DrydockLease extends DrydockDAO
 
     $this->openTransaction();
 
-      $this
-        ->setStatus(DrydockLeaseStatus::STATUS_ACTIVE)
-        ->save();
-
+    try {
       DrydockSlotLock::acquireLocks($this->getPHID(), $this->slotLocks);
       $this->slotLocks = array();
+    } catch (DrydockSlotLockException $ex) {
+      $this->killTransaction();
+
+      $this->logEvent(
+        DrydockSlotLockFailureLogType::LOGCONST,
+        array(
+          'locks' => $ex->getLockMap(),
+        ));
+
+      throw $ex;
+    }
+
+    $this
+      ->setStatus(DrydockLeaseStatus::STATUS_ACTIVE)
+      ->save();
 
     $this->saveTransaction();
 
diff --git a/src/applications/drydock/storage/DrydockResource.php b/src/applications/drydock/storage/DrydockResource.php
index da97a089d9..1ee663fa3c 100644
--- a/src/applications/drydock/storage/DrydockResource.php
+++ b/src/applications/drydock/storage/DrydockResource.php
@@ -113,13 +113,6 @@ final class DrydockResource extends DrydockDAO
   }
 
   public function allocateResource() {
-    if ($this->getID()) {
-      throw new Exception(
-        pht(
-          'Trying to allocate a resource which has already been persisted. '.
-          'Only new resources may be allocated.'));
-    }
-
     // We expect resources to have a pregenerated PHID, as they should have
     // been created by a call to DrydockBlueprint->newResourceTemplate().
     if (!$this->getPHID()) {
@@ -155,9 +148,14 @@ final class DrydockResource extends DrydockDAO
     } catch (DrydockSlotLockException $ex) {
       $this->killTransaction();
 
-      // NOTE: We have to log this on the blueprint, as the resource is not
-      // going to be saved so the PHID will vanish.
-      $this->getBlueprint()->logEvent(
+      if ($this->getID()) {
+        $log_target = $this;
+      } else {
+        // If we don't have an ID, we have to log this on the blueprint, as the
+        // resource is not going to be saved so the PHID will vanish.
+        $log_target = $this->getBlueprint();
+      }
+      $log_target->logEvent(
         DrydockSlotLockFailureLogType::LOGCONST,
         array(
           'locks' => $ex->getLockMap(),
@@ -166,14 +164,9 @@ final class DrydockResource extends DrydockDAO
       throw $ex;
     }
 
-    try {
-      $this
-        ->setStatus($new_status)
-        ->save();
-    } catch (Exception $ex) {
-      $this->killTransaction();
-      throw $ex;
-    }
+    $this
+      ->setStatus($new_status)
+      ->save();
 
     $this->saveTransaction();
 
@@ -210,12 +203,24 @@ final class DrydockResource extends DrydockDAO
 
     $this->openTransaction();
 
-      $this
-        ->setStatus(DrydockResourceStatus::STATUS_ACTIVE)
-        ->save();
-
+    try {
       DrydockSlotLock::acquireLocks($this->getPHID(), $this->slotLocks);
       $this->slotLocks = array();
+    } catch (DrydockSlotLockException $ex) {
+      $this->killTransaction();
+
+      $this->logEvent(
+        DrydockSlotLockFailureLogType::LOGCONST,
+        array(
+          'locks' => $ex->getLockMap(),
+        ));
+
+      throw $ex;
+    }
+
+    $this
+      ->setStatus(DrydockResourceStatus::STATUS_ACTIVE)
+      ->save();
 
     $this->saveTransaction();
 
diff --git a/src/applications/drydock/storage/DrydockSlotLock.php b/src/applications/drydock/storage/DrydockSlotLock.php
index 2f980660aa..7e4e320946 100644
--- a/src/applications/drydock/storage/DrydockSlotLock.php
+++ b/src/applications/drydock/storage/DrydockSlotLock.php
@@ -149,6 +149,7 @@ final class DrydockSlotLock extends DrydockDAO {
       // time we should be able to figure out which locks are already held.
       $held = self::loadHeldLocks($locks);
       $held = mpull($held, 'getOwnerPHID', 'getLockKey');
+
       throw new DrydockSlotLockException($held);
     }
   }
diff --git a/src/applications/harbormaster/artifact/HarbormasterDrydockLeaseArtifact.php b/src/applications/harbormaster/artifact/HarbormasterDrydockLeaseArtifact.php
index bd3868e7db..e7a2ac41e6 100644
--- a/src/applications/harbormaster/artifact/HarbormasterDrydockLeaseArtifact.php
+++ b/src/applications/harbormaster/artifact/HarbormasterDrydockLeaseArtifact.php
@@ -29,7 +29,9 @@ abstract class HarbormasterDrydockLeaseArtifact
   }
 
   public function willCreateArtifact(PhabricatorUser $actor) {
-    $this->loadArtifactLease($actor);
+    // We don't load the lease here because it's expected that artifacts are
+    // created before leases actually exist. This guarantees that the leases
+    // will be cleaned up.
   }
 
   public function loadArtifactLease(PhabricatorUser $viewer) {
@@ -51,7 +53,15 @@ abstract class HarbormasterDrydockLeaseArtifact
   }
 
   public function releaseArtifact(PhabricatorUser $actor) {
-    $lease = $this->loadArtifactLease($actor);
+    try {
+      $lease = $this->loadArtifactLease($actor);
+    } catch (Exception $ex) {
+      // If we can't load the lease, treat it as already released. Artifacts
+      // are generated before leases are queued, so it's possible to arrive
+      // here under normal conditions.
+      return;
+    }
+
     if (!$lease->canRelease()) {
       return;
     }
diff --git a/src/applications/harbormaster/step/HarbormasterLeaseWorkingCopyBuildStepImplementation.php b/src/applications/harbormaster/step/HarbormasterLeaseWorkingCopyBuildStepImplementation.php
index 001a981a60..91c6eb0517 100644
--- a/src/applications/harbormaster/step/HarbormasterLeaseWorkingCopyBuildStepImplementation.php
+++ b/src/applications/harbormaster/step/HarbormasterLeaseWorkingCopyBuildStepImplementation.php
@@ -41,7 +41,7 @@ final class HarbormasterLeaseWorkingCopyBuildStepImplementation
       $working_copy_type = id(new DrydockWorkingCopyBlueprintImplementation())
         ->getType();
 
-      $lease = id(new DrydockLease())
+      $lease = DrydockLease::initializeNewLease()
         ->setResourceType($working_copy_type)
         ->setOwnerPHID($build_target->getPHID());
 
@@ -54,6 +54,18 @@ final class HarbormasterLeaseWorkingCopyBuildStepImplementation
         $lease->setAwakenTaskIDs(array($task_id));
       }
 
+      // TODO: Maybe add a method to mark artifacts like this as pending?
+
+      // Create the artifact now so that the lease is always disposed of, even
+      // if this target is aborted.
+      $build_target->createArtifact(
+        $viewer,
+        $settings['name'],
+        HarbormasterWorkingCopyArtifact::ARTIFACTCONST,
+        array(
+          'drydockLeasePHID' => $lease->getPHID(),
+        ));
+
       $lease->queueForActivation();
 
       $build_target
@@ -73,14 +85,6 @@ final class HarbormasterLeaseWorkingCopyBuildStepImplementation
           'Lease "%s" never activated.',
           $lease->getPHID()));
     }
-
-    $artifact = $build_target->createArtifact(
-      $viewer,
-      $settings['name'],
-      HarbormasterWorkingCopyArtifact::ARTIFACTCONST,
-      array(
-        'drydockLeasePHID' => $lease->getPHID(),
-      ));
   }
 
   public function getArtifactOutputs() {
diff --git a/src/infrastructure/daemon/workers/management/PhabricatorWorkerManagementExecuteWorkflow.php b/src/infrastructure/daemon/workers/management/PhabricatorWorkerManagementExecuteWorkflow.php
index 3826075276..2acc8452ea 100644
--- a/src/infrastructure/daemon/workers/management/PhabricatorWorkerManagementExecuteWorkflow.php
+++ b/src/infrastructure/daemon/workers/management/PhabricatorWorkerManagementExecuteWorkflow.php
@@ -42,7 +42,8 @@ final class PhabricatorWorkerManagementExecuteWorkflow
         $task->getDataID());
       $task->setData($task_data->getData());
 
-      $console->writeOut(
+      echo tsprintf(
+        "%s\n",
         pht(
           'Executing task %d (%s)...',
           $task->getID(),
diff --git a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php
index 055b85f0df..beda39c21c 100644
--- a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php
+++ b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php
@@ -1398,6 +1398,11 @@ final class PhabricatorUSEnglishTranslation
         'Setting retention policy for "%s" to %s days.',
       ),
 
+      'Waiting %s second(s) for lease to activate.' => array(
+        'Waiting a second for lease to activate.',
+        'Waiting %s seconds for lease to activate.',
+      ),
+
     );
   }
 

From de2bbfef7d147e1ef6903392778a1415cd58a874 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 5 Oct 2015 09:46:29 -0700
Subject: [PATCH 02/11] Allow PhabricatorWorker->queueTask() to take full
 $options

Summary:
Ref T9252. Currently, `queueTask()` accepts `$priority` as its third argument. Allow it to take a full range of `$options` instead. This API just never got updated after we expanded avialable options.

Arguably this whole API should be some kind of "TaskQueueRequest" object but I'll leave that for another day.

Test Plan:
  - Grepped for `queueTask()` and verified no other callsites are affected by this API change.
  - Ran some daemons.
  - See also next diff.

Reviewers: hach-que, chad

Reviewed By: hach-que, chad

Maniphest Tasks: T9252

Differential Revision: https://secure.phabricator.com/D14235
---
 .../daemon/workers/PhabricatorWorker.php          | 12 +++++++-----
 .../storage/PhabricatorWorkerActiveTask.php       | 15 ++++++++-------
 2 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/src/infrastructure/daemon/workers/PhabricatorWorker.php b/src/infrastructure/daemon/workers/PhabricatorWorker.php
index c448e7221a..473649ba19 100644
--- a/src/infrastructure/daemon/workers/PhabricatorWorker.php
+++ b/src/infrastructure/daemon/workers/PhabricatorWorker.php
@@ -160,8 +160,7 @@ abstract class PhabricatorWorker extends Phobject {
         try {
           $worker->doWork();
           foreach ($worker->getQueuedTasks() as $queued_task) {
-            list($queued_class, $queued_data, $queued_priority) = $queued_task;
-            $queued_options = array('priority' => $queued_priority);
+            list($queued_class, $queued_data, $queued_options) = $queued_task;
             self::scheduleTask($queued_class, $queued_data, $queued_options);
           }
           break;
@@ -220,11 +219,14 @@ abstract class PhabricatorWorker extends Phobject {
    *
    * @param string    Task class to queue.
    * @param array     Data for the followup task.
-   * @param int|null  Priority for the followup task.
+   * @param array Options for the followup task.
    * @return this
    */
-  final protected function queueTask($class, array $data, $priority = null) {
-    $this->queuedTasks[] = array($class, $data, $priority);
+  final protected function queueTask(
+    $class,
+    array $data,
+    array $options = array()) {
+    $this->queuedTasks[] = array($class, $data, $options);
     return $this;
   }
 
diff --git a/src/infrastructure/daemon/workers/storage/PhabricatorWorkerActiveTask.php b/src/infrastructure/daemon/workers/storage/PhabricatorWorkerActiveTask.php
index 6c2f5b4461..57a1794ec3 100644
--- a/src/infrastructure/daemon/workers/storage/PhabricatorWorkerActiveTask.php
+++ b/src/infrastructure/daemon/workers/storage/PhabricatorWorkerActiveTask.php
@@ -217,13 +217,14 @@ final class PhabricatorWorkerActiveTask extends PhabricatorWorkerTask {
     // so execute it out here and just let the exception escape.
     if ($did_succeed) {
       foreach ($worker->getQueuedTasks() as $task) {
-        list($class, $data) = $task;
-        PhabricatorWorker::scheduleTask(
-          $class,
-          $data,
-          array(
-            'priority' => (int)$this->getPriority(),
-          ));
+        list($class, $data, $options) = $task;
+
+        // Default the new task priority to our own priority.
+        $options = $options + array(
+          'priority' => (int)$this->getPriority(),
+        );
+
+        PhabricatorWorker::scheduleTask($class, $data, $options);
       }
     }
 

From b2e89a9e48761aa33da458da75f68b67c702d6d3 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 5 Oct 2015 15:57:41 -0700
Subject: [PATCH 03/11] Fix several error handling issues with Subversion
 commits in Diffusion

Summary:
Ref T9513. I checked this briefly but didn't do a very thorough job of it.

  - Don't try to query merges for Subversion, since it doesn't support them.
  - Fix up "existsquery" to work properly (and efficiently) for both hosted and imported repositories.
  - Fix up "parentsquery" to have similar behavior on invalid commits to other VCSes (throw an exception).

Test Plan:
  - No more merges warning on SVN.
  - Hosted SVN gets the right exists result now.
  - Visiting "r23980283789287" now 404's instead of "not parsed yet".

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9513

Differential Revision: https://secure.phabricator.com/D14239
---
 .../DiffusionExistsQueryConduitAPIMethod.php  | 17 +++++++----------
 .../controller/DiffusionCommitController.php  | 19 +++++++++++--------
 .../DiffusionLowLevelParentsQuery.php         | 16 +++++++++++++++-
 3 files changed, 33 insertions(+), 19 deletions(-)

diff --git a/src/applications/diffusion/conduit/DiffusionExistsQueryConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionExistsQueryConduitAPIMethod.php
index 3c6d09ebda..4280230002 100644
--- a/src/applications/diffusion/conduit/DiffusionExistsQueryConduitAPIMethod.php
+++ b/src/applications/diffusion/conduit/DiffusionExistsQueryConduitAPIMethod.php
@@ -33,16 +33,13 @@ final class DiffusionExistsQueryConduitAPIMethod
   protected function getSVNResult(ConduitAPIRequest $request) {
     $repository = $this->getDiffusionRequest()->getRepository();
     $commit = $request->getValue('commit');
-    list($info) = $repository->execxRemoteCommand(
-      'info %s',
-      $repository->getRemoteURI());
-    $exists = false;
-    $matches = null;
-    if (preg_match('/^Revision: (\d+)$/m', $info, $matches)) {
-      $base_revision = $matches[1];
-      $exists = $base_revision >= $commit;
-    }
-    return $exists;
+
+    $refs = id(new DiffusionCachedResolveRefsQuery())
+      ->setRepository($repository)
+      ->withRefs(array($commit))
+      ->execute();
+
+    return (bool)$refs;
   }
 
   protected function getMercurialResult(ConduitAPIRequest $request) {
diff --git a/src/applications/diffusion/controller/DiffusionCommitController.php b/src/applications/diffusion/controller/DiffusionCommitController.php
index fe2f539f09..a698eba5c0 100644
--- a/src/applications/diffusion/controller/DiffusionCommitController.php
+++ b/src/applications/diffusion/controller/DiffusionCommitController.php
@@ -1136,14 +1136,17 @@ final class DiffusionCommitController extends DiffusionController {
     $merge_limit = $this->getMergeDisplayLimit();
 
     try {
-      $merges = $this->callConduitWithDiffusionRequest(
-        'diffusion.mergedcommitsquery',
-        array(
-          'commit' => $commit,
-          'limit' => $merge_limit + 1,
-        ));
-
-      $this->commitMerges = DiffusionPathChange::newFromConduit($merges);
+      if ($repository->isSVN()) {
+        $this->commitMerges = array();
+      } else {
+        $merges = $this->callConduitWithDiffusionRequest(
+          'diffusion.mergedcommitsquery',
+          array(
+            'commit' => $commit,
+            'limit' => $merge_limit + 1,
+          ));
+        $this->commitMerges = DiffusionPathChange::newFromConduit($merges);
+      }
     } catch (Exception $ex) {
       $this->commitMerges = false;
       $exceptions[] = $ex;
diff --git a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelParentsQuery.php b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelParentsQuery.php
index 60ee4b5fe2..f49f828d79 100644
--- a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelParentsQuery.php
+++ b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelParentsQuery.php
@@ -70,7 +70,21 @@ final class DiffusionLowLevelParentsQuery
   }
 
   private function loadSubversionParents() {
-    $n = (int)$this->identifier;
+    $repository = $this->getRepository();
+    $identifier = $this->identifier;
+
+    $refs = id(new DiffusionCachedResolveRefsQuery())
+      ->setRepository($repository)
+      ->withRefs(array($identifier))
+      ->execute();
+    if (!$refs) {
+      throw new Exception(
+        pht(
+          'No commit "%s" in this repository.',
+          $identifier));
+    }
+
+    $n = (int)$identifier;
     if ($n > 1) {
       $ids = array($n - 1);
     } else {

From ee937e99fb9a8310005a4f4e135590400e47df3b Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 5 Oct 2015 15:59:16 -0700
Subject: [PATCH 04/11] Fix unbounded expansion of allocating resource pool

Summary:
Ref T9252. I think there's a more complex version of this problem discussed elsewhere, but here's what we hit today:

  - 5 commits land at the same time and trigger 5 builds.
  - All of them go to acquire a working copy.
  - Working copies have a limit of 1 right now, so 1 of them gets the lease on it.
  - The other 4 all trigger allocation of //new// working copies. So now we have: 1 active, leased working copy and 4 pending, leased working copies.
  - The 4 pending working copies will never activate without manual intervention, so these 4 builds are stuck forever.

To fix this, prevent WorkingCopies from giving out leases until they activate. So now the leases won't acquire until we know the working copy is good, which solves the first problem.

However, this creates a secondary problem:

  - As above, all 5 go to acquire a working copy.
  - One gets it.
  - The other 4 trigger allocations, but no longer acquire leases. This is an improvement.
  - Every time the leases update, they trigger another allocation, but never acquire. They trigger, say, a few thousand allocations.
  - Eventually the first build finishes up and the second lease acquires the working copy. After some time, all of the builds finish.
  - However, they generated an unboundedly large number of pending working copy resources during this time.

This is technically "okay-ish", in that it did work correctly, it just generated a gigantic mess as a side effect.

To solve this, at least for now, provide a mechanism to impose allocation rate limits and put a cap on the number of allocating resources of a given type. As hard-coded, this the greater of "1" or "25% of the active resources in the pool".

So if there are 40 working copies active, we'll start allocating up to 10 more and then cut new allocations off until those allocations get sorted out. This prevents us from getting runaway queues of limitless size.

This also imposes a total active working copy resource limit of 1, which incidentally also fixes the problem, although I expect to raise this soon.

These mechanisms will need refinement, but the basic idea is:

  - Resources which aren't sure if they can actually activate should wait until they do activate before allowing leases to acquire them. I'm fairly confident this rule is a reasonable one.
  - Then we limit how many bookkeeping side effects Drydock can generate once it starts encountering limits.

Broadly, some amount of mess is inevitable because Drydock is allowed to try things that might not work. In an extreme case we could prevent this mess by setting all these limits at "1" forever, which would degrade Drydock to effectively be a synchronous, blocking queue.

The idea here is to put some amount of slack in the system (more than zero, but less than infinity) so we get the performance benefits of having a parallel, asyncronous system without a finite, manageable amount of mess.

Numbers larger than 0 but less than infinity are pretty tricky, but I think rules like "X% of active resources" seem fairly reasonable, at least for resources like working copies.

Test Plan:
Ran something like this:

```
for i in `seq 1 5`; do sh -c '(./bin/harbormaster build --plan 10 rX... &) &'; done;
```

Saw 5 plans launch, acquire leases, proceed in an orderly fashion, and eventually finish successfully.

Reviewers: hach-que, chad

Reviewed By: chad

Maniphest Tasks: T9252

Differential Revision: https://secure.phabricator.com/D14236
---
 .../DrydockBlueprintImplementation.php        | 67 +++++++++++++++++++
 ...dockWorkingCopyBlueprintImplementation.php | 23 ++++++-
 .../drydock/storage/DrydockResource.php       |  9 +++
 .../worker/DrydockLeaseUpdateWorker.php       | 11 ++-
 4 files changed, 106 insertions(+), 4 deletions(-)

diff --git a/src/applications/drydock/blueprint/DrydockBlueprintImplementation.php b/src/applications/drydock/blueprint/DrydockBlueprintImplementation.php
index e3cdff34c5..0f2e0aad44 100644
--- a/src/applications/drydock/blueprint/DrydockBlueprintImplementation.php
+++ b/src/applications/drydock/blueprint/DrydockBlueprintImplementation.php
@@ -19,6 +19,10 @@ abstract class DrydockBlueprintImplementation extends Phobject {
     return array();
   }
 
+  public function getViewer() {
+    return PhabricatorUser::getOmnipotentUser();
+  }
+
 
 /* -(  Lease Acquisition  )-------------------------------------------------- */
 
@@ -310,4 +314,67 @@ abstract class DrydockBlueprintImplementation extends Phobject {
     }
   }
 
+
+  /**
+   * Apply standard limits on resource allocation rate.
+   *
+   * @param DrydockBlueprint The blueprint requesting an allocation.
+   * @return bool True if further allocations should be limited.
+   */
+  protected function shouldLimitAllocatingPoolSize(
+    DrydockBlueprint $blueprint) {
+
+    // TODO: If this mechanism sticks around, these values should be
+    // configurable by the blueprint implementation.
+
+    // Limit on total number of active resources.
+    $total_limit = 1;
+
+    // Always allow at least this many allocations to be in flight at once.
+    $min_allowed = 1;
+
+    // Allow this fraction of allocating resources as a fraction of active
+    // resources.
+    $growth_factor = 0.25;
+
+    $resource = new DrydockResource();
+    $conn_r = $resource->establishConnection('r');
+
+    $counts = queryfx_all(
+      $conn_r,
+      'SELECT status, COUNT(*) N FROM %T WHERE blueprintPHID = %s',
+      $resource->getTableName(),
+      $blueprint->getPHID());
+    $counts = ipull($counts, 'N', 'status');
+
+    $n_alloc = idx($counts, DrydockResourceStatus::STATUS_PENDING, 0);
+    $n_active = idx($counts, DrydockResourceStatus::STATUS_ACTIVE, 0);
+    $n_broken = idx($counts, DrydockResourceStatus::STATUS_BROKEN, 0);
+    $n_released = idx($counts, DrydockResourceStatus::STATUS_RELEASED, 0);
+
+    // If we're at the limit on total active resources, limit additional
+    // allocations.
+    $n_total = ($n_alloc + $n_active + $n_broken + $n_released);
+    if ($n_total >= $total_limit) {
+      return true;
+    }
+
+    // If the number of in-flight allocations is fewer than the minimum number
+    // of allowed allocations, don't impose a limit.
+    if ($n_alloc < $min_allowed) {
+      return false;
+    }
+
+    $allowed_alloc = (int)ceil($n_active * $growth_factor);
+
+    // If the number of in-flight allocation is fewer than the number of
+    // allowed allocations according to the pool growth factor, don't impose
+    // a limit.
+    if ($n_alloc < $allowed_alloc) {
+      return false;
+    }
+
+    return true;
+  }
+
 }
diff --git a/src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php b/src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php
index e086294c4d..30dd6fe4c5 100644
--- a/src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php
+++ b/src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php
@@ -29,6 +29,17 @@ final class DrydockWorkingCopyBlueprintImplementation
   public function canAllocateResourceForLease(
     DrydockBlueprint $blueprint,
     DrydockLease $lease) {
+    $viewer = $this->getViewer();
+
+    if ($this->shouldLimitAllocatingPoolSize($blueprint)) {
+      return false;
+    }
+
+    // TODO: If we have a pending resource which is compatible with the
+    // configuration for this lease, prevent a new allocation? Otherwise the
+    // queue can fill up with copies of requests from the same lease. But
+    // maybe we can deal with this with "pre-leasing"?
+
     return true;
   }
 
@@ -37,6 +48,12 @@ final class DrydockWorkingCopyBlueprintImplementation
     DrydockResource $resource,
     DrydockLease $lease) {
 
+    // Don't hand out leases on working copies which have not activated, since
+    // it may take an arbitrarily long time for them to acquire a host.
+    if (!$resource->isActive()) {
+      return false;
+    }
+
     $need_map = $lease->getAttribute('repositories.map');
     if (!is_array($need_map)) {
       return false;
@@ -320,8 +337,10 @@ final class DrydockWorkingCopyBlueprintImplementation
   }
 
   private function loadRepositories(array $phids) {
+    $viewer = $this->getViewer();
+
     $repositories = id(new PhabricatorRepositoryQuery())
-      ->setViewer(PhabricatorUser::getOmnipotentUser())
+      ->setViewer($viewer)
       ->withPHIDs($phids)
       ->execute();
     $repositories = mpull($repositories, null, 'getPHID');
@@ -353,7 +372,7 @@ final class DrydockWorkingCopyBlueprintImplementation
   }
 
   private function loadHostLease(DrydockResource $resource) {
-    $viewer = PhabricatorUser::getOmnipotentUser();
+    $viewer = $this->getViewer();
 
     $lease_phid = $resource->getAttribute('host.leasePHID');
 
diff --git a/src/applications/drydock/storage/DrydockResource.php b/src/applications/drydock/storage/DrydockResource.php
index 1ee663fa3c..3eceec8b77 100644
--- a/src/applications/drydock/storage/DrydockResource.php
+++ b/src/applications/drydock/storage/DrydockResource.php
@@ -293,6 +293,15 @@ final class DrydockResource extends DrydockDAO
     }
   }
 
+  public function isActive() {
+    switch ($this->getStatus()) {
+      case DrydockResourceStatus::STATUS_ACTIVE:
+        return true;
+    }
+
+    return false;
+  }
+
   public function logEvent($type, array $data = array()) {
     $log = id(new DrydockLog())
       ->setEpoch(PhabricatorTime::getNow())
diff --git a/src/applications/drydock/worker/DrydockLeaseUpdateWorker.php b/src/applications/drydock/worker/DrydockLeaseUpdateWorker.php
index b6ac6cec52..a93997259d 100644
--- a/src/applications/drydock/worker/DrydockLeaseUpdateWorker.php
+++ b/src/applications/drydock/worker/DrydockLeaseUpdateWorker.php
@@ -535,7 +535,7 @@ final class DrydockLeaseUpdateWorker extends DrydockWorker {
     // If this lease has been acquired but not activated, queue a task to
     // activate it.
     if ($lease->getStatus() == DrydockLeaseStatus::STATUS_ACQUIRED) {
-      PhabricatorWorker::scheduleTask(
+      $this->queueTask(
         __CLASS__,
         array(
           'leasePHID' => $lease->getPHID(),
@@ -691,7 +691,14 @@ final class DrydockLeaseUpdateWorker extends DrydockWorker {
       ->setStatus(DrydockLeaseStatus::STATUS_BROKEN)
       ->save();
 
-    $lease->scheduleUpdate();
+    $this->queueTask(
+      __CLASS__,
+      array(
+        'leasePHID' => $lease->getPHID(),
+      ),
+      array(
+        'objectPHID' => $lease->getPHID(),
+      ));
 
     $lease->logEvent(
       DrydockLeaseActivationFailureLogType::LOGCONST,

From 4d5278af1148f211d52244e3fb61712ed24b9d1f Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 5 Oct 2015 15:59:35 -0700
Subject: [PATCH 05/11] Put Drydock build steps into their own group in
 Harbormaster

Summary: Ref T9252. Move these into a new "Drydock" group.

Test Plan: Clicked "Add Build Step", saw Drydock steps in a Drydock group.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9252

Differential Revision: https://secure.phabricator.com/D14237
---
 src/__phutil_library_map__.php                |  2 ++
 .../HarbormasterPlanEditController.php        |  2 +-
 ...rDrydockCommandBuildStepImplementation.php |  2 +-
 ...easeWorkingCopyBuildStepImplementation.php |  2 +-
 .../HarbormasterDrydockBuildStepGroup.php     | 25 +++++++++++++++++++
 5 files changed, 30 insertions(+), 3 deletions(-)
 create mode 100644 src/applications/harbormaster/stepgroup/HarbormasterDrydockBuildStepGroup.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index 61b4d1f390..184a484929 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -1011,6 +1011,7 @@ phutil_register_library_map(array(
     'HarbormasterController' => 'applications/harbormaster/controller/HarbormasterController.php',
     'HarbormasterCreateArtifactConduitAPIMethod' => 'applications/harbormaster/conduit/HarbormasterCreateArtifactConduitAPIMethod.php',
     'HarbormasterDAO' => 'applications/harbormaster/storage/HarbormasterDAO.php',
+    'HarbormasterDrydockBuildStepGroup' => 'applications/harbormaster/stepgroup/HarbormasterDrydockBuildStepGroup.php',
     'HarbormasterDrydockCommandBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterDrydockCommandBuildStepImplementation.php',
     'HarbormasterDrydockLeaseArtifact' => 'applications/harbormaster/artifact/HarbormasterDrydockLeaseArtifact.php',
     'HarbormasterExecFuture' => 'applications/harbormaster/future/HarbormasterExecFuture.php',
@@ -4813,6 +4814,7 @@ phutil_register_library_map(array(
     'HarbormasterController' => 'PhabricatorController',
     'HarbormasterCreateArtifactConduitAPIMethod' => 'HarbormasterConduitAPIMethod',
     'HarbormasterDAO' => 'PhabricatorLiskDAO',
+    'HarbormasterDrydockBuildStepGroup' => 'HarbormasterBuildStepGroup',
     'HarbormasterDrydockCommandBuildStepImplementation' => 'HarbormasterBuildStepImplementation',
     'HarbormasterDrydockLeaseArtifact' => 'HarbormasterArtifact',
     'HarbormasterExecFuture' => 'Future',
diff --git a/src/applications/harbormaster/controller/HarbormasterPlanEditController.php b/src/applications/harbormaster/controller/HarbormasterPlanEditController.php
index dd43aa0bee..98494d881d 100644
--- a/src/applications/harbormaster/controller/HarbormasterPlanEditController.php
+++ b/src/applications/harbormaster/controller/HarbormasterPlanEditController.php
@@ -61,7 +61,7 @@ final class HarbormasterPlanEditController extends HarbormasterPlanController {
     $is_new = (!$plan->getID());
     if ($is_new) {
       $title = pht('New Build Plan');
-      $cancel_uri = $this->getApplicationURI();
+      $cancel_uri = $this->getApplicationURI('plan/');
       $save_button = pht('Create Build Plan');
     } else {
       $id = $plan->getID();
diff --git a/src/applications/harbormaster/step/HarbormasterDrydockCommandBuildStepImplementation.php b/src/applications/harbormaster/step/HarbormasterDrydockCommandBuildStepImplementation.php
index 0b19704197..6eb63bb39c 100644
--- a/src/applications/harbormaster/step/HarbormasterDrydockCommandBuildStepImplementation.php
+++ b/src/applications/harbormaster/step/HarbormasterDrydockCommandBuildStepImplementation.php
@@ -12,7 +12,7 @@ final class HarbormasterDrydockCommandBuildStepImplementation
   }
 
   public function getBuildStepGroupKey() {
-    return HarbormasterPrototypeBuildStepGroup::GROUPKEY;
+    return HarbormasterDrydockBuildStepGroup::GROUPKEY;
   }
 
   public function getDescription() {
diff --git a/src/applications/harbormaster/step/HarbormasterLeaseWorkingCopyBuildStepImplementation.php b/src/applications/harbormaster/step/HarbormasterLeaseWorkingCopyBuildStepImplementation.php
index 91c6eb0517..73ea19bd7a 100644
--- a/src/applications/harbormaster/step/HarbormasterLeaseWorkingCopyBuildStepImplementation.php
+++ b/src/applications/harbormaster/step/HarbormasterLeaseWorkingCopyBuildStepImplementation.php
@@ -12,7 +12,7 @@ final class HarbormasterLeaseWorkingCopyBuildStepImplementation
   }
 
   public function getBuildStepGroupKey() {
-    return HarbormasterPrototypeBuildStepGroup::GROUPKEY;
+    return HarbormasterDrydockBuildStepGroup::GROUPKEY;
   }
 
   public function execute(
diff --git a/src/applications/harbormaster/stepgroup/HarbormasterDrydockBuildStepGroup.php b/src/applications/harbormaster/stepgroup/HarbormasterDrydockBuildStepGroup.php
new file mode 100644
index 0000000000..606a47a114
--- /dev/null
+++ b/src/applications/harbormaster/stepgroup/HarbormasterDrydockBuildStepGroup.php
@@ -0,0 +1,25 @@
+<?php
+
+final class HarbormasterDrydockBuildStepGroup
+  extends HarbormasterBuildStepGroup {
+
+  const GROUPKEY = 'harbormaster.drydock';
+
+  public function getGroupName() {
+    return pht('Drydock');
+  }
+
+  public function getGroupOrder() {
+    return 3000;
+  }
+
+  public function isEnabled() {
+    $drydock_class = 'PhabricatorDrydockApplication';
+    return PhabricatorApplication::isClassInstalled($drydock_class);
+  }
+
+  public function shouldShowIfEmpty() {
+    return false;
+  }
+
+}

From 03fea7049760b45e79bcf35f2b50abf01e4f08cd Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Tue, 6 Oct 2015 07:38:09 -0700
Subject: [PATCH 06/11] Fix some header formatting in `bin/storage probe`

Summary: Ref T9514. I missed these when I swapped out the console stuff recently.

Test Plan: Ran `bin/storage probe`, saw bold instead of escape sequences.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9514

Differential Revision: https://secure.phabricator.com/D14240
---
 .../PhabricatorStorageManagementProbeWorkflow.php    | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementProbeWorkflow.php b/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementProbeWorkflow.php
index d2fbc95326..8e373d0cf6 100644
--- a/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementProbeWorkflow.php
+++ b/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementProbeWorkflow.php
@@ -60,9 +60,9 @@ final class PhabricatorStorageManagementProbeWorkflow
         $overall);
 
       $table->addRow(array(
-        'name' => phutil_console_format('**%s**', $db),
-        'size' => phutil_console_format('**%s**', $database_size),
-        'percentage' => phutil_console_format('**%s**', $database_percentage),
+        'name' => tsprintf('**%s**', $db),
+        'size' => tsprintf('**%s**', $database_size),
+        'percentage' => tsprintf('**%s**', $database_percentage),
       ));
       $data[$db] = isort($data[$db], '_totalSize');
       foreach ($data[$db] as $table_name => $info) {
@@ -82,9 +82,9 @@ final class PhabricatorStorageManagementProbeWorkflow
       $overall,
       $overall);
     $table->addRow(array(
-      'name' => phutil_console_format('**%s**', pht('TOTAL')),
-      'size' => phutil_console_format('**%s**', $overall_size),
-      'percentage' => phutil_console_format('**%s**', $overall_percentage),
+      'name' => tsprintf('**%s**', pht('TOTAL')),
+      'size' => tsprintf('**%s**', $overall_size),
+      'percentage' => tsprintf('**%s**', $overall_percentage),
     ));
 
     $table->draw();

From 2bfa0e087e0029ea427b0286942e16e068d36511 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Tue, 6 Oct 2015 07:38:15 -0700
Subject: [PATCH 07/11] Improve consistency and Harbormaster integration of
 Diffusion

Summary:
Ref T9123. Two major Harbormaster-related UI changes in Diffusion:

  - Tags table now shows tag build status.
  - Branches table now shows branch build status.

Then some minor consistency / qualtiy of life changes:

  - Picked a nicer looking "history" icon?
  - Branches table now uses the same "history" icon as other tables.
  - Tags table now has a "history" link.
  - Browse table now has a "history" link.
  - Dates now use more consistent formatting.
  - Column order is now more consistent.
  - Use of style is now more consistent.

Test Plan:
{F865056}

{F865057}

{F865058}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9123

Differential Revision: https://secure.phabricator.com/D14242
---
 .../DiffusionLastModifiedController.php       |  5 +-
 .../view/DiffusionBranchTableView.php         | 45 ++++++-----
 .../view/DiffusionBrowseTableView.php         | 19 ++---
 .../view/DiffusionHistoryTableView.php        | 80 +++----------------
 .../diffusion/view/DiffusionTagListView.php   | 60 ++++++++++----
 .../diffusion/view/DiffusionView.php          | 80 ++++++++++++++++++-
 6 files changed, 174 insertions(+), 115 deletions(-)

diff --git a/src/applications/diffusion/controller/DiffusionLastModifiedController.php b/src/applications/diffusion/controller/DiffusionLastModifiedController.php
index 3487733c8b..b23abf1903 100644
--- a/src/applications/diffusion/controller/DiffusionLastModifiedController.php
+++ b/src/applications/diffusion/controller/DiffusionLastModifiedController.php
@@ -98,12 +98,10 @@ final class DiffusionLastModifiedController extends DiffusionController {
       $modified = DiffusionView::linkCommit(
         $drequest->getRepository(),
         $commit->getCommitIdentifier());
-      $date = phabricator_date($epoch, $viewer);
-      $time = phabricator_time($epoch, $viewer);
+      $date = phabricator_datetime($epoch, $viewer);
     } else {
       $modified = '';
       $date = '';
-      $time = '';
     }
 
     $data = $commit->getCommitData();
@@ -137,7 +135,6 @@ final class DiffusionLastModifiedController extends DiffusionController {
     $return = array(
       'commit'    => $modified,
       'date'      => $date,
-      'time'      => $time,
       'author'    => $author,
       'details'   => $details,
     );
diff --git a/src/applications/diffusion/view/DiffusionBranchTableView.php b/src/applications/diffusion/view/DiffusionBranchTableView.php
index d053d5bb33..0f4594e576 100644
--- a/src/applications/diffusion/view/DiffusionBranchTableView.php
+++ b/src/applications/diffusion/view/DiffusionBranchTableView.php
@@ -21,6 +21,11 @@ final class DiffusionBranchTableView extends DiffusionView {
     $drequest = $this->getDiffusionRequest();
     $current_branch = $drequest->getBranch();
     $repository = $drequest->getRepository();
+    $commits = $this->commits;
+    $viewer = $this->getUser();
+
+    $buildables = $this->loadBuildables($commits);
+    $have_builds = false;
 
     $can_close_branches = ($repository->isHg());
 
@@ -31,13 +36,21 @@ final class DiffusionBranchTableView extends DiffusionView {
     $rows = array();
     $rowc = array();
     foreach ($this->branches as $branch) {
-      $commit = idx($this->commits, $branch->getCommitIdentifier());
+      $commit = idx($commits, $branch->getCommitIdentifier());
       if ($commit) {
         $details = $commit->getSummary();
-        $datetime = phabricator_datetime($commit->getEpoch(), $this->user);
+        $datetime = phabricator_datetime($commit->getEpoch(), $viewer);
+        $buildable = idx($buildables, $commit->getPHID());
+        if ($buildable) {
+          $build_status = $this->renderBuildable($buildable);
+          $have_builds = true;
+        } else {
+          $build_status = null;
+        }
       } else {
         $datetime = null;
         $details = null;
+        $build_status = null;
       }
 
       switch ($repository->shouldSkipAutocloseBranch($branch->getShortName())) {
@@ -86,16 +99,7 @@ final class DiffusionBranchTableView extends DiffusionView {
       }
 
       $rows[] = array(
-        phutil_tag(
-          'a',
-          array(
-            'href' => $drequest->generateURI(
-              array(
-                'action' => 'history',
-                'branch' => $branch->getShortName(),
-              )),
-          ),
-          pht('History')),
+        $this->linkBranchHistory($branch->getShortName()),
         phutil_tag(
           'a',
           array(
@@ -109,10 +113,11 @@ final class DiffusionBranchTableView extends DiffusionView {
         self::linkCommit(
           $drequest->getRepository(),
           $branch->getCommitIdentifier()),
+        $build_status,
         $status,
+        AphrontTableView::renderSingleDisplayLine($details),
         $status_icon,
         $datetime,
-        AphrontTableView::renderSingleDisplayLine($details),
       );
       if ($branch->getShortName() == $current_branch) {
         $rowc[] = 'highlighted';
@@ -124,33 +129,37 @@ final class DiffusionBranchTableView extends DiffusionView {
     $view = new AphrontTableView($rows);
     $view->setHeaders(
       array(
-        pht('History'),
+        null,
         pht('Branch'),
         pht('Head'),
+        null,
         pht('State'),
-        pht(''),
-        pht('Modified'),
         pht('Details'),
+        null,
+        pht('Committed'),
       ));
     $view->setColumnClasses(
       array(
         '',
         'pri',
         '',
-        '',
-        '',
+        'icon',
         '',
         'wide',
+        '',
+        '',
       ));
     $view->setColumnVisibility(
       array(
         true,
         true,
         true,
+        $have_builds,
         $can_close_branches,
       ));
     $view->setRowClasses($rowc);
     return $view->render();
   }
 
+
 }
diff --git a/src/applications/diffusion/view/DiffusionBrowseTableView.php b/src/applications/diffusion/view/DiffusionBrowseTableView.php
index a8eb745fe4..71cc659897 100644
--- a/src/applications/diffusion/view/DiffusionBrowseTableView.php
+++ b/src/applications/diffusion/view/DiffusionBrowseTableView.php
@@ -31,6 +31,8 @@ final class DiffusionBrowseTableView extends DiffusionView {
     $show_edit = false;
     foreach ($this->paths as $path) {
 
+      $history_link = $this->linkHistory($path->getPath());
+
       $dir_slash = null;
       $file_type = $path->getFileType();
       if ($file_type == DifferentialChangeType::FILE_DIRECTORY) {
@@ -67,7 +69,6 @@ final class DiffusionBrowseTableView extends DiffusionView {
         'lint'      => celerity_generate_unique_node_id(),
         'commit'    => celerity_generate_unique_node_id(),
         'date'      => celerity_generate_unique_node_id(),
-        'time'      => celerity_generate_unique_node_id(),
         'author'    => celerity_generate_unique_node_id(),
         'details'   => celerity_generate_unique_node_id(),
       );
@@ -78,13 +79,13 @@ final class DiffusionBrowseTableView extends DiffusionView {
       }
 
       $rows[] = array(
+        $history_link,
         $browse_link,
         idx($dict, 'lint'),
         $dict['commit'],
         $dict['author'],
         $dict['details'],
         $dict['date'],
-        $dict['time'],
       );
     }
 
@@ -108,29 +109,29 @@ final class DiffusionBrowseTableView extends DiffusionView {
     $view = new AphrontTableView($rows);
     $view->setHeaders(
       array(
+        null,
         pht('Path'),
         ($lint ? $lint : pht('Lint')),
         pht('Modified'),
         pht('Author/Committer'),
         pht('Details'),
-        pht('Date'),
-        pht('Time'),
+        pht('Committed'),
       ));
     $view->setColumnClasses(
       array(
+        'nudgeright',
+        '',
+        '',
         '',
-        'n',
-        'n',
         '',
         'wide',
         '',
-        'right',
       ));
     $view->setColumnVisibility(
       array(
         true,
-        $show_lint,
         true,
+        $show_lint,
         true,
         true,
         true,
@@ -140,11 +141,11 @@ final class DiffusionBrowseTableView extends DiffusionView {
     $view->setDeviceVisibility(
       array(
         true,
-        false,
         true,
         false,
         true,
         false,
+        true,
         false,
       ));
 
diff --git a/src/applications/diffusion/view/DiffusionHistoryTableView.php b/src/applications/diffusion/view/DiffusionHistoryTableView.php
index 80989efdf2..314bfb435c 100644
--- a/src/applications/diffusion/view/DiffusionHistoryTableView.php
+++ b/src/applications/diffusion/view/DiffusionHistoryTableView.php
@@ -7,12 +7,10 @@ final class DiffusionHistoryTableView extends DiffusionView {
   private $handles = array();
   private $isHead;
   private $parents;
-  private $buildCache;
 
   public function setHistory(array $history) {
     assert_instances_of($history, 'DiffusionPathChange');
     $this->history = $history;
-    $this->buildCache = null;
     return $this;
   }
 
@@ -62,33 +60,14 @@ final class DiffusionHistoryTableView extends DiffusionView {
     return $this;
   }
 
-  public function loadBuildablesOnDemand() {
-    if ($this->buildCache !== null) {
-      return $this->buildCache;
-    }
-
-    $commits_to_builds = array();
-
-    $commits = mpull($this->history, 'getCommit');
-
-    $commit_phids = mpull($commits, 'getPHID');
-
-    $buildables = id(new HarbormasterBuildableQuery())
-      ->setViewer($this->getUser())
-      ->withBuildablePHIDs($commit_phids)
-      ->withManualBuildables(false)
-      ->execute();
-
-    $this->buildCache = mpull($buildables, null, 'getBuildablePHID');
-
-    return $this->buildCache;
-  }
-
   public function render() {
     $drequest = $this->getDiffusionRequest();
 
     $viewer = $this->getUser();
 
+    $buildables = $this->loadBuildables(mpull($this->history, 'getCommit'));
+    $has_any_build = false;
+
     $show_revisions = PhabricatorApplication::isClassInstalledForViewer(
       'PhabricatorDifferentialApplication',
       $viewer);
@@ -110,11 +89,9 @@ final class DiffusionHistoryTableView extends DiffusionView {
       $epoch = $history->getEpoch();
 
       if ($epoch) {
-        $date = phabricator_date($epoch, $this->user);
-        $time = phabricator_time($epoch, $this->user);
+        $committed = phabricator_datetime($epoch, $viewer);
       } else {
-        $date = null;
-        $time = null;
+        $committed = null;
       }
 
       $data = $history->getCommitData();
@@ -160,36 +137,9 @@ final class DiffusionHistoryTableView extends DiffusionView {
 
       $build = null;
       if ($show_builds) {
-        $buildable_lookup = $this->loadBuildablesOnDemand();
-        $buildable = idx($buildable_lookup, $commit->getPHID());
+        $buildable = idx($buildables, $commit->getPHID());
         if ($buildable !== null) {
-          $icon = HarbormasterBuildable::getBuildableStatusIcon(
-            $buildable->getBuildableStatus());
-          $color = HarbormasterBuildable::getBuildableStatusColor(
-            $buildable->getBuildableStatus());
-          $name = HarbormasterBuildable::getBuildableStatusName(
-            $buildable->getBuildableStatus());
-
-          $icon_view = id(new PHUIIconView())
-            ->setIconFont($icon.' '.$color);
-
-          $tooltip_view = javelin_tag(
-            'span',
-            array(
-              'sigil' => 'has-tooltip',
-              'meta' => array('tip' => $name),
-            ),
-            $icon_view);
-
-          Javelin::initBehavior('phabricator-tooltips');
-
-          $href_view = phutil_tag(
-            'a',
-            array('href' => '/'.$buildable->getMonogram()),
-            $tooltip_view);
-
-          $build = $href_view;
-
+          $build = $this->renderBuildable($buildable);
           $has_any_build = true;
         }
       }
@@ -214,8 +164,7 @@ final class DiffusionHistoryTableView extends DiffusionView {
           null),
         $author,
         $summary,
-        $date,
-        $time,
+        $committed,
       );
     }
 
@@ -226,30 +175,28 @@ final class DiffusionHistoryTableView extends DiffusionView {
         null,
         pht('Commit'),
         null,
-        pht('Revision'),
+        null,
         pht('Author/Committer'),
         pht('Details'),
-        pht('Date'),
-        pht('Time'),
+        pht('Committed'),
       ));
     $view->setColumnClasses(
       array(
         'threads',
         'nudgeright',
-        'n',
+        '',
         'icon',
-        'n',
+        '',
         '',
         'wide',
         '',
-        'right',
       ));
     $view->setColumnVisibility(
       array(
         $graph ? true : false,
         true,
         true,
-        true,
+        $has_any_build,
         $show_revisions,
       ));
     $view->setDeviceVisibility(
@@ -262,7 +209,6 @@ final class DiffusionHistoryTableView extends DiffusionView {
         false,
         true,
         false,
-        false,
       ));
     return $view->render();
   }
diff --git a/src/applications/diffusion/view/DiffusionTagListView.php b/src/applications/diffusion/view/DiffusionTagListView.php
index 668453f88e..3b27284f5e 100644
--- a/src/applications/diffusion/view/DiffusionTagListView.php
+++ b/src/applications/diffusion/view/DiffusionTagListView.php
@@ -29,6 +29,8 @@ final class DiffusionTagListView extends DiffusionView {
     $drequest = $this->getDiffusionRequest();
     $repository = $drequest->getRepository();
 
+    $buildables = $this->loadBuildables($this->commits);
+    $has_builds = false;
 
     $rows = array();
     foreach ($this->tags as $tag) {
@@ -80,30 +82,56 @@ final class DiffusionTagListView extends DiffusionView {
         }
       }
 
+      $build = null;
+      if ($commit) {
+        $buildable = idx($buildables, $commit->getPHID());
+        if ($buildable) {
+          $build = $this->renderBuildable($buildable);
+          $has_builds = true;
+        }
+      }
+
+      $history = $this->linkTagHistory($tag->getName());
+
       $rows[] = array(
+        $history,
         $tag_link,
         $commit_link,
-        $description,
+        $build,
         $author,
+        $description,
         phabricator_datetime($tag->getEpoch(), $this->user),
       );
     }
 
-    $table = new AphrontTableView($rows);
-    $table->setHeaders(
-      array(
-        pht('Tag'),
-        pht('Commit'),
-        pht('Description'),
-        pht('Author'),
-        pht('Created'),
-      ));
-    $table->setColumnClasses(
-      array(
-        'pri',
-        '',
-        'wide',
-      ));
+    $table = id(new AphrontTableView($rows))
+      ->setHeaders(
+        array(
+          null,
+          pht('Tag'),
+          pht('Commit'),
+          null,
+          pht('Author'),
+          pht('Description'),
+          pht('Created'),
+        ))
+      ->setColumnClasses(
+        array(
+          'nudgeright',
+          'pri',
+          '',
+          '',
+          '',
+          'wide',
+        ))
+      ->setColumnVisibility(
+        array(
+          true,
+          true,
+          true,
+          $has_builds,
+        ));
+
     return $table->render();
   }
 
diff --git a/src/applications/diffusion/view/DiffusionView.php b/src/applications/diffusion/view/DiffusionView.php
index b7b3599a2d..83fdc1f7e5 100644
--- a/src/applications/diffusion/view/DiffusionView.php
+++ b/src/applications/diffusion/view/DiffusionView.php
@@ -20,6 +20,30 @@ abstract class DiffusionView extends AphrontView {
         'path'   => $path,
       ));
 
+    return $this->renderHistoryLink($href);
+  }
+
+  final public function linkBranchHistory($branch) {
+    $href = $this->getDiffusionRequest()->generateURI(
+      array(
+        'action' => 'history',
+        'branch' => $branch,
+      ));
+
+    return $this->renderHistoryLink($href);
+  }
+
+  final public function linkTagHistory($tag) {
+    $href = $this->getDiffusionRequest()->generateURI(
+      array(
+        'action' => 'history',
+        'commit' => $tag,
+      ));
+
+    return $this->renderHistoryLink($href);
+  }
+
+  private function renderHistoryLink($href) {
     return javelin_tag(
       'a',
       array(
@@ -31,7 +55,7 @@ abstract class DiffusionView extends AphrontView {
           'align' => 'E',
         ),
       ),
-      id(new PHUIIconView())->setIconFont('fa-list-ul blue'));
+      id(new PHUIIconView())->setIconFont('fa-history bluegrey'));
   }
 
   final public function linkBrowse($path, array $details = array()) {
@@ -170,4 +194,58 @@ abstract class DiffusionView extends AphrontView {
     return hsprintf('%s', $name);
   }
 
+  final protected function renderBuildable(HarbormasterBuildable $buildable) {
+    $status = $buildable->getBuildableStatus();
+
+    $icon = HarbormasterBuildable::getBuildableStatusIcon($status);
+    $color = HarbormasterBuildable::getBuildableStatusColor($status);
+    $name = HarbormasterBuildable::getBuildableStatusName($status);
+
+    $icon_view = id(new PHUIIconView())
+      ->setIconFont($icon.' '.$color);
+
+    $tooltip_view = javelin_tag(
+      'span',
+      array(
+        'sigil' => 'has-tooltip',
+        'meta' => array('tip' => $name),
+      ),
+      $icon_view);
+
+    Javelin::initBehavior('phabricator-tooltips');
+
+    return phutil_tag(
+      'a',
+      array('href' => '/'.$buildable->getMonogram()),
+      $tooltip_view);
+  }
+
+  final protected function loadBuildables(array $commits) {
+    assert_instances_of($commits, 'PhabricatorRepositoryCommit');
+
+    if (!$commits) {
+      return array();
+    }
+
+    $viewer = $this->getUser();
+
+    $harbormaster_app = 'PhabricatorHarbormasterApplication';
+    $have_harbormaster = PhabricatorApplication::isClassInstalledForViewer(
+      $harbormaster_app,
+      $viewer);
+
+    if ($have_harbormaster) {
+      $buildables = id(new HarbormasterBuildableQuery())
+        ->setViewer($viewer)
+        ->withBuildablePHIDs(mpull($commits, 'getPHID'))
+        ->withManualBuildables(false)
+        ->execute();
+      $buildables = mpull($buildables, null, 'getBuildablePHID');
+    } else {
+      $buildables = array();
+    }
+
+    return $buildables;
+  }
+
 }

From 4549afbdce54841a9bf3cb0109da65923c0da2d1 Mon Sep 17 00:00:00 2001
From: Chad Little <chad@chadsdomain.com>
Date: Tue, 6 Oct 2015 14:15:50 -0700
Subject: [PATCH 08/11] Link Ponder Answer header to user

Summary: Fixes T9509

Test Plan: View a Ponder Question, hover over answerer name, get URL.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T9509

Differential Revision: https://secure.phabricator.com/D14238
---
 src/applications/ponder/view/PonderAnswerView.php | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/applications/ponder/view/PonderAnswerView.php b/src/applications/ponder/view/PonderAnswerView.php
index 6054e4f538..6f4bba539c 100644
--- a/src/applications/ponder/view/PonderAnswerView.php
+++ b/src/applications/ponder/view/PonderAnswerView.php
@@ -77,10 +77,17 @@ final class PonderAnswerView extends AphrontTagView {
       ->setIconFont('fa-bars')
       ->setDropdownMenu($actions);
 
+    $header_name = phutil_tag(
+      'a',
+      array(
+        'href' => $handle->getURI(),
+      ),
+      $handle->getName());
+
     $header = id(new PHUIHeaderView())
       ->setUser($viewer)
       ->setEpoch($answer->getDateModified())
-      ->setHeader($handle->getName())
+      ->setHeader($header_name)
       ->addActionLink($action_button)
       ->setImage($handle->getImageURI())
       ->setImageURL($handle->getURI());

From 2a355d8548e7974cfaa50a70b685e78483ed4cce Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 7 Oct 2015 07:32:27 -0700
Subject: [PATCH 09/11] Make Diffusion "Last Modified" rendering less fragile

Summary:
Ref T9524. Because fetching the last time files were modified in Diffusion can be slow, we bring it in over Ajax.

The logic to fetch and paint the table is kind of fragile because there are two different definitions of the columns right now and we break in a bad way if they differ.

In particular, calling `diffusion.updatecoverage` can populate a "lint commit" for a repository, which tries to generate lint information in one of the views (but not the other one).

In the longer run I think we're removing some of the concepts here and this rendering should be rebuilt to not have two separate column definitions, but just make it degrade gracefully for now since those are larger changes.

Test Plan:
Reproduced the issue in T9524 by calling `diffusion.updatecoverage` on a repostiory. Specifically, this has a side effect of creating a "lint commit" which triggers a "lint" column in this table, sort of.

Applied this patch, got a clean render.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9524

Differential Revision: https://secure.phabricator.com/D14243
---
 resources/celerity/map.php                    | 20 +++++++++----------
 .../diffusion/behavior-pull-lastmodified.js   | 11 +++++++++-
 2 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/resources/celerity/map.php b/resources/celerity/map.php
index cb661a9bd9..12b63a615d 100644
--- a/resources/celerity/map.php
+++ b/resources/celerity/map.php
@@ -13,7 +13,7 @@ return array(
     'differential.pkg.css' => '2de124c9',
     'differential.pkg.js' => '6223dd9d',
     'diffusion.pkg.css' => 'f45955ed',
-    'diffusion.pkg.js' => '0115b37c',
+    'diffusion.pkg.js' => 'ca1c8b5a',
     'maniphest.pkg.css' => '4845691a',
     'maniphest.pkg.js' => '3ec6a6d5',
     'rsrc/css/aphront/aphront-bars.css' => '231ac33c',
@@ -371,7 +371,7 @@ return array(
     'rsrc/js/application/diffusion/behavior-jump-to.js' => '73d09eef',
     'rsrc/js/application/diffusion/behavior-load-blame.js' => '42126667',
     'rsrc/js/application/diffusion/behavior-locate-file.js' => '6d3e1947',
-    'rsrc/js/application/diffusion/behavior-pull-lastmodified.js' => '2b228192',
+    'rsrc/js/application/diffusion/behavior-pull-lastmodified.js' => 'f01586dc',
     'rsrc/js/application/doorkeeper/behavior-doorkeeper-tag.js' => 'e5822781',
     'rsrc/js/application/files/behavior-icon-composer.js' => '8ef9ab58',
     'rsrc/js/application/files/behavior-launch-icon-composer.js' => '48086888',
@@ -574,7 +574,7 @@ return array(
     'javelin-behavior-diffusion-commit-graph' => '9007c197',
     'javelin-behavior-diffusion-jump-to' => '73d09eef',
     'javelin-behavior-diffusion-locate-file' => '6d3e1947',
-    'javelin-behavior-diffusion-pull-lastmodified' => '2b228192',
+    'javelin-behavior-diffusion-pull-lastmodified' => 'f01586dc',
     'javelin-behavior-doorkeeper-tag' => 'e5822781',
     'javelin-behavior-durable-column' => 'c72aa091',
     'javelin-behavior-error-log' => '6882e80a',
@@ -998,13 +998,6 @@ return array(
       'javelin-install',
       'javelin-util',
     ),
-    '2b228192' => array(
-      'javelin-behavior',
-      'javelin-dom',
-      'javelin-util',
-      'javelin-workflow',
-      'javelin-json',
-    ),
     '2b8de964' => array(
       'javelin-install',
       'javelin-util',
@@ -1939,6 +1932,13 @@ return array(
       'javelin-install',
       'javelin-util',
     ),
+    'f01586dc' => array(
+      'javelin-behavior',
+      'javelin-dom',
+      'javelin-util',
+      'javelin-workflow',
+      'javelin-json',
+    ),
     'f24a53cb' => array(
       'phui-fontkit-css',
     ),
diff --git a/webroot/rsrc/js/application/diffusion/behavior-pull-lastmodified.js b/webroot/rsrc/js/application/diffusion/behavior-pull-lastmodified.js
index aef3dce6b2..dc097487fd 100644
--- a/webroot/rsrc/js/application/diffusion/behavior-pull-lastmodified.js
+++ b/webroot/rsrc/js/application/diffusion/behavior-pull-lastmodified.js
@@ -16,7 +16,16 @@ JX.behavior('diffusion-pull-lastmodified', function(config) {
           if (!config.map[k][l]) {
             continue;
           }
-          JX.DOM.setContent(JX.$(config.map[k][l]), JX.$H(r[k][l]));
+          try {
+            JX.DOM.setContent(JX.$(config.map[k][l]), JX.$H(r[k][l]));
+          } catch (ex) {
+            // The way this works is weird and sometimes the components get
+            // out of sync. Fail gently until we can eventually improve the
+            // underlying mechanism.
+
+            // In particular, we currently may generate lint information
+            // without generating a lint column. See T9524.
+          }
         }
       }
     })

From 5a874ba0a84067827a30d1edd7ef6fde7223b2b2 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 8 Oct 2015 20:03:15 -0700
Subject: [PATCH 10/11] Put cows and figlet bannners in <pre> in HTML mail
 bodies

Summary: Fixes T9538. Ref T9408. `cowsay` and `figlet` Remarkup rules are being mangled in HTML mail right now. Put them in <pre> to unmangle them.

Test Plan:
Sent myself a cow + figlet in mail.

Used `bin/mail show-outbound --id ... --dump-html > dump.html` + open that HTML file in Safari to preview HTML mail.

Saw linebreaks and monospaced formatting.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9538, T9408

Differential Revision: https://secure.phabricator.com/D14248
---
 .../PhabricatorRemarkupCowsayBlockInterpreter.php         | 8 +++++++-
 .../PhabricatorRemarkupFigletBlockInterpreter.php         | 8 +++++++-
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/src/infrastructure/markup/interpreter/PhabricatorRemarkupCowsayBlockInterpreter.php b/src/infrastructure/markup/interpreter/PhabricatorRemarkupCowsayBlockInterpreter.php
index 66801d9578..f24e1e7b3d 100644
--- a/src/infrastructure/markup/interpreter/PhabricatorRemarkupCowsayBlockInterpreter.php
+++ b/src/infrastructure/markup/interpreter/PhabricatorRemarkupCowsayBlockInterpreter.php
@@ -28,10 +28,16 @@ final class PhabricatorRemarkupCowsayBlockInterpreter
       ->setText($content)
       ->renderCow();
 
-    if ($this->getEngine()->isTextMode()) {
+    $engine = $this->getEngine();
+
+    if ($engine->isTextMode()) {
       return $result;
     }
 
+    if ($engine->isHTMLMailMode()) {
+      return phutil_tag('pre', array(), $result);
+    }
+
     return phutil_tag(
       'div',
       array(
diff --git a/src/infrastructure/markup/interpreter/PhabricatorRemarkupFigletBlockInterpreter.php b/src/infrastructure/markup/interpreter/PhabricatorRemarkupFigletBlockInterpreter.php
index e823d7bdeb..ee794bcc73 100644
--- a/src/infrastructure/markup/interpreter/PhabricatorRemarkupFigletBlockInterpreter.php
+++ b/src/infrastructure/markup/interpreter/PhabricatorRemarkupFigletBlockInterpreter.php
@@ -27,10 +27,16 @@ final class PhabricatorRemarkupFigletBlockInterpreter
 
     $result = $figlet->lineEcho($content);
 
-    if ($this->getEngine()->isTextMode()) {
+    $engine = $this->getEngine();
+
+    if ($engine->isTextMode()) {
       return $result;
     }
 
+    if ($engine->isHTMLMailMode()) {
+      return phutil_tag('pre', array(), $result);
+    }
+
     return phutil_tag(
       'div',
       array(

From 3bccb0dcf2bc7f81c85ac7df629774a6871fab8b Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 8 Oct 2015 20:23:05 -0700
Subject: [PATCH 11/11] Add custom Cows and Figlet directories to .gitignore

Summary: These directories didn't get added to `.gitignore`, but should be.

Test Plan:
  - Touched a dummy file in each directory.
  - Ran `git status`.
  - Didn't see the file show up.

Reviewers: chad, cspeckmim

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D14249
---
 .gitignore | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/.gitignore b/.gitignore
index 279cb9427d..a90e868e6f 100644
--- a/.gitignore
+++ b/.gitignore
@@ -34,3 +34,7 @@
 
 # NPM local packages
 /support/aphlict/server/node_modules/
+
+# Places for users to add custom resources.
+/resources/cows/custom/*
+/resources/figlet/custom/*