Skip to content

Commit 910a5c5

Browse files
nicolas-grekasnicolas-grekas
committed
bug #865 Making conflict smarter: choose an older recipe to apply (weaverryan)
This PR was merged into the 1.x branch. Discussion ---------- Making conflict smarter: choose an older recipe to apply Hi! Flex recipes support a `conflict` key - e.g. https://github.com/symfony/recipes/blob/ee79aec15811a380b09c53d058972c74e77e25d9/doctrine/doctrine-bundle/2.4/manifest.json#L48-L50 In this case, we added that so that we could use `when@dev` in the recipe, but that requires `symfony/framework-bundle` 5.3 or greater. Anyways, the current `conflict` behavior is overly simple: if a recipe conflicts with a package you have installed, it simply skips the recipe entirely. This, for example, makes the following fail right now: ``` symfony new 4.4_app --version=4.4 cd 4.4_app composer require doctrine ``` If you try this, the `doctrine/doctrine-bundle` will be skipped **entirely**. This PR makes that behavior smarter. For each recipe that conflicts with your app (e.g. `doctrine/doctrine-bundle` 2.4 recipe), it will try the next oldest recipe version (e.g. `doctrine/doctrine-bundle` 2.3) until it finds one that does *not* conflict. So, you will always get the latest recipe version *without* any conflicts. If no compatible recipes are found, none would be installed (but I don't believe we have this situation anywhere anyways). Tested locally on an app also. Cheers! Commits ------- f921327 Making conflict smarter: choose an older recipe to apply
2 parents d40a6b1 + f921327 commit 910a5c5

File tree

3 files changed

+143
-14
lines changed

3 files changed

+143
-14
lines changed

‎src/Downloader.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,16 @@ public function getRecipes(array $operations): array
279279
return $data;
280280
}
281281

282+
/**
283+
* Used to "hide" a recipe version so that the next most-recent will be returned.
284+
*
285+
* This is used when resolving "conflicts".
286+
*/
287+
public function removeRecipeFromIndex(string $packageName, string $version)
288+
{
289+
unset($this->index[$packageName][$version]);
290+
}
291+
282292
/**
283293
* Fetches and decodes JSON HTTP response bodies.
284294
*/

‎src/Flex.php

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use Composer\Composer;
1616
use Composer\Console\Application;
1717
use Composer\DependencyResolver\Operation\InstallOperation;
18+
use Composer\DependencyResolver\Operation\OperationInterface;
1819
use Composer\DependencyResolver\Operation\UninstallOperation;
1920
use Composer\DependencyResolver\Operation\UpdateOperation;
2021
use Composer\DependencyResolver\Pool;
@@ -725,16 +726,21 @@ public function fetchRecipes(array $operations, bool $reset): array
725726
$name = $package->getName();
726727
$job = method_exists($operation, 'getOperationType') ? $operation->getOperationType() : $operation->getJobType();
727728

728-
if (!empty($manifests[$name]['manifest']['conflict']) && !$operation instanceof UninstallOperation) {
729-
$lockedRepository = $this->composer->getLocker()->getLockedRepository();
729+
while ($this->doesRecipeConflict($manifests[$name] ?? [], $operation)) {
730+
$this->downloader->removeRecipeFromIndex($name, $manifests[$name]['version']);
731+
$newData = $this->downloader->getRecipes([$operation]);
732+
$newManifests = $newData['manifests'] ?? [];
730733

731-
foreach ($manifests[$name]['manifest']['conflict'] as $conflictingPackage => $constraint) {
732-
if ($lockedRepository->findPackage($conflictingPackage, $constraint)) {
733-
$this->io->writeError(sprintf(' - Skipping recipe for %s: it conflicts with %s %s.', $name, $conflictingPackage, $constraint), true, IOInterface::VERBOSE);
734+
if (!isset($newManifests[$name])) {
735+
// no older recipe found
736+
$this->io->writeError(sprintf(' - Skipping recipe for %s: all versions of the recipe conflict with your package versions.', $name), true, IOInterface::VERBOSE);
734737

735-
continue 2;
736-
}
738+
continue 2;
737739
}
740+
741+
// push the "old" recipe into the $manifests
742+
$manifests[$name] = $newManifests[$name];
743+
$locks[$name] = $newData['locks'][$name];
738744
}
739745

740746
if ($operation instanceof InstallOperation && isset($locks[$name])) {
@@ -991,4 +997,21 @@ public static function getSubscribedEvents(): array
991997

992998
return $events;
993999
}
1000+
1001+
private function doesRecipeConflict(array $recipeData, OperationInterface $operation): bool
1002+
{
1003+
if (empty($recipeData['manifest']['conflict']) || $operation instanceof UninstallOperation) {
1004+
return false;
1005+
}
1006+
1007+
$lockedRepository = $this->composer->getLocker()->getLockedRepository();
1008+
1009+
foreach ($recipeData['manifest']['conflict'] as $conflictingPackage => $constraint) {
1010+
if ($lockedRepository->findPackage($conflictingPackage, $constraint)) {
1011+
return true;
1012+
}
1013+
}
1014+
1015+
return false;
1016+
}
9941017
}

‎tests/FlexTest.php

Lines changed: 103 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
use Composer\Package\Package;
2323
use Composer\Package\RootPackageInterface;
2424
use Composer\Plugin\PluginInterface;
25+
use Composer\Repository\RepositoryInterface;
2526
use Composer\Repository\RepositoryManager;
2627
use Composer\Repository\WritableRepositoryInterface;
2728
use Composer\Script\Event;
@@ -213,6 +214,96 @@ public function testFetchRecipesOrder()
213214
], array_keys($recipes));
214215
}
215216

