Differential D7582 Diff 17445 src/applications/harbormaster/step/UploadArtifactBuildStepImplementation.php

Changeset View

Standalone View

src/applications/harbormaster/step/UploadArtifactBuildStepImplementation.php

This file was added.

				<?php

				final class UploadArtifactBuildStepImplementation
				extends VariableBuildStepImplementation {

				public function getName() {
				return pht('Upload Artifact');
				}

				public function getGenericDescription() {
				return pht('Upload an artifact from a Drydock host to Phabricator.');
				}

				public function getDescription() {
				$settings = $this->getSettings();

				return pht(
				'Upload artifact located at \'%s\' on \'%s\'.',
				$settings['path'],
				$settings['hostartifact']);
				}

				public function execute(
				HarbormasterBuild $build,
				HarbormasterBuildTarget $build_target) {

				$settings = $this->getSettings();
				$variables = $build_target->getVariables();

				$path = $this->mergeVariables(
				'vsprintf',
				$settings['path'],
				$variables);

				$artifact = $build->loadArtifact($settings['hostartifact']);

				$lease = $artifact->loadDrydockLease();

				$interface = $lease->getInterface('filesystem');

				// TODO: Handle exceptions.
				$file = $interface->saveFile($path, $settings['name']);

				// Insert the artifact record.
				$artifact = $build->createArtifact(
				$build_target,
				$settings['name'],
				HarbormasterBuildArtifact::TYPE_FILE);
				$artifact->setArtifactData(array(
				'filePHID' => $file->getPHID()));
				$artifact->save();
				}
				epriestleyUnsubmitted Not Done Inline Actions This is missing a `-i %P`? epriestley: This is missing a `-i %P`?

				public function validateSettings() {
				$settings = $this->getSettings();

				if ($settings['path'] === null \|\| !is_string($settings['path'])) {
				return false;
				}
				if ($settings['name'] === null \|\| !is_string($settings['name'])) {
				return false;
				}
				if ($settings['hostartifact'] === null \|\|
				!is_string($settings['hostartifact'])) {
				return false;
				}

				// TODO: Check if the host artifact is provided by previous build steps.

				return true;
				}

				public function getSettingDefinitions() {
				return array(
				'path' => array(
				'name' => 'Path',
				'description' =>
				'The path of the file that should be retrieved. Note that on '.
				'Windows machines running FreeSSHD, this path will be relative '.
				'to the SFTP root path (configured under the SFTP tab). You can '.
				'not specify an absolute path for Windows machines.',
				'type' => BuildStepImplementation::SETTING_TYPE_STRING),
				'name' => array(
				'name' => 'Local Name',
				'description' =>
				'The name for the file when it is stored in Phabricator.',
				'type' => BuildStepImplementation::SETTING_TYPE_STRING),
				'hostartifact' => array(
				'name' => 'Host Artifact',
				'description' =>
				'The host artifact that determines what machine the command '.
				'will run on.',
				'type' => BuildStepImplementation::SETTING_TYPE_ARTIFACT,
				'artifact_type' => HarbormasterBuildArtifact::TYPE_HOST));
				}

				}
				epriestleyUnsubmitted Not Done Inline Actions Use `Filesystem::readRandomCharacters()`, but I think the build step should have a config option which name the artifact, which we use here. For example: Artifact Name: x86_build.exe ...would `setArtifactKey("x86_build.exe")`. This allows other build steps to refer to this artifact. (This could maybe be optional and use the basename of the file if omitted.) Or just having the "Local Name" determine this, if present, is probably cleanest. The: UNIQUE KEY `key_artifact_type` (`artifactType`,`artifactIndex`), ...that I put on the table is also probably a really dumb idea and should be removed. This one might be dumb too: UNIQUE KEY `key_artifact` (`buildablePHID`,`artifactType`,`artifactIndex`), Maybe that should be: `buildStepPHID`, `artifactType`, `artifactIndex` ...instead. In all cases, the goal is just to make sure each artifact can be uniquely identified. epriestley: Use `Filesystem::readRandomCharacters()`, but I think the build step should have a config…
				epriestleyUnsubmitted Not Done Inline Actions Pass the name in options (the second parameter). epriestley: Pass the name in options (the second parameter).