bug #1050 Fix unpack logic (nicolas-grekas)

This PR was merged into the 2.x branch.

Discussion
----------

Fix unpack logic

Fix #1030
and a few other things when unpacking packs.

Working on tests now.

Commits
-------

d66470c Fix unpack logic

Author: nicolas-grekas

src/Downloader.php

@@ -18,6 +18,7 @@
 use Composer\DependencyResolver\Operation\UpdateOperation;
 use Composer\IO\IOInterface;
 use Composer\Json\JsonFile;
+use Composer\Package\BasePackage;
 use Composer\Util\Http\Response as ComposerResponse;
 use Composer\Util\HttpDownloader;
 use Composer\Util\Loop;
@@ -316,6 +317,30 @@ public function removeRecipeFromIndex(string $packageName, string $version)
         unset($this->index[$packageName][$version]);
     }
 
+    public function getSymfonyPacks(array $packages)
+    {
+        $packs = [];
+        foreach ($this->composer->getRepositoryManager()->getRepositories() as $repo) {
+            if (!$packages) {
+                break;
+            }
+
+            $result = $repo->loadPackages($packages, BasePackage::$stabilities, []);
+
+            foreach ($result['packages'] ?? [] as $package) {
+                if (!isset($packages[$package->getName()])) {
+                    continue;
+                }
+                if ('symfony-pack' === $package->getType()) {
+                    $packs[$package->getName()] = true;
+                }
+                unset($packages[$package->getName()]);
+            }
+        }
+
+        return array_keys($packs);
+    }
+
     /**
      * Fetches and decodes JSON HTTP response bodies.
      */

src/Flex.php

@@ -77,14 +77,12 @@ class Flex implements PluginInterface, EventSubscriberInterface
     private $operations = [];
     private $lock;
     private $displayThanksReminder = 0;
-    private $dryRun = false;
     private $reinstall;
     private static $activated = true;
     private static $aliasResolveCommands = [
         'require' => true,
         'update' => false,
         'remove' => false,
-        'unpack' => true,
     ];
     private $filter;
 