217+
public function testFetchRecipesWithConflicts()
218+
{
219+
$originalRecipes = [
220+
'locks' => [
221+
'doctrine/annotations' => [
222+
'version' => '1.13',
223+
'recipe' => [
224+
'version' => '1.0',
225+
],
226+
],
227+
'doctrine/doctrine-bundle' => [
228+
'version' => '2.5',
229+
'recipe' => [
230+
'version' => '2.4',
231+
],
232+
],
233+
],
234+
'manifests' => [
235+
'doctrine/annotations' => [
236+
'version' => '1.0',
237+
'manifest' => [],
238+
],
239+
'doctrine/doctrine-bundle' => [
240+
'version' => '2.4',
241+
'manifest' => [
242+
'conflict' => [
243+
'symfony/framework-bundle' => '<5.3',
244+
],
245+
],
246+
],
247+
],
248+
];
249+
$oldRecipes = [
250+
'locks' => [
251+
'doctrine/doctrine-bundle' => [
252+
// 2.5 is still being installed, but 2.3 recipe used
253+
'version' => '2.5',
254+
'recipe' => [
255+
'version' => '2.3',
256+
],
257+
],
258+
],
259+
'manifests' => [
260+
'doctrine/doctrine-bundle' => [
261+
'version' => '2.3',
262+
'manifest' => [],
263+
],
264+
],
265+
];
266+
267+
$io = new BufferIO('', OutputInterface::VERBOSITY_VERBOSE);
268+
$rootPackage = $this->mockRootPackage(['symfony' => ['allow-contrib' => true]]);
269+
270+
$downloader = $this->getMockBuilder(Downloader::class)->disableOriginalConstructor()->getMock();
271+
$downloader->expects($this->exactly(2))
272+
->method('getRecipes')
273+
->willReturnOnConsecutiveCalls($originalRecipes, $oldRecipes);
274+
$downloader->expects($this->any())->method('isEnabled')->willReturn(true);
275+
$downloader->expects($this->once())->method('removeRecipeFromIndex')->with('doctrine/doctrine-bundle', '2.4');
276+
277+
$locker = $this->getMockBuilder(Locker::class)->disableOriginalConstructor()->getMock();
278+
$lockedRepository = $this->getMockBuilder(RepositoryInterface::class)->disableOriginalConstructor()->getMock();
279+
// make the conflicted package show up
280+
$locker->expects($this->any())
281+
->method('getLockedRepository')
282+
->willReturn($lockedRepository);
283+
$lockedRepository->expects($this->once())
284+
->method('findPackage')
285+
->with('symfony/framework-bundle', '<5.3')
286+
->willReturn(new Package('symfony/framework-bundle', '5.2.0', '5.2.0'));
287+
$composer = $this->mockComposer($locker, $rootPackage);
288+
$configurator = $this->mockConfigurator();
289+
$lock = $this->mockLock();
290+
$flex = $this->mockFlexCustom($io, $composer, $configurator, $downloader, $lock);
291+
292+
$operations = [];
293+
foreach ($originalRecipes['manifests'] as $name => $recipeData) {
294+
$package = new Package($name, $recipeData['version'], $recipeData['version']);
295+
296+
$operations[] = new InstallOperation($package);
297+
}
298+
$recipes = $flex->fetchRecipes($operations, true);
299+
300+
$this->assertSame([
301+
'doctrine/annotations',
302+
'doctrine/doctrine-bundle',
303+
], array_keys($recipes));
304+
$this->assertSame('2.3', $recipes['doctrine/doctrine-bundle']->getVersion());
305+
}
306+
216307
public static function getTestPackages(): array
217308
{
218309
return [
@@ -358,14 +449,8 @@ private function mockManager(): RepositoryManager
358449
return $manager;
359450
}
360451

361-
private function mockFlex(BufferIO $io, RootPackageInterface $package, Recipe $recipe = null, array $recipes = [], array $lockerData = []): Flex
452+
private function mockFlexCustom(BufferIO $io, Composer $composer, Configurator $configurator, Downloader $downloader, Lock $lock): Flex
362453
{
363-
$composer = $this->mockComposer($this->mockLocker($lockerData), $package);
364-
365-
$configurator = $this->mockConfigurator($recipe);
366-
$downloader = $this->mockDownloader($recipes);
367-
$lock = $this->mockLock();
368-
369454
return \Closure::bind(function () use ($composer, $io, $configurator, $downloader, $lock) {
370455
$flex = new Flex();
371456
$flex->composer = $composer;
@@ -381,4 +466,15 @@ private function mockFlex(BufferIO $io, RootPackageInterface $package, Recipe $r
381466
return $flex;
382467
}, null, Flex::class)->__invoke();
383468
}
469+
470+
private function mockFlex(BufferIO $io, RootPackageInterface $package, Recipe $recipe = null, array $recipes = [], array $lockerData = []): Flex
471+
{
472+
$composer = $this->mockComposer($this->mockLocker($lockerData), $package);
473+
474+
$configurator = $this->mockConfigurator($recipe);
475+
$downloader = $this->mockDownloader($recipes);
476+
$lock = $this->mockLock();
477+
478+
return $this->mockFlexCustom($io, $composer, $configurator, $downloader, $lock);
479+
}
384480
}

0 commit comments

Comments
 (0)