Hello community, here is the log from the commit of package arcanist for openSUSE:Factory checked in at 2020-01-13 22:22:35 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ Comparing /work/SRC/openSUSE:Factory/arcanist (Old) and /work/SRC/openSUSE:Factory/.arcanist.new.6675 (New) ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Package is "arcanist" Mon Jan 13 22:22:35 2020 rev:3 rq:764032 version:0.0~git.20191118T203151~cc850163 Changes: -------- --- /work/SRC/openSUSE:Factory/arcanist/arcanist.changes 2019-11-06 13:52:47.343940690 +0100 +++ /work/SRC/openSUSE:Factory/.arcanist.new.6675/arcanist.changes 2020-01-13 22:22:38.526549336 +0100 @@ -1,0 +2,20 @@ +Mon Jan 13 12:56:00 UTC 2020 - i...@guoyunhe.me + +- Update to version 0.0~git.20191118T203151~cc850163: + * When "arc close-revision --finalize ..." skips closing a revision, print a message + * When generating diffs in "arc diff", disable Git config option "diff.suppressBlankEmpty" + * Make "arc land --merge" an explicit error when targeting a Perforce remote + * In "arc land", when "remote/onto" does not exist locally, try to fetch it before giving up + * Update "arc help land" to reference Perforce support + * Support Perforce/Git repositories in "arc land" + * Move Git-specific "arc land" parsing of "--onto" and "--remote" into GitLandEngine + * Add a lint check for deprecated argument order to "implode()" + * When running "arc land" from a detached HEAD, don't try to delete the source ref + * Fix two "msort()" vs "msortv()" issues in "arc land" + +------------------------------------------------------------------- +Mon Jan 13 12:52:01 UTC 2020 - Yunhe Guo <i...@guoyunhe.me> + +- Change LICENSE to %license section and update template + +------------------------------------------------------------------- Old: ---- arcanist-0.0~git.20190905T053100~3cdfe1ff.obscpio arcanist-0.0~git.20190905T053100~3cdfe1ff.tar.xz arcanist.obsinfo New: ---- _servicedata arcanist-0.0~git.20191118T203151~cc850163.tar.xz ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ Other differences: ------------------ ++++++ arcanist.spec ++++++ --- /var/tmp/diff_new_pack.2Q5Qbv/_old 2020-01-13 22:22:39.670549867 +0100 +++ /var/tmp/diff_new_pack.2Q5Qbv/_new 2020-01-13 22:22:39.674549869 +0100 @@ -1,7 +1,7 @@ # # spec file for package arcanist # -# Copyright (c) 2019 SUSE LINUX GmbH, Nuernberg, Germany. +# Copyright (c) 2020 SUSE LLC # # All modifications and additions to the file contributed by third parties # remain the property of their copyright owners, unless otherwise agreed @@ -17,12 +17,12 @@ Name: arcanist -Version: 0.0~git.20190905T053100~3cdfe1ff +Version: 0.0~git.20191118T203151~cc850163 Release: 0 Summary: Command-line interface to Phabricator License: Apache-2.0 -Url: https://secure.phabricator.com/diffusion/ARC/ +URL: https://secure.phabricator.com/diffusion/ARC/ Source0: %{name}-%{version}.tar.xz Patch0: remove-arc-upgrade.patch @@ -84,7 +84,8 @@ %files %defattr(-,root,root,-) -%doc LICENSE NOTICE README.md +%doc NOTICE README.md +%license LICENSE %{_bindir}/arc %dir %{_datadir}/bash-completion %dir %{_datadir}/bash-completion/completions ++++++ _service ++++++ --- /var/tmp/diff_new_pack.2Q5Qbv/_old 2020-01-13 22:22:39.694549878 +0100 +++ /var/tmp/diff_new_pack.2Q5Qbv/_new 2020-01-13 22:22:39.694549878 +0100 @@ -1,15 +1,16 @@ <services> - <service mode="disabled" name="tar_scm"> + <service mode="localonly" name="tar_scm"> <param name="scm">git</param> <param name="url">https://github.com/phacility/arcanist</param> <param name="revision">master</param> <param name="versionformat">0~git.%ci~%h</param> <param name="versionprefix">0</param> <param name="filename">arcanist</param> + <param name="changesgenerate">enable</param> </service> - <service mode="disabled" name="recompress"> + <service mode="localonly" name="recompress"> <param name="file">*.tar</param> <param name="compression">xz</param> </service> - <service mode="disabled" name="set_version" /> + <service mode="localonly" name="set_version" /> </services> ++++++ _servicedata ++++++ <servicedata> <service name="tar_scm"> <param name="url">https://github.com/phacility/arcanist</param> <param name="changesrevision">cc850163f30c4697e925df0d6212469679600a2c</param></service></servicedata>++++++ arcanist-0.0~git.20190905T053100~3cdfe1ff.tar.xz -> arcanist-0.0~git.20191118T203151~cc850163.tar.xz ++++++ diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/arcanist-0.0~git.20190905T053100~3cdfe1ff/src/__phutil_library_map__.php new/arcanist-0.0~git.20191118T203151~cc850163/src/__phutil_library_map__.php --- old/arcanist-0.0~git.20190905T053100~3cdfe1ff/src/__phutil_library_map__.php 2019-09-05 14:31:00.000000000 +0200 +++ new/arcanist-0.0~git.20191118T203151~cc850163/src/__phutil_library_map__.php 2019-11-19 05:31:51.000000000 +0100 @@ -185,6 +185,8 @@ 'ArcanistImplicitFallthroughXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistImplicitFallthroughXHPASTLinterRuleTestCase.php', 'ArcanistImplicitVisibilityXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistImplicitVisibilityXHPASTLinterRule.php', 'ArcanistImplicitVisibilityXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistImplicitVisibilityXHPASTLinterRuleTestCase.php', + 'ArcanistImplodeArgumentOrderXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistImplodeArgumentOrderXHPASTLinterRule.php', + 'ArcanistImplodeArgumentOrderXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistImplodeArgumentOrderXHPASTLinterRuleTestCase.php', 'ArcanistInlineHTMLXHPASTLinterRule' => 'lint/linter/ArcanistInlineHTMLXHPASTLinterRule.php', 'ArcanistInlineHTMLXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistInlineHTMLXHPASTLinterRuleTestCase.php', 'ArcanistInnerFunctionXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistInnerFunctionXHPASTLinterRule.php', @@ -608,6 +610,8 @@ 'ArcanistImplicitFallthroughXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistImplicitVisibilityXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistImplicitVisibilityXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', + 'ArcanistImplodeArgumentOrderXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', + 'ArcanistImplodeArgumentOrderXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistInlineHTMLXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistInlineHTMLXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistInnerFunctionXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/arcanist-0.0~git.20190905T053100~3cdfe1ff/src/land/ArcanistGitLandEngine.php new/arcanist-0.0~git.20191118T203151~cc850163/src/land/ArcanistGitLandEngine.php --- old/arcanist-0.0~git.20190905T053100~3cdfe1ff/src/land/ArcanistGitLandEngine.php 2019-09-05 14:31:00.000000000 +0200 +++ new/arcanist-0.0~git.20191118T203151~cc850163/src/land/ArcanistGitLandEngine.php 2019-11-19 05:31:51.000000000 +0100 @@ -8,6 +8,55 @@ 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.')); + + if (!$this->getShouldSquash()) { + throw new PhutilArgumentUsageException( + pht( + 'Perforce mode does not support the "merge" land strategy. '. + 'Use the "squash" land strategy when landing to a Perforce '. + 'remote (you can use "--squash" to select this strategy).')); + } + } + + $this->setTargetRemote($remote); + } public function execute() { $this->verifySourceAndTargetExist(); @@ -98,9 +147,34 @@ $this->getTargetFullRef()); if ($err) { - throw new Exception( + $this->writeWarn( + pht('TARGET'), pht( - 'Branch "%s" does not exist in remote "%s".', + 'No local ref exists for branch "%s" in remote "%s", attempting '. + 'fetch...', + $this->getTargetOnto(), + $this->getTargetRemote())); + + $api->execManualLocal( + 'fetch %s %s --', + $this->getTargetRemote(), + $this->getTargetOnto()); + + list($err) = $api->execManualLocal( + 'rev-parse --verify %s', + $this->getTargetFullRef()); + if ($err) { + throw new Exception( + pht( + 'Branch "%s" does not exist in remote "%s".', + $this->getTargetOnto(), + $this->getTargetRemote())); + } + + $this->writeInfo( + pht('FETCHED'), + pht( + 'Fetched branch "%s" from remote "%s".', $this->getTargetOnto(), $this->getTargetRemote())); } @@ -124,25 +198,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.')); + } } } @@ -212,21 +306,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 %R --', + $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.')); + } } } @@ -332,39 +473,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. + + // 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_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; - } + $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; + } - $cascade_branches = $path->getLocalBranches(); - $cascade_branches = array_reverse($cascade_branches); + // 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); + } // First, check if any of them are ahead of the remote. @@ -428,7 +583,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( @@ -445,7 +605,10 @@ $cascade_branch)); $api->execxLocal('checkout %s --', $cascade_branch); - $api->execxLocal('pull --'); + $api->execxLocal( + 'pull %s %s --', + $this->getTargetRemote(), + $cascade_branch); } if (!$local_ahead) { @@ -569,16 +732,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 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()); + $push_command = csprintf( + '$ git push -- %R %R:%R', + $this->getTargetRemote(), + $this->mergedRef, + $this->getTargetOnto()); + } $restore_command = csprintf( '$ git checkout %R --', @@ -587,9 +761,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 '. @@ -597,7 +771,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( @@ -615,4 +789,125 @@ return !$err; } + private function getEngineOnto() { + $source_ref = $this->getSourceRef(); + + $onto = $this->getOntoArgument(); + if ($onto !== null) { + $this->writeInfo( + pht('TARGET'), + pht( + 'Landing onto "%s", selected with the "--onto" flag.', + $onto)); + return $onto; + } + + $api = $this->getRepositoryAPI(); + $path = $api->getPathToUpstream($source_ref); + + if ($path->getLength()) { + $cycle = $path->getCycle(); + if ($cycle) { + $this->writeWarn( + pht('LOCAL CYCLE'), + pht( + 'Local branch tracks an upstream, but following it leads to a '. + 'local cycle; ignoring branch upstream.')); + + echo tsprintf( + "\n %s\n\n", + implode(' -> ', $cycle)); + + } else { + if ($path->isConnectedToRemote()) { + $onto = $path->getRemoteBranchName(); + $this->writeInfo( + pht('TARGET'), + pht( + 'Landing onto "%s", selected by following tracking branches '. + 'upstream to the closest remote.', + $onto)); + return $onto; + } else { + $this->writeInfo( + pht('NO PATH TO UPSTREAM'), + pht( + 'Local branch tracks an upstream, but there is no path '. + 'to a remote; ignoring branch upstream.')); + } + } + } + + $workflow = $this->getWorkflow(); + + $config_key = 'arc.land.onto.default'; + $onto = $workflow->getConfigFromAnySource($config_key); + if ($onto !== null) { + $this->writeInfo( + pht('TARGET'), + pht( + 'Landing onto "%s", selected by "%s" configuration.', + $onto, + $config_key)); + return $onto; + } + + $onto = 'master'; + $this->writeInfo( + pht('TARGET'), + pht( + 'Landing onto "%s", the default target under git.', + $onto)); + + return $onto; + } + + private function getEngineRemote() { + $source_ref = $this->getSourceRef(); + + $remote = $this->getRemoteArgument(); + if ($remote !== null) { + $this->writeInfo( + pht('REMOTE'), + pht( + 'Using remote "%s", selected with the "--remote" flag.', + $remote)); + return $remote; + } + + $api = $this->getRepositoryAPI(); + $path = $api->getPathToUpstream($source_ref); + + $remote = $path->getRemoteRemoteName(); + if ($remote !== null) { + $this->writeInfo( + pht('REMOTE'), + pht( + 'Using remote "%s", selected by following tracking branches '. + 'upstream to the closest remote.', + $remote)); + 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.', + $remote)); + + return $remote; + } + } diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/arcanist-0.0~git.20190905T053100~3cdfe1ff/src/land/ArcanistLandEngine.php new/arcanist-0.0~git.20191118T203151~cc850163/src/land/ArcanistLandEngine.php --- old/arcanist-0.0~git.20190905T053100~3cdfe1ff/src/land/ArcanistLandEngine.php 2019-09-05 14:31:00.000000000 +0200 +++ new/arcanist-0.0~git.20191118T203151~cc850163/src/land/ArcanistLandEngine.php 2019-11-19 05:31:51.000000000 +0100 @@ -13,6 +13,8 @@ private $shouldSquash; private $shouldDeleteRemote; private $shouldPreview; + private $remoteArgument; + private $ontoArgument; // TODO: This is really grotesque. private $buildMessageCallback; @@ -117,6 +119,25 @@ return $this->commitMessageFile; } + final public function setRemoteArgument($remote_argument) { + $this->remoteArgument = $remote_argument; + return $this; + } + + final public function getRemoteArgument() { + return $this->remoteArgument; + } + + final public function setOntoArgument($onto_argument) { + $this->ontoArgument = $onto_argument; + return $this; + } + + final public function getOntoArgument() { + return $this->ontoArgument; + } + + abstract public function parseArguments(); abstract public function execute(); abstract protected function getLandingCommits(); diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/arcanist-0.0~git.20190905T053100~3cdfe1ff/src/lint/linter/xhpast/rules/ArcanistImplodeArgumentOrderXHPASTLinterRule.php new/arcanist-0.0~git.20191118T203151~cc850163/src/lint/linter/xhpast/rules/ArcanistImplodeArgumentOrderXHPASTLinterRule.php --- old/arcanist-0.0~git.20190905T053100~3cdfe1ff/src/lint/linter/xhpast/rules/ArcanistImplodeArgumentOrderXHPASTLinterRule.php 1970-01-01 01:00:00.000000000 +0100 +++ new/arcanist-0.0~git.20191118T203151~cc850163/src/lint/linter/xhpast/rules/ArcanistImplodeArgumentOrderXHPASTLinterRule.php 2019-11-19 05:31:51.000000000 +0100 @@ -0,0 +1,39 @@ +<?php + +final class ArcanistImplodeArgumentOrderXHPASTLinterRule + extends ArcanistXHPASTLinterRule { + + const ID = 129; + + public function getLintName() { + return pht('Implode With Glue First'); + } + + public function getLintSeverity() { + return ArcanistLintSeverity::SEVERITY_ERROR; + } + + public function process(XHPASTNode $root) { + $implosions = $this->getFunctionCalls($root, array('implode')); + foreach ($implosions as $implosion) { + $parameters = $implosion->getChildOfType(1, 'n_CALL_PARAMETER_LIST'); + + if (count($parameters->getChildren()) != 2) { + continue; + } + + $parameter = $parameters->getChildByIndex(1); + if (!$parameter->isStaticScalar()) { + continue; + } + + $this->raiseLintAtNode( + $implosion, + pht( + 'When calling "implode()", pass the "glue" argument first. (The '. + 'other parameter order is deprecated in PHP 7.4 and raises a '. + 'warning.)')); + } + } + +} diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/arcanist-0.0~git.20190905T053100~3cdfe1ff/src/lint/linter/xhpast/rules/__tests__/ArcanistImplodeArgumentOrderXHPASTLinterRuleTestCase.php new/arcanist-0.0~git.20191118T203151~cc850163/src/lint/linter/xhpast/rules/__tests__/ArcanistImplodeArgumentOrderXHPASTLinterRuleTestCase.php --- old/arcanist-0.0~git.20190905T053100~3cdfe1ff/src/lint/linter/xhpast/rules/__tests__/ArcanistImplodeArgumentOrderXHPASTLinterRuleTestCase.php 1970-01-01 01:00:00.000000000 +0100 +++ new/arcanist-0.0~git.20191118T203151~cc850163/src/lint/linter/xhpast/rules/__tests__/ArcanistImplodeArgumentOrderXHPASTLinterRuleTestCase.php 2019-11-19 05:31:51.000000000 +0100 @@ -0,0 +1,11 @@ +<?php + +final class ArcanistImplodeArgumentOrderXHPASTLinterRuleTestCase + extends ArcanistXHPASTLinterRuleTestCase { + + public function testLinter() { + $this->executeTestsInDirectory( + dirname(__FILE__).'/implode-argument-order/'); + } + +} diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/arcanist-0.0~git.20190905T053100~3cdfe1ff/src/lint/linter/xhpast/rules/__tests__/implode-argument-order/implode.lint-test new/arcanist-0.0~git.20191118T203151~cc850163/src/lint/linter/xhpast/rules/__tests__/implode-argument-order/implode.lint-test --- old/arcanist-0.0~git.20190905T053100~3cdfe1ff/src/lint/linter/xhpast/rules/__tests__/implode-argument-order/implode.lint-test 1970-01-01 01:00:00.000000000 +0100 +++ new/arcanist-0.0~git.20191118T203151~cc850163/src/lint/linter/xhpast/rules/__tests__/implode-argument-order/implode.lint-test 2019-11-19 05:31:51.000000000 +0100 @@ -0,0 +1,18 @@ +<?php + +// This is the correct argument order. +implode(' ', $x); + +// This is the legacy argument order which warns in PHP 7.4+. +implode($x, ' '); + +// No warning: we can't statically tell which one is the glue. +implode($x, $y); + +// No warning: these are likely wrong, but not a glue order problem. +implode(); +implode($x); +implode($x, $y, $z); + +~~~~~~~~~~ +error:7:1:XHP129:Implode With Glue First diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/arcanist-0.0~git.20190905T053100~3cdfe1ff/src/repository/api/ArcanistGitAPI.php new/arcanist-0.0~git.20191118T203151~cc850163/src/repository/api/ArcanistGitAPI.php --- old/arcanist-0.0~git.20190905T053100~3cdfe1ff/src/repository/api/ArcanistGitAPI.php 2019-09-05 14:31:00.000000000 +0200 +++ new/arcanist-0.0~git.20191118T203151~cc850163/src/repository/api/ArcanistGitAPI.php 2019-11-19 05:31:51.000000000 +0100 @@ -464,15 +464,27 @@ */ public function getFullGitDiff($base, $head = null) { $options = $this->getDiffFullOptions(); + $config_options = array(); + + // See T13432. Disable the rare "diff.suppressBlankEmpty" configuration + // option, which discards the " " (space) change type prefix on unchanged + // blank lines. At time of writing the parser does not handle these + // properly, but generating a more-standard diff is generally desirable + // even if a future parser handles this case more gracefully. + + $config_options[] = '-c'; + $config_options[] = 'diff.suppressBlankEmpty=false'; if ($head !== null) { list($stdout) = $this->execxLocal( - "diff {$options} %s %s --", + "%LR diff {$options} %s %s --", + $config_options, $base, $head); } else { list($stdout) = $this->execxLocal( - "diff {$options} %s --", + "%LR diff {$options} %s --", + $config_options, $base); } @@ -1550,4 +1562,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 -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/arcanist-0.0~git.20190905T053100~3cdfe1ff/src/workflow/ArcanistCloseRevisionWorkflow.php new/arcanist-0.0~git.20191118T203151~cc850163/src/workflow/ArcanistCloseRevisionWorkflow.php --- old/arcanist-0.0~git.20190905T053100~3cdfe1ff/src/workflow/ArcanistCloseRevisionWorkflow.php 2019-09-05 14:31:00.000000000 +0200 +++ new/arcanist-0.0~git.20191118T203151~cc850163/src/workflow/ArcanistCloseRevisionWorkflow.php 2019-11-19 05:31:51.000000000 +0100 @@ -89,11 +89,13 @@ )); $revision = head($revisions); + $object_name = "D{$revision_id}"; + if (!$revision && !$is_finalize) { throw new ArcanistUsageException( pht( 'Revision %s does not exist.', - "D{$revision_id}")); + $object_name)); } $status_accepted = ArcanistDifferentialRevisionStatus::ACCEPTED; @@ -104,15 +106,20 @@ pht( "Revision %s can not be closed. You can only close ". "revisions which have been 'accepted'.", - "D{$revision_id}")); + $object_name)); } if ($revision) { + $revision_display = sprintf( + '%s %s', + $object_name, + $revision['title']); + if (!$is_finalize && $revision['authorPHID'] != $this->getUserPHID()) { $prompt = pht( - 'You are not the author of revision %s, '. + 'You are not the author of revision "%s", '. 'are you sure you want to close it?', - "D{$revision_id}"); + $object_name); if (!phutil_console_confirm($prompt)) { throw new ArcanistUserAbortException(); } @@ -120,24 +127,42 @@ $actually_close = true; if ($is_finalize) { - if ($this->getRepositoryPHID() || - $revision['status'] != $status_accepted) { + if ($this->getRepositoryPHID()) { + $actually_close = false; + } else if ($revision['status'] != $status_accepted) { + // See T13458. The server doesn't permit a transition to "Closed" + // over the API if the revision is not "Accepted". If we won't be + // able to close the revision, skip the attempt and print a + // message. + + $this->writeWarn( + pht('OPEN REVISION'), + pht( + 'Revision "%s" is not in state "Accepted", so it will '. + 'be left open.', + $object_name)); + $actually_close = false; } } - if ($actually_close) { - $revision_name = $revision['title']; - echo pht( - "Closing revision %s '%s'...\n", - "D{$revision_id}", - $revision_name); + if ($actually_close) { + $this->writeInfo( + pht('CLOSE'), + pht( + 'Closing revision "%s"...', + $revision_display)); $conduit->callMethodSynchronous( 'differential.close', array( 'revisionID' => $revision_id, )); + + $this->writeOkay( + pht('CLOSE'), + pht( + 'Done, closed revision.')); } } diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/arcanist-0.0~git.20190905T053100~3cdfe1ff/src/workflow/ArcanistLandWorkflow.php new/arcanist-0.0~git.20191118T203151~cc850163/src/workflow/ArcanistLandWorkflow.php --- old/arcanist-0.0~git.20190905T053100~3cdfe1ff/src/workflow/ArcanistLandWorkflow.php 2019-09-05 14:31:00.000000000 +0200 +++ new/arcanist-0.0~git.20191118T203151~cc850163/src/workflow/ArcanistLandWorkflow.php 2019-11-19 05:31:51.000000000 +0100 @@ -44,10 +44,10 @@ public function getCommandHelp() { return phutil_console_format(<<<EOTEXT - Supports: git, hg + Supports: git, git/p4, hg Publish an accepted revision after review. This command is the last - step in the standard Differential pre-publish code review workflow. + step in the standard Differential code review workflow. This command merges and pushes changes associated with an accepted revision that are currently sitting in __ref__, which is usually the @@ -57,6 +57,9 @@ Under Git: branches, tags, and arbitrary commits (detached HEADs) may be landed. + Under Git/Perforce: branches, tags, and arbitrary commits may + be submitted. + Under Mercurial: branches and bookmarks may be landed, but only onto a target of the same type. See T3855. @@ -66,7 +69,8 @@ A target branch is selected by examining these sources in order: - the **--onto** flag; - - the upstream of the current branch, recursively (Git only); + - the upstream of the branch targeted by the land operation, + recursively (Git only); - the __arc.land.onto.default__ configuration setting; - or by falling back to a standard default: - "master" in Git; @@ -76,6 +80,8 @@ - the **--remote** flag; - the upstream of the current branch, recursively (Git only); + - the special "p4" remote which indicates a repository has + been synchronized with Perforce (Git only); - or by falling back to a standard default: - "origin" in Git; - the default remote in Mercurial. @@ -159,7 +165,7 @@ 'remote' => array( 'param' => 'origin', 'help' => pht( - "Push to a remote other than the default ('origin' in git)."), + 'Push to a remote other than the default.'), ), 'merge' => array( 'help' => pht( @@ -224,23 +230,36 @@ } if ($engine) { - $this->readEngineArguments(); - $this->requireCleanWorkingCopy(); - $should_hold = $this->getArgument('hold'); + $remote_arg = $this->getArgument('remote'); + $onto_arg = $this->getArgument('onto'); $engine ->setWorkflow($this) ->setRepositoryAPI($this->getRepositoryAPI()) ->setSourceRef($this->branch) - ->setTargetRemote($this->remote) - ->setTargetOnto($this->onto) ->setShouldHold($should_hold) ->setShouldKeep($this->keepBranch) ->setShouldSquash($this->useSquash) ->setShouldPreview($this->preview) + ->setRemoteArgument($remote_arg) + ->setOntoArgument($onto_arg) ->setBuildMessageCallback(array($this, 'buildEngineMessage')); + // The goal here is to raise errors with flags early (which is cheap), + // before we test if the working copy is clean (which can be slow). This + // 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(); if (!$should_hold && !$this->preview) { @@ -337,123 +356,6 @@ return $refspec; } - private function readEngineArguments() { - // NOTE: This is hard-coded for Git right now. - // TODO: Clean this up and move it into LandEngines. - - $onto = $this->getEngineOnto(); - $remote = $this->getEngineRemote(); - - // This just overwrites work we did earlier, but it has to be up in this - // class for now because other parts of the workflow still depend on it. - $this->onto = $onto; - $this->remote = $remote; - $this->ontoRemoteBranch = $this->remote.'/'.$onto; - } - - private function getEngineOnto() { - $onto = $this->getArgument('onto'); - if ($onto !== null) { - $this->writeInfo( - pht('TARGET'), - pht( - 'Landing onto "%s", selected by the --onto flag.', - $onto)); - return $onto; - } - - $api = $this->getRepositoryAPI(); - $path = $api->getPathToUpstream($this->branch); - - if ($path->getLength()) { - $cycle = $path->getCycle(); - if ($cycle) { - $this->writeWarn( - pht('LOCAL CYCLE'), - pht( - 'Local branch tracks an upstream, but following it leads to a '. - 'local cycle; ignoring branch upstream.')); - - echo tsprintf( - "\n %s\n\n", - implode(' -> ', $cycle)); - - } else { - if ($path->isConnectedToRemote()) { - $onto = $path->getRemoteBranchName(); - $this->writeInfo( - pht('TARGET'), - pht( - 'Landing onto "%s", selected by following tracking branches '. - 'upstream to the closest remote.', - $onto)); - return $onto; - } else { - $this->writeInfo( - pht('NO PATH TO UPSTREAM'), - pht( - 'Local branch tracks an upstream, but there is no path '. - 'to a remote; ignoring branch upstream.')); - } - } - } - - $config_key = 'arc.land.onto.default'; - $onto = $this->getConfigFromAnySource($config_key); - if ($onto !== null) { - $this->writeInfo( - pht('TARGET'), - pht( - 'Landing onto "%s", selected by "%s" configuration.', - $onto, - $config_key)); - return $onto; - } - - $onto = 'master'; - $this->writeInfo( - pht('TARGET'), - pht( - 'Landing onto "%s", the default target under git.', - $onto)); - return $onto; - } - - private function getEngineRemote() { - $remote = $this->getArgument('remote'); - if ($remote !== null) { - $this->writeInfo( - pht('REMOTE'), - pht( - 'Using remote "%s", selected by the --remote flag.', - $remote)); - return $remote; - } - - $api = $this->getRepositoryAPI(); - $path = $api->getPathToUpstream($this->branch); - - $remote = $path->getRemoteRemoteName(); - if ($remote !== null) { - $this->writeInfo( - pht('REMOTE'), - pht( - 'Using remote "%s", selected by following tracking branches '. - 'upstream to the closest remote.', - $remote)); - return $remote; - } - - $remote = 'origin'; - $this->writeInfo( - pht('REMOTE'), - pht( - 'Using remote "%s", the default remote under git.', - $remote)); - return $remote; - } - - private function readArguments() { $repository_api = $this->getRepositoryAPI(); $this->isGit = $repository_api instanceof ArcanistGitAPI;