@@ -108,6 +106,8 @@ class_exists(__NAMESPACE__.str_replace('/', '\\', substr($file, \strlen(__DIR__)
             }
         }
 
+        $composer->getInstallationManager()->addInstaller(new SymfonyPackInstaller($io));
+
         $this->composer = $composer;
         $this->io = $io;
         $this->config = $composer->getConfig();
@@ -122,7 +122,7 @@ class_exists(__NAMESPACE__.str_replace('/', '\\', substr($file, \strlen(__DIR__)
 
         $symfonyRequire = preg_replace('/\.x$/', '.x-dev', getenv('SYMFONY_REQUIRE') ?: ($composer->getPackage()->getExtra()['symfony']['require'] ?? ''));
 
-        $rfs = Factory::createHttpDownloader($this->io, $this->config);
+        $rfs = $composer->getLoop()->getHttpDownloader();
 
         $this->downloader = $downloader = new Downloader($composer, $io, $rfs);
 
@@ -221,14 +221,6 @@ public function configureInstaller()
         foreach ($backtrace as $trace) {
             if (isset($trace['object']) && $trace['object'] instanceof Installer) {
                 $this->installer = $trace['object']->setSuggestedPackagesReporter(new SuggestedPackagesReporter(new NullIO()));
-
-                $updateAllowList = \Closure::bind(function () {
-                    return $this->updateAllowList;
-                }, $this->installer, $this->installer)();
-
-                if (['php' => 0] === $updateAllowList) {
-                    $this->dryRun = true; // prevent recipes from being uninstalled when removing a pack
-                }
             }
 
             if (isset($trace['object']) && $trace['object'] instanceof GlobalCommand) {
@@ -254,7 +246,6 @@ public function configureProject(Event $event)
         $file = Factory::getComposerFile();
         $contents = file_get_contents($file);
         $manipulator = new JsonManipulator($contents);
-        $json = JsonFile::parseJson($contents);
 
         // new projects are most of the time proprietary
         $manipulator->addMainKey('license', 'proprietary');
@@ -351,7 +342,7 @@ public function update(Event $event, $operations = [])
 
         file_put_contents($file, $manipulator->getContents());
 
-        $this->reinstall($event, true);
+        $this->reinstall($event);
     }
 
     public function install(Event $event)
@@ -738,7 +729,7 @@ private function formatOrigin(Recipe $recipe): string
 
     private function shouldRecordOperation(OperationInterface $operation, bool $isDevMode, ?Composer $composer = null): bool
     {
-        if ($this->dryRun || $this->reinstall) {
+        if ($this->reinstall) {
             return false;
         }
 
@@ -794,31 +785,29 @@ private function unpack(Event $event)
             }
         }
 
-        $unpacker = new Unpacker($this->composer, new PackageResolver($this->downloader), $this->dryRun);
+        $unpacker = new Unpacker($this->composer, new PackageResolver($this->downloader));
         $result = $unpacker->unpack($unpackOp);
 
         if (!$result->getUnpacked()) {
             return;
         }
 
-        $this->io->writeError('<info>Unpacking Symfony packs</>');
         foreach ($result->getUnpacked() as $pkg) {
             $this->io->writeError(\sprintf('  - Unpacked <info>%s</>', $pkg->getName()));
         }
 
         $unpacker->updateLock($result, $this->io);
-
-        $this->reinstall($event, false);
     }
 
-    private function reinstall(Event $event, bool $update)
+    private function reinstall(Event $event)
     {
         $this->reinstall = false;
         $event->stopPropagation();
 
         $ed = $this->composer->getEventDispatcher();
         $disableScripts = !method_exists($ed, 'setRunScripts') || !((array) $ed)["\0*\0runScripts"];
         $composer = Factory::create($this->io, null, false, $disableScripts);
+        $composer->getInstallationManager()->addInstaller(new SymfonyPackInstaller($this->io));
 
         $installer = clone $this->installer;
         $installer->__construct(
@@ -836,10 +825,6 @@ private function reinstall(Event $event, bool $update)
             $installer->setPlatformRequirementFilter(((array) $this->installer)["\0*\0platformRequirementFilter"]);
         }
 
-        if (!$update) {
-            $installer->setUpdateAllowList(['php']);
-        }
-
         $installer->run();
 
         $this->io->write($this->postInstallOutput);

src/PackageResolver.php

@@ -14,6 +14,7 @@
 use Composer\Factory;
 use Composer\Package\Version\VersionParser;
 use Composer\Repository\PlatformRepository;
+use Composer\Semver\Constraint\MatchAllConstraint;
 
 /**
  * @author Fabien Potencier <[email protected]>
@@ -45,26 +46,41 @@ public function resolve(array $arguments = [], bool $isRequire = false): array
         // second pass to resolve versions
         $versionParser = new VersionParser();
         $requires = [];
+        $toGuess = [];
         foreach ($versionParser->parseNameVersionPairs($packages) as $package) {
-            $requires[] = $package['name'].$this->parseVersion($package['name'], $package['version'] ?? '', $isRequire);
+            $version = $this->parseVersion($package['name'], $package['version'] ?? '', $isRequire);
+            if ('' !== $version) {
+                unset($toGuess[$package['name']]);
+            } elseif (!isset($requires[$package['name']])) {
+                $toGuess[$package['name']] = new MatchAllConstraint();
+            }
+            $requires[$package['name']] = $package['name'].$version;
         }
 
-        return array_unique($requires);
+        if ($toGuess && $isRequire) {
+            foreach ($this->downloader->getSymfonyPacks($toGuess) as $package) {
+                $requires[$package] .= ':*';
+            }
+        }
+
+        return array_values($requires);
     }
 
     public function parseVersion(string $package, string $version, bool $isRequire): string
     {
+        $guess = 'guess' === ($version ?: 'guess');
+
         if (0 !== strpos($package, 'symfony/')) {
-            return $version ? ':'.$version : '';
+            return $guess ? '' : ':'.$version;
         }
 
         $versions = $this->downloader->getVersions();
 
         if (!isset($versions['splits'][$package])) {
-            return $version ? ':'.$version : '';
+            return $guess ? '' : ':'.$version;
         }
 
-        if (!$version || '*' === $version) {
+        if ($guess || '*' === $version) {
             try {
                 $config = @json_decode(file_get_contents(Factory::getComposerFile()), true);
             } finally {

src/SymfonyPackInstaller.php

@@ -0,0 +1,22 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <[email protected]>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Flex;
+
+use Composer\Installer\MetapackageInstaller;
+
+class SymfonyPackInstaller extends MetapackageInstaller
+{
+    public function supports($packageType): bool
+    {
+        return 'symfony-pack' === $packageType;
+    }
+}

src/Unpacker.php

@@ -29,14 +29,12 @@ class Unpacker
 {
     private $composer;
     private $resolver;
-    private $dryRun;
     private $versionParser;
 
-    public function __construct(Composer $composer, PackageResolver $resolver, bool $dryRun)
+    public function __construct(Composer $composer, PackageResolver $resolver)
     {
         $this->composer = $composer;
         $this->resolver = $resolver;
-        $this->dryRun = $dryRun;
         $this->versionParser = new VersionParser();
     }
 
@@ -131,7 +129,7 @@ public function unpack(Operation $op, ?Result $result = null, &$links = [], bool
             }
         }
 
-        if ($this->dryRun || 1 < \func_num_args()) {
+        if (1 < \func_num_args()) {
             return $result;
         }
 
@@ -140,6 +138,13 @@ public function unpack(Operation $op, ?Result $result = null, &$links = [], bool
         $jsonStored = json_decode($jsonContent, true);
         $jsonManipulator = new JsonManipulator($jsonContent);
 
+        foreach ($result->getUnpacked() as $pkg) {
+            $localRepo->removePackage($pkg);
+            $localRepo->setDevPackageNames(array_diff($localRepo->getDevPackageNames(), [$pkg->getName()]));
+            $jsonManipulator->removeSubNode('require', $pkg->getName());
+            $jsonManipulator->removeSubNode('require-dev', $pkg->getName());
+        }
+
         foreach ($links as $link) {
             // nothing to do, package is already present in the "require" section
             if (isset($jsonStored['require'][$link['name']])) {
@@ -197,12 +202,12 @@ public function updateLock(Result $result, IOInterface $io): void
         $lockData['content-hash'] = Locker::getContentHash($jsonContent);
         $lockFile = new JsonFile(substr($json->getPath(), 0, -4).'lock', null, $io);
 
-        if (!$this->dryRun) {
-            $lockFile->write($lockData);
-        }
+        $lockFile->write($lockData);
 
-        // force removal of files under vendor/
         $locker = new Locker($io, $lockFile, $this->composer->getInstallationManager(), $jsonContent);
         $this->composer->setLocker($locker);
+
+        $localRepo = $this->composer->getRepositoryManager()->getLocalRepository();
+        $localRepo->write($localRepo->getDevMode() ?? true, $this->composer->getInstallationManager());
     }
 }

tests/FlexTest.php

@@ -15,6 +15,7 @@
 use Composer\Config;
 use Composer\DependencyResolver\Operation\InstallOperation;
 use Composer\Factory;
+use Composer\Installer\InstallationManager;
 use Composer\Installer\PackageEvent;
 use Composer\IO\BufferIO;
 use Composer\Package\Link;
@@ -29,6 +30,8 @@
 use Composer\Script\Event;
 use Composer\Script\ScriptEvents;
 use Composer\Semver\Constraint\MatchAllConstraint;
+use Composer\Util\HttpDownloader;
+use Composer\Util\Loop;
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\Console\Output\OutputInterface;
 use Symfony\Flex\Configurator;
@@ -458,6 +461,10 @@ private function mockComposer(Locker $locker, RootPackageInterface $package, ?Co
         $composer->setConfig($config);
         $composer->setLocker($locker);
         $composer->setPackage($package);
+        $composer->setInstallationManager($this->getMockBuilder(InstallationManager::class)->disableOriginalConstructor()->getMock());
+
+        $loop = new Loop(new HttpDownloader(new BufferIO('', OutputInterface::VERBOSITY_VERBOSE), $config));
+        $composer->setLoop($loop);
 
         return $composer;
     }

tests/PackageResolverTest.php

@@ -128,6 +128,9 @@ private function getResolver()
                 'validator' => 'symfony/validator',
                 'lock' => 'symfony/lock',
             ]);
+        $downloader->expects($this->any())
+            ->method('getSymfonyPacks')
+            ->willReturn([]);
 
         return new PackageResolver($downloader);
     }

tests/UnpackerTest.php

@@ -71,7 +71,7 @@ public function testDoNotDuplicateEntry(): void
 
         $resolver = $this->getMockBuilder(PackageResolver::class)->disableOriginalConstructor()->getMock();
 
-        $unpacker = new Unpacker($composer, $resolver, false);
+        $unpacker = new Unpacker($composer, $resolver);
 
         $operation = new Operation(true, false);
         $operation->addPackage('pack_foo', '*', false);