From d66470c96047fa4e922a7cffae964eab03d551bd Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Thu, 22 May 2025 17:37:44 +0200 Subject: [PATCH 1/3] Fix unpack logic --- src/Downloader.php | 25 +++++++++++++++++++++++++ src/Flex.php | 31 ++++++++----------------------- src/PackageResolver.php | 26 +++++++++++++++++++++----- src/SymfonyPackInstaller.php | 22 ++++++++++++++++++++++ src/Unpacker.php | 21 +++++++++++++-------- tests/FlexTest.php | 7 +++++++ tests/PackageResolverTest.php | 3 +++ tests/UnpackerTest.php | 2 +- 8 files changed, 100 insertions(+), 37 deletions(-) create mode 100644 src/SymfonyPackInstaller.php diff --git a/src/Downloader.php b/src/Downloader.php index 3eda8f55e..e6bb56734 100644 --- a/src/Downloader.php +++ b/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. */ diff --git a/src/Flex.php b/src/Flex.php index b7d102092..e41e73dc9 100644 --- a/src/Flex.php +++ b/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,24 +785,21 @@ 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('Unpacking Symfony packs'); foreach ($result->getUnpacked() as $pkg) { $this->io->writeError(\sprintf(' - Unpacked %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(); @@ -819,6 +807,7 @@ private function reinstall(Event $event, bool $update) $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); diff --git a/src/PackageResolver.php b/src/PackageResolver.php index ddb054d21..37987fe84 100644 --- a/src/PackageResolver.php +++ b/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 @@ -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 { diff --git a/src/SymfonyPackInstaller.php b/src/SymfonyPackInstaller.php new file mode 100644 index 000000000..1a46bda85 --- /dev/null +++ b/src/SymfonyPackInstaller.php @@ -0,0 +1,22 @@ + + * + * 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; + } +} diff --git a/src/Unpacker.php b/src/Unpacker.php index e758fe284..da9d228df 100644 --- a/src/Unpacker.php +++ b/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()); } } diff --git a/tests/FlexTest.php b/tests/FlexTest.php index f4a315baa..1bba9deb0 100644 --- a/tests/FlexTest.php +++ b/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; } diff --git a/tests/PackageResolverTest.php b/tests/PackageResolverTest.php index 31e7af221..bb6507e1d 100644 --- a/tests/PackageResolverTest.php +++ b/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); } diff --git a/tests/UnpackerTest.php b/tests/UnpackerTest.php index 4ca774138..785da05a8 100644 --- a/tests/UnpackerTest.php +++ b/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); From 41275d619501ecdb4f95bd9af00c4d47bed5734b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jib=C3=A9=20Barth?= Date: Tue, 21 May 2024 15:53:11 +0200 Subject: [PATCH 2/3] [AssetMapper] Allow to define entrypoint in importmap.php --- src/PackageJsonSynchronizer.php | 38 +++++++++++---- .../assets/package.json | 7 +++ .../symfony/new-package/assets/package.json | 5 ++ tests/PackageJsonSynchronizerTest.php | 48 +++++++++++++++---- 4 files changed, 81 insertions(+), 17 deletions(-) create mode 100644 tests/Fixtures/packageJson/vendor/symfony/importmap-invalid-constraint-package/assets/package.json diff --git a/src/PackageJsonSynchronizer.php b/src/PackageJsonSynchronizer.php index 698894c5b..5daec3ba8 100644 --- a/src/PackageJsonSynchronizer.php +++ b/src/PackageJsonSynchronizer.php @@ -104,7 +104,7 @@ private function removeObsoletePackageJsonLinks(): bool foreach (['dependencies' => $jsDependencies, 'devDependencies' => $jsDevDependencies] as $key => $packages) { foreach ($packages as $name => $version) { - if ('@' !== $name[0] || 0 !== strpos($version, 'file:'.$this->vendorDir.'/') || false === strpos($version, '/assets')) { + if ('@' !== $name[0] || !str_starts_with($version, 'file:'.$this->vendorDir.'/') || !str_contains($version, '/assets')) { continue; } if (file_exists($this->rootDir.'/'.substr($version, 5).'/package.json')) { @@ -149,20 +149,36 @@ private function resolveImportMapPackages($phpPackage): array $dependencies = []; foreach ($packageJson->read()['symfony']['importmap'] ?? [] as $importMapName => $constraintConfig) { - if (\is_array($constraintConfig)) { - $constraint = $constraintConfig['version'] ?? []; - $package = $constraintConfig['package'] ?? $importMapName; - } else { + if (\is_string($constraintConfig)) { + // Matches string constraint, like "^3.0" or "path:%PACKAGE%/script.js" $constraint = $constraintConfig; $package = $importMapName; + $entrypoint = false; + } elseif (\is_array($constraintConfig)) { + // Matches array constraint, like {"version":"^3.0"} or {"version":"path:%PACKAGE%/script.js","entrypoint":true} + // Note that non-path assets can't be entrypoint + $constraint = $constraintConfig['version'] ?? ''; + $package = $constraintConfig['package'] ?? $importMapName; + $entrypoint = $constraintConfig['entrypoint'] ?? false; + } else { + throw new \InvalidArgumentException(\sprintf('Invalid constraint config for key "%s": "%s" given, array or string expected.', $importMapName, var_export($constraintConfig, true))); } - if (0 === strpos($constraint, 'path:')) { + // When "$constraintConfig" matches one of the following cases: + // - "entrypoint:%PACKAGE%/script.js" + // - {"version": "entrypoint:%PACKAGE%/script.js"} + if (str_starts_with($constraint, 'entrypoint:')) { + $entrypoint = true; + $constraint = substr_replace($constraint, 'path:', 0, \strlen('entrypoint:')); + } + + if (str_starts_with($constraint, 'path:')) { $path = substr($constraint, 5); $path = str_replace('%PACKAGE%', \dirname($packageJson->getPath()), $path); $dependencies[$importMapName] = [ 'path' => $path, + 'entrypoint' => $entrypoint, ]; continue; @@ -239,7 +255,7 @@ private function shouldUpdateConstraint(string $existingConstraint, string $cons } /** - * @param array $importMapEntries + * @param array $importMapEntries */ private function updateImportMap(array $importMapEntries): void { @@ -264,11 +280,15 @@ private function updateImportMap(array $importMapEntries): void continue; } - $this->io->writeError(sprintf('Updating package %s from %s to %s.', $name, $version, $versionConstraint)); + $this->io->writeError(\sprintf('Updating package %s from %s to %s.', $name, $version, $versionConstraint)); } if (isset($importMapEntry['path'])) { $arguments = [$name, '--path='.$importMapEntry['path']]; + if (isset($importMapEntry['entrypoint']) && true === $importMapEntry['entrypoint']) { + $arguments[] = '--entrypoint'; + } + $this->scriptExecutor->execute( 'symfony-cmd', 'importmap:require', @@ -293,7 +313,7 @@ private function updateImportMap(array $importMapEntries): void continue; } - throw new \InvalidArgumentException(sprintf('Invalid importmap entry: "%s".', var_export($importMapEntry, true))); + throw new \InvalidArgumentException(\sprintf('Invalid importmap entry: "%s".', var_export($importMapEntry, true))); } } diff --git a/tests/Fixtures/packageJson/vendor/symfony/importmap-invalid-constraint-package/assets/package.json b/tests/Fixtures/packageJson/vendor/symfony/importmap-invalid-constraint-package/assets/package.json new file mode 100644 index 000000000..ca57c76dc --- /dev/null +++ b/tests/Fixtures/packageJson/vendor/symfony/importmap-invalid-constraint-package/assets/package.json @@ -0,0 +1,7 @@ +{ + "symfony": { + "importmap": { + "@symfony/test": true + } + } +} diff --git a/tests/Fixtures/packageJson/vendor/symfony/new-package/assets/package.json b/tests/Fixtures/packageJson/vendor/symfony/new-package/assets/package.json index e4003ea6c..419528379 100644 --- a/tests/Fixtures/packageJson/vendor/symfony/new-package/assets/package.json +++ b/tests/Fixtures/packageJson/vendor/symfony/new-package/assets/package.json @@ -14,6 +14,11 @@ "@hotcake/foo": "^1.9.0", "@symfony/new-package": { "version": "path:%PACKAGE%/dist/loader.js" + }, + "@symfony/new-package/entry.js": "entrypoint:%PACKAGE%/entry.js", + "@symfony/new-package/entry2.js": { + "version": "path:%PACKAGE%/entry2.js", + "entrypoint": true } } }, diff --git a/tests/PackageJsonSynchronizerTest.php b/tests/PackageJsonSynchronizerTest.php index 7e6b27c0d..702a3881c 100644 --- a/tests/PackageJsonSynchronizerTest.php +++ b/tests/PackageJsonSynchronizerTest.php @@ -323,11 +323,16 @@ public function testSynchronizeAssetMapperNewPackage() file_put_contents($this->tempDir.'/importmap.php', 'tempDir.'/vendor/symfony/new-package/assets/dist/loader.js'; - $this->scriptExecutor->expects($this->exactly(2)) + $entrypointPath = $this->tempDir.'/vendor/symfony/new-package/assets/entry.js'; + $secondEntrypointPath = $this->tempDir.'/vendor/symfony/new-package/assets/entry2.js'; + + $this->scriptExecutor->expects($this->exactly(4)) ->method('execute') ->withConsecutive( ['symfony-cmd', 'importmap:require', ['@hotcake/foo@^1.9.0']], - ['symfony-cmd', 'importmap:require', ['@symfony/new-package', '--path='.$fileModulePath]] + ['symfony-cmd', 'importmap:require', ['@symfony/new-package', '--path='.$fileModulePath]], + ['symfony-cmd', 'importmap:require', ['@symfony/new-package/entry.js', '--path='.$entrypointPath, '--entrypoint']], + ['symfony-cmd', 'importmap:require', ['@symfony/new-package/entry2.js', '--path='.$secondEntrypointPath, '--entrypoint']], ); $this->synchronizer->synchronize([ @@ -396,14 +401,19 @@ public function testSynchronizeAssetMapperUpgradesPackageIfNeeded() 'version' => '1.8.0', ], ]; - file_put_contents($this->tempDir.'/importmap.php', sprintf('tempDir.'/importmap.php', \sprintf('tempDir.'/vendor/symfony/new-package/assets/dist/loader.js'; - $this->scriptExecutor->expects($this->exactly(2)) + $entrypointPath = $this->tempDir.'/vendor/symfony/new-package/assets/entry.js'; + $secondEntrypointPath = $this->tempDir.'/vendor/symfony/new-package/assets/entry2.js'; + + $this->scriptExecutor->expects($this->exactly(4)) ->method('execute') ->withConsecutive( ['symfony-cmd', 'importmap:require', ['@hotcake/foo@^1.9.0']], - ['symfony-cmd', 'importmap:require', ['@symfony/new-package', '--path='.$fileModulePath]] + ['symfony-cmd', 'importmap:require', ['@symfony/new-package', '--path='.$fileModulePath]], + ['symfony-cmd', 'importmap:require', ['@symfony/new-package/entry.js', '--path='.$entrypointPath, '--entrypoint']], + ['symfony-cmd', 'importmap:require', ['@symfony/new-package/entry2.js', '--path='.$secondEntrypointPath, '--entrypoint']] ); $this->synchronizer->synchronize([ @@ -421,14 +431,21 @@ public function testSynchronizeAssetMapperSkipsUpgradeIfAlreadySatisfied() // constraint in package.json is ^1.9.0 'version' => '1.9.1', ], + '@symfony/new-package/entry2.js' => [ + 'path' => './vendor/symfony/new-package/assets/entry2.js', + 'entrypoint' => true, + ], ]; - file_put_contents($this->tempDir.'/importmap.php', sprintf('tempDir.'/importmap.php', \sprintf('tempDir.'/vendor/symfony/new-package/assets/dist/loader.js'; - $this->scriptExecutor->expects($this->once()) + $entrypointPath = $this->tempDir.'/vendor/symfony/new-package/assets/entry.js'; + + $this->scriptExecutor->expects($this->exactly(2)) ->method('execute') ->withConsecutive( - ['symfony-cmd', 'importmap:require', ['@symfony/new-package', '--path='.$fileModulePath]] + ['symfony-cmd', 'importmap:require', ['@symfony/new-package', '--path='.$fileModulePath]], + ['symfony-cmd', 'importmap:require', ['@symfony/new-package/entry.js', '--path='.$entrypointPath, '--entrypoint']], ); $this->synchronizer->synchronize([ @@ -438,4 +455,19 @@ public function testSynchronizeAssetMapperSkipsUpgradeIfAlreadySatisfied() ], ]); } + + public function testExceptionWhenInvalidImportMapConstraint() + { + file_put_contents($this->tempDir.'/importmap.php', 'expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('Invalid constraint config for key "@symfony/test": "true" given, array or string expected.'); + + $this->synchronizer->synchronize([ + [ + 'name' => 'symfony/importmap-invalid-constraint-package', + 'keywords' => ['symfony-ux'], + ], + ]); + } } From 5d743b3b78fabe9f3146586d77b0a1f9292851fc Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Fri, 23 May 2025 13:41:40 +0200 Subject: [PATCH 3/3] Fix flex upgrades --- src/Flex.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Flex.php b/src/Flex.php index e41e73dc9..a17aedec9 100644 --- a/src/Flex.php +++ b/src/Flex.php @@ -785,7 +785,7 @@ private function unpack(Event $event) } } - $unpacker = new Unpacker($this->composer, new PackageResolver($this->downloader)); + $unpacker = new Unpacker($this->composer, new PackageResolver($this->downloader), false); // 3rd arg to ease upgrading from flex <= 2.6.0 $result = $unpacker->unpack($unpackOp); if (!$result->getUnpacked()) {