mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-26 08:42:41 +01:00
Land Revision button for hosted git repos
Summary: ref T182. Simple approach of clone, patch, push. While waiting for drydock, implement a hackish mutex setup for the workspace, which should work ok as long as there's only one committer who is carefull about theses things. Less obvious note: This is taking the both author and commiter's 'primary email' for the commit - which might rub some people wrong. Test Plan: With a hosted repo, created some diffs and landed them. Also clicked button for some error cases, got the right error message. Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley CC: hach-que, Korvin, epriestley, aran Maniphest Tasks: T182 Differential Revision: https://secure.phabricator.com/D7486
This commit is contained in:
parent
ca5400d14b
commit
5c0edc9351
7 changed files with 413 additions and 0 deletions
|
@ -369,6 +369,7 @@ phutil_register_library_map(array(
|
|||
'DifferentialFieldValidationException' => 'applications/differential/field/exception/DifferentialFieldValidationException.php',
|
||||
'DifferentialFreeformFieldSpecification' => 'applications/differential/field/specification/DifferentialFreeformFieldSpecification.php',
|
||||
'DifferentialFreeformFieldTestCase' => 'applications/differential/field/specification/__tests__/DifferentialFreeformFieldTestCase.php',
|
||||
'DifferentialGetWorkingCopy' => 'applications/differential/DifferentialGetWorkingCopy.php',
|
||||
'DifferentialGitSVNIDFieldSpecification' => 'applications/differential/field/specification/DifferentialGitSVNIDFieldSpecification.php',
|
||||
'DifferentialHostFieldSpecification' => 'applications/differential/field/specification/DifferentialHostFieldSpecification.php',
|
||||
'DifferentialHovercardEventListener' => 'applications/differential/event/DifferentialHovercardEventListener.php',
|
||||
|
@ -383,6 +384,9 @@ phutil_register_library_map(array(
|
|||
'DifferentialInlineCommentQuery' => 'applications/differential/query/DifferentialInlineCommentQuery.php',
|
||||
'DifferentialInlineCommentView' => 'applications/differential/view/DifferentialInlineCommentView.php',
|
||||
'DifferentialJIRAIssuesFieldSpecification' => 'applications/differential/field/specification/DifferentialJIRAIssuesFieldSpecification.php',
|
||||
'DifferentialLandingActionMenuEventListener' => 'applications/differential/landing/DifferentialLandingActionMenuEventListener.php',
|
||||
'DifferentialLandingStrategy' => 'applications/differential/landing/DifferentialLandingStrategy.php',
|
||||
'DifferentialLandingToHostedGit' => 'applications/differential/landing/DifferentialLandingToHostedGit.php',
|
||||
'DifferentialLinesFieldSpecification' => 'applications/differential/field/specification/DifferentialLinesFieldSpecification.php',
|
||||
'DifferentialLintFieldSpecification' => 'applications/differential/field/specification/DifferentialLintFieldSpecification.php',
|
||||
'DifferentialLintStatus' => 'applications/differential/constants/DifferentialLintStatus.php',
|
||||
|
@ -420,6 +424,7 @@ phutil_register_library_map(array(
|
|||
'DifferentialRevisionEditor' => 'applications/differential/editor/DifferentialRevisionEditor.php',
|
||||
'DifferentialRevisionIDFieldParserTestCase' => 'applications/differential/field/specification/__tests__/DifferentialRevisionIDFieldParserTestCase.php',
|
||||
'DifferentialRevisionIDFieldSpecification' => 'applications/differential/field/specification/DifferentialRevisionIDFieldSpecification.php',
|
||||
'DifferentialRevisionLandController' => 'applications/differential/controller/DifferentialRevisionLandController.php',
|
||||
'DifferentialRevisionListController' => 'applications/differential/controller/DifferentialRevisionListController.php',
|
||||
'DifferentialRevisionListView' => 'applications/differential/view/DifferentialRevisionListView.php',
|
||||
'DifferentialRevisionMailReceiver' => 'applications/differential/mail/DifferentialRevisionMailReceiver.php',
|
||||
|
@ -2586,6 +2591,8 @@ phutil_register_library_map(array(
|
|||
'DifferentialInlineCommentQuery' => 'PhabricatorOffsetPagedQuery',
|
||||
'DifferentialInlineCommentView' => 'AphrontView',
|
||||
'DifferentialJIRAIssuesFieldSpecification' => 'DifferentialFieldSpecification',
|
||||
'DifferentialLandingActionMenuEventListener' => 'PhabricatorEventListener',
|
||||
'DifferentialLandingToHostedGit' => 'DifferentialLandingStrategy',
|
||||
'DifferentialLinesFieldSpecification' => 'DifferentialFieldSpecification',
|
||||
'DifferentialLintFieldSpecification' => 'DifferentialFieldSpecification',
|
||||
'DifferentialLocalCommitsView' => 'AphrontView',
|
||||
|
@ -2623,6 +2630,7 @@ phutil_register_library_map(array(
|
|||
'DifferentialRevisionEditor' => 'PhabricatorEditor',
|
||||
'DifferentialRevisionIDFieldParserTestCase' => 'PhabricatorTestCase',
|
||||
'DifferentialRevisionIDFieldSpecification' => 'DifferentialFieldSpecification',
|
||||
'DifferentialRevisionLandController' => 'DifferentialController',
|
||||
'DifferentialRevisionListController' =>
|
||||
array(
|
||||
0 => 'DifferentialController',
|
||||
|
|
39
src/applications/differential/DifferentialGetWorkingCopy.php
Normal file
39
src/applications/differential/DifferentialGetWorkingCopy.php
Normal file
|
@ -0,0 +1,39 @@
|
|||
<?php
|
||||
|
||||
/**
|
||||
* Can't find a good place for this, so I'm putting it in the most notably
|
||||
* wrong place.
|
||||
*/
|
||||
final class DifferentialGetWorkingCopy {
|
||||
|
||||
/**
|
||||
* Creates and/or cleans a workspace for the requested repo.
|
||||
*
|
||||
* return ArcanistGitAPI
|
||||
*/
|
||||
public static function getCleanGitWorkspace(
|
||||
PhabricatorRepository $repo) {
|
||||
|
||||
$origin_path = $repo->getLocalPath();
|
||||
|
||||
$path = rtrim($origin_path, '/');
|
||||
$path = $path . '__workspace';
|
||||
|
||||
if (!Filesystem::pathExists($path)) {
|
||||
$repo->execxLocalCommand(
|
||||
'clone -- file://%s %s',
|
||||
$origin_path,
|
||||
$path);
|
||||
}
|
||||
|
||||
$workspace = new ArcanistGitAPI($path);
|
||||
$workspace->execxLocal('clean -f -d');
|
||||
$workspace->execxLocal('checkout master');
|
||||
$workspace->execxLocal('fetch');
|
||||
$workspace->execxLocal('reset --hard origin/master');
|
||||
$workspace->reloadWorkingCopy();
|
||||
|
||||
return $workspace;
|
||||
}
|
||||
|
||||
}
|
|
@ -32,6 +32,7 @@ final class PhabricatorApplicationDifferential extends PhabricatorApplication {
|
|||
return array(
|
||||
new DifferentialActionMenuEventListener(),
|
||||
new DifferentialHovercardEventListener(),
|
||||
new DifferentialLandingActionMenuEventListener(),
|
||||
);
|
||||
}
|
||||
|
||||
|
@ -48,6 +49,8 @@ final class PhabricatorApplicationDifferential extends PhabricatorApplication {
|
|||
'changeset/' => 'DifferentialChangesetViewController',
|
||||
'revision/edit/(?:(?P<id>[1-9]\d*)/)?'
|
||||
=> 'DifferentialRevisionEditController',
|
||||
'revision/land/(?:(?P<id>[1-9]\d*))/(?P<strategy>[^/]+)/'
|
||||
=> 'DifferentialRevisionLandController',
|
||||
'comment/' => array(
|
||||
'preview/(?P<id>[1-9]\d*)/' => 'DifferentialCommentPreviewController',
|
||||
'save/' => 'DifferentialCommentSaveController',
|
||||
|
|
|
@ -0,0 +1,130 @@
|
|||
<?php
|
||||
|
||||
final class DifferentialRevisionLandController extends DifferentialController {
|
||||
|
||||
private $revisionID;
|
||||
private $strategyClass;
|
||||
private $pushStrategy;
|
||||
|
||||
public function willProcessRequest(array $data) {
|
||||
$this->revisionID = $data['id'];
|
||||
$this->strategyClass = $data['strategy'];
|
||||
}
|
||||
|
||||
public function processRequest() {
|
||||
$request = $this->getRequest();
|
||||
$viewer = $request->getUser();
|
||||
|
||||
$revision_id = $this->revisionID;
|
||||
|
||||
$revision = id(new DifferentialRevisionQuery())
|
||||
->withIDs(array($revision_id))
|
||||
->setViewer($viewer)
|
||||
->executeOne();
|
||||
if (!$revision) {
|
||||
return new Aphront404Response();
|
||||
}
|
||||
|
||||
if (is_subclass_of($this->strategyClass, 'DifferentialLandingStrategy')) {
|
||||
$this->pushStrategy = newv($this->strategyClass, array());
|
||||
} else {
|
||||
throw new Exception(
|
||||
"Strategy type must be a valid class name and must subclass ".
|
||||
"DifferentialLandingStrategy. ".
|
||||
"'{$this->strategyClass}' is not a subclass of ".
|
||||
"DifferentialLandingStrategy.");
|
||||
}
|
||||
|
||||
if ($request->isDialogFormPost()) {
|
||||
try {
|
||||
$this->attemptLand($revision, $request);
|
||||
$title = pht("Success!");
|
||||
$text = pht("Revision was successfully landed.");
|
||||
} catch (Exception $ex) {
|
||||
$title = pht("Failed to land revision");
|
||||
$text = 'moo';
|
||||
if ($ex instanceof PhutilProxyException) {
|
||||
$text = hsprintf(
|
||||
'%s:<br><pre>%s</pre>',
|
||||
$ex->getMessage(),
|
||||
$ex->getPreviousException()->getMessage());
|
||||
} else {
|
||||
$text = hsprintf('<pre>%s</pre>', $ex->getMessage());
|
||||
}
|
||||
$text = id(new AphrontErrorView())
|
||||
->appendChild($text);
|
||||
}
|
||||
|
||||
$dialog = id(new AphrontDialogView())
|
||||
->setUser($viewer)
|
||||
->setTitle($title)
|
||||
->appendChild(phutil_tag('p', array(), $text))
|
||||
->setSubmitURI('/D'.$revision_id)
|
||||
->addSubmitButton(pht('Done'));
|
||||
|
||||
return id(new AphrontDialogResponse())->setDialog($dialog);
|
||||
}
|
||||
|
||||
$prompt = hsprintf('%s<br><br>%s',
|
||||
pht(
|
||||
'This will squash and rebase revision %s, and push it to '.
|
||||
'origin/master.',
|
||||
$revision_id),
|
||||
pht('It is an experimental feature and may not work.'));
|
||||
|
||||
$dialog = id(new AphrontDialogView())
|
||||
->setUser($viewer)
|
||||
->setTitle(pht("Land Revision %s?", $revision_id))
|
||||
->appendChild($prompt)
|
||||
->setSubmitURI($request->getRequestURI())
|
||||
->addSubmitButton(pht('Land it!'))
|
||||
->addCancelButton('/D'.$revision_id);
|
||||
|
||||
return id(new AphrontDialogResponse())->setDialog($dialog);
|
||||
}
|
||||
|
||||
private function attemptLand($revision, $request) {
|
||||
$status = $revision->getStatus();
|
||||
if ($status != ArcanistDifferentialRevisionStatus::ACCEPTED) {
|
||||
throw new Exception("Only Accepted revisions can be landed.");
|
||||
}
|
||||
|
||||
$repository = $revision->getRepository();
|
||||
|
||||
if ($repository === null) {
|
||||
throw new Exception("revision is not attached to a repository.");
|
||||
}
|
||||
|
||||
$can_push = PhabricatorPolicyFilter::hasCapability(
|
||||
$request->getUser(),
|
||||
$repository,
|
||||
DiffusionCapabilityPush::CAPABILITY);
|
||||
|
||||
if (!$can_push) {
|
||||
throw new Exception(
|
||||
pht('You do not have permission to push to this repository.'));
|
||||
}
|
||||
|
||||
$lock = $this->lockRepository($repository);
|
||||
|
||||
try {
|
||||
$this->pushStrategy->processLandRequest(
|
||||
$request,
|
||||
$revision,
|
||||
$repository);
|
||||
} catch (Exception $e) {
|
||||
$lock->unlock();
|
||||
throw $e;
|
||||
}
|
||||
|
||||
$lock->unlock();
|
||||
}
|
||||
|
||||
private function lockRepository($repository) {
|
||||
$lock_name = __CLASS__.':'.($repository->getCallsign());
|
||||
$lock = PhabricatorGlobalLock::newLock($lock_name);
|
||||
$lock->lock();
|
||||
return $lock;
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,54 @@
|
|||
<?php
|
||||
|
||||
final class DifferentialLandingActionMenuEventListener
|
||||
extends PhabricatorEventListener {
|
||||
|
||||
public function register() {
|
||||
$this->listen(PhabricatorEventType::TYPE_UI_DIDRENDERACTIONS);
|
||||
}
|
||||
|
||||
public function handleEvent(PhutilEvent $event) {
|
||||
switch ($event->getType()) {
|
||||
case PhabricatorEventType::TYPE_UI_DIDRENDERACTIONS:
|
||||
$this->handleActionsEvent($event);
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
private function handleActionsEvent(PhutilEvent $event) {
|
||||
$object = $event->getValue('object');
|
||||
|
||||
$actions = null;
|
||||
if ($object instanceof DifferentialRevision) {
|
||||
$actions = $this->renderRevisionAction($event);
|
||||
}
|
||||
|
||||
$this->addActionMenuItems($event, $actions);
|
||||
}
|
||||
|
||||
private function renderRevisionAction(PhutilEvent $event) {
|
||||
if (!$this->canUseApplication($event->getUser())) {
|
||||
return null;
|
||||
}
|
||||
|
||||
$revision = $event->getValue('object');
|
||||
|
||||
$repository = $revision->getRepository();
|
||||
if ($repository === null) {
|
||||
return null;
|
||||
}
|
||||
|
||||
$strategies = id(new PhutilSymbolLoader())
|
||||
->setAncestorClass('DifferentialLandingStrategy')
|
||||
->loadObjects();
|
||||
foreach ($strategies as $strategy) {
|
||||
$actions = $strategy->createMenuItems(
|
||||
$event->getUser(),
|
||||
$revision,
|
||||
$repository);
|
||||
$this->addActionMenuItems($event, $actions);
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
|
|
@ -0,0 +1,43 @@
|
|||
<?php
|
||||
|
||||
abstract class DifferentialLandingStrategy {
|
||||
|
||||
public abstract function processLandRequest(
|
||||
AphrontRequest $request,
|
||||
DifferentialRevision $revision,
|
||||
PhabricatorRepository $repository);
|
||||
|
||||
/**
|
||||
* returns PhabricatorActionView or an array of PhabricatorActionView or null.
|
||||
*/
|
||||
abstract function createMenuItems(
|
||||
PhabricatorUser $viewer,
|
||||
DifferentialRevision $revision,
|
||||
PhabricatorRepository $repository);
|
||||
|
||||
/**
|
||||
* returns PhabricatorActionView which can be attached to the revision view.
|
||||
*/
|
||||
protected function createActionView($revision, $name, $disabled = false) {
|
||||
$strategy = get_class($this);
|
||||
$revision_id = $revision->getId();
|
||||
return id(new PhabricatorActionView())
|
||||
->setRenderAsForm(true)
|
||||
->setName($name)
|
||||
->setHref("/differential/revision/land/{$revision_id}/{$strategy}/")
|
||||
->setDisabled($disabled);
|
||||
}
|
||||
|
||||
/**
|
||||
* might break if repository is not Git.
|
||||
*/
|
||||
protected function getGitWorkspace(PhabricatorRepository $repository) {
|
||||
try {
|
||||
return DifferentialGetWorkingCopy::getCleanGitWorkspace($repository);
|
||||
} catch (Exception $e) {
|
||||
throw new PhutilProxyException (
|
||||
'Failed to allocate a workspace',
|
||||
$e);
|
||||
}
|
||||
}
|
||||
}
|
|
@ -0,0 +1,136 @@
|
|||
<?php
|
||||
|
||||
final class DifferentialLandingToHostedGit
|
||||
extends DifferentialLandingStrategy {
|
||||
|
||||
public function processLandRequest(
|
||||
AphrontRequest $request,
|
||||
DifferentialRevision $revision,
|
||||
PhabricatorRepository $repository) {
|
||||
|
||||
$viewer = $request->getUser();
|
||||
|
||||
$workspace = $this->getGitWorkspace($repository);
|
||||
|
||||
try {
|
||||
$this->commitRevisionToWorkspace(
|
||||
$revision,
|
||||
$workspace,
|
||||
$viewer);
|
||||
} catch (Exception $e) {
|
||||
throw new PhutilProxyException(
|
||||
'Failed to commit patch',
|
||||
$e);
|
||||
}
|
||||
|
||||
try {
|
||||
$this->pushWorkspaceRepository(
|
||||
$repository,
|
||||
$workspace,
|
||||
$viewer);
|
||||
} catch (Exception $e) {
|
||||
throw new PhutilProxyException(
|
||||
'Failed to push changes upstream',
|
||||
$e);
|
||||
}
|
||||
}
|
||||
|
||||
public function commitRevisionToWorkspace(
|
||||
DifferentialRevision $revision,
|
||||
ArcanistRepositoryAPI $workspace,
|
||||
PhabricatorUser $user) {
|
||||
|
||||
$diff_id = $revision->loadActiveDiff()->getID();
|
||||
|
||||
$call = new ConduitCall(
|
||||
'differential.getrawdiff',
|
||||
array(
|
||||
'diffID' => $diff_id,
|
||||
));
|
||||
|
||||
$call->setUser($user);
|
||||
$raw_diff = $call->execute();
|
||||
|
||||
$missing_binary =
|
||||
"\nindex "
|
||||
. "0000000000000000000000000000000000000000.."
|
||||
. "0000000000000000000000000000000000000000\n";
|
||||
if (strpos($raw_diff, $missing_binary) !== false) {
|
||||
throw new Exception("Patch is missing content for a binary file");
|
||||
}
|
||||
|
||||
$future = $workspace->execFutureLocal('apply --index -');
|
||||
$future->write($raw_diff);
|
||||
$future->resolvex();
|
||||
|
||||
$workspace->reloadWorkingCopy();
|
||||
|
||||
$call = new ConduitCall(
|
||||
'differential.getcommitmessage',
|
||||
array(
|
||||
'revision_id' => $revision->getID(),
|
||||
));
|
||||
|
||||
$call->setUser($user);
|
||||
$message = $call->execute();
|
||||
|
||||
$author = id(new PhabricatorUser())->loadOneWhere(
|
||||
'phid = %s',
|
||||
$revision->getAuthorPHID());
|
||||
|
||||
$author_string = sprintf(
|
||||
'%s <%s>',
|
||||
$author->getRealName(),
|
||||
$author->loadPrimaryEmailAddress());
|
||||
$author_date = $revision->getDateCreated();
|
||||
|
||||
$workspace->execxLocal(
|
||||
'-c user.name=%s -c user.email=%s ' .
|
||||
'commit --date=%s --author=%s '.
|
||||
'--message=%s',
|
||||
// -c will set the 'committer'
|
||||
$user->getRealName(),
|
||||
$user->loadPrimaryEmailAddress(),
|
||||
$author_date,
|
||||
$author_string,
|
||||
$message);
|
||||
}
|
||||
|
||||
|
||||
public function pushWorkspaceRepository(
|
||||
PhabricatorRepository $repository,
|
||||
ArcanistRepositoryAPI $workspace,
|
||||
PhabricatorUser $user) {
|
||||
|
||||
$workspace->execxLocal("push origin HEAD:master");
|
||||
}
|
||||
|
||||
public function createMenuItems(
|
||||
PhabricatorUser $viewer,
|
||||
DifferentialRevision $revision,
|
||||
PhabricatorRepository $repository) {
|
||||
|
||||
$vcs = $repository->getVersionControlSystem();
|
||||
if ($vcs !== PhabricatorRepositoryType::REPOSITORY_TYPE_GIT) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (!$repository->isHosted()) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (!$repository->isWorkingCopyBare()) {
|
||||
return;
|
||||
}
|
||||
|
||||
$can_push = PhabricatorPolicyFilter::hasCapability(
|
||||
$viewer,
|
||||
$repository,
|
||||
DiffusionCapabilityPush::CAPABILITY);
|
||||
|
||||
return $this->createActionView(
|
||||
$revision,
|
||||
pht('Land to Hosted Repository'),
|
||||
!$can_push);
|
||||
}
|
||||
}
|
Loading…
Reference in a new issue