mirror of
https://we.phorge.it/source/arcanist.git
synced 2024-11-22 23:02:41 +01:00
When arc
pushes to the staging area, tell Phabricator what we did
Summary: Ref T10093. Right now, Phabricator kind of guesses that `arc` probably pushed stuff to the staging area. This can cause confusing/misleading errors later, if it didn't actually push. Instead, tell Phabricator that we pushed, so we can raise more tailored messages in the web UI (e.g., make "Land Revision" say "this wasn't pushed to the staging area" instead of "whoops, error!!~"). Test Plan: Ran `arc diff` a few times, then looked in the database for properties. {F1161655} Reviewers: chad Reviewed By: chad Maniphest Tasks: T10093 Differential Revision: https://secure.phabricator.com/D15426
This commit is contained in:
parent
4a1160e0c3
commit
ccbaee585e
1 changed files with 67 additions and 13 deletions
|
@ -22,6 +22,15 @@ final class ArcanistDiffWorkflow extends ArcanistWorkflow {
|
||||||
private $commitMessageFromRevision;
|
private $commitMessageFromRevision;
|
||||||
private $hitAutotargets;
|
private $hitAutotargets;
|
||||||
|
|
||||||
|
const STAGING_PUSHED = 'pushed';
|
||||||
|
const STAGING_USER_SKIP = 'user.skip';
|
||||||
|
const STAGING_DIFF_RAW = 'diff.raw';
|
||||||
|
const STAGING_REPOSITORY_UNKNOWN = 'repository.unknown';
|
||||||
|
const STAGING_REPOSITORY_UNAVAILABLE = 'repository.unavailable';
|
||||||
|
const STAGING_REPOSITORY_UNSUPPORTED = 'repository.unsupported';
|
||||||
|
const STAGING_REPOSITORY_UNCONFIGURED = 'repository.unconfigured';
|
||||||
|
const STAGING_CLIENT_UNSUPPORTED = 'client.unsupported';
|
||||||
|
|
||||||
public function getWorkflowName() {
|
public function getWorkflowName() {
|
||||||
return 'diff';
|
return 'diff';
|
||||||
}
|
}
|
||||||
|
@ -519,7 +528,7 @@ EOTEXT
|
||||||
'unitResult' => $unit_result,
|
'unitResult' => $unit_result,
|
||||||
));
|
));
|
||||||
|
|
||||||
$this->pushChangesToStagingArea($this->diffID);
|
$this->submitChangesToStagingArea($this->diffID);
|
||||||
|
|
||||||
$phid = idx($diff_info, 'phid');
|
$phid = idx($diff_info, 'phid');
|
||||||
if ($phid) {
|
if ($phid) {
|
||||||
|
@ -2657,26 +2666,50 @@ EOTEXT
|
||||||
return $this->getArgument('browse');
|
return $this->getArgument('browse');
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private function submitChangesToStagingArea($id) {
|
||||||
|
$result = $this->pushChangesToStagingArea($id);
|
||||||
|
|
||||||
|
// We'll either get a failure constant on error, or a list of pushed
|
||||||
|
// refs on success.
|
||||||
|
$ok = is_array($result);
|
||||||
|
|
||||||
|
if ($ok) {
|
||||||
|
$staging = array(
|
||||||
|
'status' => self::STAGING_PUSHED,
|
||||||
|
'refs' => $result,
|
||||||
|
);
|
||||||
|
} else {
|
||||||
|
$staging = array(
|
||||||
|
'status' => $result,
|
||||||
|
'refs' => array(),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
$this->updateDiffProperty(
|
||||||
|
'arc.staging',
|
||||||
|
phutil_json_encode($staging));
|
||||||
|
}
|
||||||
|
|
||||||
private function pushChangesToStagingArea($id) {
|
private function pushChangesToStagingArea($id) {
|
||||||
if ($this->getArgument('skip-staging')) {
|
if ($this->getArgument('skip-staging')) {
|
||||||
$this->writeInfo(
|
$this->writeInfo(
|
||||||
pht('SKIP STAGING'),
|
pht('SKIP STAGING'),
|
||||||
pht('Flag --skip-staging was specified.'));
|
pht('Flag --skip-staging was specified.'));
|
||||||
return;
|
return self::STAGING_USER_SKIP;
|
||||||
}
|
}
|
||||||
|
|
||||||
if ($this->isRawDiffSource()) {
|
if ($this->isRawDiffSource()) {
|
||||||
$this->writeInfo(
|
$this->writeInfo(
|
||||||
pht('SKIP STAGING'),
|
pht('SKIP STAGING'),
|
||||||
pht('Raw changes can not be pushed to a staging area.'));
|
pht('Raw changes can not be pushed to a staging area.'));
|
||||||
return;
|
return self::STAGING_DIFF_RAW;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (!$this->getRepositoryPHID()) {
|
if (!$this->getRepositoryPHID()) {
|
||||||
$this->writeInfo(
|
$this->writeInfo(
|
||||||
pht('SKIP STAGING'),
|
pht('SKIP STAGING'),
|
||||||
pht('Unable to determine repository for this change.'));
|
pht('Unable to determine repository for this change.'));
|
||||||
return;
|
return self::STAGING_REPOSITORY_UNKNOWN;
|
||||||
}
|
}
|
||||||
|
|
||||||
$staging = $this->getRepositoryStagingConfiguration();
|
$staging = $this->getRepositoryStagingConfiguration();
|
||||||
|
@ -2684,7 +2717,7 @@ EOTEXT
|
||||||
$this->writeInfo(
|
$this->writeInfo(
|
||||||
pht('SKIP STAGING'),
|
pht('SKIP STAGING'),
|
||||||
pht('The server does not support staging areas.'));
|
pht('The server does not support staging areas.'));
|
||||||
return;
|
return self::STAGING_REPOSITORY_UNAVAILABLE;
|
||||||
}
|
}
|
||||||
|
|
||||||
$supported = idx($staging, 'supported');
|
$supported = idx($staging, 'supported');
|
||||||
|
@ -2692,7 +2725,7 @@ EOTEXT
|
||||||
$this->writeInfo(
|
$this->writeInfo(
|
||||||
pht('SKIP STAGING'),
|
pht('SKIP STAGING'),
|
||||||
pht('Phabricator does not support staging areas for this repository.'));
|
pht('Phabricator does not support staging areas for this repository.'));
|
||||||
return;
|
return self::STAGING_REPOSITORY_UNSUPPORTED;
|
||||||
}
|
}
|
||||||
|
|
||||||
$staging_uri = idx($staging, 'uri');
|
$staging_uri = idx($staging, 'uri');
|
||||||
|
@ -2700,7 +2733,7 @@ EOTEXT
|
||||||
$this->writeInfo(
|
$this->writeInfo(
|
||||||
pht('SKIP STAGING'),
|
pht('SKIP STAGING'),
|
||||||
pht('No staging area is configured for this repository.'));
|
pht('No staging area is configured for this repository.'));
|
||||||
return;
|
return self::STAGING_REPOSITORY_UNCONFIGURED;
|
||||||
}
|
}
|
||||||
|
|
||||||
$api = $this->getRepositoryAPI();
|
$api = $this->getRepositoryAPI();
|
||||||
|
@ -2708,14 +2741,14 @@ EOTEXT
|
||||||
$this->writeInfo(
|
$this->writeInfo(
|
||||||
pht('SKIP STAGING'),
|
pht('SKIP STAGING'),
|
||||||
pht('This client version does not support staging this repository.'));
|
pht('This client version does not support staging this repository.'));
|
||||||
return;
|
return self::STAGING_CLIENT_UNSUPPORTED;
|
||||||
}
|
}
|
||||||
|
|
||||||
$commit = $api->getHeadCommit();
|
$commit = $api->getHeadCommit();
|
||||||
$prefix = idx($staging, 'prefix', 'phabricator');
|
$prefix = idx($staging, 'prefix', 'phabricator');
|
||||||
|
|
||||||
$base_tag = $prefix.'/base/'.$id;
|
$base_tag = "refs/tags/{$prefix}/base/{$id}";
|
||||||
$diff_tag = $prefix.'/diff/'.$id;
|
$diff_tag = "refs/tags/{$prefix}/diff/{$id}";
|
||||||
|
|
||||||
$this->writeOkay(
|
$this->writeOkay(
|
||||||
pht('PUSH STAGING'),
|
pht('PUSH STAGING'),
|
||||||
|
@ -2728,6 +2761,10 @@ EOTEXT
|
||||||
|
|
||||||
$refs = array();
|
$refs = array();
|
||||||
|
|
||||||
|
$remote = array(
|
||||||
|
'uri' => $staging_uri,
|
||||||
|
);
|
||||||
|
|
||||||
// If the base commit is a real commit, we're going to push it. We don't
|
// If the base commit is a real commit, we're going to push it. We don't
|
||||||
// use this, but pushing it to a ref reduces the amount of redundant work
|
// use this, but pushing it to a ref reduces the amount of redundant work
|
||||||
// that Git does on later pushes by helping it figure out that the remote
|
// that Git does on later pushes by helping it figure out that the remote
|
||||||
|
@ -2739,17 +2776,32 @@ EOTEXT
|
||||||
// refs.
|
// refs.
|
||||||
$base_commit = $api->getSourceControlBaseRevision();
|
$base_commit = $api->getSourceControlBaseRevision();
|
||||||
if ($base_commit !== ArcanistGitAPI::GIT_MAGIC_ROOT_COMMIT) {
|
if ($base_commit !== ArcanistGitAPI::GIT_MAGIC_ROOT_COMMIT) {
|
||||||
$refs[] = "{$base_commit}:refs/tags/{$base_tag}";
|
$refs[] = array(
|
||||||
|
'ref' => $base_tag,
|
||||||
|
'type' => 'base',
|
||||||
|
'commit' => $base_commit,
|
||||||
|
'remote' => $remote,
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
// We're always going to push the change itself.
|
// We're always going to push the change itself.
|
||||||
$refs[] = "{$commit}:refs/tags/{$diff_tag}";
|
$refs[] = array(
|
||||||
|
'ref' => $diff_tag,
|
||||||
|
'type' => 'diff',
|
||||||
|
'commit' => $commit,
|
||||||
|
'remote' => $remote,
|
||||||
|
);
|
||||||
|
|
||||||
|
$ref_list = array();
|
||||||
|
foreach ($refs as $ref) {
|
||||||
|
$ref_list[] = $ref['commit'].':'.$ref['ref'];
|
||||||
|
}
|
||||||
|
|
||||||
$err = phutil_passthru(
|
$err = phutil_passthru(
|
||||||
'git push %Ls -- %s %Ls',
|
'git push %Ls -- %s %Ls',
|
||||||
$push_flags,
|
$push_flags,
|
||||||
$staging_uri,
|
$staging_uri,
|
||||||
$refs);
|
$ref_list);
|
||||||
|
|
||||||
if ($err) {
|
if ($err) {
|
||||||
$this->writeWarn(
|
$this->writeWarn(
|
||||||
|
@ -2761,6 +2813,8 @@ EOTEXT
|
||||||
'Failed to push changes to staging area. Correct the issue, or '.
|
'Failed to push changes to staging area. Correct the issue, or '.
|
||||||
'use --skip-staging to skip this step.'));
|
'use --skip-staging to skip this step.'));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
return $refs;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue