1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2024-11-29 10:12:41 +01:00

Arc - add a sanity check to arc patch workflows to make sure the vcs base

revision is the correct base revision relative to the patch.

Summary: What the title says.   If not correct, warn the user.   This check
honors the --force flag to skip all these checks.   This change also includes
moving some Differential constants into Arc so they can be used for both
projects.   There is a corresponding phabricator diff (incoming) to address this
part of the change.

Test Plan:
For a project with actual diffs, a git repository tracked by phabricator, *AND*
development in master branch only, do some...
- git reset --hard HEAD^1
- arc patch DX, where X is what got us to HEAD in the first place
- verify successful patch
...then...
- git reset --hard HEAD^^
- arc patch DX, where X is what got us to HEAD in the first place
- verify warning
- verify Y versus N continues versus stops appropriately
Note if development were done outside the master branch this warning message
will fire early / often as git commit hashes are based on the commit *and* the
rest of the source code the commit is made against.  This is (unfortunately) the
"typical" case so this warning is pretty active at the moment.   T201 will
eventually land and when parsing a given commit update the corresponding diff.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, btrahan, epriestley

Differential Revision: https://secure.phabricator.com/D1328
This commit is contained in:
Bob Trahan 2011-12-02 16:21:14 -08:00
parent f3eccfbe81
commit b61e4eacf1
14 changed files with 229 additions and 14 deletions

View file

