Page MenuHomePhabricator

D7486.id16872.diff

D7486.id16872.diff

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -367,6 +367,7 @@
'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',
@@ -381,6 +382,8 @@
'DifferentialInlineCommentQuery' => 'applications/differential/query/DifferentialInlineCommentQuery.php',
'DifferentialInlineCommentView' => 'applications/differential/view/DifferentialInlineCommentView.php',
'DifferentialJIRAIssuesFieldSpecification' => 'applications/differential/field/specification/DifferentialJIRAIssuesFieldSpecification.php',
+ 'DifferentialLandingStrategyInterface' => 'applications/differential/landing/DifferentialLandingAdapterInterface.php',
+ 'DifferentialLandingToHostedGit' => 'applications/differential/landing/DifferentialLandingPushToHostedGit.php',
'DifferentialLinesFieldSpecification' => 'applications/differential/field/specification/DifferentialLinesFieldSpecification.php',
'DifferentialLintFieldSpecification' => 'applications/differential/field/specification/DifferentialLintFieldSpecification.php',
'DifferentialLintStatus' => 'applications/differential/constants/DifferentialLintStatus.php',
@@ -418,6 +421,7 @@
'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',
@@ -2588,6 +2592,7 @@
'DifferentialInlineCommentQuery' => 'PhabricatorOffsetPagedQuery',
'DifferentialInlineCommentView' => 'AphrontView',
'DifferentialJIRAIssuesFieldSpecification' => 'DifferentialFieldSpecification',
+ 'DifferentialLandingToHostedGit' => 'DifferentialLandingStrategyInterface',
'DifferentialLinesFieldSpecification' => 'DifferentialFieldSpecification',
'DifferentialLintFieldSpecification' => 'DifferentialFieldSpecification',
'DifferentialLocalCommitsView' => 'AphrontView',
@@ -2625,6 +2630,7 @@
'DifferentialRevisionEditor' => 'PhabricatorEditor',
'DifferentialRevisionIDFieldParserTestCase' => 'PhabricatorTestCase',
'DifferentialRevisionIDFieldSpecification' => 'DifferentialFieldSpecification',
+ 'DifferentialRevisionLandController' => 'DifferentialController',
'DifferentialRevisionListController' =>
array(
0 => 'DifferentialController',
diff --git a/src/applications/differential/DifferentialGetWorkingCopy.php b/src/applications/differential/DifferentialGetWorkingCopy.php
new file mode 100644
--- /dev/null
+++ b/src/applications/differential/DifferentialGetWorkingCopy.php
@@ -0,0 +1,54 @@
+<?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.
+ * tries to handle locking.
+ *
+ * return ArcanistGitAPI TODO is this the right abstraction for server side?
+ */
+ public static function getCleanGitWorkspace(
+ PhabricatorRepository $repo) {
+
+ $origin_path = $repo->getLocalPath();
+
+ $path = rtrim($origin_path, '/');
+ $path = $path . '__workspace';
+
+ if (! Filesystem::pathExists($path)) {
+ $future = new ExecFuture(
+ 'git clone file://%s %s',
+ $origin_path,
+ $path);
+ $future->resolvex();
+ }
+
+ self::obtainLock($path .'/.git/phabricator_lockfile');
+
+ $workspace = new ArcanistGitAPI($path);
+ $workspace->execxLocal('clean -fd');
+ $workspace->execxLocal('checkout master');
+ $workspace->execxLocal('fetch');
+ $workspace->execxLocal('reset --hard origin/master');
+ $workspace->reloadWorkingCopy();
+
+ return $workspace;
+ }
+
+ private static function obtainLock($filename) {
+ $file = fopen($filename, 'c');
+ if ($file === false) {
+ throw new Exception("Could not create lockfile");
+ }
+ $locked = flock($file, LOCK_EX | LOCK_NB);
+ if ($locked !== true) {
+ throw new Exception("Could not lock lockfile");
+ }
+ // ...and keep the lock for the remainder of run.
+ }
+}
diff --git a/src/applications/differential/application/PhabricatorApplicationDifferential.php b/src/applications/differential/application/PhabricatorApplicationDifferential.php
--- a/src/applications/differential/application/PhabricatorApplicationDifferential.php
+++ b/src/applications/differential/application/PhabricatorApplicationDifferential.php
@@ -48,6 +48,8 @@
'changeset/' => 'DifferentialChangesetViewController',
'revision/edit/(?:(?P<id>[1-9]\d*)/)?'
=> 'DifferentialRevisionEditController',
+ 'revision/land/'
+ => 'DifferentialRevisionLandController',
'comment/' => array(
'preview/(?P<id>[1-9]\d*)/' => 'DifferentialCommentPreviewController',
'save/' => 'DifferentialCommentSaveController',
diff --git a/src/applications/differential/config/PhabricatorDifferentialConfigOptions.php b/src/applications/differential/config/PhabricatorDifferentialConfigOptions.php
--- a/src/applications/differential/config/PhabricatorDifferentialConfigOptions.php
+++ b/src/applications/differential/config/PhabricatorDifferentialConfigOptions.php
@@ -36,6 +36,12 @@
'DifferentialDefaultFieldSelector')
->setBaseClass('DifferentialFieldSelector')
->setDescription(pht('Field selector class')),
+ $this->newOption(
+ 'differential.land-strategy',
+ 'class',
+ 'DifferentialLandingToHostedGit')
+ ->setBaseClass('DifferentialLandingStrategyInterface')
+ ->setDescription(pht('Implementation for landing revisions')),
$this->newOption('differential.show-host-field', 'bool', false)
->setBoolOptions(
array(
diff --git a/src/applications/differential/controller/DifferentialRevisionLandController.php b/src/applications/differential/controller/DifferentialRevisionLandController.php
new file mode 100644
--- /dev/null
+++ b/src/applications/differential/controller/DifferentialRevisionLandController.php
@@ -0,0 +1,103 @@
+<?php
+
+final class DifferentialRevisionLandController extends DifferentialController {
+
+ public function processRequest() {
+ $request = $this->getRequest();
+ $viewer = $request->getUser();
+
+ $revision_id = $request->getInt('revisionID');
+
+ if ($revision_id == null) {
+ throw new Exception('"');
+ }
+
+ list($error, $details) = $this->attemptLand($revision_id, $viewer);
+
+ if ($error === null) {
+ $title = "Success!";
+ $text = "Revision was successfully landed.";
+ } else {
+ $title = "Failed to land revision";
+ $text = hsprintf('%s<br>Details:<br><pre>%s</pre>', $error, $details);
+ }
+
+ $dialog = id(new AphrontDialogView())
+ ->setUser($viewer)
+ ->setTitle($title)
+ ->appendChild(phutil_tag('p', array(), $text));
+
+ return id(new AphrontDialogResponse())->setDialog($dialog);
+ }
+
+ private function attemptLand($revision_id, $viewer) {
+ $revision = id(new DifferentialRevisionQuery())
+ ->withIDs(array($revision_id))
+ ->setViewer($viewer)
+ ->executeOne();
+ if (!$revision) {
+ return array("revision $revision_id not found");
+ }
+
+ $status = $revision->getStatus();
+ if ($status != ArcanistDifferentialRevisionStatus::ACCEPTED) {
+ return array("Can only land Accepted revisions.");
+ }
+
+ $repository = $revision->getRepository();
+
+ if ($repository === null) {
+ return array("revision is not attached to a repository.");
+ }
+
+ $can_push = PhabricatorPolicyFilter::hasCapability(
+ $viewer,
+ $repository,
+ DiffusionCapabilityPush::CAPABILITY);
+
+ if (!$can_push) {
+ return array(
+ pht('You do not have permission to push to this repository.'));
+ }
+
+ $push_strategy = PhabricatorEnv::newObjectFromConfig(
+ 'differential.land-strategy');
+
+ try {
+ $push_strategy->assertSupportForRepository($repository);
+ } catch (Exception $e) {
+ return array('Repository does not support landing:', $e->getMessage());
+ }
+
+ try {
+ $workspace = $this->getWorkspace($repository);
+ } catch (Exception $e) {
+ return array('Failed to allocate a workspace.', $e->getMessage());
+ }
+
+ try {
+ $push_strategy->commitRevisionToWorkspace(
+ $revision,
+ $workspace,
+ $viewer);
+ } catch (Exception $e) {
+ return array('Failed to commit patch.', $e->getMessage());
+ }
+
+ try {
+ $push_strategy->pushWorkspaceRepository(
+ $repository,
+ $workspace,
+ $viewer);
+ } catch (Exception $e) {
+ return array('Failed to push changes upstream.', $e->getMessage());
+ }
+
+ return array();
+ }
+
+ function getWorkspace($repo) {
+ return DifferentialGetWorkingCopy::getCleanGitWorkspace($repo);
+ }
+}
+
diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php
--- a/src/applications/differential/controller/DifferentialRevisionViewController.php
+++ b/src/applications/differential/controller/DifferentialRevisionViewController.php
@@ -535,6 +535,32 @@
'href' => $request_uri->alter('download', 'true')
);
+ $pushable = false;
+ $repository = $revision->getRepository();
+ if ($repository !== null) {
+ $landing_strategy = PhabricatorEnv::newObjectFromConfig(
+ 'differential.land-strategy');
+ try {
+ $landing_strategy->assertSupportForRepository($repository);
+ $pushable = true;
+ } catch (Exception $e) {
+ $pushable = false;
+ }
+ }
+
+ if ($pushable) {
+ $can_push = PhabricatorPolicyFilter::hasCapability(
+ $user,
+ $repository,
+ DiffusionCapabilityPush::CAPABILITY);
+ $land_action = array(
+ 'icon' => 'none',
+ 'name' => pht('Land this revision'),
+ 'href' => "/differential/revision/land/?revisionID=${revision_id}",
+ 'disabled' => !$can_push
+ );
+ $links[] = $land_action;
+ }
return $links;
}
diff --git a/src/applications/differential/landing/DifferentialLandingStrategyInterface.php b/src/applications/differential/landing/DifferentialLandingStrategyInterface.php
new file mode 100755
--- /dev/null
+++ b/src/applications/differential/landing/DifferentialLandingStrategyInterface.php
@@ -0,0 +1,17 @@
+<?php
+
+interface DifferentialLandingStrategyInterface {
+
+ function assertSupportForRepository(
+ PhabricatorRepository $repository);
+
+ function commitRevisionToWorkspace(
+ DifferentialRevision $revision,
+ ArcanistRepositoryAPI $workspace,
+ PhabricatorUser $user);
+
+ function pushWorkspaceRepository(
+ PhabricatorRepository $repository,
+ ArcanistRepositoryAPI $workspace,
+ PhabricatorUser $user);
+}
diff --git a/src/applications/differential/landing/DifferentialLandingToHostedGit.php b/src/applications/differential/landing/DifferentialLandingToHostedGit.php
new file mode 100755
--- /dev/null
+++ b/src/applications/differential/landing/DifferentialLandingToHostedGit.php
@@ -0,0 +1,90 @@
+<?php
+
+final class DifferentialLandingToHostedGit
+ implements DifferentialLandingStrategyInterface {
+
+ public function assertSupportForRepository(
+ PhabricatorRepository $repository) {
+
+ $vcs = $repository->getVersionControlSystem();
+ if ($vcs !== PhabricatorRepositoryType::REPOSITORY_TYPE_GIT) {
+ throw new Exception('Only Git repositories are supported');
+ }
+
+ if (!$repository->isHosted()) {
+ throw new Exception('Repository must be Hosted');
+ }
+
+ if (!$repository->isWorkingCopyBare()) {
+ throw new Exception('Repository must be hosted as "bare".');
+ }
+ }
+
+ public function commitRevisionToWorkspace(
+ DifferentialRevision $revision,
+ ArcanistRepositoryAPI $workspace,
+ PhabricatorUser $user) {
+
+ $diff = $revision->loadActiveDiff();
+ $diff_id = $diff->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");
+ }
+
+ $tmp_file = new TempFile();
+ Filesystem::writeFile($tmp_file, $raw_diff);
+
+ $workspace->execxLocal('apply --index %s', $tmp_file);
+
+ $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());
+
+ $workspace->execxLocal(
+ '-c user.name=%s -c user.email=%s commit --message=%s --author=%s',
+ // -c will set the 'committer'
+ $user->getRealName(),
+ $user->loadPrimaryEmailAddress(),
+ $message,
+ $author_string);
+ }
+
+
+ public function pushWorkspaceRepository(
+ PhabricatorRepository $repository,
+ ArcanistRepositoryAPI $workspace,
+ PhabricatorUser $user) {
+
+ $workspace->execxLocal("push origin HEAD:master");
+ }
+}

File Metadata

Mime Type
text/x-diff
Storage Engine
amazon-s3
Storage Format
Raw Data
Storage Handle
phabricator/jq/ef/kgbdftqxdqjahyyh
Default Alt Text
D7486.id16872.diff (15 KB)

Event Timeline