Browse Source

Merge branch '1.7'

Jordi Boggiano 6 years ago
parent
commit
df2f2dc113

+ 2 - 2
src/Composer/DependencyResolver/Pool.php

@@ -317,12 +317,12 @@ class Pool implements \Countable
      * Checks if the package matches the given constraint directly or through
      * provided or replaced packages
      *
-     * @param  array|PackageInterface $candidate
+     * @param  PackageInterface       $candidate
      * @param  string                 $name       Name of the package to be matched
      * @param  ConstraintInterface    $constraint The constraint to verify
      * @return int                    One of the MATCH* constants of this class or 0 if there is no match
      */
-    private function match($candidate, $name, ConstraintInterface $constraint = null, $bypassFilters)
+    public function match($candidate, $name, ConstraintInterface $constraint = null, $bypassFilters)
     {
         $candidateName = $candidate->getName();
         $candidateVersion = $candidate->getVersion();

+ 55 - 21
src/Composer/DependencyResolver/RuleSetGenerator.php

@@ -28,6 +28,9 @@ class RuleSetGenerator
     protected $installedMap;
     protected $whitelistedMap;
     protected $addedMap;
+    protected $conflictAddedMap;
+    protected $addedPackages;
+    protected $addedPackagesByNames;
 
     public function __construct(PolicyInterface $policy, Pool $pool)
     {
@@ -185,6 +188,7 @@ class RuleSetGenerator
         $workQueue->enqueue($package);
 
         while (!$workQueue->isEmpty()) {
+            /** @var PackageInterface $package */
             $package = $workQueue->dequeue();
             if (isset($this->addedMap[$package->id])) {
                 continue;
@@ -192,6 +196,11 @@ class RuleSetGenerator
 
             $this->addedMap[$package->id] = true;
 
+            $this->addedPackages[] = $package;
+            foreach ($package->getNames() as $name) {
+                $this->addedPackagesByNames[$name][] = $package;
+            }
+
             foreach ($package->getRequires() as $link) {
                 if ($ignorePlatformReqs && preg_match(PlatformRepository::PLATFORM_PACKAGE_REGEX, $link->getTarget())) {
                     continue;
@@ -206,11 +215,41 @@ class RuleSetGenerator
                 }
             }
 
+            $packageName = $package->getName();
+            $obsoleteProviders = $this->pool->whatProvides($packageName, null);
+
+            foreach ($obsoleteProviders as $provider) {
+                if ($provider === $package) {
+                    continue;
+                }
+
+                if (($package instanceof AliasPackage) && $package->getAliasOf() === $provider) {
+                    $this->addRule(RuleSet::TYPE_PACKAGE, $this->createRequireRule($package, array($provider), Rule::RULE_PACKAGE_ALIAS, $package));
+                } elseif (!$this->obsoleteImpossibleForAlias($package, $provider)) {
+                    $reason = ($packageName == $provider->getName()) ? Rule::RULE_PACKAGE_SAME_NAME : Rule::RULE_PACKAGE_IMPLICIT_OBSOLETES;
+                    $this->addRule(RuleSet::TYPE_PACKAGE, $this->createRule2Literals($package, $provider, $reason, $package));
+                }
+            }
+        }
+    }
+
+    protected function addConflictRules()
+    {
+        /** @var PackageInterface $package */
+        foreach ($this->addedPackages as $package) {
             foreach ($package->getConflicts() as $link) {
-                $possibleConflicts = $this->pool->whatProvides($link->getTarget(), $link->getConstraint());
+                if (!isset($this->addedPackagesByNames[$link->getTarget()])) {
+                    continue;
+                }
+
+                /** @var PackageInterface $possibleConflict */
+                foreach ($this->addedPackagesByNames[$link->getTarget()] as $possibleConflict) {
+                    $conflictMatch = $this->pool->match($possibleConflict, $link->getTarget(), $link->getConstraint(), true);
+
+                    if ($conflictMatch === Pool::MATCH || $conflictMatch === Pool::MATCH_REPLACE) {
+                        $this->addRule(RuleSet::TYPE_PACKAGE, $this->createRule2Literals($package, $possibleConflict, Rule::RULE_PACKAGE_CONFLICT, $link));
+                    }
 
-                foreach ($possibleConflicts as $conflict) {
-                    $this->addRule(RuleSet::TYPE_PACKAGE, $this->createRule2Literals($package, $conflict, Rule::RULE_PACKAGE_CONFLICT, $link));
                 }
             }
 
@@ -218,9 +257,12 @@ class RuleSetGenerator
             $isInstalled = isset($this->installedMap[$package->id]);
 
             foreach ($package->getReplaces() as $link) {
-                $obsoleteProviders = $this->pool->whatProvides($link->getTarget(), $link->getConstraint());
+                if (!isset($this->addedPackagesByNames[$link->getTarget()])) {
+                    continue;
+                }
 
-                foreach ($obsoleteProviders as $provider) {
+                /** @var PackageInterface $possibleConflict */
+                foreach ($this->addedPackagesByNames[$link->getTarget()] as $provider) {
                     if ($provider === $package) {
                         continue;
                     }
@@ -231,22 +273,6 @@ class RuleSetGenerator
                     }
                 }
             }
-
-            $packageName = $package->getName();
-            $obsoleteProviders = $this->pool->whatProvides($packageName, null);
-
-            foreach ($obsoleteProviders as $provider) {
-                if ($provider === $package) {
-                    continue;
-                }
-
-                if (($package instanceof AliasPackage) && $package->getAliasOf() === $provider) {
-                    $this->addRule(RuleSet::TYPE_PACKAGE, $this->createRequireRule($package, array($provider), Rule::RULE_PACKAGE_ALIAS, $package));
-                } elseif (!$this->obsoleteImpossibleForAlias($package, $provider)) {
-                    $reason = ($packageName == $provider->getName()) ? Rule::RULE_PACKAGE_SAME_NAME : Rule::RULE_PACKAGE_IMPLICIT_OBSOLETES;
-                    $this->addRule(RuleSet::TYPE_PACKAGE, $this->createRule2Literals($package, $provider, $reason, $package));
-                }
-            }
         }
     }
 
@@ -327,12 +353,20 @@ class RuleSetGenerator
         $this->pool->setWhitelist($this->whitelistedMap);
 
         $this->addedMap = array();
+        $this->conflictAddedMap = array();
+        $this->addedPackages = array();
+        $this->addedPackagesByNames = array();
         foreach ($this->installedMap as $package) {
             $this->addRulesForPackage($package, $ignorePlatformReqs);
         }
 
         $this->addRulesForJobs($ignorePlatformReqs);
 
+        $this->addConflictRules();
+
+        // Remove references to packages
+        $this->addedPackages = $this->addedPackagesByNames = null;
+
         return $this->rules;
     }
 }

+ 7 - 24
src/Composer/Repository/Vcs/GitHubDriver.php

@@ -183,30 +183,13 @@ class GitHubDriver extends VcsDriver
             return $this->gitDriver->getFileContent($file, $identifier);
         }
 
