1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2024-11-10 08:52:39 +01:00

Add an "arc merge" workflow

Summary:
This should support conservative rewrite policies in git fairly well, under an
assumed workflow of:

  - Develop in local branches, never rewrite history.
  - Commit with "-m" or by typing a brief, non-template commit message
describing the checkpoint.
  - Provide rich information in the web console (reviewers, etc.)
  - Finalize with "git checkout master && arc merge branch && git push" or some
flavor thereof.

This supports Mercurial somewhat. The major problem is that "hg merge" fails if
the local is a fastforward of the remote, at which point there's nowhere we can
throw the commit message. Oh well. Just push it and we'll do our best to link
them up based on local commit info.

I am increasingly forming an opinion that Mercurial is "saftey-scissors git".
But also maybe I have no clue what I'm doing. I just don't understand why anyone
would think it's a good idea to have a trunk consisting of ~50% known-broken
revisions, random checkpoint parts, whitespace changes, typo fixes, etc. If you
use git with branching you can avoid this by making a trunk out of merges or
with rebase/amend, but there seems to be no way to have "one commit = one idea"
in any real sense in Mercurial.

Test Plan: Execute "arc merge" in git and mercurial.

Reviewers: fratrik, Makinde, aran, jungejason, tuomaspelkonen

Reviewed By: Makinde

CC: aran, epriestley, Makinde

Differential Revision: 860
This commit is contained in:
epriestley 2011-08-25 16:02:03 -07:00
parent 31ec011922
commit 44959afd4b
10 changed files with 285 additions and 2 deletions

View file

