From 3eaac9eda9d5e08aaf9266b8079d880a012a563f Mon Sep 17 00:00:00 2001
From: James Rhodes <jrhodes@redpointsoftware.com.au>
Date: Fri, 8 Nov 2013 19:06:45 -0800
Subject: [PATCH] Added `build.id` variable and other improvements / fixes

Summary: This adds a `build.id` variable, cleans up the naming convention of other variables and also fixes an issue in the remote command to read the buffers after the command finishes.

Test Plan: Ran a build with `/bin/echo ${build.id}` and saw the build ID come through.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Maniphest Tasks: T1049

Differential Revision: https://secure.phabricator.com/D7540
---
 .../RemoteCommandBuildStepImplementation.php  | 10 ++++-
 .../step/VariableBuildStepImplementation.php  | 43 +++++++++++--------
 2 files changed, 33 insertions(+), 20 deletions(-)

diff --git a/src/applications/harbormaster/step/RemoteCommandBuildStepImplementation.php b/src/applications/harbormaster/step/RemoteCommandBuildStepImplementation.php
index a84ca21635..ce0e85fc9d 100644
--- a/src/applications/harbormaster/step/RemoteCommandBuildStepImplementation.php
+++ b/src/applications/harbormaster/step/RemoteCommandBuildStepImplementation.php
@@ -31,14 +31,14 @@ final class RemoteCommandBuildStepImplementation
     $variables = $this->retrieveVariablesFromBuild($build);
     $command = $settings['command'];
     preg_match_all(
-      "/\\\$\\{(?P<name>[a-z]+)\\}/",
+      "/\\\$\\{(?P<name>[a-z\.]+)\\}/",
       $command,
       $matches);
     foreach ($matches["name"] as $match) {
       $parameters[] = idx($variables, $match, "");
     }
     $command = str_replace("%", "%%", $command);
-    $command = preg_replace("/\\\$\\{(?P<name>[a-z]+)\\}/", "%s", $command);
+    $command = preg_replace("/\\\$\\{(?P<name>[a-z\.]+)\\}/", "%s", $command);
 
     $command = vcsprintf(
       $command,
@@ -90,6 +90,12 @@ final class RemoteCommandBuildStepImplementation
     // Get the return value so we can log that as well.
     list($err) = $future->resolve();
 
+    // Retrieve the last few bits of information.
+    list($stdout, $stderr) = $future->read();
+    $log_stdout->append($stdout);
+    $log_stderr->append($stderr);
+    $future->discardBuffers();
+
     $log_stdout->finalize($start_stdout);
     $log_stderr->finalize($start_stderr);
 
diff --git a/src/applications/harbormaster/step/VariableBuildStepImplementation.php b/src/applications/harbormaster/step/VariableBuildStepImplementation.php
index 7f91518282..26054db07c 100644
--- a/src/applications/harbormaster/step/VariableBuildStepImplementation.php
+++ b/src/applications/harbormaster/step/VariableBuildStepImplementation.php
@@ -4,29 +4,31 @@ abstract class VariableBuildStepImplementation extends BuildStepImplementation {
 
   public function retrieveVariablesFromBuild(HarbormasterBuild $build) {
     $results = array(
-      'revision' => null,
-      'commit' => null,
-      'repository' => null,
-      'vcs' => null,
-      'uri' => null,
-      'timestamp' => null);
+      'buildable.revision' => null,
+      'buildable.commit' => null,
+      'repository.callsign' => null,
+      'repository.vcs' => null,
+      'repository.uri' => null,
+      'step.timestamp' => null,
+      'build.id' => null);
 
     $buildable = $build->getBuildable();
     $object = $buildable->getBuildableObject();
 
     $repo = null;
     if ($object instanceof DifferentialRevision) {
-      $results['revision'] = $object->getID();
+      $results['buildable.revision'] = $object->getID();
       $repo = $object->getRepository();
     } else if ($object instanceof PhabricatorRepositoryCommit) {
-      $results['commit'] = $object->getCommitIdentifier();
+      $results['buildable.commit'] = $object->getCommitIdentifier();
       $repo = $object->getRepository();
     }
 
-    $results['repository'] = $repo->getCallsign();
-    $results['vcs'] = $repo->getVersionControlSystem();
-    $results['uri'] = $repo->getPublicRemoteURI();
-    $results['timestamp'] = time();
+    $results['repository.callsign'] = $repo->getCallsign();
+    $results['repository.vcs'] = $repo->getVersionControlSystem();
+    $results['repository.uri'] = $repo->getPublicRemoteURI();
+    $results['step.timestamp'] = time();
+    $results['build.id'] = $build->getID();
 
     return $results;
   }
@@ -44,12 +46,17 @@ abstract class VariableBuildStepImplementation extends BuildStepImplementation {
 
   public function getAvailableVariables() {
     return array(
-      'revision' => pht('The differential revision ID, if applicable.'),
-      'commit' => pht('The commit identifier, if applicable.'),
-      'repository' => pht('The callsign of the repository in Phabricator.'),
-      'vcs' => pht('The version control system, either "svn", "hg" or "git".'),
-      'uri' => pht('The URI to clone or checkout the repository from.'),
-      'timestamp' => pht('The current UNIX timestamp.'));
+      'buildable.revision' =>
+        pht('The differential revision ID, if applicable.'),
+      'buildable.commit' => pht('The commit identifier, if applicable.'),
+      'repository.callsign' =>
+        pht('The callsign of the repository in Phabricator.'),
+      'repository.vcs' =>
+        pht('The version control system, either "svn", "hg" or "git".'),
+      'repository.uri' =>
+        pht('The URI to clone or checkout the repository from.'),
+      'step.timestamp' => pht('The current UNIX timestamp.'),
+      'build.id' => pht('The ID of the current build.'));
   }
 
   public function getSettingRemarkupInstructions() {