Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F15430457
D20868.id49752.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
13 KB
Referenced Files
None
Subscribers
None
D20868.id49752.diff
View Options
diff --git a/src/land/ArcanistGitLandEngine.php b/src/land/ArcanistGitLandEngine.php
--- a/src/land/ArcanistGitLandEngine.php
+++ b/src/land/ArcanistGitLandEngine.php
@@ -8,12 +8,45 @@
private $sourceCommit;
private $mergedRef;
private $restoreWhenDestroyed;
+ private $isGitPerforce;
+
+ private function setIsGitPerforce($is_git_perforce) {
+ $this->isGitPerforce = $is_git_perforce;
+ return $this;
+ }
+
+ private function getIsGitPerforce() {
+ return $this->isGitPerforce;
+ }
public function parseArguments() {
+ $api = $this->getRepositoryAPI();
+
$onto = $this->getEngineOnto();
$this->setTargetOnto($onto);
$remote = $this->getEngineRemote();
+
+ $is_pushable = $api->isPushableRemote($remote);
+ $is_perforce = $api->isPerforceRemote($remote);
+
+ if (!$is_pushable && !$is_perforce) {
+ throw new PhutilArgumentUsageException(
+ pht(
+ 'No pushable remote "%s" exists. Use the "--remote" flag to choose '.
+ 'a valid, pushable remote to land changes onto.',
+ $remote));
+ }
+
+ if ($is_perforce) {
+ $this->setIsGitPerforce(true);
+ $this->writeWarn(
+ pht('P4 MODE'),
+ pht(
+ 'Operating in Git/Perforce mode after selecting a Perforce '.
+ 'remote.'));
+ }
+
$this->setTargetRemote($remote);
}
@@ -132,25 +165,45 @@
$ref = $this->getTargetFullRef();
- $this->writeInfo(
- pht('FETCH'),
- pht('Fetching %s...', $ref));
-
// NOTE: Although this output isn't hugely useful, we need to passthru
// instead of using a subprocess here because `git fetch` may prompt the
// user to enter a password if they're fetching over HTTP with basic
// authentication. See T10314.
- $err = $api->execPassthru(
- 'fetch --quiet -- %s %s',
- $this->getTargetRemote(),
- $this->getTargetOnto());
+ if ($this->getIsGitPerforce()) {
+ $this->writeInfo(
+ pht('P4 SYNC'),
+ pht('Synchronizing "%s" from Perforce...', $ref));
- if ($err) {
- throw new ArcanistUsageException(
- pht(
- 'Fetch failed! Fix the error and run "%s" again.',
- 'arc land'));
+ $sync_ref = sprintf(
+ 'refs/remotes/%s/%s',
+ $this->getTargetRemote(),
+ $this->getTargetOnto());
+
+ $err = $api->execPassthru(
+ 'p4 sync --silent --branch %R --',
+ $sync_ref);
+
+ if ($err) {
+ throw new ArcanistUsageException(
+ pht(
+ 'Perforce sync failed! Fix the error and run "arc land" again.'));
+ }
+ } else {
+ $this->writeInfo(
+ pht('FETCH'),
+ pht('Fetching "%s"...', $ref));
+
+ $err = $api->execPassthru(
+ 'fetch --quiet -- %s %s',
+ $this->getTargetRemote(),
+ $this->getTargetOnto());
+
+ if ($err) {
+ throw new ArcanistUsageException(
+ pht(
+ 'Fetch failed! Fix the error and run "arc land" again.'));
+ }
}
}
@@ -220,21 +273,68 @@
private function pushChange() {
$api = $this->getRepositoryAPI();
- $this->writeInfo(
- pht('PUSHING'),
- pht('Pushing changes to "%s".', $this->getTargetFullRef()));
+ if ($this->getIsGitPerforce()) {
+ $this->writeInfo(
+ pht('SUBMITTING'),
+ pht('Submitting changes to "%s".', $this->getTargetFullRef()));
- $err = $api->execPassthru(
- 'push -- %s %s:%s',
- $this->getTargetRemote(),
- $this->mergedRef,
- $this->getTargetOnto());
+ $config_argv = array();
- if ($err) {
- throw new ArcanistUsageException(
- pht(
- 'Push failed! Fix the error and run "%s" again.',
- 'arc land'));
+ // Skip the "git p4 submit" interactive editor workflow. We expect
+ // the commit message that "arc land" has built to be satisfactory.
+ $config_argv[] = '-c';
+ $config_argv[] = 'git-p4.skipSubmitEdit=true';
+
+ // Skip the "git p4 submit" confirmation prompt if the user does not edit
+ // the submit message.
+ $config_argv[] = '-c';
+ $config_argv[] = 'git-p4.skipSubmitEditCheck=true';
+
+ $flags_argv = array();
+
+ // Disable implicit "git p4 rebase" as part of submit. We're allowing
+ // the implicit "git p4 sync" to go through since this puts us in a
+ // state which is generally similar to the state after "git push", with
+ // updated remotes.
+
+ // We could do a manual "git p4 sync" with a more narrow "--branch"
+ // instead, but it's not clear that this is beneficial.
+ $flags_argv[] = '--disable-rebase';
+
+ // Detect moves and submit them to Perforce as move operations.
+ $flags_argv[] = '-M';
+
+ // If we run into a conflict, abort the operation. We expect users to
+ // fix conflicts and run "arc land" again.
+ $flags_argv[] = '--conflict=quit';
+
+ $err = $api->execPassthru(
+ '%LR p4 submit %LR --commit %s --',
+ $config_argv,
+ $flags_argv,
+ $this->mergedRef);
+
+ if ($err) {
+ throw new ArcanistUsageException(
+ pht(
+ 'Submit failed! Fix the error and run "arc land" again.'));
+ }
+ } else {
+ $this->writeInfo(
+ pht('PUSHING'),
+ pht('Pushing changes to "%s".', $this->getTargetFullRef()));
+
+ $err = $api->execPassthru(
+ 'push -- %s %s:%s',
+ $this->getTargetRemote(),
+ $this->mergedRef,
+ $this->getTargetOnto());
+
+ if ($err) {
+ throw new ArcanistUsageException(
+ pht(
+ 'Push failed! Fix the error and run "arc land" again.'));
+ }
}
}
@@ -340,39 +440,53 @@
return;
}
- if (!$path->isConnectedToRemote()) {
- $this->writeInfo(
- pht('UPDATE'),
- pht(
- 'Local branch "%s" is not connected to a remote, staying on '.
- 'detached HEAD.',
- $local_branch));
- return;
- }
+ $is_perforce = $this->getIsGitPerforce();
- $remote_remote = $path->getRemoteRemoteName();
- $remote_branch = $path->getRemoteBranchName();
+ if ($is_perforce) {
+ // If we're in Perforce mode, we don't expect to have a meaningful
+ // path to the remote: the "p4" remote is not a real remote, and
+ // "git p4" commands do not configure branch upstreams to provide
+ // a path.
- $remote_actual = $remote_remote.'/'.$remote_branch;
- $remote_expect = $this->getTargetFullRef();
- if ($remote_actual != $remote_expect) {
- $this->writeInfo(
- pht('UPDATE'),
- pht(
- 'Local branch "%s" is connected to a remote ("%s") other than '.
- 'the target remote ("%s"), staying on detached HEAD.',
- $local_branch,
- $remote_actual,
- $remote_expect));
- return;
- }
+ // Just pretend the target branch is connected directly to the remote,
+ // since this is effectively the behavior of Perforce and appears to
+ // do the right thing.
+ $cascade_branches = array($local_branch);
+ } else {
+ if (!$path->isConnectedToRemote()) {
+ $this->writeInfo(
+ pht('UPDATE'),
+ pht(
+ 'Local branch "%s" is not connected to a remote, staying on '.
+ 'detached HEAD.',
+ $local_branch));
+ return;
+ }
+
+ $remote_remote = $path->getRemoteRemoteName();
+ $remote_branch = $path->getRemoteBranchName();
- // If we get this far, we have a sequence of branches which ultimately
- // connect to the remote. We're going to try to update them all in reverse
- // order, from most-upstream to most-local.
+ $remote_actual = $remote_remote.'/'.$remote_branch;
+ $remote_expect = $this->getTargetFullRef();
+ if ($remote_actual != $remote_expect) {
+ $this->writeInfo(
+ pht('UPDATE'),
+ pht(
+ 'Local branch "%s" is connected to a remote ("%s") other than '.
+ 'the target remote ("%s"), staying on detached HEAD.',
+ $local_branch,
+ $remote_actual,
+ $remote_expect));
+ return;
+ }
+
+ // If we get this far, we have a sequence of branches which ultimately
+ // connect to the remote. We're going to try to update them all in reverse
+ // order, from most-upstream to most-local.
- $cascade_branches = $path->getLocalBranches();
- $cascade_branches = array_reverse($cascade_branches);
+ $cascade_branches = $path->getLocalBranches();
+ $cascade_branches = array_reverse($cascade_branches);
+ }
// First, check if any of them are ahead of the remote.
@@ -436,7 +550,12 @@
}
}
- if ($cascade_targets) {
+ if ($is_perforce) {
+ // In Perforce, we've already set the remote to the right state with an
+ // implicit "git p4 sync" during "git p4 submit", and "git pull" isn't a
+ // meaningful operation. We're going to skip this step and jump down to
+ // the "git reset --hard" below to get everything into the right state.
+ } else if ($cascade_targets) {
$this->writeInfo(
pht('UPDATE'),
pht(
@@ -453,7 +572,10 @@
$cascade_branch));
$api->execxLocal('checkout %s --', $cascade_branch);
- $api->execxLocal('pull --');
+ $api->execxLocal(
+ 'pull %s %s --',
+ $this->getTargetRemote(),
+ $cascade_branch);
}
if (!$local_ahead) {
@@ -577,16 +699,27 @@
}
private function didHoldChanges() {
- $this->writeInfo(
- pht('HOLD'),
- pht(
- 'Holding change locally, it has not been pushed.'));
+ if ($this->getIsGitPerforce()) {
+ $this->writeInfo(
+ pht('HOLD'),
+ pht(
+ 'Holding change locally, it has not been submitted.'));
- $push_command = csprintf(
- '$ git push -- %R %R:%R',
- $this->getTargetRemote(),
- $this->mergedRef,
- $this->getTargetOnto());
+ $push_command = csprintf(
+ '$ git p4 submit -M --commit %R --',
+ $this->mergedRef);
+ } else {
+ $this->writeInfo(
+ pht('HOLD'),
+ pht(
+ 'Holding change locally, it has not been pushed.'));
+
+ $push_command = csprintf(
+ '$ git push -- %R %R:%R',
+ $this->getTargetRemote(),
+ $this->mergedRef,
+ $this->getTargetOnto());
+ }
$restore_command = csprintf(
'$ git checkout %R --',
@@ -595,9 +728,9 @@
echo tsprintf(
"\n%s\n\n".
"%s\n\n".
- " %s\n\n".
+ " **%s**\n\n".
"%s\n\n".
- " %s\n\n".
+ " **%s**\n\n".
"%s\n",
pht(
'This local working copy now contains the merged changes in a '.
@@ -605,7 +738,7 @@
pht('You can push the changes manually with this command:'),
$push_command,
pht(
- 'You can go back to how things were before you ran `arc land` with '.
+ 'You can go back to how things were before you ran "arc land" with '.
'this command:'),
$restore_command,
pht(
@@ -723,11 +856,22 @@
return $remote;
}
+ $remote = 'p4';
+ if ($api->isPerforceRemote($remote)) {
+ $this->writeInfo(
+ pht('REMOTE'),
+ pht(
+ 'Using Perforce remote "%s". The existence of this remote implies '.
+ 'this working copy was synchronized from a Perforce repository.',
+ $remote));
+ return $remote;
+ }
+
$remote = 'origin';
$this->writeInfo(
pht('REMOTE'),
pht(
- 'Using remote "%s", the default remote under git.',
+ 'Using remote "%s", the default remote under Git.',
$remote));
return $remote;
diff --git a/src/repository/api/ArcanistGitAPI.php b/src/repository/api/ArcanistGitAPI.php
--- a/src/repository/api/ArcanistGitAPI.php
+++ b/src/repository/api/ArcanistGitAPI.php
@@ -1550,4 +1550,31 @@
return $path;
}
+ public function isPerforceRemote($remote_name) {
+ // See T13434. In Perforce workflows, "git p4 clone" creates "p4" refs
+ // under "refs/remotes/", but does not define a real remote named "p4".
+
+ // We treat this remote as though it were a real remote during "arc land",
+ // but it does not respond to commands like "git remote show p4", so we
+ // need to handle it specially.
+
+ if ($remote_name !== 'p4') {
+ return false;
+ }
+
+ $remote_dir = $this->getMetadataPath().'/refs/remotes/p4';
+ if (!Filesystem::pathExists($remote_dir)) {
+ return false;
+ }
+
+ return true;
+ }
+
+ public function isPushableRemote($remote_name) {
+ list($err, $stdout) = $this->execManualLocal(
+ 'remote get-url --push -- %s',
+ $remote_name);
+ return !$err;
+ }
+
}
diff --git a/src/workflow/ArcanistLandWorkflow.php b/src/workflow/ArcanistLandWorkflow.php
--- a/src/workflow/ArcanistLandWorkflow.php
+++ b/src/workflow/ArcanistLandWorkflow.php
@@ -245,6 +245,14 @@
// could probably be structured more cleanly.
$engine->parseArguments();
+
+ // This must be configured or we fail later inside "buildEngineMessage()".
+ // This is less than ideal.
+ $this->ontoRemoteBranch = sprintf(
+ '%s/%s',
+ $engine->getTargetRemote(),
+ $engine->getTargetOnto());
+
$this->requireCleanWorkingCopy();
$engine->execute();
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Tue, Mar 25, 7:46 AM (4 w, 1 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7706357
Default Alt Text
D20868.id49752.diff (13 KB)
Attached To
Mode
D20868: Support Perforce/Git repositories in "arc land"
Attached
Detach File
Event Timeline
Log In to Comment