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();