From d1ce9f6246f797b4c65f47e4c156fdb064b9f8b0 Mon Sep 17 00:00:00 2001 From: Arnout Boks Date: Wed, 30 Jan 2019 14:18:06 +0100 Subject: [PATCH 1/4] Fix defaultRepos fallback does not use auth config When a full 'composer' cannot be constructed (because there is no local composer.json and no global composer.json), some commands (e.g. `show -a`) fall back to the default repositories from the `$COMPOSER_HOME/config.json` file. Without this fix, any auth configuration from `$COMPOSER_HOME/auth.json` is not used for these repositories in such a fallback scenario. Steps to reproduce: * Configure a password-protected composer repository in `$COMPOSER_HOME/config.json`. * Configure valid credentials for that repository in `$COMPOSER_HOME/auth.json`. * Make sure there is no file `$COMPOSER_HOME/composer.json`. * Ensure the current working directory has no `composer.json`. * Run `composer show -a some/package`. Expected: Information about `some/package` is shown without needing to enter credentials. Actual: A prompt "Authentication required" is shown for the private repository. When running the same command in a dir that has a `composer.json`, or when `$COMPOSER_HOME/composer.json` exists, things work as expected. --- src/Composer/Repository/RepositoryFactory.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Composer/Repository/RepositoryFactory.php b/src/Composer/Repository/RepositoryFactory.php index ca479a7fd..3c7e837c3 100644 --- a/src/Composer/Repository/RepositoryFactory.php +++ b/src/Composer/Repository/RepositoryFactory.php @@ -93,6 +93,7 @@ class RepositoryFactory { if (!$config) { $config = Factory::createConfig($io); + $io->loadConfiguration($config); } if (!$rm) { if (!$io) { From e151a6c51c08e638d271c64b5384cbbd5ecc6333 Mon Sep 17 00:00:00 2001 From: Arnout Boks Date: Thu, 31 Jan 2019 09:37:28 +0100 Subject: [PATCH 2/4] Only load configuration into IO if IO is available --- src/Composer/Repository/RepositoryFactory.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Composer/Repository/RepositoryFactory.php b/src/Composer/Repository/RepositoryFactory.php index 3c7e837c3..d8e917147 100644 --- a/src/Composer/Repository/RepositoryFactory.php +++ b/src/Composer/Repository/RepositoryFactory.php @@ -93,7 +93,9 @@ class RepositoryFactory { if (!$config) { $config = Factory::createConfig($io); - $io->loadConfiguration($config); + if ($io) { + $io->loadConfiguration($config); + } } if (!$rm) { if (!$io) { From 82b010782d1c1a3553862621d4eae83e31cdb3d1 Mon Sep 17 00:00:00 2001 From: Arnout Boks Date: Thu, 31 Jan 2019 13:38:20 +0100 Subject: [PATCH 3/4] Also load config into IO if not freshly created --- src/Composer/Repository/RepositoryFactory.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Composer/Repository/RepositoryFactory.php b/src/Composer/Repository/RepositoryFactory.php index d8e917147..5d3a2a61f 100644 --- a/src/Composer/Repository/RepositoryFactory.php +++ b/src/Composer/Repository/RepositoryFactory.php @@ -93,9 +93,9 @@ class RepositoryFactory { if (!$config) { $config = Factory::createConfig($io); - if ($io) { - $io->loadConfiguration($config); - } + } + if ($io) { + $io->loadConfiguration($config); } if (!$rm) { if (!$io) { From 6b2edeae56d83b84ba60957aee18c314257d98fa Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Sun, 3 Feb 2019 01:53:17 +0100 Subject: [PATCH 4/4] Fix solver problem exceptions with unexpected contradictory "Conclusions" This 5 character fix comes with a solver test as well as a functional installer test essentially verifying the same thing. The solver test is more useful when working on the solver. But the functional test is less likely to be accidentally modified incorrectly during refactoring, as every single package, version and link in the rather complex test scenario is essential, and a modified version of the test may very well still result in a successful installation but no longer verify the bug described below. Background: In commit 451bab1c2cd58e05af6e21639b829408ad023463 from May 19, 2012 I refactored literals from complex objects into pure integers to reduce memory consumption. The absolute value of an integer literal is the id of the package it refers to in the package pool. The sign indicates whether the package should be installed (positive) or removed (negative), So a major part of the refactoring was swapping this call: $literal->getPackageId() For this: abs($literal) Unintentionally in line 554/523 I incorrectly applied this change to the line: $this->literalFromId(-$literal->getPackageId()); It was converted to: -abs($literal); The function literalFromId used to create a new literal object. By using the abs() function this change essentially forces the resulting literal to be negative, while the minus sign previously inverted the literal, so positive into negative and vice versa. This particular line is in a function meant to analyze a conflicting decision during dependency resolution and to draw a conclusion from it, then revert the state of the solver to an earlier position, and attempt to solve the rest of the rules again with this new "learned" conclusion. Because of this bug these conclusions could only ever occur in the negative, e.g. "don't install package X". This is by far the most likely scenario when the solver reaches this particular line, but there are exceptions. If you experienced a solver problem description that contained a statement like "Conclusion: don't install vendor/package 1.2.3" which directly contradicted other statements listed as part of the problem, this could likely have been the cause. --- src/Composer/DependencyResolver/Decisions.php | 12 +++ src/Composer/DependencyResolver/Solver.php | 2 +- .../Test/DependencyResolver/SolverTest.php | 64 +++++++++++ ...everts-and-learning-positive-literals.test | 100 ++++++++++++++++++ 4 files changed, 177 insertions(+), 1 deletion(-) create mode 100644 tests/Composer/Test/Fixtures/installer/update-requiring-decision-reverts-and-learning-positive-literals.test diff --git a/src/Composer/DependencyResolver/Decisions.php b/src/Composer/DependencyResolver/Decisions.php index a9808e60e..86b62c3d3 100644 --- a/src/Composer/DependencyResolver/Decisions.php +++ b/src/Composer/DependencyResolver/Decisions.php @@ -196,4 +196,16 @@ class Decisions implements \Iterator, \Countable $this->decisionMap[$packageId] = -$level; } } + + public function __toString() + { + $decisionMap = $this->decisionMap; + ksort($decisionMap); + $str = '['; + foreach ($decisionMap as $packageId => $level) { + $str .= $packageId.':'.$level.','; + } + $str .= ']'; + return $str; + } } diff --git a/src/Composer/DependencyResolver/Solver.php b/src/Composer/DependencyResolver/Solver.php index 1ed35ad9c..c40789c1f 100644 --- a/src/Composer/DependencyResolver/Solver.php +++ b/src/Composer/DependencyResolver/Solver.php @@ -470,7 +470,7 @@ class Solver unset($seen[abs($literal)]); if ($num && 0 === --$num) { - $learnedLiterals[0] = -abs($literal); + $learnedLiterals[0] = -$literal; if (!$l1num) { break 2; diff --git a/tests/Composer/Test/DependencyResolver/SolverTest.php b/tests/Composer/Test/DependencyResolver/SolverTest.php index 24147e6ad..7094f412d 100644 --- a/tests/Composer/Test/DependencyResolver/SolverTest.php +++ b/tests/Composer/Test/DependencyResolver/SolverTest.php @@ -838,6 +838,70 @@ class SolverTest extends TestCase )); } + /** + * Tests for a bug introduced in commit 451bab1c2cd58e05af6e21639b829408ad023463 Solver.php line 554/523 + * + * Every package and link in this test matters, only a combination this complex will run into the situation in which + * a negatively decided literal will need to be learned inverted as a positive assertion. + * + * In particular in this case the goal is to first have the solver decide X 2.0 should not be installed to later + * decide to learn that X 2.0 must be installed and revert decisions to retry solving with this new assumption. + */ + public function testLearnPositiveLiteral() + { + $this->repo->addPackage($packageA = $this->getPackage('A', '1.0')); + $this->repo->addPackage($packageB = $this->getPackage('B', '1.0')); + $this->repo->addPackage($packageC1 = $this->getPackage('C', '1.0')); + $this->repo->addPackage($packageC2 = $this->getPackage('C', '2.0')); + $this->repo->addPackage($packageD = $this->getPackage('D', '1.0')); + $this->repo->addPackage($packageE = $this->getPackage('E', '1.0')); + $this->repo->addPackage($packageF1 = $this->getPackage('F', '1.0')); + $this->repo->addPackage($packageF2 = $this->getPackage('F', '2.0')); + $this->repo->addPackage($packageG1 = $this->getPackage('G', '1.0')); + $this->repo->addPackage($packageG2 = $this->getPackage('G', '2.0')); + $this->repo->addPackage($packageG3 = $this->getPackage('G', '3.0')); + + $packageA->setRequires(array( + 'b' => new Link('A', 'B', $this->getVersionConstraint('==', '1.0'), 'requires'), + 'c' => new Link('A', 'C', $this->getVersionConstraint('>=', '1.0'), 'requires'), + 'd' => new Link('A', 'D', $this->getVersionConstraint('==', '1.0'), 'requires'), + )); + + $packageB->setRequires(array( + 'e' => new Link('B', 'E', $this->getVersionConstraint('==', '1.0'), 'requires'), + )); + + $packageC1->setRequires(array( + 'f' => new Link('C', 'F', $this->getVersionConstraint('==', '1.0'), 'requires'), + )); + $packageC2->setRequires(array( + 'f' => new Link('C', 'F', $this->getVersionConstraint('==', '1.0'), 'requires'), + 'g' => new Link('C', 'G', $this->getVersionConstraint('>=', '1.0'), 'requires'), + )); + + $packageD->setRequires(array( + 'f' => new Link('D', 'F', $this->getVersionConstraint('>=', '1.0'), 'requires'), + )); + + $packageE->setRequires(array( + 'g' => new Link('E', 'G', $this->getVersionConstraint('<=', '2.0'), 'requires'), + )); + + $this->reposComplete(); + + $this->request->install('A'); + + $this->checkSolverResult(array( + array('job' => 'install', 'package' => $packageF1), + array('job' => 'install', 'package' => $packageD), + array('job' => 'install', 'package' => $packageG2), + array('job' => 'install', 'package' => $packageC2), + array('job' => 'install', 'package' => $packageE), + array('job' => 'install', 'package' => $packageB), + array('job' => 'install', 'package' => $packageA), + )); + } + protected function reposComplete() { $this->pool->addRepository($this->repoInstalled); diff --git a/tests/Composer/Test/Fixtures/installer/update-requiring-decision-reverts-and-learning-positive-literals.test b/tests/Composer/Test/Fixtures/installer/update-requiring-decision-reverts-and-learning-positive-literals.test new file mode 100644 index 000000000..3f5667823 --- /dev/null +++ b/tests/Composer/Test/Fixtures/installer/update-requiring-decision-reverts-and-learning-positive-literals.test @@ -0,0 +1,100 @@ +--TEST-- +Update a project which requires decision reverts and learning a positive literal to arrive at a correct solution. + +Tests for solver regression in commit 451bab1c2cd58e05af6e21639b829408ad023463. See also SolverTest testLearnPositiveLiteral +--COMPOSER-- +{ + "repositories": [ + { + "type": "package", + "package": [ + { + "name": "spryker-feature/product", + "require": { + "spryker-feature/spryker-core": "1.0.0", + "spryker-shop/product-search-widget": ">=1.0.0", + "spryker/product-category-filter-gui": "1.0.0" + }, + "version": "1.0.0" + }, + { + "name": "spryker-feature/spryker-core", + "version": "1.0.0", + "require": { + "spryker/store": "1.0.0" + } + }, + { + "name": "spryker/store", + "version": "1.0.0", + "require": { + "spryker/kernel": "<=2.0.0" + } + }, + { + "name": "spryker-shop/product-search-widget", + "version": "1.0.0", + "require": { + "spryker/catalog": "1.0.0" + } + }, + { + "name": "spryker-shop/product-search-widget", + "version": "2.0.0", + "require": { + "spryker/catalog": "1.0.0", + "spryker/kernel": ">=1.0.0" + } + }, + { + "name": "spryker/product-category-filter-gui", + "version": "1.0.0", + "require": { + "spryker/catalog": ">=1.0.0" + } + }, + { + "name": "spryker/catalog", + "version": "1.0.0", + "require": { } + }, + { + "name": "spryker/catalog", + "version": "2.0.0", + "require": { } + }, + + { + "name": "spryker/kernel", + "version": "1.0.0", + "require": { } + }, + { + "name": "spryker/kernel", + "version": "2.0.0", + "require": { + } + }, + { + "name": "spryker/kernel", + "version": "3.0.0", + "require": { } + } + ] + } + ], + "require": { + "spryker-feature/product": "1.0.0" + } +} +--RUN-- +update +--EXPECT-- +Installing spryker/catalog (1.0.0) +Installing spryker/product-category-filter-gui (1.0.0) +Installing spryker/kernel (2.0.0) +Installing spryker-shop/product-search-widget (2.0.0) +Installing spryker/store (1.0.0) +Installing spryker-feature/spryker-core (1.0.0) +Installing spryker-feature/product (1.0.0) +