revi-archive/phorge-arcanist
1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2024-12-23 05:50:54 +01:00
Code Issues Releases Wiki Activity

Improve arc commit with SVN branches

Browse source 
Summary:
If you are trying to commit someone else's diff, arc commit gives warnings about path mismatch. This changes the path comparison to be based on the repo url rather than the local working directory. E.g. if both the author and committer are working in branches/release/2013_08_07 despite being checked out in ~/dev/2013_08_07 (system user being different, of course) it no longer warns that the WC path is different

Original behavior:

	eric@Eric-MBP ~/dev/2013_07_31: arc commit --revision 21
		You are not the author of 'D21: WeMerge Automatic Request'. Commit this
		revision anyway? [y/N] y

		Revision 'D21: WeMerge Automatic Request' was generated from '', but
		current working copy root is '/Users/eric/dev/2013_07_31/'. Commit this
		revision anyway? [y/N] y

	Committing 'D21: WeMerge Automatic Request'...
	Adding         test
	Transmitting file data .
	Committed revision 52676.
	Closing revision D21 'WeMerge Automatic Request'...
	Exception
	ERR-CONDUIT-CORE: You can not mark a revision you don't own as closed.
	(Run with --trace for a full exception trace.)

New behavior:

	eric@Eric-MBP ~/dev/2013_07_31: arc commit --revision 24

		You are not the author of 'D24: WeMerge Automatic Request'. Commit this
		revision anyway? [y/N] y

	Committing 'D24: WeMerge Automatic Request'...
	Adding         test
	Transmitting file data .
	Committed revision 52679.
	Closing revision D24 'WeMerge Automatic Request'...
	Exception
	ERR-CONDUIT-CORE: You can not mark a revision you don't own as closed.
	(Run with --trace for a full exception trace.)

Test Plan: 'arc diff' changes with one user. 'arc patch Dxx' on a different working copy by a different user to review and test changes. accept review. 'arc commit --revision xx' as reviewer to land the patch. complaint goes away.

Reviewers: epriestley, ghostwriter78

Reviewed By: epriestley

CC: aran, epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D6665
This commit is contained in:
Eric Stern 2013-08-04 09:01:22 -07:00 committed by epriestley
parent 5e3b436099
commit dccc8571d7
3 changed files with 45 additions and 17 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
src/repository/api/ArcanistSubversionAPI.php
View file

@ -233,6 +233,13 @@ final class ArcanistSubversionAPI extends ArcanistRepositoryAPI {
}
public function getBranchName() {
$info = $this->getSVNInfo('/');
$repo_root = idx($info, 'Repository Root');
$repo_root_length = strlen($repo_root);
$url = idx($info, 'URL');
if (substr($url, 0, $repo_root_length) == $repo_root) {
return substr($url, $repo_root_length);
}
return 'svn';
}
@ -329,6 +336,7 @@ final class ArcanistSubversionAPI extends ArcanistRepositoryAPI {
'/^(Last Changed Date): (.+) \(.+\)$/m',
'/^(Copied From URL): (\S+)$/m',
'/^(Copied From Rev): (\d+)$/m',
'/^(Repository Root): (\S+)$/m',
'/^(Repository UUID): (\S+)$/m',
'/^(Node Kind): (\S+)$/m',
);

50
src/repository/api/__tests__/ArcanistRepositoryAPIStateTestCase.php
View file

@ -2,24 +2,44 @@
final class ArcanistRepositoryAPIStateTestCase extends ArcanistTestCase {
public function testStateParsing() {
$dir = dirname(__FILE__).'/state/';
$tests = Filesystem::listDirectory($dir, $include_hidden = false);
foreach ($tests as $test) {
$fixture = PhutilDirectoryFixture::newFromArchive($dir.'/'.$test);
$fixture_path = $fixture->getPath();
$working_copy = ArcanistWorkingCopyIdentity::newFromPath($fixture_path);
$api = ArcanistRepositoryAPI::newAPIFromWorkingCopyIdentity(
$working_copy);
$api->setBaseCommitArgumentRules('arc:this');
$this->assertCorrectState($test, $api);
public function testGitStateParsing() {
if (Filesystem::binaryExists('git')) {
$this->parseState('git_basic.git.tgz');
} else {
$this->assertSkipped('Git is not installed');
}
}
public function testHgStateParsing() {
if (Filesystem::binaryExists('hg')) {
$this->parseState('hg_basic.hg.tgz');
} else {
$this->assertSkipped('Mercurial is not installed');
}
}
public function testSvnStateParsing() {
if (Filesystem::binaryExists('svn')) {
$this->parseState('svn_basic.svn.tgz');
} else {
$this->assertSkipped('Subversion is not installed');
}
}
private function parseState($test) {
$dir = dirname(__FILE__) . '/state/';
$fixture = PhutilDirectoryFixture::newFromArchive($dir.'/'.$test);
$fixture_path = $fixture->getPath();
$working_copy = ArcanistWorkingCopyIdentity::newFromPath($fixture_path);
$api = ArcanistRepositoryAPI::newAPIFromWorkingCopyIdentity(
$working_copy);
$api->setBaseCommitArgumentRules('arc:this');
$this->assertCorrectState($test, $api);
}
private function assertCorrectState($test, ArcanistRepositoryAPI $api) {
$f_mod = ArcanistRepositoryAPI::FLAG_MODIFIED;
$f_add = ArcanistRepositoryAPI::FLAG_ADDED;

4
src/workflow/ArcanistCommitWorkflow.php
View file

@ -320,8 +320,8 @@ EOTEXT
"Commit this revision anyway?";
}
$revision_source = idx($revision, 'sourcePath');
$current_source = $repository_api->getPath();
$revision_source = idx($revision, 'branch');
$current_source = $repository_api->getSourceControlPath();
if ($revision_source != $current_source) {
$confirm[] =
"Revision 'D{$revision_id}: {$revision_title}' was generated from ".