Rewrite git unpushed status checks, fixes #4987

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) {

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)) {
-        // we are on a detached HEAD tag so no need to check for changes
+            // could not match the HEAD for some reason
-        if ($branch === 'HEAD') {
             return;
         }
 
-        // check that the branch exists in composer remote, if not then we should assume it is an unpushed branch
+        $headRef = $match[1];
-        $command = sprintf('git rev-parse --verify composer/%s', $branch);
+        if (!preg_match_all('{^'.$headRef.' refs/heads/(.+)$}mi', $refs, $matches)) {
-        if (0 !== $this->process->execute($command, $output, $path)) {
+            // not on a branch, we are either on a not-modified tag or some sort of detached head, so skip this
-            $composerBranch = preg_replace('{(?:^dev-|(?:\.x)?-dev$)}i', '', $package->getPrettyVersion());
+            return;
-            $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)) {
+        // use the first match as branch name for now
-                $composerBranch = 'v' . $composerBranch;
+        $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;
                 }
+            }
+
+            // 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 {
-            $composerBranch = $branch;
+                $command = sprintf('git diff --name-status %s...%s --', $remoteBranch, $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 (trim($output)) {
+                $unpushedChanges = trim($output) ?: null;
-            // fetch from both to make sure we have up to date remotes
+            }
+
+            // 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)) {
+            // abort after first pass if we didn't find anything
-                throw new \RuntimeException('Failed to execute ' . $command . "\n\n" . $this->process->getErrorOutput());
+            if (!$unpushedChanges) {
+                break;
             }
         }
 
-        return trim($output) ?: null;
+        return $unpushedChanges;
     }
 
     /**

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/")))
+            ->with($this->equalTo($this->winCompat("git status --porcelain --untracked-files=no")))
             ->will($this->returnValue(0));
         $processExecutor->expects($this->at(2))
             ->method('execute')
-            ->with($this->equalTo($this->winCompat("git diff --name-status composer/...")))
+            ->with($this->equalTo($this->winCompat("git remote -v")))
             ->will($this->returnValue(0));
         $processExecutor->expects($this->at(3))
             ->method('execute')
-            ->with($this->equalTo($this->winCompat("git status --porcelain --untracked-files=no")))
+            ->with($this->equalTo($this->winCompat($expectedGitUpdateCommand)), $this->equalTo(null), $this->equalTo($this->winCompat($this->workingDir)))
             ->will($this->returnValue(0));
         $processExecutor->expects($this->at(4))
-            ->method('execute')
-            ->with($this->equalTo($this->winCompat("git remote -v")))
-            ->will($this->returnValue(0));
-        $processExecutor->expects($this->at(5))
-            ->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))
             ->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/")))
+            ->with($this->equalTo($this->winCompat("git status --porcelain --untracked-files=no")))
             ->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))
             ->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/")))
+            ->with($this->equalTo($this->winCompat("git status --porcelain --untracked-files=no")))
             ->will($this->returnValue(0));
         $processExecutor->expects($this->at(2))
            ->method('execute')
-            ->with($this->equalTo($this->winCompat("git diff --name-status composer/...")))
+            ->with($this->equalTo($this->winCompat("git remote -v")))
             ->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))
-           ->method('execute')
-            ->with($this->equalTo($this->winCompat("git remote -v")))
-            ->will($this->returnValue(0));
-        $processExecutor->expects($this->at(5))
             ->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));