From b93b73e8360c50c70031d45034dd8f8bb1bed8c4 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Wed, 2 Mar 2016 13:00:05 +0000 Subject: [PATCH] Rewrite git unpushed status checks, fixes #4987 --- src/Composer/Command/StatusCommand.php | 2 +- src/Composer/Downloader/GitDownloader.php | 72 +++++++++++-------- .../Test/Downloader/GitDownloaderTest.php | 54 ++++---------- 3 files changed, 60 insertions(+), 68 deletions(-) diff --git a/src/Composer/Command/StatusCommand.php b/src/Composer/Command/StatusCommand.php index f86077458..976e9cb41 100644 --- a/src/Composer/Command/StatusCommand.php +++ b/src/Composer/Command/StatusCommand.php @@ -65,7 +65,7 @@ EOT $unpushedChanges = array(); // list packages - foreach ($installedRepo->getPackages() as $package) { + foreach ($installedRepo->getCanonicalPackages() as $package) { $downloader = $dm->getDownloaderForInstalledPackage($package); if ($downloader instanceof ChangeReportInterface) { diff --git a/src/Composer/Downloader/GitDownloader.php b/src/Composer/Downloader/GitDownloader.php index 90431a327..e1eef2a46 100644 --- a/src/Composer/Downloader/GitDownloader.php +++ b/src/Composer/Downloader/GitDownloader.php @@ -120,49 +120,65 @@ class GitDownloader extends VcsDownloader implements DvcsDownloaderInterface return; } - $command = 'git rev-parse --abbrev-ref HEAD'; + $command = 'git show-ref --head -d'; if (0 !== $this->process->execute($command, $output, $path)) { throw new \RuntimeException('Failed to execute ' . $command . "\n\n" . $this->process->getErrorOutput()); } - $branch = trim($output); + $refs = trim($output); + if (!preg_match('{^([a-f0-9]+) HEAD$}mi', $refs, $match)) { + // could not match the HEAD for some reason + return; + } - // we are on a detached HEAD tag so no need to check for changes - if ($branch === 'HEAD') { + $headRef = $match[1]; + if (!preg_match_all('{^'.$headRef.' refs/heads/(.+)$}mi', $refs, $matches)) { + // not on a branch, we are either on a not-modified tag or some sort of detached head, so skip this return; } - // check that the branch exists in composer remote, if not then we should assume it is an unpushed branch - $command = sprintf('git rev-parse --verify composer/%s', $branch); - if (0 !== $this->process->execute($command, $output, $path)) { - $composerBranch = preg_replace('{(?:^dev-|(?:\.x)?-dev$)}i', '', $package->getPrettyVersion()); - $branches = ''; - if (0 === $this->process->execute('git branch -r', $output, $path)) { - $branches = $output; - } - // add 'v' in front of the branch if it was stripped when generating the pretty name - if (!preg_match('{^\s+composer/'.preg_quote($composerBranch).'$}m', $branches) && preg_match('{^\s+composer/v'.preg_quote($composerBranch).'$}m', $branches)) { - $composerBranch = 'v' . $composerBranch; + // use the first match as branch name for now + $branch = $matches[1][0]; + $unpushedChanges = null; + + // do two passes, as if we find anything we want to fetch and then re-try + for ($i = 0; $i <= 1; $i++) { + // try to find the a matching branch name in the composer remote + foreach ($matches[1] as $candidate) { + if (preg_match('{^[a-f0-9]+ refs/remotes/((?:composer|origin)/'.preg_quote($candidate).')$}mi', $refs, $match)) { + $branch = $candidate; + $remoteBranch = $match[1]; + break; + } } - } else { - $composerBranch = $branch; - } - $command = sprintf('git diff --name-status composer/%s...%s', $composerBranch, $branch); - if (0 !== $this->process->execute($command, $output, $path)) { - throw new \RuntimeException('Failed to execute ' . $command . "\n\n" . $this->process->getErrorOutput()); - } + // if it doesn't exist, then we assume it is an unpushed branch + // this is bad as we have no reference point to do a diff so we just bail listing + // the branch as being unpushed + if (!isset($remoteBranch)) { + $unpushedChanges = 'Branch ' . $branch . ' could not be found on the origin remote and appears to be unpushed'; + } else { + $command = sprintf('git diff --name-status %s...%s --', $remoteBranch, $branch); + if (0 !== $this->process->execute($command, $output, $path)) { + throw new \RuntimeException('Failed to execute ' . $command . "\n\n" . $this->process->getErrorOutput()); + } - if (trim($output)) { - // fetch from both to make sure we have up to date remotes - $this->process->execute('git fetch composer && git fetch origin', $output, $path); + $unpushedChanges = trim($output) ?: null; + } + + // first pass and we found unpushed changes, fetch from both remotes to make sure we have up to date + // remotes and then try again as outdated remotes can sometimes cause false-positives + if ($unpushedChanges && $i === 0) { + $this->process->execute('git fetch composer && git fetch origin', $output, $path); + } - if (0 !== $this->process->execute($command, $output, $path)) { - throw new \RuntimeException('Failed to execute ' . $command . "\n\n" . $this->process->getErrorOutput()); + // abort after first pass if we didn't find anything + if (!$unpushedChanges) { + break; } } - return trim($output) ?: null; + return $unpushedChanges; } /** diff --git a/tests/Composer/Test/Downloader/GitDownloaderTest.php b/tests/Composer/Test/Downloader/GitDownloaderTest.php index 08ec2ca41..d5b57a287 100644 --- a/tests/Composer/Test/Downloader/GitDownloaderTest.php +++ b/tests/Composer/Test/Downloader/GitDownloaderTest.php @@ -276,33 +276,25 @@ class GitDownloaderTest extends TestCase $processExecutor = $this->getMock('Composer\Util\ProcessExecutor'); $processExecutor->expects($this->at(0)) ->method('execute') - ->with($this->equalTo($this->winCompat("git rev-parse --abbrev-ref HEAD"))) + ->with($this->equalTo($this->winCompat("git show-ref --head -d"))) ->will($this->returnValue(0)); $processExecutor->expects($this->at(1)) - ->method('execute') - ->with($this->equalTo($this->winCompat("git rev-parse --verify composer/"))) - ->will($this->returnValue(0)); - $processExecutor->expects($this->at(2)) - ->method('execute') - ->with($this->equalTo($this->winCompat("git diff --name-status composer/..."))) - ->will($this->returnValue(0)); - $processExecutor->expects($this->at(3)) ->method('execute') ->with($this->equalTo($this->winCompat("git status --porcelain --untracked-files=no"))) ->will($this->returnValue(0)); - $processExecutor->expects($this->at(4)) + $processExecutor->expects($this->at(2)) ->method('execute') ->with($this->equalTo($this->winCompat("git remote -v"))) ->will($this->returnValue(0)); - $processExecutor->expects($this->at(5)) + $processExecutor->expects($this->at(3)) ->method('execute') ->with($this->equalTo($this->winCompat($expectedGitUpdateCommand)), $this->equalTo(null), $this->equalTo($this->winCompat($this->workingDir))) ->will($this->returnValue(0)); - $processExecutor->expects($this->at(6)) + $processExecutor->expects($this->at(4)) ->method('execute') ->with($this->equalTo('git branch -r')) ->will($this->returnValue(0)); - $processExecutor->expects($this->at(7)) + $processExecutor->expects($this->at(5)) ->method('execute') ->with($this->equalTo($this->winCompat("git checkout 'ref' -- && git reset --hard 'ref' --")), $this->equalTo(null), $this->equalTo($this->winCompat($this->workingDir))) ->will($this->returnValue(0)); @@ -330,25 +322,17 @@ class GitDownloaderTest extends TestCase $processExecutor = $this->getMock('Composer\Util\ProcessExecutor'); $processExecutor->expects($this->at(0)) ->method('execute') - ->with($this->equalTo($this->winCompat("git rev-parse --abbrev-ref HEAD"))) + ->with($this->equalTo($this->winCompat("git show-ref --head -d"))) ->will($this->returnValue(0)); $processExecutor->expects($this->at(1)) - ->method('execute') - ->with($this->equalTo($this->winCompat("git rev-parse --verify composer/"))) - ->will($this->returnValue(0)); - $processExecutor->expects($this->at(2)) - ->method('execute') - ->with($this->equalTo($this->winCompat("git diff --name-status composer/..."))) - ->will($this->returnValue(0)); - $processExecutor->expects($this->at(3)) ->method('execute') ->with($this->equalTo($this->winCompat("git status --porcelain --untracked-files=no"))) ->will($this->returnValue(0)); - $processExecutor->expects($this->at(4)) + $processExecutor->expects($this->at(2)) ->method('execute') ->with($this->equalTo($this->winCompat("git remote -v"))) ->will($this->returnValue(0)); - $processExecutor->expects($this->at(5)) + $processExecutor->expects($this->at(3)) ->method('execute') ->with($this->equalTo($expectedGitUpdateCommand)) ->will($this->returnValue(1)); @@ -373,41 +357,33 @@ class GitDownloaderTest extends TestCase $processExecutor = $this->getMock('Composer\Util\ProcessExecutor'); $processExecutor->expects($this->at(0)) ->method('execute') - ->with($this->equalTo($this->winCompat("git rev-parse --abbrev-ref HEAD"))) + ->with($this->equalTo($this->winCompat("git show-ref --head -d"))) ->will($this->returnValue(0)); $processExecutor->expects($this->at(1)) - ->method('execute') - ->with($this->equalTo($this->winCompat("git rev-parse --verify composer/"))) - ->will($this->returnValue(0)); - $processExecutor->expects($this->at(2)) - ->method('execute') - ->with($this->equalTo($this->winCompat("git diff --name-status composer/..."))) - ->will($this->returnValue(0)); - $processExecutor->expects($this->at(3)) ->method('execute') ->with($this->equalTo($this->winCompat("git status --porcelain --untracked-files=no"))) ->will($this->returnValue(0)); - $processExecutor->expects($this->at(4)) + $processExecutor->expects($this->at(2)) ->method('execute') ->with($this->equalTo($this->winCompat("git remote -v"))) ->will($this->returnValue(0)); - $processExecutor->expects($this->at(5)) + $processExecutor->expects($this->at(3)) ->method('execute') ->with($this->equalTo($expectedFirstGitUpdateCommand)) ->will($this->returnValue(1)); - $processExecutor->expects($this->at(7)) + $processExecutor->expects($this->at(5)) ->method('execute') ->with($this->equalTo($this->winCompat("git --version"))) ->will($this->returnValue(0)); - $processExecutor->expects($this->at(8)) + $processExecutor->expects($this->at(6)) ->method('execute') ->with($this->equalTo($this->winCompat("git remote -v"))) ->will($this->returnValue(0)); - $processExecutor->expects($this->at(9)) + $processExecutor->expects($this->at(7)) ->method('execute') ->with($this->equalTo($expectedSecondGitUpdateCommand)) ->will($this->returnValue(0)); - $processExecutor->expects($this->at(11)) + $processExecutor->expects($this->at(9)) ->method('execute') ->with($this->equalTo($this->winCompat("git checkout 'ref' -- && git reset --hard 'ref' --")), $this->equalTo(null), $this->equalTo($this->winCompat($this->workingDir))) ->will($this->returnValue(0));