@ -16,6 +16,7 @@ phutil_register_library_map(array(
'ArcanistBranchWorkflow' => 'workflow/branch',
'ArcanistBundle' => 'parser/bundle',
'ArcanistCallConduitWorkflow' => 'workflow/call-conduit',
'ArcanistCapabilityNotSupportedException' => 'workflow/exception/notsupported',
'ArcanistChooseInvalidRevisionException' => 'exception',
'ArcanistChooseNoRevisionsException' => 'exception',
'ArcanistCommitWorkflow' => 'workflow/commit',
@ -56,6 +57,7 @@ phutil_register_library_map(array(
'ArcanistListWorkflow' => 'workflow/list',
'ArcanistMarkCommittedWorkflow' => 'workflow/mark-committed',
'ArcanistMercurialAPI' => 'repository/api/mercurial',
'ArcanistMergeWorkflow' => 'workflow/merge',
'ArcanistNoEffectException' => 'exception/usage/noeffect',
'ArcanistNoEngineException' => 'exception/usage/noengine',
'ArcanistNoLintLinter' => 'lint/linter/nolint',
@ -121,6 +123,7 @@ phutil_register_library_map(array(
'ArcanistListWorkflow' => 'ArcanistBaseWorkflow',
'ArcanistMarkCommittedWorkflow' => 'ArcanistBaseWorkflow',
'ArcanistMercurialAPI' => 'ArcanistRepositoryAPI',
'ArcanistMergeWorkflow' => 'ArcanistBaseWorkflow',
'ArcanistNoEffectException' => 'ArcanistUsageException',
'ArcanistNoEngineException' => 'ArcanistUsageException',
'ArcanistNoLintLinter' => 'ArcanistLinter',

View file

@ -163,11 +163,21 @@ abstract class ArcanistRepositoryAPI {
abstract public function supportsRelativeLocalCommits();
public function parseRelativeLocalCommit(array $argv) {
throw new Exception("This VCS does not support relative local commits.");
throw new ArcanistCapabilityNotSupportedException($this);
}
public function getAllLocalChanges() {
throw new Exception("This VCS does not support getting all local changes.");
throw new ArcanistCapabilityNotSupportedException($this);
}
abstract public function supportsLocalBranchMerge();
public function performLocalBranchMerge($branch, $message) {
throw new ArcanistCapabilityNotSupportedException($this);
}
public function getFinalizedRevisionMessage() {
throw new ArcanistCapabilityNotSupportedException($this);
}
}

View file

@ -7,6 +7,7 @@
phutil_require_module('arcanist', 'exception/usage');
phutil_require_module('arcanist', 'workflow/exception/notsupported');
phutil_require_module('phutil', 'console');
phutil_require_module('phutil', 'filesystem');

View file

@ -545,4 +545,29 @@ class ArcanistGitAPI extends ArcanistRepositoryAPI {
return $parser->parseDiff($diff);
}
public function supportsLocalBranchMerge() {
return true;
}
public function performLocalBranchMerge($branch, $message) {
if (!$branch) {
throw new ArcanistUsageException(
"Under git, you must specify the branch you want to merge.");
}
$err = phutil_passthru(
'(cd %s && git merge --no-ff -m %s %s)',
$this->getPath(),
$message,
$branch);
if ($err) {
throw new ArcanistUsageException("Merge failed!");
}
}
public function getFinalizedRevisionMessage() {
return "You may now push this commit upstream, as appropriate (e.g. with ".
"'git push', or 'git svn dcommit', or by printing and faxing it).";
}
}

View file

@ -396,4 +396,32 @@ class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
return $parser->parseDiff($diff);
}
public function supportsLocalBranchMerge() {
return true;
}
public function performLocalBranchMerge($branch, $message) {
if ($branch) {
$err = phutil_passthru(
'(cd %s && hg merge --rev %s && hg commit -m %s)',
$this->getPath(),
$branch,
$message);
} else {
$err = phutil_passthru(
'(cd %s && hg merge && hg commit -m %s)',
$this->getPath(),
$message);
}
if ($err) {
throw new ArcanistUsageException("Merge failed!");
}
}
public function getFinalizedRevisionMessage() {
return "You may now push this commit upstream, as appropriate (e.g. with ".
"'hg push' or by printing and faxing it).";
}
}

View file

@ -486,4 +486,8 @@ EODIFF;
return false;
}
public function supportsLocalBranchMerge() {
return false;
}
}

View file

@ -0,0 +1,28 @@
<?php
/*
* Copyright 2011 Facebook, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
final class ArcanistCapabilityNotSupportedException extends Exception {
public function __construct(ArcanistRepositoryAPI $api) {
$name = $api->getSourceControlSystemName();
parent::__construct(
"This repository API ('{$name}') does not support the requested ".
"capability.");
}
}

View file

@ -0,0 +1,10 @@
<?php
/**
* This file is automatically generated. Lint this module to rebuild it.
* @generated
*/
phutil_require_source('ArcanistCapabilityNotSupportedException.php');

View file

@ -0,0 +1,158 @@
<?php
/*
* Copyright 2011 Facebook, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
/**
* Merges a branch using "git merge" or "hg merge", using a template commit
* message from Differential.
*
* @group workflow
*/
class ArcanistMergeWorkflow extends ArcanistBaseWorkflow {
public function getCommandHelp() {
return phutil_console_format(<<<EOTEXT
**merge** [__branch__] [--revision __revision_id__] [--show]
Supports: git, hg
Execute a "git merge <branch>" or "hg merge --rev <branch>" of a
reviewed branch, but give the merge commit a useful commit message
with information from Differential.
In Git, this operates like "git merge <branch>" and should be executed
from the branch you want to merge __into__, just like "git merge".
Branch is required.
In Mercurial, this operates like "hg merge" (default) or
"hg merge --rev <branch>" and should be executed from the branch you
want to merge __from__, just like "hg merge". It will also effect an
"hg commit" with a rich commit message.
EOTEXT
);
}
public function requiresWorkingCopy() {
return true;
}
public function requiresConduit() {
return true;
}
public function requiresAuthentication() {
return true;
}
public function requiresRepositoryAPI() {
return true;
}
public function getArguments() {
return array(
'show' => array(
'help' =>
"Don't merge, just show the commit message."
),
'revision' => array(
'param' => 'revision',
'help' =>
"Use the message for a specific revision. If 'arc' can't figure ".
"out which revision you want, you can tell it explicitly.",
),
'*' => 'branch',
);
}
public function run() {
$this->writeStatusMessage(
phutil_console_format(
"**WARNING:** 'arc merge' is new and experimental.\n"));
$repository_api = $this->getRepositoryAPI();
if (!$repository_api->supportsLocalBranchMerge()) {
$name = $repository_api->getSourceControlSystemName();
throw new ArcanistUsageException(
"This source control system ('{$name}') does not support 'arc merge'.");
}
if ($repository_api->getUncommittedChanges()) {
throw new ArcanistUsageException(
"You have uncommitted changes in this working copy. Commit ".
"(or revert) them before proceeding.");
}
$branch = $this->getArgument('branch');
if (count($branch) > 1) {
throw new ArcanistUsageException("Specify only one branch to merge.");
} else {
$branch = head($branch);
}
$conduit = $this->getConduit();
$revisions = $conduit->callMethodSynchronous(
'differential.find',
array(
'guids' => array($this->getUserPHID()),
'query' => 'committable',
));
// TODO: Make an effort to guess which revision the user means here. Branch
// name is a very strong heuristic but Conduit doesn't make it easy to get
// right now. We now also have "commits:local" after D857. Between these
// we should be able to get this right automatically in essentially every
// reasonable case.
try {
$revision = $this->chooseRevision(
$revisions,
$this->getArgument('revision'),
'Which revision do you want to merge?');
$revision_id = $revision->getID();
} catch (ArcanistChooseInvalidRevisionException $ex) {
throw new ArcanistUsageException(
"You can only merge Differential revisions which have been accepted.");
} catch (ArcanistChooseNoRevisionsException $ex) {
throw new ArcanistUsageException(
"You have no accepted Differential revisions.");
}
$message = $conduit->callMethodSynchronous(
'differential.getcommitmessage',
array(
'revision_id' => $revision_id,
'edit' => false,
));
if ($this->getArgument('show')) {
echo $message."\n";
} else {
$repository_api->performLocalBranchMerge($branch, $message);
echo "Merged '{$branch}'.\n";
$done_message = $repository_api->getFinalizedRevisionMessage();
echo phutil_console_wrap($done_message."\n");
}
return 0;
}
protected function getSupportedRevisionControlSystems() {
return array('git');
}
}

View file

@ -0,0 +1,16 @@
<?php
/**
* This file is automatically generated. Lint this module to rebuild it.
* @generated
*/
phutil_require_module('arcanist', 'exception/usage');
phutil_require_module('arcanist', 'workflow/base');
phutil_require_module('phutil', 'console');
phutil_require_module('phutil', 'utils');
phutil_require_source('ArcanistMergeWorkflow.php');