-        $notFoundRetries = 2;
-        while ($notFoundRetries) {
-            try {
-                $resource = $this->getApiUrl() . '/repos/'.$this->owner.'/'.$this->repository.'/contents/' . $file . '?ref='.urlencode($identifier);
-                $resource = JsonFile::parseJson($this->getContents($resource));
-                if (empty($resource['content']) || $resource['encoding'] !== 'base64' || !($content = base64_decode($resource['content']))) {
-                    throw new \RuntimeException('Could not retrieve ' . $file . ' for '.$identifier);
-                }
-
-                return $content;
-            } catch (TransportException $e) {
-                if (404 !== $e->getCode()) {
-                    throw $e;
-                }
-
-                // TODO should be removed when possible
-                // retry fetching if github returns a 404 since they happen randomly
-                $notFoundRetries--;
-
-                return null;
-            }
+        $resource = $this->getApiUrl() . '/repos/'.$this->owner.'/'.$this->repository.'/contents/' . $file . '?ref='.urlencode($identifier);
+        $resource = JsonFile::parseJson($this->getContents($resource));
+        if (empty($resource['content']) || $resource['encoding'] !== 'base64' || !($content = base64_decode($resource['content']))) {
+            throw new \RuntimeException('Could not retrieve ' . $file . ' for '.$identifier);
         }
 
-        return null;
+        return $content;
     }
 
     /**
@@ -378,7 +361,7 @@ class GitHubDriver extends VcsDriver
                         return $this->attemptCloneFallback();
                     }
 
-                    $rateLimited = $githubUtil->isRateLimited($e->getHeaders());
+                    $rateLimited = $gitHubUtil->isRateLimited($e->getHeaders());
 
                     if (!$this->io->hasAuthentication($this->originUrl)) {
                         if (!$this->io->isInteractive()) {
@@ -392,7 +375,7 @@ class GitHubDriver extends VcsDriver
                     }
 
                     if ($rateLimited) {
-                        $rateLimit = $githubUtil->getRateLimit($e->getHeaders());
+                        $rateLimit = $gitHubUtil->getRateLimit($e->getHeaders());
                         $this->io->writeError(sprintf(
                             '<error>GitHub API limit (%d calls/hr) is exhausted. You are already authorized so you have to wait until %s before doing more requests</error>',
                             $rateLimit['limit'],

+ 5 - 7
tests/Composer/Test/Fixtures/installer/replaced-packages-wrong-version-install.test → tests/Composer/Test/Fixtures/installer/replaced-packages-should-not-be-installed-when-installing-from-lock.test

@@ -1,5 +1,5 @@
 --TEST--
-Requiring a replaced package in a version, that is not provided by the replacing package, should install correctly (although that is not a very smart idea)
+Requiring a replaced package in a version, that is not provided by the replacing package, should result in a conflict, when installing from lock
 --COMPOSER--
 {
     "repositories": [
@@ -17,9 +17,7 @@ Requiring a replaced package in a version, that is not provided by the replacing
         "foo/replaced": "2.0.0"
     }
 }
---RUN--
-install
---EXPECT-LOCK--
+--LOCK--
 {
     "packages": [
         { "name": "foo/original", "version": "1.0.0", "replace": {"foo/replaced": "1.0.0"}, "type": "library" },
@@ -34,8 +32,8 @@ install
     "platform": [],
     "platform-dev": []
 }
+--RUN--
+install
 --EXPECT-EXIT-CODE--
-0
+2
 --EXPECT--
-Installing foo/original (1.0.0)
-Installing foo/replaced (2.0.0)

+ 24 - 0
tests/Composer/Test/Fixtures/installer/replaced-packages-should-not-be-installed.test

@@ -0,0 +1,24 @@
+--TEST--
+Requiring a replaced package in a version, that is not provided by the replacing package, should result in a conflict
+--COMPOSER--
+{
+    "repositories": [
+        {
+            "type": "package",
+            "package": [
+                { "name": "foo/original", "version": "1.0.0", "replace": {"foo/replaced": "1.0.0"} },
+                { "name": "foo/replaced", "version": "1.0.0" },
+                { "name": "foo/replaced", "version": "2.0.0" }
+            ]
+        }
+    ],
+    "require": {
+        "foo/original": "1.0.0",
+        "foo/replaced": "2.0.0"
+    }
+}
+--RUN--
+install
+--EXPECT-EXIT-CODE--
+2
+--EXPECT--

+ 0 - 41
tests/Composer/Test/Fixtures/installer/replaced-packages-wrong-version-install-from-lock.test

@@ -1,41 +0,0 @@
---TEST--
-Requiring a replaced package in a version, that is not provided by the replacing package, should install correctly (although that is not a very smart idea) also when installing from lock
---COMPOSER--
-{
-    "repositories": [
-        {
-            "type": "package",
-            "package": [
-                { "name": "foo/original", "version": "1.0.0", "replace": {"foo/replaced": "1.0.0"} },
-                { "name": "foo/replaced", "version": "1.0.0" },
-                { "name": "foo/replaced", "version": "2.0.0" }
-            ]
-        }
-    ],
-    "require": {
-        "foo/original": "1.0.0",
-        "foo/replaced": "2.0.0"
-    }
-}
---LOCK--
-{
-    "packages": [
-        { "name": "foo/original", "version": "1.0.0", "replace": {"foo/replaced": "1.0.0"}, "type": "library" },
-        { "name": "foo/replaced", "version": "2.0.0", "type": "library" }
-    ],
-    "packages-dev": [],
-    "aliases": [],
-    "minimum-stability": "stable",
-    "stability-flags": {},
-    "prefer-stable": false,
-    "prefer-lowest": false,
-    "platform": [],
-    "platform-dev": []
-}
---RUN--
-install
---EXPECT-EXIT-CODE--
-0
---EXPECT--
-Installing foo/original (1.0.0)
-Installing foo/replaced (2.0.0)