@ -33,7 +33,9 @@ phutil_register_library_map(array(
'ArcanistDiffWorkflow' => 'workflow/diff',
'ArcanistDifferentialCommitMessage' => 'differential/commitmessage',
'ArcanistDifferentialCommitMessageParserException' => 'differential/commitmessage',
'ArcanistDifferentialRevisionHash' => 'differential/constants/revisionhash',
'ArcanistDifferentialRevisionRef' => 'differential/revision',
'ArcanistDifferentialRevisionStatus' => 'differential/constants/revisionstatus',
'ArcanistDownloadWorkflow' => 'workflow/download',
'ArcanistEventType' => 'events/constant/type',
'ArcanistExportWorkflow' => 'workflow/export',

View file

@ -0,0 +1,35 @@
<?php
/*
* Copyright 2012 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 ArcanistDifferentialRevisionHash {
const TABLE_NAME = 'differential_revisionhash';
const HASH_GIT_COMMIT = 'gtcm';
const HASH_GIT_TREE = 'gttr';
const HASH_MERCURIAL_COMMIT = 'hgcm';
public static function getTypes() {
return array(
ArcanistDifferentialRevisionHash::HASH_GIT_COMMIT,
ArcanistDifferentialRevisionHash::HASH_GIT_TREE,
ArcanistDifferentialRevisionHash::HASH_MERCURIAL_COMMIT,
);
}
}

View file

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

View file

@ -0,0 +1,39 @@
<?php
/*
* Copyright 2012 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 ArcanistDifferentialRevisionStatus {
const NEEDS_REVIEW = 0;
const NEEDS_REVISION = 1;
const ACCEPTED = 2;
const COMMITTED = 3;
const ABANDONED = 4;
public static function getNameForRevisionStatus($status) {
static $map = array(
self::NEEDS_REVIEW => 'Needs Review',
self::NEEDS_REVISION => 'Needs Revision',
self::ACCEPTED => 'Accepted',
self::COMMITTED => 'Committed',
self::ABANDONED => 'Abandoned',
);
return idx($map, coalesce($status, '?'), 'Unknown');
}
}

View file

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

View file

@ -28,6 +28,7 @@ class ArcanistBundle {
private $blobs = array();
private $diskPath;
private $projectID;
private $baseRevision;
public function setConduit(ConduitClient $conduit) {
$this->conduit = $conduit;
@ -41,6 +42,14 @@ class ArcanistBundle {
return $this->projectID;
}
public function setBaseRevision($base_revision) {
$this->baseRevision = $base_revision;
}
public function getBaseRevision() {
return $this->baseRevision;
}
public static function newFromChanges(array $changes) {
$obj = new ArcanistBundle();
$obj->changes = $changes;
@ -65,10 +74,12 @@ class ArcanistBundle {
$meta_info = $future->resolveJSON();
$version = idx($meta_info, 'version', 0);
$project_name = idx($meta_info, 'projectName');
$base_revision = idx($meta_info, 'baseRevision');
// this arc bundle was probably made before we started storing meta info
} else {
$version = 0;
$project_name = null;
$base_revision = null;
}
$future = new ExecFuture(
@ -92,6 +103,7 @@ class ArcanistBundle {
$obj->changes = $changes;
$obj->diskPath = $path;
$obj->setProjectID($project_name);
$obj->setBaseRevision($base_revision);
return $obj;
}
@ -138,8 +150,11 @@ class ArcanistBundle {
$blobs[$phid] = $this->getBlob($phid);
}
$meta_info = array('version' => 1,
'projectName' => $this->getProjectID());
$meta_info = array(
'version' => 2,
'projectName' => $this->getProjectID(),
'baseRevision' => $this->getBaseRevision(),
);
$dir = Filesystem::createTemporaryDirectory();
Filesystem::createDirectory($dir.'/hunks');

View file

@ -1,7 +1,7 @@
<?php
/*
* Copyright 2011 Facebook, Inc.
* Copyright 2012 Facebook, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -151,9 +151,9 @@ abstract class ArcanistRepositoryAPI {
abstract public function getOriginalFileData($path);
abstract public function getCurrentFileData($path);
abstract public function getLocalCommitInformation();
abstract public function getSourceControlBaseRevision();
abstract public function supportsRelativeLocalCommits();
abstract public function getWorkingCopyRevision();
public function parseRelativeLocalCommit(array $argv) {
throw new ArcanistCapabilityNotSupportedException($this);

View file

@ -1,7 +1,7 @@
<?php
/*
* Copyright 2011 Facebook, Inc.
* Copyright 2012 Facebook, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -544,6 +544,14 @@ class ArcanistGitAPI extends ArcanistRepositoryAPI {
return trim($owner);
}
public function getWorkingCopyRevision() {
list($stdout) = execx(
'(cd %s; git rev-parse %s)',
$this->getPath(),
'HEAD');
return rtrim($stdout, "\n");
}
public function supportsRelativeLocalCommits() {
return true;
}

View file

@ -304,7 +304,7 @@ class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
}
}
private function getWorkingCopyRevision() {
public function getWorkingCopyRevision() {
// In Mercurial, "tip" means the tip of the current branch, not what's in
// the working copy. The tip may be ahead of the working copy. We need to
// use "hg summary" to figure out what is actually in the working copy.

View file

@ -1,7 +1,7 @@
<?php
/*
* Copyright 2011 Facebook, Inc.
* Copyright 2012 Facebook, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -492,6 +492,10 @@ EODIFF;
return false;
}
public function getWorkingCopyRevision() {
return $this->getSourceControlBaseRevision();
}
public function supportsLocalBranchMerge() {
return false;
}

View file

@ -752,6 +752,7 @@ class ArcanistBaseWorkflow {
$bundle = ArcanistBundle::newFromChanges($changes);
$bundle->setConduit($conduit);
$bundle->setProjectID($diff['projectName']);
$bundle->setBaseRevision($diff['sourceControlBaseRevision']);
return $bundle;
}

View file

@ -1,7 +1,7 @@
<?php
/*
* Copyright 2011 Facebook, Inc.
* Copyright 2012 Facebook, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -187,6 +187,8 @@ EOTEXT
$bundle = ArcanistBundle::newFromChanges($changes);
$bundle->setProjectID($this->getWorkingCopy()->getProjectID());
$bundle->setBaseRevision(
$repository_api->getSourceControlBaseRevision());
break;
case self::SOURCE_REVISION:
$bundle = $this->loadRevisionBundleFromConduit(

View file

@ -1,7 +1,7 @@
<?php
/*
* Copyright 2011 Facebook, Inc.
* Copyright 2012 Facebook, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -135,8 +135,7 @@ EOTEXT
}
public function requiresConduit() {
return ($this->getSource() == self::SOURCE_REVISION) ||
($this->getSource() == self::SOURCE_DIFF);
return ($this->getSource() != self::SOURCE_PATCH);
}
public function requiresRepositoryAPI() {
@ -428,12 +427,12 @@ EOTEXT
*/
private function sanityCheckPatch(ArcanistBundle $bundle) {
// Check to see if the bundle project id matches the working copy
// Check to see if the bundle's project id matches the working copy
// project id
$bundle_project_id = $bundle->getProjectID();
$working_copy_project_id = $this->getWorkingCopy()->getProjectID();
if (empty($bundle_project_id)) {
// this means $source is SOURCE_PATCH || SOURCE_BUNDLE
// this means $source is SOURCE_PATCH || SOURCE_BUNDLE w/ $version = 0
// they don't come with a project id so just do nothing
} else if ($bundle_project_id != $working_copy_project_id) {
$ok = phutil_console_confirm(
@ -447,6 +446,69 @@ EOTEXT
}
}
// Check to see if the bundle's base revision matches the working copy
// base revision
$bundle_base_rev = $bundle->getBaseRevision();
if (empty($bundle_base_rev)) {
// this means $source is SOURCE_PATCH || SOURCE_BUNDLE w/ $version < 2
// they don't have a base rev so just do nothing
} else {
$repository_api = $this->getRepositoryAPI();
$source_base_rev = $repository_api->getWorkingCopyRevision();
if ($source_base_rev != $bundle_base_rev) {
// we have a problem...! lots of work because we need to ask
// differential for revision information for these base revisions
// to improve our error message.
$bundle_base_rev_str = null;
$source_base_rev_str = null;
// SVN doesn't store these hashes, so we're basically done already
// and will have a relatively "lame" error message
if ($repository_api instanceof ArcanistSubversionAPI) {
$hash_type = null;
} else if ($repository_api instanceof ArcanistGitAPI) {
$hash_type = ArcanistDifferentialRevisionHash::HASH_GIT_COMMIT;
} else if ($repository_api instanceof ArcanistMercurialAPI) {
$hash_type = ArcanistDifferentialRevisionHash::HASH_MERCURIAL_COMMIT;
} else {
$hash_type = null;
}
if ($hash_type) {
// 2 round trips because even though we could send off one query
// we wouldn't be able to tell which revisions were for which hash
$hash = array($hash_type, $bundle_base_rev);
$bundle_revision = $this->loadRevisionFromHash($hash);
$hash = array($hash_type, $source_base_rev);
$source_revision = $this->loadRevisionFromHash($hash);
if ($bundle_revision) {
$bundle_base_rev_str = $bundle_base_rev .
' \ D' . $bundle_revision['id'];
}
if ($source_revision) {
$source_base_rev_str = $source_base_rev .
' \ D' . $source_revision['id'];
}
}
$bundle_base_rev_str = nonempty($bundle_base_rev_str,
$bundle_base_rev);
$source_base_rev_str = nonempty($source_base_rev_str,
$source_base_rev);
$ok = phutil_console_confirm(
"This diff is against commit {$bundle_base_rev_str}, but the ".
"working copy is at {$source_base_rev_str}. ".
"Still try to apply it?",
$default_no = false
);
if (!$ok) {
throw new ArcanistUserAbortException();
}
}
}
// TODO -- more sanity checks here
}
@ -473,4 +535,27 @@ EOTEXT
$dir));
}
}
private function loadRevisionFromHash($hash) {
$conduit = $this->getConduit();
$revisions = $conduit->callMethodSynchronous(
'differential.query',
array(
'commitHashes' => array($hash),
)
);
// grab the latest committed revision only
$found_revision = null;
$revisions = isort($revisions, 'dateModified');
foreach ($revisions as $revision) {
if ($revision['status'] ==
ArcanistDifferentialRevisionStatus::COMMITTED) {
$found_revision = $revision;
}
}
return $found_revision;
}
}

View file

@ -6,6 +6,8 @@
phutil_require_module('arcanist', 'differential/constants/revisionhash');
phutil_require_module('arcanist', 'differential/constants/revisionstatus');
phutil_require_module('arcanist', 'exception/usage');
phutil_require_module('arcanist', 'exception/usage/userabort');
phutil_require_module('arcanist', 'parser/bundle');