diff --git a/e2e/scenario8/README.md b/e2e/scenario8/README.md index 9cdf3ed..9af1b6d 100644 --- a/e2e/scenario8/README.md +++ b/e2e/scenario8/README.md @@ -1,3 +1 @@ Tests that extra arguments and options are not lost when forwarding the command to a bin namespace. -In this scenario the dependencies are not install in the bin namespace since the parsed (forwarded) -command is invalid. diff --git a/e2e/scenario9/.gitignore b/e2e/scenario9/.gitignore new file mode 100644 index 0000000..cac9c03 --- /dev/null +++ b/e2e/scenario9/.gitignore @@ -0,0 +1,5 @@ +/actual.txt +/composer.lock +/vendor/ +/vendor-bin/*/composer.lock +/vendor-bin/*/vendor/ diff --git a/e2e/scenario9/README.md b/e2e/scenario9/README.md new file mode 100644 index 0000000..9af1b6d --- /dev/null +++ b/e2e/scenario9/README.md @@ -0,0 +1 @@ +Tests that extra arguments and options are not lost when forwarding the command to a bin namespace. diff --git a/e2e/scenario9/composer.json b/e2e/scenario9/composer.json new file mode 100644 index 0000000..3061d0c --- /dev/null +++ b/e2e/scenario9/composer.json @@ -0,0 +1,21 @@ +{ + "repositories": [ + { + "type": "path", + "url": "../../" + } + ], + "require": { + "bamarni/composer-bin-plugin": "dev-master" + }, + "config": { + "allow-plugins": { + "bamarni/composer-bin-plugin": true + } + }, + "extra": { + "bamarni-bin": { + "forward-command": true + } + } +} diff --git a/e2e/scenario9/expected.txt b/e2e/scenario9/expected.txt new file mode 100644 index 0000000..0515c41 --- /dev/null +++ b/e2e/scenario9/expected.txt @@ -0,0 +1,9 @@ +[bamarni-bin] Checking namespace vendor-bin/composer-unused + +Loading packages +---------------- + + ! [NOTE] Found 0 package(s) to be checked. + + [OK] Done. No required packages to scan. + diff --git a/e2e/scenario9/script.sh b/e2e/scenario9/script.sh new file mode 100755 index 0000000..4decef4 --- /dev/null +++ b/e2e/scenario9/script.sh @@ -0,0 +1,28 @@ +#!/usr/bin/env bash + +set -Eeuo pipefail + +# Set env envariables in order to experience a behaviour closer to what happens +# in the CI locally. It should not hurt to set those in the CI as the CI should +# contain those values. +export CI=1 +export COMPOSER_NO_INTERACTION=1 + +readonly ORIGINAL_WORKING_DIR=$(pwd) + +trap "cd ${ORIGINAL_WORKING_DIR}" err exit + +# Change to script directory +cd "$(dirname "$0")" + +# Ensure we have a clean state +rm -rf actual.txt || true +rm -rf composer.lock || true +rm -rf vendor || true +rm -rf vendor-bin/*/composer.lock || true +rm -rf vendor-bin/*/vendor || true + +composer update + +# Actual command to execute the test itself +composer bin composer-unused unused --no-progress 2>&1 | tee > actual.txt || true diff --git a/e2e/scenario9/vendor-bin/composer-unused/.gitignore b/e2e/scenario9/vendor-bin/composer-unused/.gitignore new file mode 100644 index 0000000..57494c2 --- /dev/null +++ b/e2e/scenario9/vendor-bin/composer-unused/.gitignore @@ -0,0 +1,2 @@ +/composer-unused-dump-* + diff --git a/e2e/scenario9/vendor-bin/composer-unused/composer.json b/e2e/scenario9/vendor-bin/composer-unused/composer.json new file mode 100644 index 0000000..3c5a312 --- /dev/null +++ b/e2e/scenario9/vendor-bin/composer-unused/composer.json @@ -0,0 +1,11 @@ +{ + "require": { + "composer-runtime-api": "^2.2", + "icanhazstring/composer-unused": "~0.7.12" + }, + "config": { + "allow-plugins": { + "icanhazstring/composer-unused": true + } + } +} diff --git a/src/BinCommand.php b/src/BinCommand.php index 9d7e9cc..2e97f5d 100644 --- a/src/BinCommand.php +++ b/src/BinCommand.php @@ -8,8 +8,6 @@ use Composer\Factory; use Composer\IO\IOInterface; use Composer\IO\NullIO; -use ReflectionClass; -use ReflectionProperty; use Symfony\Component\Console\Application; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; @@ -36,15 +34,23 @@ class BinCommand extends BaseCommand private const NAMESPACE_ARG = 'namespace'; + /** + * @var NamespaceApplicationFactory + */ + private $applicationFactory; + /** * @var Logger */ private $logger; - public function __construct(?Logger $logger = null) - { + public function __construct( + ?NamespaceApplicationFactory $applicationFactory = null, + ?Logger $logger = null + ) { parent::__construct('bin'); + $this->applicationFactory = $applicationFactory ?? new FreshInstanceApplicationFactory(); $this->logger = $logger ?? new Logger(new NullIO()); } @@ -106,24 +112,18 @@ public function execute(InputInterface $input, OutputInterface $output): int $input ); - $applicationReflection = new ReflectionClass(Application::class); - $commandsReflection = $applicationReflection->getProperty('commands'); - $commandsReflection->setAccessible(true); - return (self::ALL_NAMESPACES !== $namespace) ? $this->executeInNamespace( $currentWorkingDir, $vendorRoot.'/'.$namespace, $binInput, - $output, - $commandsReflection + $output ) : $this->executeAllNamespaces( $currentWorkingDir, $vendorRoot, $binInput, - $output, - $commandsReflection + $output ); } @@ -139,8 +139,7 @@ private function executeAllNamespaces( string $originalWorkingDir, string $binVendorRoot, InputInterface $input, - OutputInterface $output, - ReflectionProperty $commandsReflection + OutputInterface $output ): int { $namespaces = self::getBinNamespaces($binVendorRoot); @@ -159,8 +158,7 @@ private function executeAllNamespaces( $originalWorkingDir, $namespace, $input, - $output, - $commandsReflection + $output ); } @@ -171,8 +169,7 @@ private function executeInNamespace( string $originalWorkingDir, string $namespace, InputInterface $input, - OutputInterface $output, - ReflectionProperty $commandsReflection + OutputInterface $output ): int { $this->logger->logStandard( sprintf( @@ -194,25 +191,21 @@ private function executeInNamespace( return self::FAILURE; } - $application = $this->getApplication(); - $commands = $application->all(); + // Use a new application: this avoids a variety of issues: + // - A command may be added in a namespace which may cause side effects + // when executed in another namespace afterwards (since it is the same + // process). + // - Different plugins may be registered in the namespace in which case + // an already executed application will not pick that up. + $namespaceApplication = $this->applicationFactory->create( + $this->getApplication() + ); // It is important to clean up the state either for follow-up plugins // or for example the execution in the next namespace. - $cleanUp = function () use ( - $originalWorkingDir, - $commandsReflection, - $application, - $commands - ): void { + $cleanUp = function () use ($originalWorkingDir): void { $this->chdir($originalWorkingDir); $this->resetComposers(); - - // When executing composer in a namespace, some commands may be - // registered. - // For example when scripts are registered in the composer.json, - // Composer adds them as commands to the application. - $commandsReflection->setValue($application, $commands); }; $this->chdir($namespace); @@ -229,7 +222,7 @@ private function executeInNamespace( ); try { - $exitCode = $application->doRun($namespaceInput, $output); + $exitCode = $namespaceApplication->doRun($namespaceInput, $output); } catch (Throwable $executionFailed) { // Ensure we do the cleanup even in case of failure $cleanUp(); diff --git a/src/FreshInstanceApplicationFactory.php b/src/FreshInstanceApplicationFactory.php new file mode 100644 index 0000000..d791340 --- /dev/null +++ b/src/FreshInstanceApplicationFactory.php @@ -0,0 +1,15 @@ +application = new Application(); $this->application->addCommands([ - new BinCommand(), + new BinCommand(new ReuseApplicationFactory()), $this->testCommand, ]); } diff --git a/tests/EndToEndTest.php b/tests/EndToEndTest.php index 81ed11e..c35bf51 100644 --- a/tests/EndToEndTest.php +++ b/tests/EndToEndTest.php @@ -7,16 +7,20 @@ use PHPUnit\Framework\TestCase; use Symfony\Component\Finder\Finder; use Symfony\Component\Process\Process; +use function array_map; use function basename; use function dirname; +use function explode; use function file_exists; use function file_get_contents; use function getcwd; +use function implode; use function preg_replace; use function realpath; use function sprintf; use function str_replace; use function trim; +use const PHP_EOL; /** * @group e2e @@ -39,7 +43,9 @@ public function test_it_passes_the_e2e_test(string $scenarioPath): void 10 ); - $expected = file_get_contents($scenarioPath.'/expected.txt'); + $expected = self::normalizeTrailingWhitespacesAndLineReturns( + file_get_contents($scenarioPath.'/expected.txt') + ); $scenarioProcess->run(); @@ -167,6 +173,14 @@ private static function retrieveActualOutput( $normalizedContent ); - return $normalizedContent; + return self::normalizeTrailingWhitespacesAndLineReturns($normalizedContent); + } + + private static function normalizeTrailingWhitespacesAndLineReturns(string $value): string + { + return implode( + "\n", + array_map('rtrim', explode(PHP_EOL, $value)) + ); } } diff --git a/tests/Fixtures/ReuseApplicationFactory.php b/tests/Fixtures/ReuseApplicationFactory.php new file mode 100644 index 0000000..eeec010 --- /dev/null +++ b/tests/Fixtures/ReuseApplicationFactory.php @@ -0,0 +1,16 @@ +