From d1b9343e107ee4d2d564a3b5e5e1d4315959b579 Mon Sep 17 00:00:00 2001 From: Markus Staab <47448731+clxmstaab@users.noreply.github.com> Date: Wed, 6 Jul 2022 22:18:08 +0200 Subject: [PATCH 01/57] Fix typo in doc (#69) --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 46c6349..e26339c 100644 --- a/README.md +++ b/README.md @@ -206,7 +206,7 @@ There is a `forward mode` which is disabled by default. This can be activated by } ``` -If this mode is activated, all your `composer install` and `composer update` commands are forwared to all bin directories. +If this mode is activated, all your `composer install` and `composer update` commands are forwarded to all bin directories. This is an replacement for the tasks shown in section [Auto-installation](#auto-installation). ### Reduce clutter From 64495ff086ac7c0454f049f07b154a19777c4517 Mon Sep 17 00:00:00 2001 From: Markus Staab <47448731+clxmstaab@users.noreply.github.com> Date: Wed, 6 Jul 2022 22:18:28 +0200 Subject: [PATCH 02/57] Fix missing phpdoc-type (#70) --- src/BinCommand.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/BinCommand.php b/src/BinCommand.php index e7fd540..64a9dc2 100644 --- a/src/BinCommand.php +++ b/src/BinCommand.php @@ -150,7 +150,7 @@ private function resetComposers(ComposerApplication $application) } /** - * @param $dir + * @param string $dir * @return void */ private function chdir($dir) From 6232a7be205a058150997cde936198d775d8ac6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= Date: Wed, 6 Jul 2022 22:27:14 +0200 Subject: [PATCH 03/57] Add GitHub Actions workflow (#74) --- .github/workflows/ci.yaml | 46 +++++++++++++++++++++++++++++++++++ .travis.yml | 51 --------------------------------------- 2 files changed, 46 insertions(+), 51 deletions(-) create mode 100644 .github/workflows/ci.yaml delete mode 100644 .travis.yml diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml new file mode 100644 index 0000000..e71e8d6 --- /dev/null +++ b/.github/workflows/ci.yaml @@ -0,0 +1,46 @@ +name: "CI" + +on: + push: + branches: + - "main" + - "master" + pull_request: null + +jobs: + tests: + runs-on: "ubuntu-latest" + name: "Test PHP${{ matrix.php }}" + strategy: + fail-fast: true + matrix: + php: + - "7.2" + - "7.3" + - "7.4" + - "8.0" + - "8.1" + + steps: + - name: "Check out repository code" + uses: "actions/checkout@v2" + + - name: "Setup PHP" + uses: "shivammathur/setup-php@v2" + with: + php-version: "${{ matrix.php }}" + tools: "composer" + + - name: "Configure Composer bin directory" + run: "composer global config repositories.bin path $PWD" + + - name: "Install Composer bin plugin" + run: "composer global require bamarni/composer-bin-plugin:dev-master" + + - name: "Install Composer dependencies" + uses: "ramsey/composer-install@v2" + with: + dependency-versions: "highest" + + - name: "Run tests" + run: "vendor/bin/phpunit" diff --git a/.travis.yml b/.travis.yml deleted file mode 100644 index b1da73c..0000000 --- a/.travis.yml +++ /dev/null @@ -1,51 +0,0 @@ -language: php - -cache: - directories: - - "$HOME/.composer/cache" - -jobs: - include: - - name: PHP 5.5.9 - php: 5.5.9 - dist: trusty - env: COMPOSER_FLAGS='--prefer-lowest' - - name: PHP 5.5 - php: 5.5 - dist: trusty - - name: PHP 5.6 - php: 5.6 - dist: xenial - - name: PHP 7.0 - php: 7.0 - dist: xenial - - name: PHP 7.1 - php: 7.1 - dist: bionic - - name: PHP 7.2 - php: 7.2 - dist: bionic - - name: PHP 7.3 - php: 7.3 - dist: bionic - - name: PHP 7.4 - php: 7.4 - dist: bionic - - name: PHP 8.0 - php: nightly - dist: bionic - allow_failures: - - php: nightly - -before_install: - - composer global config repositories.bin path $PWD - - composer global require bamarni/composer-bin-plugin:dev-master - -install: - - composer update --no-interaction - -script: - - vendor/bin/phpunit - -notifications: - email: false From d917dca60ef842c8a180f718d10cb4a8ee4b9473 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= Date: Wed, 6 Jul 2022 23:08:17 +0200 Subject: [PATCH 04/57] Tweak the CI (#75) --- .github/workflows/{ci.yaml => tests.yaml} | 19 +++++++++++-------- README.md | 12 ++++++++++++ composer.json | 6 +++++- 3 files changed, 28 insertions(+), 9 deletions(-) rename .github/workflows/{ci.yaml => tests.yaml} (63%) diff --git a/.github/workflows/ci.yaml b/.github/workflows/tests.yaml similarity index 63% rename from .github/workflows/ci.yaml rename to .github/workflows/tests.yaml index e71e8d6..a99484a 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/tests.yaml @@ -1,4 +1,4 @@ -name: "CI" +name: "Tests" on: push: @@ -10,9 +10,9 @@ on: jobs: tests: runs-on: "ubuntu-latest" - name: "Test PHP${{ matrix.php }}" + name: "Test PHP ${{ matrix.php }}" strategy: - fail-fast: true + fail-fast: false matrix: php: - "7.2" @@ -31,16 +31,19 @@ jobs: php-version: "${{ matrix.php }}" tools: "composer" - - name: "Configure Composer bin directory" - run: "composer global config repositories.bin path $PWD" - - - name: "Install Composer bin plugin" - run: "composer global require bamarni/composer-bin-plugin:dev-master" + - name: "Configure global composer" + run: | + composer global config repositories.bin path $PWD + composer global config allow-plugins.bamarni/composer-bin-plugin true + composer global require bamarni/composer-bin-plugin:dev-${GITHUB_SHA} - name: "Install Composer dependencies" uses: "ramsey/composer-install@v2" with: dependency-versions: "highest" + - name: "Validate composer.json" + run: "composer validate --strict --no-check-lock" + - name: "Run tests" run: "vendor/bin/phpunit" diff --git a/README.md b/README.md index e26339c..4ce1cbb 100644 --- a/README.md +++ b/README.md @@ -21,6 +21,7 @@ 1. [Forward mode](#forward-mode) 1. [Reduce clutter](#reduce-clutter) 1. [Related plugins](#related-plugins) +1. [Contributing](#contributing) ## Why? @@ -229,6 +230,17 @@ vendor-bin/**/composer.lock binary * [theofidry/composer-inheritance-plugin][7]: Opinionated version of [Wikimedia composer-merge-plugin][8] to work in pair with this plugin. +## Contributing + +Before getting started, you need to install the plugin globally: + +```bash +$ composer global require --dev bamarni/composer-bin-plugin +$ composer install # now works +$ vendor/bin/phpunit # run the tests +``` + + [1]: https://github.com/etsy/phan [2]: https://github.com/phpmetrics/PhpMetrics [3]: https://getcomposer.org/doc/06-config.md#bin-dir diff --git a/composer.json b/composer.json index 89752c4..df57899 100644 --- a/composer.json +++ b/composer.json @@ -17,10 +17,14 @@ }, "require-dev": { "composer/composer": "^1.0 || ^2.0", + "phpstan/extension-installer": "^1.1", "symfony/console": "^2.5 || ^3.0 || ^4.0" }, "config": { - "sort-packages": true + "sort-packages": true, + "allow-plugins": { + "phpstan/extension-installer": true + } }, "extra": { "class": "Bamarni\\Composer\\Bin\\Plugin" From af3670351d031d7f167b266fda4a3a95346b43cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= Date: Wed, 6 Jul 2022 23:27:00 +0200 Subject: [PATCH 05/57] Bump dependencies (#76) - Drop support for Composer 1.x - Drop support for PHP <7.2.5 (like Composer 2.3) --- .gitignore | 1 + composer.json | 8 ++++---- tests/BinCommandTest.php | 7 ++++--- tests/Fixtures/MyTestCommand.php | 3 ++- vendor-bin/phpunit/composer.json | 2 +- 5 files changed, 12 insertions(+), 9 deletions(-) diff --git a/.gitignore b/.gitignore index d45ca06..2ad881b 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ composer.lock vendor vendor-bin/*/vendor +.phpunit.result.cache diff --git a/composer.json b/composer.json index df57899..b711aaf 100644 --- a/composer.json +++ b/composer.json @@ -12,13 +12,13 @@ ], "license": "MIT", "require": { - "php": "^5.5.9 || ^7.0 || ^8.0", - "composer-plugin-api": "^1.0 || ^2.0" + "php": "^7.2.5 || ^8.0", + "composer-plugin-api": "^2.0" }, "require-dev": { - "composer/composer": "^1.0 || ^2.0", + "composer/composer": "^2.0", "phpstan/extension-installer": "^1.1", - "symfony/console": "^2.5 || ^3.0 || ^4.0" + "symfony/console": "^5.4.7 || ^6.0.7" }, "config": { "sort-packages": true, diff --git a/tests/BinCommandTest.php b/tests/BinCommandTest.php index 7e1320f..54ab363 100644 --- a/tests/BinCommandTest.php +++ b/tests/BinCommandTest.php @@ -5,16 +5,17 @@ use Composer\Console\Application; use Bamarni\Composer\Bin\BinCommand; use Bamarni\Composer\Bin\Tests\Fixtures\MyTestCommand; +use PHPUnit\Framework\TestCase; use Symfony\Component\Console\Input\StringInput; use Symfony\Component\Console\Output\NullOutput; -class BinCommandTest extends \PHPUnit_Framework_TestCase +class BinCommandTest extends TestCase { private $application; private $myTestCommand; private $rootDir; - protected function setUp() + protected function setUp(): void { $this->rootDir = sys_get_temp_dir().'/'.uniqid('composer_bin_plugin_tests_'); mkdir($this->rootDir); @@ -29,7 +30,7 @@ protected function setUp() ]); } - public function tearDown() + public function tearDown(): void { putenv('COMPOSER_BIN_DIR'); $this->myTestCommand->data = []; diff --git a/tests/Fixtures/MyTestCommand.php b/tests/Fixtures/MyTestCommand.php index 9c9691f..ad7a118 100644 --- a/tests/Fixtures/MyTestCommand.php +++ b/tests/Fixtures/MyTestCommand.php @@ -2,6 +2,7 @@ namespace Bamarni\Composer\Bin\Tests\Fixtures; +use PHPUnit\Framework\Assert; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; @@ -15,7 +16,7 @@ class MyTestCommand extends BaseCommand private $assert; - public function __construct(\PHPUnit_Framework_Assert $assert) + public function __construct(Assert $assert) { $this->assert = $assert; diff --git a/vendor-bin/phpunit/composer.json b/vendor-bin/phpunit/composer.json index 4920ed5..53850b5 100644 --- a/vendor-bin/phpunit/composer.json +++ b/vendor-bin/phpunit/composer.json @@ -1,5 +1,5 @@ { "require": { - "phpunit/phpunit": "^4.8||^5.0" + "phpunit/phpunit": "^8.5 || ^9.5" } } From 80b2f1ca19ff81e9aea495e7cf70443e70d12048 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= Date: Thu, 7 Jul 2022 00:16:05 +0200 Subject: [PATCH 06/57] Rewrite the tests (#77) - Remove removed property from the PHPUnit config file - Clarify the types in the tests - Make the tests more explicit - Introduced helper assertions - Remove the assert from the test command - Cleanup the generated directories after running the tests - No longer generate a new directory for each test run - Fix running the tests locally on OSX --- phpunit.xml.dist | 6 +- tests/BinCommandTest.php | 197 +++++++++++++++++++++++++------ tests/Fixtures/MyTestCommand.php | 29 +++-- 3 files changed, 178 insertions(+), 54 deletions(-) diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 9a40036..866c96b 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -1,5 +1,4 @@ - - ./tests/ + tests - \ No newline at end of file + diff --git a/tests/BinCommandTest.php b/tests/BinCommandTest.php index 54ab363..f84781f 100644 --- a/tests/BinCommandTest.php +++ b/tests/BinCommandTest.php @@ -2,95 +2,151 @@ namespace Bamarni\Composer\Bin\Tests; +use Composer\Composer; use Composer\Console\Application; use Bamarni\Composer\Bin\BinCommand; use Bamarni\Composer\Bin\Tests\Fixtures\MyTestCommand; use PHPUnit\Framework\TestCase; use Symfony\Component\Console\Input\StringInput; use Symfony\Component\Console\Output\NullOutput; +use function array_shift; +use function chdir; +use function count; +use function file_put_contents; +use function getcwd; +use function json_encode; +use function mkdir; +use function putenv; +use function realpath; +use function sprintf; +use function sys_get_temp_dir; +use function uniqid; +use function var_dump; class BinCommandTest extends TestCase { + /** + * @var Application + */ private $application; - private $myTestCommand; - private $rootDir; + + /** + * @var MyTestCommand + */ + private $testCommand; + + /** + * @var non-empty-string + */ + private $tmpDir; + + /** + * @var string + */ + private $previousCwd; protected function setUp(): void { - $this->rootDir = sys_get_temp_dir().'/'.uniqid('composer_bin_plugin_tests_'); - mkdir($this->rootDir); - chdir($this->rootDir); + $this->previousCwd = getcwd(); - file_put_contents($this->rootDir.'/composer.json', '{}'); + $tmpDir = sys_get_temp_dir().'/composer_bin_plugin_tests'; + mkdir($tmpDir); + chdir($tmpDir); + // On OSX sys_get_temp_dir() may return a symlink + $this->tmpDir = realpath($tmpDir); + + file_put_contents( + $this->tmpDir.'/composer.json', + '{}' + ); + + $this->testCommand = new MyTestCommand(); $this->application = new Application(); $this->application->addCommands([ new BinCommand(), - $this->myTestCommand = new MyTestCommand($this), + $this->testCommand, ]); } public function tearDown(): void { putenv('COMPOSER_BIN_DIR'); - $this->myTestCommand->data = []; + + chdir($this->previousCwd); + exec('rm -rf ' . $this->tmpDir); + + unset($this->application); + unset($this->testCommand); + unset($this->previousCwd); + unset($this->tmpDir); } /** * @dataProvider namespaceProvider */ - public function testNamespaceCommand($input) + public function test_it_can_execute_the_bin_command( + string $input, + string $expectedRelativeBinDir, + string $expectedRelativeCwd, + string $expectedRelativeVendorDir + ): void { $input = new StringInput($input); $output = new NullOutput(); - $this->application->doRun($input, $output); - $this->assertCount(1, $this->myTestCommand->data); - $dataSet = array_shift($this->myTestCommand->data); - $this->assertEquals($dataSet['bin-dir'], $this->rootDir.'/vendor/bin'); - $this->assertEquals($dataSet['cwd'], $this->rootDir.'/vendor-bin/mynamespace'); - $this->assertEquals($dataSet['vendor-dir'], $this->rootDir.'/vendor-bin/mynamespace/vendor'); - } + $this->application->doRun($input, $output); - public static function namespaceProvider() - { - return [ - ['bin mynamespace mytest'], - ['bin mynamespace mytest --myoption'], - ]; + $this->assertHasAccessToComposer(); + $this->assertDataSetRecordedIs( + $this->tmpDir.'/'.$expectedRelativeBinDir, + $this->tmpDir.'/'.$expectedRelativeCwd, + $this->tmpDir.'/'.$expectedRelativeVendorDir + ); + $this->assertNoMoreDataFound(); } - public function testAllNamespaceWithoutAnyNamespace() + public function test_the_all_namespace_can_be_called(): void { $input = new StringInput('bin all mytest'); $output = new NullOutput(); + $this->application->doRun($input, $output); - $this->assertEmpty($this->myTestCommand->data); + $this->assertNoMoreDataFound(); } - public function testAllNamespaceCommand() + public function test_a_command_can_be_executed_in_each_namespace_via_the_all_namespace(): void { - $namespaces = ['mynamespace', 'yournamespace']; - foreach ($namespaces as $ns) { - mkdir($this->rootDir.'/vendor-bin/'.$ns, 0777, true); + $namespaces = ['namespace1', 'namespace2']; + + foreach ($namespaces as $namespace) { + mkdir( + $this->tmpDir.'/vendor-bin/'.$namespace, + 0777, + true + ); } $input = new StringInput('bin all mytest'); $output = new NullOutput(); + $this->application->doRun($input, $output); - $this->assertCount(count($namespaces), $this->myTestCommand->data); + $this->assertHasAccessToComposer(); - foreach ($namespaces as $ns) { - $dataSet = array_shift($this->myTestCommand->data); - $this->assertEquals($dataSet['bin-dir'], $this->rootDir . '/vendor/bin'); - $this->assertEquals($dataSet['cwd'], $this->rootDir . '/vendor-bin/'.$ns); - $this->assertEquals($dataSet['vendor-dir'], $this->rootDir . '/vendor-bin/'.$ns.'/vendor'); + foreach ($namespaces as $namespace) { + $this->assertDataSetRecordedIs( + $this->tmpDir . '/vendor/bin', + $this->tmpDir . '/vendor-bin/'.$namespace, + $this->tmpDir . '/vendor-bin/'.$namespace.'/vendor' + ); } + + $this->assertNoMoreDataFound(); } - public function testBinDirFromLocalConfig() + public function test_the_bin_dir_can_be_changed(): void { $binDir = 'bin'; $composer = [ @@ -98,14 +154,77 @@ public function testBinDirFromLocalConfig() 'bin-dir' => $binDir ] ]; - file_put_contents($this->rootDir.'/composer.json', json_encode($composer)); + + file_put_contents( + $this->tmpDir.'/composer.json', + json_encode($composer) + ); $input = new StringInput('bin theirspace mytest'); $output = new NullOutput(); + $this->application->doRun($input, $output); - $this->assertCount(1, $this->myTestCommand->data); - $dataSet = array_shift($this->myTestCommand->data); - $this->assertEquals($dataSet['bin-dir'], $this->rootDir.'/'.$binDir); + $this->assertHasAccessToComposer(); + $this->assertDataSetRecordedIs( + $this->tmpDir.'/'.$binDir, + $this->tmpDir.'/'.'vendor-bin/theirspace', + $this->tmpDir.'/'.'vendor-bin/theirspace/vendor', + '' + ); + $this->assertNoMoreDataFound(); + } + + public static function namespaceProvider(): iterable + { + yield 'execute command from namespace' => [ + 'bin testnamespace mytest', + 'vendor/bin', + 'vendor-bin/testnamespace', + 'vendor-bin/testnamespace/vendor', + ]; + + yield 'execute command with options from namespace' => [ + 'bin testnamespace mytest --myoption', + 'vendor/bin', + 'vendor-bin/testnamespace', + 'vendor-bin/testnamespace/vendor', + ]; + } + + private function assertHasAccessToComposer(): void + { + self::assertInstanceOf( + Composer::class, + $this->testCommand->composer, + 'Some plugins may require access to composer file e.g. Symfony Flex' + ); + } + + private function assertDataSetRecordedIs( + string $expectedBinDir, + string $expectedCwd, + string $expectedVendorDir + ): void + { + $data = array_shift($this->testCommand->data); + + self::assertNotNull( + $data, + 'Expected test command to contain at least one data entry' + ); + self::assertSame($expectedBinDir, $data['bin-dir']); + self::assertSame($expectedCwd, $data['cwd']); + self::assertSame($expectedVendorDir, $data['vendor-dir']); + } + + private function assertNoMoreDataFound(): void + { + $data = array_shift($this->testCommand->data); + + self::assertNull( + $data, + 'Expected test command to contain not contain any more data entries.' + ); } } diff --git a/tests/Fixtures/MyTestCommand.php b/tests/Fixtures/MyTestCommand.php index ad7a118..12667a7 100644 --- a/tests/Fixtures/MyTestCommand.php +++ b/tests/Fixtures/MyTestCommand.php @@ -2,7 +2,9 @@ namespace Bamarni\Composer\Bin\Tests\Fixtures; +use Composer\Composer; use PHPUnit\Framework\Assert; +use PHPUnit\Framework\TestCase; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; @@ -12,27 +14,32 @@ class MyTestCommand extends BaseCommand { + /** + * @var mixed|Composer + */ + public $composer; + + /** + * @var list + */ public $data = []; - private $assert; - - public function __construct(Assert $assert) + public function __construct() { - $this->assert = $assert; - parent::__construct('mytest'); + $this->setDefinition([ - new InputOption('myoption', null, InputOption::VALUE_NONE), + new InputOption( + 'myoption', + null, + InputOption::VALUE_NONE + ), ]); } public function execute(InputInterface $input, OutputInterface $output) { - $this->assert->assertInstanceOf( - '\Composer\Composer', - $this->getComposer(), - "Some plugins may require access to composer file e.g. Symfony Flex" - ); + $this->composer = $this->getComposer(); $factory = Factory::create(new NullIO()); $config = $factory->getConfig(); From 755bd96bac4d8bc31ab11ef9c163c550c1c97c82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= Date: Thu, 7 Jul 2022 00:29:33 +0200 Subject: [PATCH 07/57] Leverage PHP types (#78) - Add PHP types when possible - Remove unnecessary `@inheritdoc` and equivalents - Remove redundant PHPDoc - Import symbols when possible --- src/BinCommand.php | 76 +++++++++++++++++------------------------ src/CommandProvider.php | 5 +-- src/Config.php | 21 +++++------- src/Plugin.php | 44 +++++++----------------- 4 files changed, 53 insertions(+), 93 deletions(-) diff --git a/src/BinCommand.php b/src/BinCommand.php index 64a9dc2..575d1d8 100644 --- a/src/BinCommand.php +++ b/src/BinCommand.php @@ -2,6 +2,7 @@ namespace Bamarni\Composer\Bin; +use Composer\Config as ComposerConfig; use Composer\Console\Application as ComposerApplication; use Composer\Factory; use Composer\IO\IOInterface; @@ -11,13 +12,20 @@ use Symfony\Component\Console\Output\OutputInterface; use Composer\Command\BaseCommand; use Composer\Json\JsonFile; +use function chdir; +use function file_exists; +use function file_put_contents; +use function glob; +use function min; +use function mkdir; +use function preg_quote; +use function preg_replace; +use function putenv; +use function sprintf; class BinCommand extends BaseCommand { - /** - * {@inheritDoc} - */ - protected function configure() + protected function configure(): void { $this ->setName('bin') @@ -30,10 +38,7 @@ protected function configure() ; } - /** - * {@inheritDoc} - */ - public function execute(InputInterface $input, OutputInterface $output) + public function execute(InputInterface $input, OutputInterface $output): int { $config = new Config($this->getComposer()); $this->resetComposers($application = $this->getApplication()); @@ -59,15 +64,12 @@ public function execute(InputInterface $input, OutputInterface $output) ; } - /** - * @param ComposerApplication $application - * @param string $binVendorRoot - * @param InputInterface $input - * @param OutputInterface $output - * - * @return int Exit code - */ - private function executeAllNamespaces(ComposerApplication $application, $binVendorRoot, InputInterface $input, OutputInterface $output) + private function executeAllNamespaces( + ComposerApplication $application, + string $binVendorRoot, + InputInterface $input, + OutputInterface $output + ): int { $binRoots = glob($binVendorRoot.'/*', GLOB_ONLYDIR); if (empty($binRoots)) { @@ -91,16 +93,12 @@ private function executeAllNamespaces(ComposerApplication $application, $binVend return min($exitCode, 255); } - - /** - * @param ComposerApplication $application - * @param string $namespace - * @param InputInterface $input - * @param OutputInterface $output - * - * @return int Exit code - */ - private function executeInNamespace(ComposerApplication $application, $namespace, InputInterface $input, OutputInterface $output) + private function executeInNamespace( + ComposerApplication $application, + string $namespace, + InputInterface $input, + OutputInterface $output + ): int { if (!file_exists($namespace)) { mkdir($namespace, 0777, true); @@ -124,21 +122,12 @@ private function executeInNamespace(ComposerApplication $application, $namespace return $application->doRun($input, $output); } - /** - * {@inheritDoc} - */ - public function isProxyCommand() + public function isProxyCommand(): bool { return true; } - /** - * Resets all Composer references in the application. - * - * @param ComposerApplication $application - * @return void - */ - private function resetComposers(ComposerApplication $application) + private function resetComposers(ComposerApplication $application): void { $application->resetComposer(); @@ -149,11 +138,7 @@ private function resetComposers(ComposerApplication $application) } } - /** - * @param string $dir - * @return void - */ - private function chdir($dir) + private function chdir(string $dir): void { chdir($dir); @@ -165,11 +150,12 @@ private function chdir($dir) } /** - * @return \Composer\Config * @throws \Composer\Json\JsonValidationException * @throws \Seld\JsonLint\ParsingException + * + * @return ComposerConfig */ - private function createConfig() + private function createConfig(): ComposerConfig { $config = Factory::createConfig(); diff --git a/src/CommandProvider.php b/src/CommandProvider.php index 2e8235f..f86593d 100644 --- a/src/CommandProvider.php +++ b/src/CommandProvider.php @@ -6,10 +6,7 @@ class CommandProvider implements CommandProviderCapability { - /** - * {@inheritDoc} - */ - public function getCommands() + public function getCommands(): array { return [new BinCommand]; } diff --git a/src/Config.php b/src/Config.php index 4e98a65..1405194 100644 --- a/src/Config.php +++ b/src/Config.php @@ -3,9 +3,13 @@ namespace Bamarni\Composer\Bin; use Composer\Composer; +use function array_merge; final class Config { + /** + * @var array{'bin-links': bool, 'target-directory': string, 'forward-command': bool} + */ private $config; public function __construct(Composer $composer) @@ -17,30 +21,21 @@ public function __construct(Composer $composer) 'target-directory' => 'vendor-bin', 'forward-command' => false, ], - isset($extra['bamarni-bin']) ? $extra['bamarni-bin'] : [] + $extra['bamarni-bin'] ?? [] ); } - /** - * @return bool - */ - public function binLinksAreEnabled() + public function binLinksAreEnabled(): bool { return true === $this->config['bin-links']; } - /** - * @return string - */ - public function getTargetDirectory() + public function getTargetDirectory(): string { return $this->config['target-directory']; } - /** - * @return bool - */ - public function isCommandForwarded() + public function isCommandForwarded(): bool { return $this->config['forward-command']; } diff --git a/src/Plugin.php b/src/Plugin.php index 06ea90e..05b4d49 100644 --- a/src/Plugin.php +++ b/src/Plugin.php @@ -2,6 +2,7 @@ namespace Bamarni\Composer\Bin; +use Bamarni\Composer\Bin\CommandProvider as BarmaniCommandProvider; use Composer\Composer; use Composer\Console\Application; use Composer\EventDispatcher\EventSubscriberInterface; @@ -13,6 +14,10 @@ use Symfony\Component\Console\Input\ArrayInput; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputDefinition; +use Composer\Plugin\Capability\CommandProvider as ComposerPluginCommandProvider; +use Throwable; +use function array_filter; +use function array_keys; class Plugin implements PluginInterface, Capable, EventSubscriberInterface { @@ -26,42 +31,27 @@ class Plugin implements PluginInterface, Capable, EventSubscriberInterface */ protected $io; - /** - * @return void - */ - public function activate(Composer $composer, IOInterface $io) + public function activate(Composer $composer, IOInterface $io): void { $this->composer = $composer; $this->io = $io; } - /** - * @return string[] - */ - public function getCapabilities() + public function getCapabilities(): array { return [ - 'Composer\Plugin\Capability\CommandProvider' => 'Bamarni\Composer\Bin\CommandProvider', + ComposerPluginCommandProvider::class => BarmaniCommandProvider::class, ]; } - /** - * @return void - */ - public function deactivate(Composer $composer, IOInterface $io) + public function deactivate(Composer $composer, IOInterface $io): void { } - /** - * @return void - */ - public function uninstall(Composer $composer, IOInterface $io) + public function uninstall(Composer $composer, IOInterface $io): void { } - /** - * @return string[] - */ public static function getSubscribedEvents() { return [ @@ -69,11 +59,7 @@ public static function getSubscribedEvents() ]; } - /** - * @param CommandEvent $event - * @return bool - */ - public function onCommandEvent(CommandEvent $event) + public function onCommandEvent(CommandEvent $event): bool { $config = new Config($this->composer); @@ -88,11 +74,7 @@ public function onCommandEvent(CommandEvent $event) return true; } - /** - * @param CommandEvent $event - * @return bool - */ - protected function onCommandEventInstallUpdate(CommandEvent $event) + protected function onCommandEventInstallUpdate(CommandEvent $event): bool { $command = new BinCommand(); $command->setComposer($this->composer); @@ -121,7 +103,7 @@ protected function onCommandEventInstallUpdate(CommandEvent $event) try { $returnCode = $command->run($input, $event->getOutput()); - } catch (\Exception $e) { + } catch (Throwable $throwable) { return false; } From 6b1bdf3850ecdbe4b286cd7ba1feb159d133f101 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= Date: Thu, 7 Jul 2022 01:16:26 +0200 Subject: [PATCH 08/57] Fix GitHubActions for master (#80) --- .github/workflows/tests.yaml | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index a99484a..34d0e35 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -35,7 +35,14 @@ jobs: run: | composer global config repositories.bin path $PWD composer global config allow-plugins.bamarni/composer-bin-plugin true - composer global require bamarni/composer-bin-plugin:dev-${GITHUB_SHA} + + - name: "Install bin plugin globally (PR)" + if: github.event_name == 'pull_request' + run: composer global require bamarni/composer-bin-plugin:dev-${GITHUB_SHA} + + - name: "Install bin plugin globally (master)" + if: github.event_name != 'pull_request' + run: composer global require bamarni/composer-bin-plugin:dev-master - name: "Install Composer dependencies" uses: "ramsey/composer-install@v2" From 58d1bbfec66470b3c29db87b027b6f5a9b758ab5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= Date: Thu, 7 Jul 2022 10:19:24 +0200 Subject: [PATCH 09/57] Enable PHP strict types (#81) --- src/BinCommand.php | 2 +- src/CommandProvider.php | 2 +- src/Config.php | 2 +- src/Plugin.php | 2 +- tests/BinCommandTest.php | 2 +- tests/Fixtures/MyTestCommand.php | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/BinCommand.php b/src/BinCommand.php index 575d1d8..f4dc52f 100644 --- a/src/BinCommand.php +++ b/src/BinCommand.php @@ -1,4 +1,4 @@ - Date: Thu, 7 Jul 2022 10:23:19 +0200 Subject: [PATCH 10/57] Add missing ext-json requirement (#82) --- composer.json | 1 + 1 file changed, 1 insertion(+) diff --git a/composer.json b/composer.json index b711aaf..5b5ee10 100644 --- a/composer.json +++ b/composer.json @@ -16,6 +16,7 @@ "composer-plugin-api": "^2.0" }, "require-dev": { + "ext-json": "*", "composer/composer": "^2.0", "phpstan/extension-installer": "^1.1", "symfony/console": "^5.4.7 || ^6.0.7" From ff6ff69c768cbb7f81e9fb6944f24252c8295973 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= Date: Thu, 7 Jul 2022 10:30:25 +0200 Subject: [PATCH 11/57] Extract the input creation into a dedicated utility class (#83) --- src/BinCommand.php | 16 ++++---- src/BinInputFactory.php | 36 +++++++++++++++++ tests/BinInputFactoryTest.php | 73 +++++++++++++++++++++++++++++++++++ 3 files changed, 116 insertions(+), 9 deletions(-) create mode 100644 src/BinInputFactory.php create mode 100644 tests/BinInputFactoryTest.php diff --git a/src/BinCommand.php b/src/BinCommand.php index f4dc52f..7542626 100644 --- a/src/BinCommand.php +++ b/src/BinCommand.php @@ -51,12 +51,10 @@ public function execute(InputInterface $input, OutputInterface $output): int $vendorRoot = $config->getTargetDirectory(); $namespace = $input->getArgument('namespace'); - $input = new StringInput(preg_replace( - sprintf('/bin\s+(--ansi\s)?%s(\s.+)/', preg_quote($namespace, '/')), - '$1$2', - (string) $input, - 1 - )); + $input = BinInputFactory::createInput( + $namespace, + $input + ); return ('all' !== $namespace) ? $this->executeInNamespace($application, $vendorRoot.'/'.$namespace, $input, $output) @@ -111,15 +109,15 @@ private function executeInNamespace( file_put_contents(Factory::getComposerFile(), '{}'); } - $input = new StringInput((string) $input . ' --working-dir=.'); + $namespaceInput = BinInputFactory::createNamespaceInput($input); $this->getIO()->writeError( - sprintf('Run with %s', $input->__toString()), + sprintf('Run with %s', $namespaceInput), true, IOInterface::VERBOSE ); - return $application->doRun($input, $output); + return $application->doRun($namespaceInput, $output); } public function isProxyCommand(): bool diff --git a/src/BinInputFactory.php b/src/BinInputFactory.php new file mode 100644 index 0000000..249dbab --- /dev/null +++ b/src/BinInputFactory.php @@ -0,0 +1,36 @@ + Date: Thu, 7 Jul 2022 10:35:57 +0200 Subject: [PATCH 12/57] Leverage constants (#84) --- src/BinCommand.php | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/BinCommand.php b/src/BinCommand.php index 7542626..f07d1df 100644 --- a/src/BinCommand.php +++ b/src/BinCommand.php @@ -25,13 +25,17 @@ class BinCommand extends BaseCommand { + private const ALL_NAMESPACES = 'all'; + + private const NAMESPACE_ARG = 'namespace'; + protected function configure(): void { $this ->setName('bin') ->setDescription('Run a command inside a bin namespace') ->setDefinition([ - new InputArgument('namespace', InputArgument::REQUIRED), + new InputArgument(self::NAMESPACE_ARG, InputArgument::REQUIRED), new InputArgument('args', InputArgument::REQUIRED | InputArgument::IS_ARRAY), ]) ->ignoreValidationErrors() @@ -49,14 +53,14 @@ public function execute(InputInterface $input, OutputInterface $output): int } $vendorRoot = $config->getTargetDirectory(); - $namespace = $input->getArgument('namespace'); + $namespace = $input->getArgument(self::NAMESPACE_ARG); $input = BinInputFactory::createInput( $namespace, $input ); - return ('all' !== $namespace) + return (self::ALL_NAMESPACES !== $namespace) ? $this->executeInNamespace($application, $vendorRoot.'/'.$namespace, $input, $output) : $this->executeAllNamespaces($application, $vendorRoot, $input, $output) ; @@ -73,11 +77,11 @@ private function executeAllNamespaces( if (empty($binRoots)) { $this->getIO()->writeError('Couldn\'t find any bin namespace.'); - return 0; // Is a valid scenario: the user may not have setup any bin namespace yet + return self::SUCCESS; // Is a valid scenario: the user may not have setup any bin namespace yet } $originalWorkingDir = getcwd(); - $exitCode = 0; + $exitCode = self::SUCCESS; foreach ($binRoots as $namespace) { $output->writeln( sprintf('Run in namespace %s', $namespace), @@ -89,7 +93,7 @@ private function executeAllNamespaces( $this->resetComposers($application); } - return min($exitCode, 255); + return min($exitCode, self::FAILURE); } private function executeInNamespace( ComposerApplication $application, From 1e929276fadb99546ce5d60b0f62f90db35dfedb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= Date: Thu, 7 Jul 2022 11:05:24 +0200 Subject: [PATCH 13/57] Introduce PHP-CS-Fixer (#85) --- .github/workflows/tests.yaml | 5 +++++ .gitignore | 1 + .php-cs-fixer.php | 17 +++++++++++++++++ composer.json | 1 + src/BinCommand.php | 16 ++++++---------- src/BinInputFactory.php | 7 ++++--- src/CommandProvider.php | 6 ++++-- src/Config.php | 4 +++- src/Plugin.php | 8 +++++--- tests/BinCommandTest.php | 14 +++++--------- tests/BinInputFactoryTest.php | 10 +++++----- tests/Fixtures/MyTestCommand.php | 7 +++---- 12 files changed, 59 insertions(+), 37 deletions(-) create mode 100644 .php-cs-fixer.php diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 34d0e35..e2295cb 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -36,6 +36,11 @@ jobs: composer global config repositories.bin path $PWD composer global config allow-plugins.bamarni/composer-bin-plugin true + # It is not used in the CI and the PHP constraints may not match + # the CI. + - name: "Remove PHP-CS-Fixer" + run: composer remove --dev --no-update php-cs-fixer/shim + - name: "Install bin plugin globally (PR)" if: github.event_name == 'pull_request' run: composer global require bamarni/composer-bin-plugin:dev-${GITHUB_SHA} diff --git a/.gitignore b/.gitignore index 2ad881b..1919fa1 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,4 @@ composer.lock vendor vendor-bin/*/vendor .phpunit.result.cache +.php-cs-fixer.cache diff --git a/.php-cs-fixer.php b/.php-cs-fixer.php new file mode 100644 index 0000000..507763a --- /dev/null +++ b/.php-cs-fixer.php @@ -0,0 +1,17 @@ +files() + ->in(['src', 'tests']); + +$config = new PhpCsFixer\Config(); + +return $config + ->setRules([ + '@PSR12' => true, + 'strict_param' => true, + 'array_syntax' => ['syntax' => 'short'], + 'no_unused_imports' => true, + ]) + ->setRiskyAllowed(true) + ->setFinder($finder); diff --git a/composer.json b/composer.json index 5b5ee10..8ba4759 100644 --- a/composer.json +++ b/composer.json @@ -18,6 +18,7 @@ "require-dev": { "ext-json": "*", "composer/composer": "^2.0", + "php-cs-fixer/shim": "^3.8", "phpstan/extension-installer": "^1.1", "symfony/console": "^5.4.7 || ^6.0.7" }, diff --git a/src/BinCommand.php b/src/BinCommand.php index f07d1df..8031b75 100644 --- a/src/BinCommand.php +++ b/src/BinCommand.php @@ -1,4 +1,6 @@ -getIO()->writeError('Couldn\'t find any bin namespace.'); @@ -95,13 +93,13 @@ private function executeAllNamespaces( return min($exitCode, self::FAILURE); } + private function executeInNamespace( ComposerApplication $application, string $namespace, InputInterface $input, OutputInterface $output - ): int - { + ): int { if (!file_exists($namespace)) { mkdir($namespace, 0777, true); } @@ -154,8 +152,6 @@ private function chdir(string $dir): void /** * @throws \Composer\Json\JsonValidationException * @throws \Seld\JsonLint\ParsingException - * - * @return ComposerConfig */ private function createConfig(): ComposerConfig { diff --git a/src/BinInputFactory.php b/src/BinInputFactory.php index 249dbab..ea9265d 100644 --- a/src/BinInputFactory.php +++ b/src/BinInputFactory.php @@ -1,4 +1,6 @@ - BarmaniCommandProvider::class, + ComposerPluginCommandProvider::class => BamarniCommandProvider::class, ]; } diff --git a/tests/BinCommandTest.php b/tests/BinCommandTest.php index 2baba51..be5b6ce 100644 --- a/tests/BinCommandTest.php +++ b/tests/BinCommandTest.php @@ -1,4 +1,6 @@ -testCommand->data); self::assertNotNull( diff --git a/tests/BinInputFactoryTest.php b/tests/BinInputFactoryTest.php index afad0b8..24aa973 100644 --- a/tests/BinInputFactoryTest.php +++ b/tests/BinInputFactoryTest.php @@ -1,4 +1,6 @@ -getApplication()->resetComposer(); } } - From b3476c360570e99f4ee46f44ef4e47c113477656 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= Date: Thu, 7 Jul 2022 11:08:37 +0200 Subject: [PATCH 14/57] Use the new requireComposer() instead of the deprecated getComposer() (#86) --- src/BinCommand.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/BinCommand.php b/src/BinCommand.php index 8031b75..80ef6d4 100644 --- a/src/BinCommand.php +++ b/src/BinCommand.php @@ -43,7 +43,7 @@ protected function configure(): void public function execute(InputInterface $input, OutputInterface $output): int { - $config = new Config($this->getComposer()); + $config = new Config($this->requireComposer()); $this->resetComposers($application = $this->getApplication()); /** @var ComposerApplication $application */ From d4e4223b0d65171bdfb7a0b32edab6dfaadfa41d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= Date: Thu, 7 Jul 2022 11:19:10 +0200 Subject: [PATCH 15/57] Fail the command if mkdir fails (#87) --- src/BinCommand.php | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/BinCommand.php b/src/BinCommand.php index 80ef6d4..c99239b 100644 --- a/src/BinCommand.php +++ b/src/BinCommand.php @@ -17,6 +17,7 @@ use function file_exists; use function file_put_contents; use function glob; +use function is_dir; use function min; use function mkdir; use function putenv; @@ -101,7 +102,18 @@ private function executeInNamespace( OutputInterface $output ): int { if (!file_exists($namespace)) { - mkdir($namespace, 0777, true); + $mkdirResult = mkdir($namespace, 0777, true); + + if (!$mkdirResult && !is_dir($namespace)) { + $this + ->getIO() + ->writeError(sprintf( + 'Could not create the directory "%s".', + $namespace + )); + + return self::FAILURE; + } } $this->chdir($namespace); From 7c8b7cbcf66e35a38683312f357b920d82f79a13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= Date: Thu, 7 Jul 2022 11:25:28 +0200 Subject: [PATCH 16/57] Fail the command if mkdir fails (#88) From 7a44c0091987bf34a678275b7b219f31b89c88cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= Date: Thu, 7 Jul 2022 11:42:22 +0200 Subject: [PATCH 17/57] Extract config creation into a dedicated class (#89) --- src/BinCommand.php | 23 +---------------------- src/ConfigFactory.php | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 22 deletions(-) create mode 100644 src/ConfigFactory.php diff --git a/src/BinCommand.php b/src/BinCommand.php index c99239b..2376759 100644 --- a/src/BinCommand.php +++ b/src/BinCommand.php @@ -4,7 +4,6 @@ namespace Bamarni\Composer\Bin; -use Composer\Config as ComposerConfig; use Composer\Console\Application as ComposerApplication; use Composer\Factory; use Composer\IO\IOInterface; @@ -12,7 +11,6 @@ use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Output\OutputInterface; use Composer\Command\BaseCommand; -use Composer\Json\JsonFile; use function chdir; use function file_exists; use function file_put_contents; @@ -49,7 +47,7 @@ public function execute(InputInterface $input, OutputInterface $output): int /** @var ComposerApplication $application */ if ($config->binLinksAreEnabled()) { - putenv('COMPOSER_BIN_DIR='.$this->createConfig()->get('bin-dir')); + putenv('COMPOSER_BIN_DIR='.ConfigFactory::createConfig()->get('bin-dir')); } $vendorRoot = $config->getTargetDirectory(); @@ -160,23 +158,4 @@ private function chdir(string $dir): void IOInterface::VERBOSE ); } - - /** - * @throws \Composer\Json\JsonValidationException - * @throws \Seld\JsonLint\ParsingException - */ - private function createConfig(): ComposerConfig - { - $config = Factory::createConfig(); - - $file = new JsonFile(Factory::getComposerFile()); - if (!$file->exists()) { - return $config; - } - $file->validateSchema(JsonFile::LAX_SCHEMA); - - $config->merge($file->read()); - - return $config; - } } diff --git a/src/ConfigFactory.php b/src/ConfigFactory.php new file mode 100644 index 0000000..5efde10 --- /dev/null +++ b/src/ConfigFactory.php @@ -0,0 +1,37 @@ +exists()) { + return $config; + } + + $file->validateSchema(JsonFile::LAX_SCHEMA); + + $config->merge($file->read()); + + return $config; + } + + private function __construct() + { + } +} From 7757471d43b38e6dd3de2afdfbbf2a23dcaefbb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= Date: Thu, 7 Jul 2022 11:51:03 +0200 Subject: [PATCH 18/57] Tweak the Config (#90) --- src/BinCommand.php | 2 +- src/Config.php | 11 +++++++++-- src/Plugin.php | 2 +- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/BinCommand.php b/src/BinCommand.php index 2376759..ba507f9 100644 --- a/src/BinCommand.php +++ b/src/BinCommand.php @@ -42,7 +42,7 @@ protected function configure(): void public function execute(InputInterface $input, OutputInterface $output): int { - $config = new Config($this->requireComposer()); + $config = Config::fromComposer($this->requireComposer()); $this->resetComposers($application = $this->getApplication()); /** @var ComposerApplication $application */ diff --git a/src/Config.php b/src/Config.php index 3512d84..81cebf4 100644 --- a/src/Config.php +++ b/src/Config.php @@ -14,9 +14,16 @@ final class Config */ private $config; - public function __construct(Composer $composer) + public static function fromComposer(Composer $composer): self + { + return new self($composer->getPackage()->getExtra()); + } + + /** + * @param mixed[] $extra + */ + public function __construct(array $extra) { - $extra = $composer->getPackage()->getExtra(); $this->config = array_merge( [ 'bin-links' => true, diff --git a/src/Plugin.php b/src/Plugin.php index bf5b890..9106c22 100644 --- a/src/Plugin.php +++ b/src/Plugin.php @@ -63,7 +63,7 @@ public static function getSubscribedEvents() public function onCommandEvent(CommandEvent $event): bool { - $config = new Config($this->composer); + $config = Config::fromComposer($this->composer); if ($config->isCommandForwarded()) { switch ($event->getCommandName()) { From a9a26945c781b2a4b7b681bd535ec9d23b25e8d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= Date: Thu, 7 Jul 2022 12:03:16 +0200 Subject: [PATCH 19/57] Various minor refactoring (#79) --- src/BinCommand.php | 78 +++++++++++++++++++++++--------- src/Plugin.php | 2 +- tests/BinCommandTest.php | 11 +++-- tests/Fixtures/MyTestCommand.php | 2 +- 4 files changed, 67 insertions(+), 26 deletions(-) diff --git a/src/BinCommand.php b/src/BinCommand.php index ba507f9..aead287 100644 --- a/src/BinCommand.php +++ b/src/BinCommand.php @@ -4,22 +4,24 @@ namespace Bamarni\Composer\Bin; +use Composer\Command\BaseCommand; use Composer\Console\Application as ComposerApplication; use Composer\Factory; use Composer\IO\IOInterface; -use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputArgument; +use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; -use Composer\Command\BaseCommand; use function chdir; +use function count; use function file_exists; use function file_put_contents; +use function getcwd; use function glob; -use function is_dir; use function min; use function mkdir; use function putenv; use function sprintf; +use const GLOB_ONLYDIR; class BinCommand extends BaseCommand { @@ -36,32 +38,54 @@ protected function configure(): void new InputArgument(self::NAMESPACE_ARG, InputArgument::REQUIRED), new InputArgument('args', InputArgument::REQUIRED | InputArgument::IS_ARRAY), ]) - ->ignoreValidationErrors() - ; + ->ignoreValidationErrors(); } public function execute(InputInterface $input, OutputInterface $output): int { $config = Config::fromComposer($this->requireComposer()); - $this->resetComposers($application = $this->getApplication()); - /** @var ComposerApplication $application */ + $application = $this->getApplication(); + + // Ensures Composer is reset – we are setting some environment variables + // & co. so a fresh Composer instance is required. + $this->resetComposers($application); if ($config->binLinksAreEnabled()) { - putenv('COMPOSER_BIN_DIR='.ConfigFactory::createConfig()->get('bin-dir')); + putenv(sprintf( + 'COMPOSER_BIN_DIR=%s', + ConfigFactory::createConfig()->get('bin-dir') + )); } $vendorRoot = $config->getTargetDirectory(); $namespace = $input->getArgument(self::NAMESPACE_ARG); - $input = BinInputFactory::createInput( + $binInput = BinInputFactory::createInput( $namespace, $input ); return (self::ALL_NAMESPACES !== $namespace) - ? $this->executeInNamespace($application, $vendorRoot.'/'.$namespace, $input, $output) - : $this->executeAllNamespaces($application, $vendorRoot, $input, $output) - ; + ? $this->executeInNamespace( + $application, + $vendorRoot.'/'.$namespace, + $binInput, + $output + ) + : $this->executeAllNamespaces( + $application, + $vendorRoot, + $binInput, + $output + ); + } + + /** + * @return list + */ + private static function getBinNamespaces(string $binVendorRoot): array + { + return glob($binVendorRoot.'/*', GLOB_ONLYDIR); } private function executeAllNamespaces( @@ -70,22 +94,30 @@ private function executeAllNamespaces( InputInterface $input, OutputInterface $output ): int { - $binRoots = glob($binVendorRoot.'/*', GLOB_ONLYDIR); - if (empty($binRoots)) { - $this->getIO()->writeError('Couldn\'t find any bin namespace.'); + $namespaces = self::getBinNamespaces($binVendorRoot); - return self::SUCCESS; // Is a valid scenario: the user may not have setup any bin namespace yet + if (count($namespaces) === 0) { + $this + ->getIO() + ->writeError('Could not find any bin namespace.'); + + // Is a valid scenario: the user may not have set up any bin + // namespace yet + return self::SUCCESS; } $originalWorkingDir = getcwd(); $exitCode = self::SUCCESS; - foreach ($binRoots as $namespace) { + + foreach ($namespaces as $namespace) { $output->writeln( sprintf('Run in namespace %s', $namespace), OutputInterface::VERBOSITY_VERBOSE ); + $exitCode += $this->executeInNamespace($application, $namespace, $input, $output); + // Ensure we have a clean state in-between each namespace execution chdir($originalWorkingDir); $this->resetComposers($application); } @@ -116,15 +148,19 @@ private function executeInNamespace( $this->chdir($namespace); - // some plugins require access to composer file e.g. Symfony Flex - if (!file_exists(Factory::getComposerFile())) { - file_put_contents(Factory::getComposerFile(), '{}'); + // Some plugins require access to the Composer file e.g. Symfony Flex + $namespaceComposerFile = Factory::getComposerFile(); + if (!file_exists($namespaceComposerFile)) { + file_put_contents($namespaceComposerFile, '{}'); } $namespaceInput = BinInputFactory::createNamespaceInput($input); $this->getIO()->writeError( - sprintf('Run with %s', $namespaceInput), + sprintf( + 'Run with %s', + $namespaceInput + ), true, IOInterface::VERBOSE ); diff --git a/src/Plugin.php b/src/Plugin.php index 9106c22..b506661 100644 --- a/src/Plugin.php +++ b/src/Plugin.php @@ -54,7 +54,7 @@ public function uninstall(Composer $composer, IOInterface $io): void { } - public static function getSubscribedEvents() + public static function getSubscribedEvents(): array { return [ PluginEvents::COMMAND => 'onCommandEvent', diff --git a/tests/BinCommandTest.php b/tests/BinCommandTest.php index be5b6ce..c91dc0a 100644 --- a/tests/BinCommandTest.php +++ b/tests/BinCommandTest.php @@ -4,15 +4,16 @@ namespace Bamarni\Composer\Bin\Tests; -use Composer\Composer; -use Composer\Console\Application; use Bamarni\Composer\Bin\BinCommand; use Bamarni\Composer\Bin\Tests\Fixtures\MyTestCommand; +use Composer\Composer; +use Composer\Console\Application; use PHPUnit\Framework\TestCase; use Symfony\Component\Console\Input\StringInput; use Symfony\Component\Console\Output\NullOutput; use function array_shift; use function chdir; +use function file_exists; use function file_put_contents; use function getcwd; use function json_encode; @@ -48,7 +49,11 @@ protected function setUp(): void $this->previousCwd = getcwd(); $tmpDir = sys_get_temp_dir().'/composer_bin_plugin_tests'; - mkdir($tmpDir); + + if (!file_exists($tmpDir)) { + mkdir($tmpDir); + } + chdir($tmpDir); // On OSX sys_get_temp_dir() may return a symlink $this->tmpDir = realpath($tmpDir); diff --git a/tests/Fixtures/MyTestCommand.php b/tests/Fixtures/MyTestCommand.php index 2b89aad..19fb77d 100644 --- a/tests/Fixtures/MyTestCommand.php +++ b/tests/Fixtures/MyTestCommand.php @@ -39,7 +39,7 @@ public function __construct() public function execute(InputInterface $input, OutputInterface $output) { - $this->composer = $this->getComposer(); + $this->composer = $this->tryComposer(); $factory = Factory::create(new NullIO()); $config = $factory->getConfig(); From 2b787fe59d7cfa16ff864fc259b178d27eefccf8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= Date: Fri, 8 Jul 2022 00:50:57 +0200 Subject: [PATCH 20/57] Improve the logging experience (#92) - The logging messages are now all prefixed with `[bamarni-bin-plugin]` - All the debug messages are logged in verbose mode (`-v|--verbose`) or higher - Added a few logging messages - logs the original working directory - logs the bin directory configured - logs the change of directory done for each change of namespace - logs when an empty Composer file has been created - Rephrased the logging messages to be a bit more consistent --- src/BinCommand.php | 90 ++++++++++++++++++++++++++++++++-------------- 1 file changed, 64 insertions(+), 26 deletions(-) diff --git a/src/BinCommand.php b/src/BinCommand.php index aead287..d414998 100644 --- a/src/BinCommand.php +++ b/src/BinCommand.php @@ -45,18 +45,37 @@ public function execute(InputInterface $input, OutputInterface $output): int { $config = Config::fromComposer($this->requireComposer()); $application = $this->getApplication(); + $currentWorkingDir = getcwd(); // Ensures Composer is reset – we are setting some environment variables // & co. so a fresh Composer instance is required. $this->resetComposers($application); if ($config->binLinksAreEnabled()) { - putenv(sprintf( - 'COMPOSER_BIN_DIR=%s', - ConfigFactory::createConfig()->get('bin-dir') - )); + $binDir = ConfigFactory::createConfig()->get('bin-dir'); + + putenv( + sprintf( + 'COMPOSER_BIN_DIR=%s', + $binDir + ) + ); + + $this->log( + sprintf( + 'Configuring bin directory to %s.', + $binDir + ) + ); } + $this->log( + sprintf( + 'Current working directory: %s', + $currentWorkingDir + ) + ); + $vendorRoot = $config->getTargetDirectory(); $namespace = $input->getArgument(self::NAMESPACE_ARG); @@ -97,9 +116,10 @@ private function executeAllNamespaces( $namespaces = self::getBinNamespaces($binVendorRoot); if (count($namespaces) === 0) { - $this - ->getIO() - ->writeError('Could not find any bin namespace.'); + $this->log( + 'Could not find any bin namespace.', + false + ); // Is a valid scenario: the user may not have set up any bin // namespace yet @@ -110,15 +130,10 @@ private function executeAllNamespaces( $exitCode = self::SUCCESS; foreach ($namespaces as $namespace) { - $output->writeln( - sprintf('Run in namespace %s', $namespace), - OutputInterface::VERBOSITY_VERBOSE - ); - $exitCode += $this->executeInNamespace($application, $namespace, $input, $output); // Ensure we have a clean state in-between each namespace execution - chdir($originalWorkingDir); + $this->chdir($originalWorkingDir); $this->resetComposers($application); } @@ -131,16 +146,24 @@ private function executeInNamespace( InputInterface $input, OutputInterface $output ): int { + $this->log( + sprintf( + 'Checking namespace %s', + $namespace + ) + ); + if (!file_exists($namespace)) { $mkdirResult = mkdir($namespace, 0777, true); if (!$mkdirResult && !is_dir($namespace)) { - $this - ->getIO() - ->writeError(sprintf( + $this->log( + sprintf( 'Could not create the directory "%s".', $namespace - )); + ), + false + ); return self::FAILURE; } @@ -152,17 +175,22 @@ private function executeInNamespace( $namespaceComposerFile = Factory::getComposerFile(); if (!file_exists($namespaceComposerFile)) { file_put_contents($namespaceComposerFile, '{}'); + + $this->log( + sprintf( + 'Created the file %s.', + $namespaceComposerFile + ) + ); } $namespaceInput = BinInputFactory::createNamespaceInput($input); - $this->getIO()->writeError( + $this->log( sprintf( - 'Run with %s', + 'Running `@composer %s`.', $namespaceInput - ), - true, - IOInterface::VERBOSE + ) ); return $application->doRun($namespaceInput, $output); @@ -188,10 +216,20 @@ private function chdir(string $dir): void { chdir($dir); - $this->getIO()->writeError( - sprintf('Changed current directory to %s', $dir), - true, - IOInterface::VERBOSE + $this->log( + sprintf( + 'Changed current directory to %s.', + $dir + ) ); } + + private function log(string $message, bool $debug = true): void + { + $verbosity = $debug + ? IOInterface::VERBOSE + : IOInterface::NORMAL; + + $this->getIO()->writeError('[bamarni-bin-plugin] '.$message, true, $verbosity); + } } From 8d63dae0d4cbc26da8f2b169f1dcd8c31fb9cc1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= Date: Fri, 8 Jul 2022 00:56:20 +0200 Subject: [PATCH 21/57] Use getApplication() instead of passing the instance around (#93) By passing it like so as a parameter it gives the impression that the instance itself may change (different references). This is however not the case and since the application state does mutate across the execution I do not think this helps out in the understanding of the code. --- src/BinCommand.php | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/BinCommand.php b/src/BinCommand.php index d414998..8045fe4 100644 --- a/src/BinCommand.php +++ b/src/BinCommand.php @@ -5,7 +5,6 @@ namespace Bamarni\Composer\Bin; use Composer\Command\BaseCommand; -use Composer\Console\Application as ComposerApplication; use Composer\Factory; use Composer\IO\IOInterface; use Symfony\Component\Console\Input\InputArgument; @@ -44,12 +43,11 @@ protected function configure(): void public function execute(InputInterface $input, OutputInterface $output): int { $config = Config::fromComposer($this->requireComposer()); - $application = $this->getApplication(); $currentWorkingDir = getcwd(); // Ensures Composer is reset – we are setting some environment variables // & co. so a fresh Composer instance is required. - $this->resetComposers($application); + $this->resetComposers(); if ($config->binLinksAreEnabled()) { $binDir = ConfigFactory::createConfig()->get('bin-dir'); @@ -86,13 +84,11 @@ public function execute(InputInterface $input, OutputInterface $output): int return (self::ALL_NAMESPACES !== $namespace) ? $this->executeInNamespace( - $application, $vendorRoot.'/'.$namespace, $binInput, $output ) : $this->executeAllNamespaces( - $application, $vendorRoot, $binInput, $output @@ -108,7 +104,6 @@ private static function getBinNamespaces(string $binVendorRoot): array } private function executeAllNamespaces( - ComposerApplication $application, string $binVendorRoot, InputInterface $input, OutputInterface $output @@ -130,18 +125,17 @@ private function executeAllNamespaces( $exitCode = self::SUCCESS; foreach ($namespaces as $namespace) { - $exitCode += $this->executeInNamespace($application, $namespace, $input, $output); + $exitCode += $this->executeInNamespace($namespace, $input, $output); // Ensure we have a clean state in-between each namespace execution $this->chdir($originalWorkingDir); - $this->resetComposers($application); + $this->resetComposers(); } return min($exitCode, self::FAILURE); } private function executeInNamespace( - ComposerApplication $application, string $namespace, InputInterface $input, OutputInterface $output @@ -193,7 +187,7 @@ private function executeInNamespace( ) ); - return $application->doRun($namespaceInput, $output); + return $this->getApplication()->doRun($namespaceInput, $output); } public function isProxyCommand(): bool @@ -201,9 +195,9 @@ public function isProxyCommand(): bool return true; } - private function resetComposers(ComposerApplication $application): void + private function resetComposers(): void { - $application->resetComposer(); + $this->getApplication()->resetComposer(); foreach ($this->getApplication()->all() as $command) { if ($command instanceof BaseCommand) { From aa6a52493feea8c027d32ba43572f35aa483908e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= Date: Fri, 8 Jul 2022 01:02:28 +0200 Subject: [PATCH 22/57] Ensure we clean up the state even when executing only one namespace (#94) Another plugin being executed afterwards could still rely on the state (e.g. current working directory) in which case it is best we leave a clean state. --- src/BinCommand.php | 37 ++++++++++++++++++++++++++++++------- 1 file changed, 30 insertions(+), 7 deletions(-) diff --git a/src/BinCommand.php b/src/BinCommand.php index 8045fe4..1bb195d 100644 --- a/src/BinCommand.php +++ b/src/BinCommand.php @@ -10,6 +10,7 @@ use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; +use Throwable; use function chdir; use function count; use function file_exists; @@ -84,11 +85,13 @@ public function execute(InputInterface $input, OutputInterface $output): int return (self::ALL_NAMESPACES !== $namespace) ? $this->executeInNamespace( + $currentWorkingDir, $vendorRoot.'/'.$namespace, $binInput, $output ) : $this->executeAllNamespaces( + $currentWorkingDir, $vendorRoot, $binInput, $output @@ -104,6 +107,7 @@ private static function getBinNamespaces(string $binVendorRoot): array } private function executeAllNamespaces( + string $originalWorkingDir, string $binVendorRoot, InputInterface $input, OutputInterface $output @@ -121,21 +125,22 @@ private function executeAllNamespaces( return self::SUCCESS; } - $originalWorkingDir = getcwd(); $exitCode = self::SUCCESS; foreach ($namespaces as $namespace) { - $exitCode += $this->executeInNamespace($namespace, $input, $output); - - // Ensure we have a clean state in-between each namespace execution - $this->chdir($originalWorkingDir); - $this->resetComposers(); + $exitCode += $this->executeInNamespace( + $originalWorkingDir, + $namespace, + $input, + $output + ); } return min($exitCode, self::FAILURE); } private function executeInNamespace( + string $originalWorkingDir, string $namespace, InputInterface $input, OutputInterface $output @@ -163,6 +168,13 @@ private function executeInNamespace( } } + // 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): void { + $this->chdir($originalWorkingDir); + $this->resetComposers(); + }; + $this->chdir($namespace); // Some plugins require access to the Composer file e.g. Symfony Flex @@ -187,7 +199,18 @@ private function executeInNamespace( ) ); - return $this->getApplication()->doRun($namespaceInput, $output); + try { + $exitCode = $this->getApplication()->doRun($namespaceInput, $output); + } catch (Throwable $executionFailed) { + // Ensure we do the cleanup even in case of failure + $cleanUp(); + + throw $executionFailed; + } + + $cleanUp(); + + return $exitCode; } public function isProxyCommand(): bool From 57f9aa9f934631055e6a566c32d06b61539fb3ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= Date: Fri, 8 Jul 2022 01:08:10 +0200 Subject: [PATCH 23/57] Code readability improvements (#91) --- src/BinCommand.php | 127 ++++++++++++++++++----------- src/CouldNotCreateNamespaceDir.php | 21 +++++ 2 files changed, 99 insertions(+), 49 deletions(-) create mode 100644 src/CouldNotCreateNamespaceDir.php diff --git a/src/BinCommand.php b/src/BinCommand.php index 1bb195d..ab37d97 100644 --- a/src/BinCommand.php +++ b/src/BinCommand.php @@ -41,33 +41,16 @@ protected function configure(): void ->ignoreValidationErrors(); } + public function isProxyCommand(): bool + { + return true; + } + public function execute(InputInterface $input, OutputInterface $output): int { $config = Config::fromComposer($this->requireComposer()); $currentWorkingDir = getcwd(); - // Ensures Composer is reset – we are setting some environment variables - // & co. so a fresh Composer instance is required. - $this->resetComposers(); - - if ($config->binLinksAreEnabled()) { - $binDir = ConfigFactory::createConfig()->get('bin-dir'); - - putenv( - sprintf( - 'COMPOSER_BIN_DIR=%s', - $binDir - ) - ); - - $this->log( - sprintf( - 'Configuring bin directory to %s.', - $binDir - ) - ); - } - $this->log( sprintf( 'Current working directory: %s', @@ -75,6 +58,12 @@ public function execute(InputInterface $input, OutputInterface $output): int ) ); + // Ensures Composer is reset – we are setting some environment variables + // & co. so a fresh Composer instance is required. + $this->resetComposers(); + + $this->configureBinLinksDir($config); + $vendorRoot = $config->getTargetDirectory(); $namespace = $input->getArgument(self::NAMESPACE_ARG); @@ -152,20 +141,18 @@ private function executeInNamespace( ) ); - if (!file_exists($namespace)) { - $mkdirResult = mkdir($namespace, 0777, true); - - if (!$mkdirResult && !is_dir($namespace)) { - $this->log( - sprintf( - 'Could not create the directory "%s".', - $namespace - ), - false - ); + try { + self::createNamespaceDirIfDoesNotExist($namespace); + } catch (CouldNotCreateNamespaceDir $exception) { + $this->log( + sprintf( + '%s', + $exception->getMessage() + ), + false + ); - return self::FAILURE; - } + return self::FAILURE; } // It is important to clean up the state either for follow-up plugins @@ -177,18 +164,7 @@ private function executeInNamespace( $this->chdir($namespace); - // Some plugins require access to the Composer file e.g. Symfony Flex - $namespaceComposerFile = Factory::getComposerFile(); - if (!file_exists($namespaceComposerFile)) { - file_put_contents($namespaceComposerFile, '{}'); - - $this->log( - sprintf( - 'Created the file %s.', - $namespaceComposerFile - ) - ); - } + $this->ensureComposerFileExists(); $namespaceInput = BinInputFactory::createNamespaceInput($input); @@ -213,9 +189,62 @@ private function executeInNamespace( return $exitCode; } - public function isProxyCommand(): bool + /** + * @throws CouldNotCreateNamespaceDir + */ + private static function createNamespaceDirIfDoesNotExist(string $namespace): void { - return true; + if (file_exists($namespace)) { + return; + } + + $mkdirResult = mkdir($namespace, 0777, true); + + if (!$mkdirResult && !is_dir($namespace)) { + throw CouldNotCreateNamespaceDir::forNamespace($namespace); + } + } + + private function configureBinLinksDir(Config $config): void + { + if (!$config->binLinksAreEnabled()) { + return; + } + + $binDir = ConfigFactory::createConfig()->get('bin-dir'); + + putenv( + sprintf( + 'COMPOSER_BIN_DIR=%s', + $binDir + ) + ); + + $this->log( + sprintf( + 'Configuring bin directory to %s.', + $binDir + ) + ); + } + + private function ensureComposerFileExists(): void + { + // Some plugins require access to the Composer file e.g. Symfony Flex + $namespaceComposerFile = Factory::getComposerFile(); + + if (file_exists($namespaceComposerFile)) { + return; + } + + file_put_contents($namespaceComposerFile, '{}'); + + $this->log( + sprintf( + 'Created the file %s.', + $namespaceComposerFile + ) + ); } private function resetComposers(): void diff --git a/src/CouldNotCreateNamespaceDir.php b/src/CouldNotCreateNamespaceDir.php new file mode 100644 index 0000000..17683e2 --- /dev/null +++ b/src/CouldNotCreateNamespaceDir.php @@ -0,0 +1,21 @@ + Date: Fri, 8 Jul 2022 09:11:24 +0200 Subject: [PATCH 24/57] Add end-to-end tests (#95) --- .github/workflows/tests.yaml | 64 +++++++++-- composer.json | 4 +- e2e/scenario0/.gitignore | 5 + e2e/scenario0/README.md | 1 + e2e/scenario0/composer.json | 16 +++ e2e/scenario0/expected.txt | 14 +++ e2e/scenario0/script.sh | 22 ++++ e2e/scenario0/vendor-bin/ns1/composer.json | 1 + e2e/scenario0/vendor-bin/ns2/composer.json | 1 + e2e/scenario1/.gitignore | 5 + e2e/scenario1/README.md | 1 + e2e/scenario1/composer.json | 16 +++ e2e/scenario1/expected.txt | 30 ++++++ e2e/scenario1/script.sh | 22 ++++ e2e/scenario1/vendor-bin/ns1/composer.json | 1 + e2e/scenario1/vendor-bin/ns2/composer.json | 1 + tests/EndToEndTest.php | 117 +++++++++++++++++++++ 17 files changed, 314 insertions(+), 7 deletions(-) create mode 100644 e2e/scenario0/.gitignore create mode 100644 e2e/scenario0/README.md create mode 100644 e2e/scenario0/composer.json create mode 100644 e2e/scenario0/expected.txt create mode 100755 e2e/scenario0/script.sh create mode 100644 e2e/scenario0/vendor-bin/ns1/composer.json create mode 100644 e2e/scenario0/vendor-bin/ns2/composer.json create mode 100644 e2e/scenario1/.gitignore create mode 100644 e2e/scenario1/README.md create mode 100644 e2e/scenario1/composer.json create mode 100644 e2e/scenario1/expected.txt create mode 100755 e2e/scenario1/script.sh create mode 100644 e2e/scenario1/vendor-bin/ns1/composer.json create mode 100644 e2e/scenario1/vendor-bin/ns2/composer.json create mode 100644 tests/EndToEndTest.php diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index e2295cb..cd0f96d 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -1,4 +1,4 @@ -name: "Tests" +name: "Unit Tests" on: push: @@ -8,9 +8,9 @@ on: pull_request: null jobs: - tests: + unit-tests: runs-on: "ubuntu-latest" - name: "Test PHP ${{ matrix.php }}" + name: "Unit Tests on PHP ${{ matrix.php }}" strategy: fail-fast: false matrix: @@ -41,11 +41,11 @@ jobs: - name: "Remove PHP-CS-Fixer" run: composer remove --dev --no-update php-cs-fixer/shim - - name: "Install bin plugin globally (PR)" + - name: "Install bin plugin globally (PR-only)" if: github.event_name == 'pull_request' run: composer global require bamarni/composer-bin-plugin:dev-${GITHUB_SHA} - - name: "Install bin plugin globally (master)" + - name: "Install bin plugin globally (master-only)" if: github.event_name != 'pull_request' run: composer global require bamarni/composer-bin-plugin:dev-master @@ -58,4 +58,56 @@ jobs: run: "composer validate --strict --no-check-lock" - name: "Run tests" - run: "vendor/bin/phpunit" + run: "vendor/bin/phpunit --group default" + + e2e-tests: + runs-on: "ubuntu-latest" + name: "E2E Tests on PHP ${{ matrix.php }}" + strategy: + fail-fast: false + matrix: + php: + - "8.1" + + steps: + - name: "Check out repository code" + uses: "actions/checkout@v2" + + - name: "Setup PHP" + uses: "shivammathur/setup-php@v2" + with: + php-version: "${{ matrix.php }}" + tools: "composer" + + - name: "Configure global composer" + run: | + composer global config repositories.bin path $PWD + composer global config allow-plugins.bamarni/composer-bin-plugin true + + # It is not used in the CI and the PHP constraints may not match + # the CI. + - name: "Remove PHP-CS-Fixer" + run: composer remove --dev --no-update php-cs-fixer/shim + + - name: "Install bin plugin globally (PR-only)" + if: github.event_name == 'pull_request' + run: composer global require bamarni/composer-bin-plugin:dev-${GITHUB_SHA} + + - name: "Correct bin plugin version for e2e scenarios (PR-only)" + if: github.event_name == 'pull_request' + run: find e2e -maxdepth 1 -mindepth 1 -type d -exec bash -c "cd {} && composer require --dev bamarni/composer-bin-plugin:dev-${GITHUB_SHA} --no-update" \; + + - name: "Install bin plugin globally (master-only)" + if: github.event_name != 'pull_request' + run: composer global require bamarni/composer-bin-plugin:dev-master + + - name: "Install Composer dependencies" + uses: "ramsey/composer-install@v2" + with: + dependency-versions: "highest" + + - name: "Validate composer.json" + run: "composer validate --strict --no-check-lock" + + - name: "Run tests" + run: "vendor/bin/phpunit --group e2e" diff --git a/composer.json b/composer.json index 8ba4759..6463bc0 100644 --- a/composer.json +++ b/composer.json @@ -20,7 +20,9 @@ "composer/composer": "^2.0", "php-cs-fixer/shim": "^3.8", "phpstan/extension-installer": "^1.1", - "symfony/console": "^5.4.7 || ^6.0.7" + "symfony/console": "^5.4.7 || ^6.0.7", + "symfony/finder": "^5.4.7 || ^6.0.7", + "symfony/process": "^5.4.7 || ^6.0.7" }, "config": { "sort-packages": true, diff --git a/e2e/scenario0/.gitignore b/e2e/scenario0/.gitignore new file mode 100644 index 0000000..cac9c03 --- /dev/null +++ b/e2e/scenario0/.gitignore @@ -0,0 +1,5 @@ +/actual.txt +/composer.lock +/vendor/ +/vendor-bin/*/composer.lock +/vendor-bin/*/vendor/ diff --git a/e2e/scenario0/README.md b/e2e/scenario0/README.md new file mode 100644 index 0000000..6b2a3c9 --- /dev/null +++ b/e2e/scenario0/README.md @@ -0,0 +1 @@ +Regular installation on a project with multiple namespaces. diff --git a/e2e/scenario0/composer.json b/e2e/scenario0/composer.json new file mode 100644 index 0000000..dff4cca --- /dev/null +++ b/e2e/scenario0/composer.json @@ -0,0 +1,16 @@ +{ + "repositories": [ + { + "type": "path", + "url": "../../" + } + ], + "require-dev": { + "bamarni/composer-bin-plugin": "dev-master" + }, + "config": { + "allow-plugins": { + "bamarni/composer-bin-plugin": true + } + } +} diff --git a/e2e/scenario0/expected.txt b/e2e/scenario0/expected.txt new file mode 100644 index 0000000..e1a8e3f --- /dev/null +++ b/e2e/scenario0/expected.txt @@ -0,0 +1,14 @@ +Loading composer repositories with package information +Updating dependencies +Nothing to modify in lock file +Writing lock file +Installing dependencies from lock file (including require-dev) +Nothing to install, update or remove +Generating autoload files +Loading composer repositories with package information +Updating dependencies +Nothing to modify in lock file +Writing lock file +Installing dependencies from lock file (including require-dev) +Nothing to install, update or remove +Generating autoload files diff --git a/e2e/scenario0/script.sh b/e2e/scenario0/script.sh new file mode 100755 index 0000000..3b03638 --- /dev/null +++ b/e2e/scenario0/script.sh @@ -0,0 +1,22 @@ +#!/usr/bin/env bash + +set -Eeuo pipefail + +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 all update 2>&1 | tee > actual.txt diff --git a/e2e/scenario0/vendor-bin/ns1/composer.json b/e2e/scenario0/vendor-bin/ns1/composer.json new file mode 100644 index 0000000..0967ef4 --- /dev/null +++ b/e2e/scenario0/vendor-bin/ns1/composer.json @@ -0,0 +1 @@ +{} diff --git a/e2e/scenario0/vendor-bin/ns2/composer.json b/e2e/scenario0/vendor-bin/ns2/composer.json new file mode 100644 index 0000000..0967ef4 --- /dev/null +++ b/e2e/scenario0/vendor-bin/ns2/composer.json @@ -0,0 +1 @@ +{} diff --git a/e2e/scenario1/.gitignore b/e2e/scenario1/.gitignore new file mode 100644 index 0000000..cac9c03 --- /dev/null +++ b/e2e/scenario1/.gitignore @@ -0,0 +1,5 @@ +/actual.txt +/composer.lock +/vendor/ +/vendor-bin/*/composer.lock +/vendor-bin/*/vendor/ diff --git a/e2e/scenario1/README.md b/e2e/scenario1/README.md new file mode 100644 index 0000000..4552c26 --- /dev/null +++ b/e2e/scenario1/README.md @@ -0,0 +1 @@ +Regular installation on a project with multiple namespaces in verbose mode. diff --git a/e2e/scenario1/composer.json b/e2e/scenario1/composer.json new file mode 100644 index 0000000..dff4cca --- /dev/null +++ b/e2e/scenario1/composer.json @@ -0,0 +1,16 @@ +{ + "repositories": [ + { + "type": "path", + "url": "../../" + } + ], + "require-dev": { + "bamarni/composer-bin-plugin": "dev-master" + }, + "config": { + "allow-plugins": { + "bamarni/composer-bin-plugin": true + } + } +} diff --git a/e2e/scenario1/expected.txt b/e2e/scenario1/expected.txt new file mode 100644 index 0000000..23cc600 --- /dev/null +++ b/e2e/scenario1/expected.txt @@ -0,0 +1,30 @@ +[bamarni-bin-plugin] Current working directory: /path/to/project/e2e/scenario1 +[bamarni-bin-plugin] Configuring bin directory to /path/to/project/e2e/scenario1/vendor/bin. +[bamarni-bin-plugin] Checking namespace vendor-bin/ns1 +[bamarni-bin-plugin] Changed current directory to vendor-bin/ns1. +[bamarni-bin-plugin] Running `@composer update --verbose --working-dir='.'`. +Loading composer repositories with package information +Updating dependencies +Dependency resolution completed in 0.000 seconds +Analyzed 90 packages to resolve dependencies +Analyzed 90 rules to resolve dependencies +Nothing to modify in lock file +Writing lock file +Installing dependencies from lock file (including require-dev) +Nothing to install, update or remove +Generating autoload files +[bamarni-bin-plugin] Changed current directory to /path/to/project/e2e/scenario1. +[bamarni-bin-plugin] Checking namespace vendor-bin/ns2 +[bamarni-bin-plugin] Changed current directory to vendor-bin/ns2. +[bamarni-bin-plugin] Running `@composer update --verbose --working-dir='.'`. +Loading composer repositories with package information +Updating dependencies +Dependency resolution completed in 0.000 seconds +Analyzed 90 packages to resolve dependencies +Analyzed 90 rules to resolve dependencies +Nothing to modify in lock file +Writing lock file +Installing dependencies from lock file (including require-dev) +Nothing to install, update or remove +Generating autoload files +[bamarni-bin-plugin] Changed current directory to /path/to/project/e2e/scenario1. diff --git a/e2e/scenario1/script.sh b/e2e/scenario1/script.sh new file mode 100755 index 0000000..3917ea0 --- /dev/null +++ b/e2e/scenario1/script.sh @@ -0,0 +1,22 @@ +#!/usr/bin/env bash + +set -Eeuo pipefail + +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 all update --verbose 2>&1 | tee > actual.txt diff --git a/e2e/scenario1/vendor-bin/ns1/composer.json b/e2e/scenario1/vendor-bin/ns1/composer.json new file mode 100644 index 0000000..0967ef4 --- /dev/null +++ b/e2e/scenario1/vendor-bin/ns1/composer.json @@ -0,0 +1 @@ +{} diff --git a/e2e/scenario1/vendor-bin/ns2/composer.json b/e2e/scenario1/vendor-bin/ns2/composer.json new file mode 100644 index 0000000..0967ef4 --- /dev/null +++ b/e2e/scenario1/vendor-bin/ns2/composer.json @@ -0,0 +1 @@ +{} diff --git a/tests/EndToEndTest.php b/tests/EndToEndTest.php new file mode 100644 index 0000000..8a560e5 --- /dev/null +++ b/tests/EndToEndTest.php @@ -0,0 +1,117 @@ +run(); + + self::assertTrue( + $scenarioProcess->isSuccessful(), + $scenarioProcess->getOutput().'|'.$scenarioProcess->getErrorOutput() + ); + + $actual = self::retrieveActualOutput( + getcwd(), + $scenarioPath + ); + ; + + self::assertSame($expected, $actual); + } + + public static function scenarioProvider(): iterable + { + $scenarios = Finder::create() + ->files() + ->depth(1) + ->in(self::E2E_DIR) + ->name('README.md'); + + foreach ($scenarios as $scenario) { + $scenarioPath = dirname($scenario->getPathname()); + $scenarioName = basename($scenarioPath); + $description = trim($scenario->getContents()); + + $title = sprintf( + '[%s] %s', + $scenarioName, + $description + ); + + yield $title => [ + realpath($scenarioPath), + ]; + } + } + + private static function retrieveActualOutput( + string $cwd, + string $scenarioPath + ): string { + $originalContent = file_get_contents($scenarioPath.'/actual.txt'); + + $normalizedContent = str_replace( + $cwd, + '/path/to/project', + $originalContent + ); + + // Sometimes in the CI a different log is shown, e.g. in https://github.com/bamarni/composer-bin-plugin/runs/7246889244 + $normalizedContent = preg_replace( + '/> command: .+\n/', + '', + $normalizedContent + ); + + // Those values come from the expected.txt, it actually does matter how + // many they are at instant t. + $normalizedContent = preg_replace( + '/Analyzed (\d+) packages to resolve dependencies/', + 'Analyzed 90 packages to resolve dependencies', + $normalizedContent + ); + $normalizedContent = preg_replace( + '/Analyzed (\d+) rules to resolve dependencies/', + 'Analyzed 90 rules to resolve dependencies', + $normalizedContent + ); + + return $normalizedContent; + } +} From 0b5068c6daabe17414cae1e64dda28bf633b6cf0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= Date: Fri, 8 Jul 2022 09:31:50 +0200 Subject: [PATCH 25/57] Cleanup application commands between each executions (#96) --- e2e/scenario2/.gitignore | 5 +++ e2e/scenario2/README.md | 1 + e2e/scenario2/composer.json | 16 +++++++++ e2e/scenario2/expected.txt | 4 +++ e2e/scenario2/script.sh | 22 ++++++++++++ e2e/scenario2/vendor-bin/ns1/composer.json | 5 +++ e2e/scenario2/vendor-bin/ns2/composer.json | 5 +++ src/BinCommand.php | 40 ++++++++++++++++++---- tests/EndToEndTest.php | 23 +++++++++---- 9 files changed, 108 insertions(+), 13 deletions(-) create mode 100644 e2e/scenario2/.gitignore create mode 100644 e2e/scenario2/README.md create mode 100644 e2e/scenario2/composer.json create mode 100644 e2e/scenario2/expected.txt create mode 100755 e2e/scenario2/script.sh create mode 100644 e2e/scenario2/vendor-bin/ns1/composer.json create mode 100644 e2e/scenario2/vendor-bin/ns2/composer.json diff --git a/e2e/scenario2/.gitignore b/e2e/scenario2/.gitignore new file mode 100644 index 0000000..cac9c03 --- /dev/null +++ b/e2e/scenario2/.gitignore @@ -0,0 +1,5 @@ +/actual.txt +/composer.lock +/vendor/ +/vendor-bin/*/composer.lock +/vendor-bin/*/vendor/ diff --git a/e2e/scenario2/README.md b/e2e/scenario2/README.md new file mode 100644 index 0000000..432e6c8 --- /dev/null +++ b/e2e/scenario2/README.md @@ -0,0 +1 @@ +Regular script (same name) in different namespaces diff --git a/e2e/scenario2/composer.json b/e2e/scenario2/composer.json new file mode 100644 index 0000000..dff4cca --- /dev/null +++ b/e2e/scenario2/composer.json @@ -0,0 +1,16 @@ +{ + "repositories": [ + { + "type": "path", + "url": "../../" + } + ], + "require-dev": { + "bamarni/composer-bin-plugin": "dev-master" + }, + "config": { + "allow-plugins": { + "bamarni/composer-bin-plugin": true + } + } +} diff --git a/e2e/scenario2/expected.txt b/e2e/scenario2/expected.txt new file mode 100644 index 0000000..195d503 --- /dev/null +++ b/e2e/scenario2/expected.txt @@ -0,0 +1,4 @@ +> echo ns1 +ns1 +> echo ns2 +ns2 diff --git a/e2e/scenario2/script.sh b/e2e/scenario2/script.sh new file mode 100755 index 0000000..7894dab --- /dev/null +++ b/e2e/scenario2/script.sh @@ -0,0 +1,22 @@ +#!/usr/bin/env bash + +set -Eeuo pipefail + +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 all run-script foo 2>&1 | tee > actual.txt diff --git a/e2e/scenario2/vendor-bin/ns1/composer.json b/e2e/scenario2/vendor-bin/ns1/composer.json new file mode 100644 index 0000000..7f276e9 --- /dev/null +++ b/e2e/scenario2/vendor-bin/ns1/composer.json @@ -0,0 +1,5 @@ +{ + "scripts": { + "foo": "echo ns1" + } +} diff --git a/e2e/scenario2/vendor-bin/ns2/composer.json b/e2e/scenario2/vendor-bin/ns2/composer.json new file mode 100644 index 0000000..3b8933b --- /dev/null +++ b/e2e/scenario2/vendor-bin/ns2/composer.json @@ -0,0 +1,5 @@ +{ + "scripts": { + "foo": "echo ns2" + } +} diff --git a/src/BinCommand.php b/src/BinCommand.php index ab37d97..a959221 100644 --- a/src/BinCommand.php +++ b/src/BinCommand.php @@ -7,6 +7,9 @@ use Composer\Command\BaseCommand; use Composer\Factory; use Composer\IO\IOInterface; +use ReflectionClass; +use ReflectionProperty; +use Symfony\Component\Console\Application; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; @@ -72,18 +75,24 @@ 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 + $output, + $commandsReflection ) : $this->executeAllNamespaces( $currentWorkingDir, $vendorRoot, $binInput, - $output + $output, + $commandsReflection ); } @@ -99,7 +108,8 @@ private function executeAllNamespaces( string $originalWorkingDir, string $binVendorRoot, InputInterface $input, - OutputInterface $output + OutputInterface $output, + ReflectionProperty $commandsReflection ): int { $namespaces = self::getBinNamespaces($binVendorRoot); @@ -121,7 +131,8 @@ private function executeAllNamespaces( $originalWorkingDir, $namespace, $input, - $output + $output, + $commandsReflection ); } @@ -132,7 +143,8 @@ private function executeInNamespace( string $originalWorkingDir, string $namespace, InputInterface $input, - OutputInterface $output + OutputInterface $output, + ReflectionProperty $commandsReflection ): int { $this->log( sprintf( @@ -155,11 +167,25 @@ private function executeInNamespace( return self::FAILURE; } + $application = $this->getApplication(); + $commands = $application->all(); + // 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): void { + $cleanUp = function () use ( + $originalWorkingDir, + $commandsReflection, + $application, + $commands + ): 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); @@ -176,7 +202,7 @@ private function executeInNamespace( ); try { - $exitCode = $this->getApplication()->doRun($namespaceInput, $output); + $exitCode = $application->doRun($namespaceInput, $output); } catch (Throwable $executionFailed) { // Ensure we do the cleanup even in case of failure $cleanUp(); diff --git a/tests/EndToEndTest.php b/tests/EndToEndTest.php index 8a560e5..cf3c096 100644 --- a/tests/EndToEndTest.php +++ b/tests/EndToEndTest.php @@ -41,16 +41,29 @@ public function test_it_passes_the_e2e_test(string $scenarioPath): void $scenarioProcess->run(); + $standardOutput = $scenarioProcess->getOutput(); + $errorOutput = $scenarioProcess->getErrorOutput(); + + $originalContent = file_get_contents($scenarioPath.'/actual.txt'); + self::assertTrue( $scenarioProcess->isSuccessful(), - $scenarioProcess->getOutput().'|'.$scenarioProcess->getErrorOutput() + << Date: Fri, 8 Jul 2022 10:02:27 +0200 Subject: [PATCH 26/57] Add test (#97) --- tests/BinInputFactoryTest.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/BinInputFactoryTest.php b/tests/BinInputFactoryTest.php index 24aa973..52ad36b 100644 --- a/tests/BinInputFactoryTest.php +++ b/tests/BinInputFactoryTest.php @@ -44,6 +44,13 @@ public static function inputProvider(): iterable new StringInput('bin --ansi foo-namespace flex:update --prefer-lowest'), new StringInput('--ansi flex:update --prefer-lowest'), ]; + + // See https://github.com/bamarni/composer-bin-plugin/pull/73 + yield [ + 'irrelevant', + new StringInput('update --dry-run --no-plugins roave/security-advisories'), + new StringInput('update --dry-run --no-plugins roave/security-advisories'), + ]; } /** From 5222278dfe808fcd95f27dfea1fc2ad472c788f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= Date: Fri, 8 Jul 2022 20:33:05 +0200 Subject: [PATCH 27/57] Mark classes as final (#98) --- src/BinCommand.php | 3 +++ src/CommandProvider.php | 3 +++ src/ConfigFactory.php | 6 ++++-- src/Plugin.php | 3 +++ 4 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/BinCommand.php b/src/BinCommand.php index a959221..3b0733d 100644 --- a/src/BinCommand.php +++ b/src/BinCommand.php @@ -26,6 +26,9 @@ use function sprintf; use const GLOB_ONLYDIR; +/** + * @final Will be final in 2.x. + */ class BinCommand extends BaseCommand { private const ALL_NAMESPACES = 'all'; diff --git a/src/CommandProvider.php b/src/CommandProvider.php index c5266fd..842f5ad 100644 --- a/src/CommandProvider.php +++ b/src/CommandProvider.php @@ -6,6 +6,9 @@ use Composer\Plugin\Capability\CommandProvider as CommandProviderCapability; +/** + * @final Will be final in 2.x. + */ class CommandProvider implements CommandProviderCapability { public function getCommands(): array diff --git a/src/ConfigFactory.php b/src/ConfigFactory.php index 5efde10..2243934 100644 --- a/src/ConfigFactory.php +++ b/src/ConfigFactory.php @@ -7,12 +7,14 @@ use Composer\Config as ComposerConfig; use Composer\Factory; use Composer\Json\JsonFile; +use Composer\Json\JsonValidationException; +use Seld\JsonLint\ParsingException; final class ConfigFactory { /** - * @throws \Composer\Json\JsonValidationException - * @throws \Seld\JsonLint\ParsingException + * @throws JsonValidationException + * @throws ParsingException */ public static function createConfig(): ComposerConfig { diff --git a/src/Plugin.php b/src/Plugin.php index b506661..ff97deb 100644 --- a/src/Plugin.php +++ b/src/Plugin.php @@ -21,6 +21,9 @@ use function array_filter; use function array_keys; +/** + * @final Will be final in 2.x. + */ class Plugin implements PluginInterface, Capable, EventSubscriberInterface { /** From 09a868e3d716b0565daf1f2937f27ea0ab490588 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= Date: Fri, 8 Jul 2022 21:24:31 +0200 Subject: [PATCH 28/57] Add a Logger class (#99) --- src/BinCommand.php | 61 ++++++++++++-------- src/Logger.php | 39 +++++++++++++ tests/BinCommandTest.php | 3 + tests/BinInputFactoryTest.php | 3 + tests/EndToEndTest.php | 1 + tests/LoggerTest.php | 105 ++++++++++++++++++++++++++++++++++ 6 files changed, 189 insertions(+), 23 deletions(-) create mode 100644 src/Logger.php create mode 100644 tests/LoggerTest.php diff --git a/src/BinCommand.php b/src/BinCommand.php index 3b0733d..73c5da3 100644 --- a/src/BinCommand.php +++ b/src/BinCommand.php @@ -7,6 +7,7 @@ use Composer\Command\BaseCommand; use Composer\Factory; use Composer\IO\IOInterface; +use Composer\IO\NullIO; use ReflectionClass; use ReflectionProperty; use Symfony\Component\Console\Application; @@ -35,10 +36,21 @@ class BinCommand extends BaseCommand private const NAMESPACE_ARG = 'namespace'; + /** + * @var Logger + */ + private $logger; + + public function __construct(?Logger $logger = null) + { + parent::__construct('bin'); + + $this->logger = $logger ?? new Logger(new NullIO()); + } + protected function configure(): void { $this - ->setName('bin') ->setDescription('Run a command inside a bin namespace') ->setDefinition([ new InputArgument(self::NAMESPACE_ARG, InputArgument::REQUIRED), @@ -47,6 +59,22 @@ protected function configure(): void ->ignoreValidationErrors(); } + public function setIO(IOInterface $io): void + { + parent::setIO($io); + + $this->logger = new Logger($io); + } + + public function getIO(): IOInterface + { + $io = parent::getIO(); + + $this->logger = new Logger($io); + + return $io; + } + public function isProxyCommand(): bool { return true; @@ -57,7 +85,7 @@ public function execute(InputInterface $input, OutputInterface $output): int $config = Config::fromComposer($this->requireComposer()); $currentWorkingDir = getcwd(); - $this->log( + $this->logger->logDebug( sprintf( 'Current working directory: %s', $currentWorkingDir @@ -117,10 +145,7 @@ private function executeAllNamespaces( $namespaces = self::getBinNamespaces($binVendorRoot); if (count($namespaces) === 0) { - $this->log( - 'Could not find any bin namespace.', - false - ); + $this->logger->logStandard('Could not find any bin namespace.'); // Is a valid scenario: the user may not have set up any bin // namespace yet @@ -149,7 +174,7 @@ private function executeInNamespace( OutputInterface $output, ReflectionProperty $commandsReflection ): int { - $this->log( + $this->logger->logDebug( sprintf( 'Checking namespace %s', $namespace @@ -159,12 +184,11 @@ private function executeInNamespace( try { self::createNamespaceDirIfDoesNotExist($namespace); } catch (CouldNotCreateNamespaceDir $exception) { - $this->log( + $this->logger->logStandard( sprintf( '%s', $exception->getMessage() - ), - false + ) ); return self::FAILURE; @@ -197,7 +221,7 @@ private function executeInNamespace( $namespaceInput = BinInputFactory::createNamespaceInput($input); - $this->log( + $this->logger->logDebug( sprintf( 'Running `@composer %s`.', $namespaceInput @@ -249,7 +273,7 @@ private function configureBinLinksDir(Config $config): void ) ); - $this->log( + $this->logger->logDebug( sprintf( 'Configuring bin directory to %s.', $binDir @@ -268,7 +292,7 @@ private function ensureComposerFileExists(): void file_put_contents($namespaceComposerFile, '{}'); - $this->log( + $this->logger->logDebug( sprintf( 'Created the file %s.', $namespaceComposerFile @@ -291,20 +315,11 @@ private function chdir(string $dir): void { chdir($dir); - $this->log( + $this->logger->logDebug( sprintf( 'Changed current directory to %s.', $dir ) ); } - - private function log(string $message, bool $debug = true): void - { - $verbosity = $debug - ? IOInterface::VERBOSE - : IOInterface::NORMAL; - - $this->getIO()->writeError('[bamarni-bin-plugin] '.$message, true, $verbosity); - } } diff --git a/src/Logger.php b/src/Logger.php new file mode 100644 index 0000000..a71f857 --- /dev/null +++ b/src/Logger.php @@ -0,0 +1,39 @@ +io = $io; + } + + public function logStandard(string $message): void + { + $this->log($message, false); + } + + public function logDebug(string $message): void + { + $this->log($message, true); + } + + private function log(string $message, bool $debug): void + { + $verbosity = $debug + ? IOInterface::VERBOSE + : IOInterface::NORMAL; + + $this->io->writeError('[bamarni-bin-plugin] '.$message, true, $verbosity); + } +} diff --git a/tests/BinCommandTest.php b/tests/BinCommandTest.php index c91dc0a..462bbec 100644 --- a/tests/BinCommandTest.php +++ b/tests/BinCommandTest.php @@ -22,6 +22,9 @@ use function realpath; use function sys_get_temp_dir; +/** + * @covers \Bamarni\Composer\Bin\BinCommand + */ class BinCommandTest extends TestCase { /** diff --git a/tests/BinInputFactoryTest.php b/tests/BinInputFactoryTest.php index 52ad36b..e43c9a3 100644 --- a/tests/BinInputFactoryTest.php +++ b/tests/BinInputFactoryTest.php @@ -9,6 +9,9 @@ use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\StringInput; +/** + * @covers \Bamarni\Composer\Bin\BinInputFactory + */ final class BinInputFactoryTest extends TestCase { /** diff --git a/tests/EndToEndTest.php b/tests/EndToEndTest.php index cf3c096..22b93b7 100644 --- a/tests/EndToEndTest.php +++ b/tests/EndToEndTest.php @@ -19,6 +19,7 @@ /** * @group e2e + * @coversNothing */ final class EndToEndTest extends TestCase { diff --git a/tests/LoggerTest.php b/tests/LoggerTest.php new file mode 100644 index 0000000..6452f69 --- /dev/null +++ b/tests/LoggerTest.php @@ -0,0 +1,105 @@ +logStandard($message); + + self::assertSame($expected, $io->getOutput()); + } + + public static function standardMessageProvider(): iterable + { + $notLoggedVerbosities = [ + OutputInterface::VERBOSITY_QUIET, + ]; + + $loggedVerbosities = array_diff( + self::VERBOSITIES, + $notLoggedVerbosities + ); + + $message = 'Hello world!'; + $expected = '[bamarni-bin-plugin] Hello world!'.PHP_EOL; + + foreach ($notLoggedVerbosities as $verbosity) { + yield [$verbosity, $message, '']; + } + + foreach ($loggedVerbosities as $verbosity) { + yield [$verbosity, $message, $expected]; + } + } + + /** + * @dataProvider standardMessageProvider + */ + public function test_it_can_log_debug_messages( + int $verbosity, + string $message, + string $expected + ): void { + $io = new BufferIO('', $verbosity); + $logger = new Logger($io); + + $logger->logStandard($message); + + self::assertSame($expected, $io->getOutput()); + } + + public static function debugMessageProvider(): iterable + { + $notLoggedVerbosities = [ + OutputInterface::VERBOSITY_QUIET, + OutputInterface::VERBOSITY_NORMAL, + ]; + + $loggedVerbosities = array_diff( + self::VERBOSITIES, + $notLoggedVerbosities + ); + + $message = 'Hello world!'; + $expected = '[bamarni-bin-plugin] Hello world!'.PHP_EOL; + + foreach ($notLoggedVerbosities as $verbosity) { + yield [$verbosity, $message, '']; + } + + foreach ($loggedVerbosities as $verbosity) { + yield [$verbosity, $message, $expected]; + } + } +} From 6788f2343ed6b5a5b26894073a17171d3668eb66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= Date: Fri, 8 Jul 2022 21:30:24 +0200 Subject: [PATCH 29/57] Rename plugin log prefix (#100) Since the config in `extra` is `bamarni-bin`, I think it's better to keep the prefix `[bamarni-bin]` instead of `[bamarni-bin-plugin]`. --- e2e/scenario1/expected.txt | 20 ++++++++++---------- src/Logger.php | 2 +- tests/LoggerTest.php | 4 ++-- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/e2e/scenario1/expected.txt b/e2e/scenario1/expected.txt index 23cc600..161b70a 100644 --- a/e2e/scenario1/expected.txt +++ b/e2e/scenario1/expected.txt @@ -1,8 +1,8 @@ -[bamarni-bin-plugin] Current working directory: /path/to/project/e2e/scenario1 -[bamarni-bin-plugin] Configuring bin directory to /path/to/project/e2e/scenario1/vendor/bin. -[bamarni-bin-plugin] Checking namespace vendor-bin/ns1 -[bamarni-bin-plugin] Changed current directory to vendor-bin/ns1. -[bamarni-bin-plugin] Running `@composer update --verbose --working-dir='.'`. +[bamarni-bin] Current working directory: /path/to/project/e2e/scenario1 +[bamarni-bin] Configuring bin directory to /path/to/project/e2e/scenario1/vendor/bin. +[bamarni-bin] Checking namespace vendor-bin/ns1 +[bamarni-bin] Changed current directory to vendor-bin/ns1. +[bamarni-bin] Running `@composer update --verbose --working-dir='.'`. Loading composer repositories with package information Updating dependencies Dependency resolution completed in 0.000 seconds @@ -13,10 +13,10 @@ Writing lock file Installing dependencies from lock file (including require-dev) Nothing to install, update or remove Generating autoload files -[bamarni-bin-plugin] Changed current directory to /path/to/project/e2e/scenario1. -[bamarni-bin-plugin] Checking namespace vendor-bin/ns2 -[bamarni-bin-plugin] Changed current directory to vendor-bin/ns2. -[bamarni-bin-plugin] Running `@composer update --verbose --working-dir='.'`. +[bamarni-bin] Changed current directory to /path/to/project/e2e/scenario1. +[bamarni-bin] Checking namespace vendor-bin/ns2 +[bamarni-bin] Changed current directory to vendor-bin/ns2. +[bamarni-bin] Running `@composer update --verbose --working-dir='.'`. Loading composer repositories with package information Updating dependencies Dependency resolution completed in 0.000 seconds @@ -27,4 +27,4 @@ Writing lock file Installing dependencies from lock file (including require-dev) Nothing to install, update or remove Generating autoload files -[bamarni-bin-plugin] Changed current directory to /path/to/project/e2e/scenario1. +[bamarni-bin] Changed current directory to /path/to/project/e2e/scenario1. diff --git a/src/Logger.php b/src/Logger.php index a71f857..1905719 100644 --- a/src/Logger.php +++ b/src/Logger.php @@ -34,6 +34,6 @@ private function log(string $message, bool $debug): void ? IOInterface::VERBOSE : IOInterface::NORMAL; - $this->io->writeError('[bamarni-bin-plugin] '.$message, true, $verbosity); + $this->io->writeError('[bamarni-bin] '.$message, true, $verbosity); } } diff --git a/tests/LoggerTest.php b/tests/LoggerTest.php index 6452f69..0f7a64f 100644 --- a/tests/LoggerTest.php +++ b/tests/LoggerTest.php @@ -52,7 +52,7 @@ public static function standardMessageProvider(): iterable ); $message = 'Hello world!'; - $expected = '[bamarni-bin-plugin] Hello world!'.PHP_EOL; + $expected = '[bamarni-bin] Hello world!'.PHP_EOL; foreach ($notLoggedVerbosities as $verbosity) { yield [$verbosity, $message, '']; @@ -92,7 +92,7 @@ public static function debugMessageProvider(): iterable ); $message = 'Hello world!'; - $expected = '[bamarni-bin-plugin] Hello world!'.PHP_EOL; + $expected = '[bamarni-bin] Hello world!'.PHP_EOL; foreach ($notLoggedVerbosities as $verbosity) { yield [$verbosity, $message, '']; From 6512428d7948b1d28f23012ee1439c2c180ba106 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= Date: Fri, 8 Jul 2022 21:44:49 +0200 Subject: [PATCH 30/57] Simplify workflow (#101) With the introducing of end-to-end tests, I do not feel it is needed anymore to use the plugin on itself for PHPUnit. Since using the plugin on itself actually comes with a non-negligible cost: - the CI is a bit more complex - when developing on the project you need to install the plugin globally (not ideal) --- .github/workflows/tests.yaml | 29 ----------------------------- README.md | 4 ++-- composer.json | 9 +-------- vendor-bin/phpunit/composer.json | 5 ----- 4 files changed, 3 insertions(+), 44 deletions(-) delete mode 100644 vendor-bin/phpunit/composer.json diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index cd0f96d..99dcccc 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -31,24 +31,11 @@ jobs: php-version: "${{ matrix.php }}" tools: "composer" - - name: "Configure global composer" - run: | - composer global config repositories.bin path $PWD - composer global config allow-plugins.bamarni/composer-bin-plugin true - # It is not used in the CI and the PHP constraints may not match # the CI. - name: "Remove PHP-CS-Fixer" run: composer remove --dev --no-update php-cs-fixer/shim - - name: "Install bin plugin globally (PR-only)" - if: github.event_name == 'pull_request' - run: composer global require bamarni/composer-bin-plugin:dev-${GITHUB_SHA} - - - name: "Install bin plugin globally (master-only)" - if: github.event_name != 'pull_request' - run: composer global require bamarni/composer-bin-plugin:dev-master - - name: "Install Composer dependencies" uses: "ramsey/composer-install@v2" with: @@ -79,35 +66,19 @@ jobs: php-version: "${{ matrix.php }}" tools: "composer" - - name: "Configure global composer" - run: | - composer global config repositories.bin path $PWD - composer global config allow-plugins.bamarni/composer-bin-plugin true - # It is not used in the CI and the PHP constraints may not match # the CI. - name: "Remove PHP-CS-Fixer" run: composer remove --dev --no-update php-cs-fixer/shim - - name: "Install bin plugin globally (PR-only)" - if: github.event_name == 'pull_request' - run: composer global require bamarni/composer-bin-plugin:dev-${GITHUB_SHA} - - name: "Correct bin plugin version for e2e scenarios (PR-only)" if: github.event_name == 'pull_request' run: find e2e -maxdepth 1 -mindepth 1 -type d -exec bash -c "cd {} && composer require --dev bamarni/composer-bin-plugin:dev-${GITHUB_SHA} --no-update" \; - - name: "Install bin plugin globally (master-only)" - if: github.event_name != 'pull_request' - run: composer global require bamarni/composer-bin-plugin:dev-master - - name: "Install Composer dependencies" uses: "ramsey/composer-install@v2" with: dependency-versions: "highest" - - name: "Validate composer.json" - run: "composer validate --strict --no-check-lock" - - name: "Run tests" run: "vendor/bin/phpunit --group e2e" diff --git a/README.md b/README.md index 4ce1cbb..16a96f5 100644 --- a/README.md +++ b/README.md @@ -235,9 +235,9 @@ vendor-bin/**/composer.lock binary Before getting started, you need to install the plugin globally: ```bash -$ composer global require --dev bamarni/composer-bin-plugin -$ composer install # now works +$ composer install $ vendor/bin/phpunit # run the tests +$ vendor/bin/php-cs-fixer # run PHP-CS-Fixer ``` diff --git a/composer.json b/composer.json index 6463bc0..621c385 100644 --- a/composer.json +++ b/composer.json @@ -20,6 +20,7 @@ "composer/composer": "^2.0", "php-cs-fixer/shim": "^3.8", "phpstan/extension-installer": "^1.1", + "phpunit/phpunit": "^8.5 || ^9.5", "symfony/console": "^5.4.7 || ^6.0.7", "symfony/finder": "^5.4.7 || ^6.0.7", "symfony/process": "^5.4.7 || ^6.0.7" @@ -42,13 +43,5 @@ "psr-4": { "Bamarni\\Composer\\Bin\\Tests\\": "tests" } - }, - "scripts": { - "post-install-cmd": [ - "@composer bin phpunit install" - ], - "post-update-cmd": [ - "@post-install-cmd" - ] } } diff --git a/vendor-bin/phpunit/composer.json b/vendor-bin/phpunit/composer.json deleted file mode 100644 index 53850b5..0000000 --- a/vendor-bin/phpunit/composer.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "require": { - "phpunit/phpunit": "^8.5 || ^9.5" - } -} From 047bb4bf4a70cd30bb5314b16a0e53f807451d71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= Date: Fri, 8 Jul 2022 22:51:32 +0200 Subject: [PATCH 31/57] Add tests for the forward command setting (#102) Related to #72 --- e2e/scenario3/.gitignore | 5 ++ e2e/scenario3/README.md | 1 + e2e/scenario3/composer.json | 21 ++++++ e2e/scenario3/expected.txt | 39 +++++++++++ e2e/scenario3/script.sh | 22 ++++++ e2e/scenario3/vendor-bin/ns1/composer.json | 1 + src/BinInputFactory.php | 10 +++ src/Plugin.php | 79 ++++++++++++---------- tests/BinInputFactoryTest.php | 25 +++++++ tests/EndToEndTest.php | 24 ++++++- 10 files changed, 189 insertions(+), 38 deletions(-) create mode 100644 e2e/scenario3/.gitignore create mode 100644 e2e/scenario3/README.md create mode 100644 e2e/scenario3/composer.json create mode 100644 e2e/scenario3/expected.txt create mode 100755 e2e/scenario3/script.sh create mode 100644 e2e/scenario3/vendor-bin/ns1/composer.json diff --git a/e2e/scenario3/.gitignore b/e2e/scenario3/.gitignore new file mode 100644 index 0000000..cac9c03 --- /dev/null +++ b/e2e/scenario3/.gitignore @@ -0,0 +1,5 @@ +/actual.txt +/composer.lock +/vendor/ +/vendor-bin/*/composer.lock +/vendor-bin/*/vendor/ diff --git a/e2e/scenario3/README.md b/e2e/scenario3/README.md new file mode 100644 index 0000000..ad1c3b8 --- /dev/null +++ b/e2e/scenario3/README.md @@ -0,0 +1 @@ +Regular installation on a project with forwarded mode enabled. diff --git a/e2e/scenario3/composer.json b/e2e/scenario3/composer.json new file mode 100644 index 0000000..d449a2e --- /dev/null +++ b/e2e/scenario3/composer.json @@ -0,0 +1,21 @@ +{ + "repositories": [ + { + "type": "path", + "url": "../../" + } + ], + "require-dev": { + "bamarni/composer-bin-plugin": "dev-master" + }, + "config": { + "allow-plugins": { + "bamarni/composer-bin-plugin": true + } + }, + "extra": { + "bamarni-bin": { + "forward-command": true + } + } +} diff --git a/e2e/scenario3/expected.txt b/e2e/scenario3/expected.txt new file mode 100644 index 0000000..3325663 --- /dev/null +++ b/e2e/scenario3/expected.txt @@ -0,0 +1,39 @@ +Loading composer repositories with package information +Updating dependencies +Dependency resolution completed in 0.000 seconds +Analyzed 90 packages to resolve dependencies +Analyzed 90 rules to resolve dependencies +Dependency resolution completed in 0.000 seconds +Lock file operations: 1 install, 0 updates, 0 removals +Installs: bamarni/composer-bin-plugin:dev-hash + - Locking bamarni/composer-bin-plugin (dev-hash) +Writing lock file +Installing dependencies from lock file (including require-dev) +Package operations: 1 install, 0 updates, 0 removals +Installs: bamarni/composer-bin-plugin:dev-hash + - Installing bamarni/composer-bin-plugin (dev-hash): Symlinking from ../.. +Generating autoload files +––––––––––––––––––––– +[bamarni-bin] Calling onCommandEvent(). +[bamarni-bin] The command is being forwarded. +[bamarni-bin] Original input: update --verbose. +Loading composer repositories with package information +Updating dependencies +Dependency resolution completed in 0.000 seconds +Analyzed 90 packages to resolve dependencies +Analyzed 90 rules to resolve dependencies +Nothing to modify in lock file +Writing lock file +Installing dependencies from lock file (including require-dev) +Nothing to install, update or remove +Generating autoload files +Loading composer repositories with package information +Updating dependencies +Dependency resolution completed in 0.000 seconds +Analyzed 90 packages to resolve dependencies +Analyzed 90 rules to resolve dependencies +Nothing to modify in lock file +Dependency resolution completed in 0.000 seconds +Installing dependencies from lock file (including require-dev) +Nothing to install, update or remove +Generating autoload files diff --git a/e2e/scenario3/script.sh b/e2e/scenario3/script.sh new file mode 100755 index 0000000..2b31397 --- /dev/null +++ b/e2e/scenario3/script.sh @@ -0,0 +1,22 @@ +#!/usr/bin/env bash + +set -Eeuo pipefail + +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 + +# Actual command to execute the test itself +composer update --verbose 2>&1 | tee > actual.txt +echo "–––––––––––––––––––––" >> actual.txt +composer update --verbose 2>&1 | tee >> actual.txt diff --git a/e2e/scenario3/vendor-bin/ns1/composer.json b/e2e/scenario3/vendor-bin/ns1/composer.json new file mode 100644 index 0000000..0967ef4 --- /dev/null +++ b/e2e/scenario3/vendor-bin/ns1/composer.json @@ -0,0 +1 @@ +{} diff --git a/src/BinInputFactory.php b/src/BinInputFactory.php index ea9265d..2faac72 100644 --- a/src/BinInputFactory.php +++ b/src/BinInputFactory.php @@ -31,6 +31,16 @@ public static function createNamespaceInput(InputInterface $previousInput): Inpu return new StringInput((string) $previousInput . ' --working-dir=.'); } + public static function createForwardedCommandInput(InputInterface $input): InputInterface + { + return new StringInput( + sprintf( + 'bin all %s', + (string) $input + ) + ); + } + private function __construct() { } diff --git a/src/Plugin.php b/src/Plugin.php index ff97deb..6891e74 100644 --- a/src/Plugin.php +++ b/src/Plugin.php @@ -13,19 +13,19 @@ use Composer\Plugin\PluginEvents; use Composer\Plugin\PluginInterface; use Composer\Plugin\Capable; -use Symfony\Component\Console\Input\ArrayInput; -use Symfony\Component\Console\Input\InputArgument; -use Symfony\Component\Console\Input\InputDefinition; +use Symfony\Component\Console\Command\Command; use Composer\Plugin\Capability\CommandProvider as ComposerPluginCommandProvider; use Throwable; -use function array_filter; -use function array_keys; +use function in_array; +use function sprintf; /** * @final Will be final in 2.x. */ class Plugin implements PluginInterface, Capable, EventSubscriberInterface { + private const FORWARDED_COMMANDS = ['install', 'update']; + /** * @var Composer */ @@ -36,10 +36,16 @@ class Plugin implements PluginInterface, Capable, EventSubscriberInterface */ protected $io; + /** + * @var Logger + */ + private $logger; + public function activate(Composer $composer, IOInterface $io): void { $this->composer = $composer; $this->io = $io; + $this->logger = new Logger($io); } public function getCapabilities(): array @@ -66,52 +72,51 @@ public static function getSubscribedEvents(): array public function onCommandEvent(CommandEvent $event): bool { + $this->logger->logDebug('Calling onCommandEvent().'); + $config = Config::fromComposer($this->composer); - if ($config->isCommandForwarded()) { - switch ($event->getCommandName()) { - case 'update': - case 'install': - return $this->onCommandEventInstallUpdate($event); - } + if ($config->isCommandForwarded() + && in_array($event->getCommandName(), self::FORWARDED_COMMANDS, true) + ) { + return $this->onForwardedCommand($event); } return true; } - protected function onCommandEventInstallUpdate(CommandEvent $event): bool + protected function onForwardedCommand(CommandEvent $event): bool { - $command = new BinCommand(); + $this->logger->logDebug('The command is being forwarded.'); + $this->logger->logDebug( + sprintf( + 'Original input: %s.', + $event->getInput() + ) + ); + + // Note that the input & output of $io should be the same as the event + // input & output. + $io = $this->io; + $logger = new Logger($io); + + $application = new Application(); + + $command = new BinCommand($logger); $command->setComposer($this->composer); - $command->setApplication(new Application()); + $command->setApplication($application); - $arguments = [ - 'command' => $command->getName(), - 'namespace' => 'all', - 'args' => [], - ]; - - foreach (array_filter($event->getInput()->getArguments()) as $argument) { - $arguments['args'][] = $argument; - } - - foreach (array_keys(array_filter($event->getInput()->getOptions())) as $option) { - $arguments['args'][] = '--' . $option; - } - - $definition = new InputDefinition(); - $definition->addArgument(new InputArgument('command', InputArgument::REQUIRED)); - $definition->addArguments($command->getDefinition()->getArguments()); - $definition->addOptions($command->getDefinition()->getOptions()); - - $input = new ArrayInput($arguments, $definition); + $forwardedCommandInput = BinInputFactory::createForwardedCommandInput( + $event->getInput() + ); try { - $returnCode = $command->run($input, $event->getOutput()); + return Command::SUCCESS === $command->run( + $forwardedCommandInput, + $event->getOutput() + ); } catch (Throwable $throwable) { return false; } - - return $returnCode === 0; } } diff --git a/tests/BinInputFactoryTest.php b/tests/BinInputFactoryTest.php index e43c9a3..8bf6b97 100644 --- a/tests/BinInputFactoryTest.php +++ b/tests/BinInputFactoryTest.php @@ -80,4 +80,29 @@ public static function namespaceInputProvider(): iterable new StringInput('flex:update --prefer-lowest --ansi --working-dir=.'), ]; } + + /** + * @dataProvider forwardedCommandInputProvider + */ + public function test_it_can_create_a_new_input_for_forwarded_command( + InputInterface $previousInput, + InputInterface $expected + ): void { + $actual = BinInputFactory::createForwardedCommandInput($previousInput); + + self::assertEquals($expected, $actual); + } + + public static function forwardedCommandInputProvider(): iterable + { + yield [ + new StringInput('install --verbose'), + new StringInput('bin all install --verbose'), + ]; + + yield [ + new StringInput('flex:update --prefer-lowest --ansi'), + new StringInput('bin all flex:update --prefer-lowest --ansi'), + ]; + } } diff --git a/tests/EndToEndTest.php b/tests/EndToEndTest.php index 22b93b7..ac2799c 100644 --- a/tests/EndToEndTest.php +++ b/tests/EndToEndTest.php @@ -9,6 +9,7 @@ use Symfony\Component\Process\Process; use function basename; use function dirname; +use function file_exists; use function file_get_contents; use function getcwd; use function preg_replace; @@ -45,7 +46,13 @@ public function test_it_passes_the_e2e_test(string $scenarioPath): void $standardOutput = $scenarioProcess->getOutput(); $errorOutput = $scenarioProcess->getErrorOutput(); - $originalContent = file_get_contents($scenarioPath.'/actual.txt'); + $actualPath = $scenarioPath.'/actual.txt'; + + if (file_exists($actualPath)) { + $originalContent = file_get_contents($scenarioPath.'/actual.txt'); + } else { + $originalContent = 'File was not created.'; + } self::assertTrue( $scenarioProcess->isSuccessful(), @@ -123,6 +130,21 @@ private static function retrieveActualOutput( 'Analyzed 90 rules to resolve dependencies', $normalizedContent ); + $normalizedContent = preg_replace( + '/Installs: bamarni\/composer-bin-plugin:dev-.+/', + 'Installs: bamarni/composer-bin-plugin:dev-hash', + $normalizedContent + ); + $normalizedContent = preg_replace( + '/Locking bamarni\/composer-bin-plugin \(dev-.+\)/', + 'Locking bamarni/composer-bin-plugin (dev-hash)', + $normalizedContent + ); + $normalizedContent = preg_replace( + '/Installing bamarni\/composer-bin-plugin \(dev-.+\): Symlinking from \.\.\/\.\./', + 'Installing bamarni/composer-bin-plugin (dev-hash): Symlinking from ../..', + $normalizedContent + ); return $normalizedContent; } From bb0a30202986d0183ff4b4e266da115133f5c6a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= Date: Fri, 8 Jul 2022 23:50:37 +0200 Subject: [PATCH 32/57] Fix logging when forwarding the command (#104) Although the logger was correctly configured, when initializing the command the IO was not configured properly and as a result `getIO()` resulted in a `NullIO`, which reconfigured the logger... --- e2e/scenario3/expected.txt | 6 ++++++ src/Plugin.php | 8 +++----- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/e2e/scenario3/expected.txt b/e2e/scenario3/expected.txt index 3325663..b3bc6a5 100644 --- a/e2e/scenario3/expected.txt +++ b/e2e/scenario3/expected.txt @@ -17,6 +17,11 @@ Generating autoload files [bamarni-bin] Calling onCommandEvent(). [bamarni-bin] The command is being forwarded. [bamarni-bin] Original input: update --verbose. +[bamarni-bin] Current working directory: /path/to/project/e2e/scenario3 +[bamarni-bin] Configuring bin directory to /path/to/project/e2e/scenario3/vendor/bin. +[bamarni-bin] Checking namespace vendor-bin/ns1 +[bamarni-bin] Changed current directory to vendor-bin/ns1. +[bamarni-bin] Running `@composer update --verbose --working-dir='.'`. Loading composer repositories with package information Updating dependencies Dependency resolution completed in 0.000 seconds @@ -27,6 +32,7 @@ Writing lock file Installing dependencies from lock file (including require-dev) Nothing to install, update or remove Generating autoload files +[bamarni-bin] Changed current directory to /path/to/project/e2e/scenario3. Loading composer repositories with package information Updating dependencies Dependency resolution completed in 0.000 seconds diff --git a/src/Plugin.php b/src/Plugin.php index 6891e74..404be36 100644 --- a/src/Plugin.php +++ b/src/Plugin.php @@ -98,13 +98,11 @@ protected function onForwardedCommand(CommandEvent $event): bool // Note that the input & output of $io should be the same as the event // input & output. $io = $this->io; - $logger = new Logger($io); - $application = new Application(); - - $command = new BinCommand($logger); + $command = new BinCommand(); $command->setComposer($this->composer); - $command->setApplication($application); + $command->setIO($io); + $command->setApplication(new Application()); $forwardedCommandInput = BinInputFactory::createForwardedCommandInput( $event->getInput() From 7dd88b60b082a1ea3f7a0ced44943cb87eb6a0fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= Date: Fri, 8 Jul 2022 23:56:52 +0200 Subject: [PATCH 33/57] Fix forwarded command does not work on first install (#103) Closes #72 The issue was that `PluginEvents::COMMAND` is not triggered at all on the first install. The trick done here is to hook on the post dump autoload instead. This requires a few hack: - extend `ConsoleIO` to expose the input & output - ensure we are not forwarding the command twice But I think overall this delivers a better user experience. --- e2e/scenario3/expected.txt | 24 ++++++++++++- src/Plugin.php | 74 ++++++++++++++++++++++++++++++++------ src/PublicIO.php | 31 ++++++++++++++++ tests/EndToEndTest.php | 5 +++ 4 files changed, 122 insertions(+), 12 deletions(-) create mode 100644 src/PublicIO.php diff --git a/e2e/scenario3/expected.txt b/e2e/scenario3/expected.txt index b3bc6a5..9e29669 100644 --- a/e2e/scenario3/expected.txt +++ b/e2e/scenario3/expected.txt @@ -13,6 +13,26 @@ Package operations: 1 install, 0 updates, 0 removals Installs: bamarni/composer-bin-plugin:dev-hash - Installing bamarni/composer-bin-plugin (dev-hash): Symlinking from ../.. Generating autoload files +> post-autoload-dump: Bamarni\Composer\Bin\Plugin->onPostAutoloadDump +[bamarni-bin] Calling onPostAutoloadDump(). +[bamarni-bin] The command is being forwarded. +[bamarni-bin] Original input: update --verbose. +[bamarni-bin] Current working directory: /path/to/project/e2e/scenario3 +[bamarni-bin] Configuring bin directory to /path/to/project/e2e/scenario3/vendor/bin. +[bamarni-bin] Checking namespace vendor-bin/ns1 +[bamarni-bin] Changed current directory to vendor-bin/ns1. +[bamarni-bin] Running `@composer update --verbose --working-dir='.'`. +Loading composer repositories with package information +Updating dependencies +Dependency resolution completed in 0.000 seconds +Analyzed 90 packages to resolve dependencies +Analyzed 90 rules to resolve dependencies +Nothing to modify in lock file +Writing lock file +Installing dependencies from lock file (including require-dev) +Nothing to install, update or remove +Generating autoload files +[bamarni-bin] Changed current directory to /path/to/project/e2e/scenario3. ––––––––––––––––––––– [bamarni-bin] Calling onCommandEvent(). [bamarni-bin] The command is being forwarded. @@ -28,7 +48,6 @@ Dependency resolution completed in 0.000 seconds Analyzed 90 packages to resolve dependencies Analyzed 90 rules to resolve dependencies Nothing to modify in lock file -Writing lock file Installing dependencies from lock file (including require-dev) Nothing to install, update or remove Generating autoload files @@ -43,3 +62,6 @@ Dependency resolution completed in 0.000 seconds Installing dependencies from lock file (including require-dev) Nothing to install, update or remove Generating autoload files +> post-autoload-dump: Bamarni\Composer\Bin\Plugin->onPostAutoloadDump +[bamarni-bin] Calling onPostAutoloadDump(). +[bamarni-bin] Command already forwarded within the process: skipping. diff --git a/src/Plugin.php b/src/Plugin.php index 404be36..dc65124 100644 --- a/src/Plugin.php +++ b/src/Plugin.php @@ -8,13 +8,18 @@ use Composer\Composer; use Composer\Console\Application; use Composer\EventDispatcher\EventSubscriberInterface; +use Composer\IO\ConsoleIO; use Composer\IO\IOInterface; use Composer\Plugin\CommandEvent; use Composer\Plugin\PluginEvents; use Composer\Plugin\PluginInterface; use Composer\Plugin\Capable; +use Composer\Script\Event; +use Composer\Script\ScriptEvents; use Symfony\Component\Console\Command\Command; use Composer\Plugin\Capability\CommandProvider as ComposerPluginCommandProvider; +use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Output\OutputInterface; use Throwable; use function in_array; use function sprintf; @@ -41,6 +46,8 @@ class Plugin implements PluginInterface, Capable, EventSubscriberInterface */ private $logger; + private $forwarded = false; + public function activate(Composer $composer, IOInterface $io): void { $this->composer = $composer; @@ -67,31 +74,76 @@ public static function getSubscribedEvents(): array { return [ PluginEvents::COMMAND => 'onCommandEvent', + ScriptEvents::POST_AUTOLOAD_DUMP => 'onPostAutoloadDump', ]; } + public function onPostAutoloadDump(Event $event): void + { + $this->logger->logDebug('Calling onPostAutoloadDump().'); + + $eventIO = $event->getIO(); + + if (!($eventIO instanceof ConsoleIO)) { + return; + } + + // This is a bit convoluted but Event does not expose the input unlike + // CommandEvent. + $publicIO = PublicIO::fromConsoleIO($eventIO); + $eventInput = $publicIO->getInput(); + + $this->onEvent( + $eventInput->getArgument('command'), + $eventInput, + $publicIO->getOutput() + ); + } + public function onCommandEvent(CommandEvent $event): bool { $this->logger->logDebug('Calling onCommandEvent().'); + return $this->onEvent( + $event->getCommandName(), + $event->getInput(), + $event->getOutput() + ); + } + + private function onEvent( + string $commandName, + InputInterface $input, + OutputInterface $output + ): bool { $config = Config::fromComposer($this->composer); if ($config->isCommandForwarded() - && in_array($event->getCommandName(), self::FORWARDED_COMMANDS, true) + && in_array($commandName, self::FORWARDED_COMMANDS, true) ) { - return $this->onForwardedCommand($event); + return $this->onForwardedCommand($input, $output); } return true; } - protected function onForwardedCommand(CommandEvent $event): bool - { - $this->logger->logDebug('The command is being forwarded.'); + protected function onForwardedCommand( + InputInterface $input, + OutputInterface $output + ): bool { + if ($this->forwarded) { + $this->logger->logDebug('Command already forwarded within the process: skipping.'); + + return true; + } + + $this->forwarded = true; + + $this->logger->logStandard('The command is being forwarded.'); $this->logger->logDebug( sprintf( 'Original input: %s.', - $event->getInput() + $input ) ); @@ -99,19 +151,19 @@ protected function onForwardedCommand(CommandEvent $event): bool // input & output. $io = $this->io; + $application = new Application(); + $command = new BinCommand(); $command->setComposer($this->composer); + $command->setApplication($application); $command->setIO($io); - $command->setApplication(new Application()); - $forwardedCommandInput = BinInputFactory::createForwardedCommandInput( - $event->getInput() - ); + $forwardedCommandInput = BinInputFactory::createForwardedCommandInput($input); try { return Command::SUCCESS === $command->run( $forwardedCommandInput, - $event->getOutput() + $output ); } catch (Throwable $throwable) { return false; diff --git a/src/PublicIO.php b/src/PublicIO.php new file mode 100644 index 0000000..6c90d84 --- /dev/null +++ b/src/PublicIO.php @@ -0,0 +1,31 @@ +input, + $io->output, + $io->helperSet + ); + } + + public function getInput(): InputInterface + { + return $this->input; + } + + public function getOutput(): OutputInterface + { + return $this->output; + } +} diff --git a/tests/EndToEndTest.php b/tests/EndToEndTest.php index ac2799c..51c3311 100644 --- a/tests/EndToEndTest.php +++ b/tests/EndToEndTest.php @@ -145,6 +145,11 @@ private static function retrieveActualOutput( 'Installing bamarni/composer-bin-plugin (dev-hash): Symlinking from ../..', $normalizedContent ); + $normalizedContent = preg_replace( + '/Dependency resolution completed in \d\.\d{3} seconds/', + 'Dependency resolution completed in 0.000 seconds', + $normalizedContent + ); return $normalizedContent; } From 3ebe2c462db7352311f191c8e35a9a1335f66f51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= Date: Sat, 9 Jul 2022 10:27:48 +0200 Subject: [PATCH 34/57] Clarify behaviour when there is conflicting symlinks (#105) Closes #16. --- README.md | 4 ++++ e2e/scenario5/.gitignore | 5 +++++ e2e/scenario5/README.md | 1 + e2e/scenario5/composer.json | 21 +++++++++++++++++++ e2e/scenario5/expected.txt | 5 +++++ e2e/scenario5/script.sh | 23 +++++++++++++++++++++ e2e/scenario5/vendor-bin/ns1/composer.json | 5 +++++ e2e/scenario5/vendor-bin/ns2/composer.json | 5 +++++ e2e/scenario6/.gitignore | 5 +++++ e2e/scenario6/README.md | 1 + e2e/scenario6/composer.json | 21 +++++++++++++++++++ e2e/scenario6/expected.txt | 4 ++++ e2e/scenario6/script.sh | 24 ++++++++++++++++++++++ e2e/scenario6/vendor-bin/ns1/composer.json | 5 +++++ e2e/scenario6/vendor-bin/ns2/composer.json | 5 +++++ tests/EndToEndTest.php | 14 +++++++++++++ 16 files changed, 148 insertions(+) create mode 100644 e2e/scenario5/.gitignore create mode 100644 e2e/scenario5/README.md create mode 100644 e2e/scenario5/composer.json create mode 100644 e2e/scenario5/expected.txt create mode 100755 e2e/scenario5/script.sh create mode 100644 e2e/scenario5/vendor-bin/ns1/composer.json create mode 100644 e2e/scenario5/vendor-bin/ns2/composer.json create mode 100644 e2e/scenario6/.gitignore create mode 100644 e2e/scenario6/README.md create mode 100644 e2e/scenario6/composer.json create mode 100644 e2e/scenario6/expected.txt create mode 100755 e2e/scenario6/script.sh create mode 100644 e2e/scenario6/vendor-bin/ns1/composer.json create mode 100644 e2e/scenario6/vendor-bin/ns2/composer.json diff --git a/README.md b/README.md index 16a96f5..8a64ecb 100644 --- a/README.md +++ b/README.md @@ -179,6 +179,10 @@ wish to disable that behaviour, you can do so by adding a little setting in the } ``` +Note that otherwise, in case of conflicts (e.g. `phpstan` is present in two namespaces), only the +first one is linked and the second one is ignored. + + ### Change directory By default, the packages are looked for in the `vendor-bin` directory. The location can be changed using `target-directory` value in the extra config: diff --git a/e2e/scenario5/.gitignore b/e2e/scenario5/.gitignore new file mode 100644 index 0000000..cac9c03 --- /dev/null +++ b/e2e/scenario5/.gitignore @@ -0,0 +1,5 @@ +/actual.txt +/composer.lock +/vendor/ +/vendor-bin/*/composer.lock +/vendor-bin/*/vendor/ diff --git a/e2e/scenario5/README.md b/e2e/scenario5/README.md new file mode 100644 index 0000000..71f75d8 --- /dev/null +++ b/e2e/scenario5/README.md @@ -0,0 +1 @@ +Regular installation on a project with multiple namespaces and with links disabled. diff --git a/e2e/scenario5/composer.json b/e2e/scenario5/composer.json new file mode 100644 index 0000000..3d06a59 --- /dev/null +++ b/e2e/scenario5/composer.json @@ -0,0 +1,21 @@ +{ + "repositories": [ + { + "type": "path", + "url": "../../" + } + ], + "require-dev": { + "bamarni/composer-bin-plugin": "dev-master" + }, + "config": { + "allow-plugins": { + "bamarni/composer-bin-plugin": true + } + }, + "extra": { + "bamarni-bin": { + "bin-links": false + } + } +} diff --git a/e2e/scenario5/expected.txt b/e2e/scenario5/expected.txt new file mode 100644 index 0000000..3cdb7f2 --- /dev/null +++ b/e2e/scenario5/expected.txt @@ -0,0 +1,5 @@ +find: vendor/bin: No such file or directory +vendor-bin/ns1/vendor/bin/phpstan +vendor-bin/ns1/vendor/bin/phpstan.phar +vendor-bin/ns2/vendor/bin/phpstan +vendor-bin/ns2/vendor/bin/phpstan.phar diff --git a/e2e/scenario5/script.sh b/e2e/scenario5/script.sh new file mode 100755 index 0000000..27d9f0f --- /dev/null +++ b/e2e/scenario5/script.sh @@ -0,0 +1,23 @@ +#!/usr/bin/env bash + +set -Eeuo pipefail + +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 +composer bin all update + +# Actual command to execute the test itself +find vendor/bin vendor-bin/*/vendor/bin -maxdepth 1 -type f 2>&1 | tee > actual.txt || true diff --git a/e2e/scenario5/vendor-bin/ns1/composer.json b/e2e/scenario5/vendor-bin/ns1/composer.json new file mode 100644 index 0000000..ba924f7 --- /dev/null +++ b/e2e/scenario5/vendor-bin/ns1/composer.json @@ -0,0 +1,5 @@ +{ + "require": { + "phpstan/phpstan": "1.8.0" + } +} diff --git a/e2e/scenario5/vendor-bin/ns2/composer.json b/e2e/scenario5/vendor-bin/ns2/composer.json new file mode 100644 index 0000000..ba924f7 --- /dev/null +++ b/e2e/scenario5/vendor-bin/ns2/composer.json @@ -0,0 +1,5 @@ +{ + "require": { + "phpstan/phpstan": "1.8.0" + } +} diff --git a/e2e/scenario6/.gitignore b/e2e/scenario6/.gitignore new file mode 100644 index 0000000..cac9c03 --- /dev/null +++ b/e2e/scenario6/.gitignore @@ -0,0 +1,5 @@ +/actual.txt +/composer.lock +/vendor/ +/vendor-bin/*/composer.lock +/vendor-bin/*/vendor/ diff --git a/e2e/scenario6/README.md b/e2e/scenario6/README.md new file mode 100644 index 0000000..2174f99 --- /dev/null +++ b/e2e/scenario6/README.md @@ -0,0 +1 @@ +Regular installation on a project with multiple namespaces and with links enabled and conflicting symlinks. diff --git a/e2e/scenario6/composer.json b/e2e/scenario6/composer.json new file mode 100644 index 0000000..23f275b --- /dev/null +++ b/e2e/scenario6/composer.json @@ -0,0 +1,21 @@ +{ + "repositories": [ + { + "type": "path", + "url": "../../" + } + ], + "require-dev": { + "bamarni/composer-bin-plugin": "dev-master" + }, + "config": { + "allow-plugins": { + "bamarni/composer-bin-plugin": true + } + }, + "extra": { + "bamarni-bin": { + "bin-links": true + } + } +} diff --git a/e2e/scenario6/expected.txt b/e2e/scenario6/expected.txt new file mode 100644 index 0000000..623e1e1 --- /dev/null +++ b/e2e/scenario6/expected.txt @@ -0,0 +1,4 @@ +vendor/bin/phpstan +vendor/bin/phpstan.phar +find: vendor-bin/*/vendor/bin: No such file or directory +PHPStan - PHP Static Analysis Tool 1.8.0 diff --git a/e2e/scenario6/script.sh b/e2e/scenario6/script.sh new file mode 100755 index 0000000..b253a7d --- /dev/null +++ b/e2e/scenario6/script.sh @@ -0,0 +1,24 @@ +#!/usr/bin/env bash + +set -Eeuo pipefail + +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 +composer bin all update + +# Actual command to execute the test itself +find vendor/bin vendor-bin/*/vendor/bin -maxdepth 1 -type f 2>&1 | tee > actual.txt || true +vendor/bin/phpstan --version >> actual.txt diff --git a/e2e/scenario6/vendor-bin/ns1/composer.json b/e2e/scenario6/vendor-bin/ns1/composer.json new file mode 100644 index 0000000..ba924f7 --- /dev/null +++ b/e2e/scenario6/vendor-bin/ns1/composer.json @@ -0,0 +1,5 @@ +{ + "require": { + "phpstan/phpstan": "1.8.0" + } +} diff --git a/e2e/scenario6/vendor-bin/ns2/composer.json b/e2e/scenario6/vendor-bin/ns2/composer.json new file mode 100644 index 0000000..7826cef --- /dev/null +++ b/e2e/scenario6/vendor-bin/ns2/composer.json @@ -0,0 +1,5 @@ +{ + "require": { + "phpstan/phpstan": "1.6.0" + } +} diff --git a/tests/EndToEndTest.php b/tests/EndToEndTest.php index 51c3311..f4940ba 100644 --- a/tests/EndToEndTest.php +++ b/tests/EndToEndTest.php @@ -130,6 +130,9 @@ private static function retrieveActualOutput( 'Analyzed 90 rules to resolve dependencies', $normalizedContent ); + + // We are not interested in the exact version installed especially since + // in a PR the version will be `dev-commithash` instead of `dev-master`. $normalizedContent = preg_replace( '/Installs: bamarni\/composer-bin-plugin:dev-.+/', 'Installs: bamarni/composer-bin-plugin:dev-hash', @@ -145,12 +148,23 @@ private static function retrieveActualOutput( 'Installing bamarni/composer-bin-plugin (dev-hash): Symlinking from ../..', $normalizedContent ); + + // We are not interested in the time taken which can vary from locally + // and on the CI. $normalizedContent = preg_replace( '/Dependency resolution completed in \d\.\d{3} seconds/', 'Dependency resolution completed in 0.000 seconds', $normalizedContent ); + // Normalize the find directory: on some versions of OSX it does not come + // with ticks but it does on Linux (at least Ubuntu). + $normalizedContent = preg_replace( + '/find: ‘?(.+?)’?: No such file or directory/u', + 'find: $1: No such file or directory', + $normalizedContent + ); + return $normalizedContent; } } From 679a7b9163bdb8f2568477c10200aaf823b03693 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= Date: Sun, 10 Jul 2022 10:58:25 +0200 Subject: [PATCH 35/57] Normalize composer.json (#108) --- composer.json | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/composer.json b/composer.json index 621c385..6f8d7b8 100644 --- a/composer.json +++ b/composer.json @@ -1,7 +1,8 @@ { "name": "bamarni/composer-bin-plugin", - "type": "composer-plugin", "description": "No conflicts for your bin dependencies", + "license": "MIT", + "type": "composer-plugin", "keywords": [ "composer", "dependency", @@ -10,7 +11,6 @@ "conflict", "executable" ], - "license": "MIT", "require": { "php": "^7.2.5 || ^8.0", "composer-plugin-api": "^2.0" @@ -18,6 +18,7 @@ "require-dev": { "ext-json": "*", "composer/composer": "^2.0", + "ergebnis/composer-normalize": "~2.9 || ^2.28", "php-cs-fixer/shim": "^3.8", "phpstan/extension-installer": "^1.1", "phpunit/phpunit": "^8.5 || ^9.5", @@ -25,15 +26,6 @@ "symfony/finder": "^5.4.7 || ^6.0.7", "symfony/process": "^5.4.7 || ^6.0.7" }, - "config": { - "sort-packages": true, - "allow-plugins": { - "phpstan/extension-installer": true - } - }, - "extra": { - "class": "Bamarni\\Composer\\Bin\\Plugin" - }, "autoload": { "psr-4": { "Bamarni\\Composer\\Bin\\": "src" @@ -43,5 +35,15 @@ "psr-4": { "Bamarni\\Composer\\Bin\\Tests\\": "tests" } + }, + "config": { + "allow-plugins": { + "phpstan/extension-installer": true, + "ergebnis/composer-normalize": true + }, + "sort-packages": true + }, + "extra": { + "class": "Bamarni\\Composer\\Bin\\Plugin" } } From 0fc009af91e19f3b75bc2056adc928c6545ae886 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= Date: Sun, 10 Jul 2022 11:01:09 +0200 Subject: [PATCH 36/57] Add PHPStan (#109) --- composer.json | 2 ++ 1 file changed, 2 insertions(+) diff --git a/composer.json b/composer.json index 6f8d7b8..57d84f8 100644 --- a/composer.json +++ b/composer.json @@ -21,6 +21,8 @@ "ergebnis/composer-normalize": "~2.9 || ^2.28", "php-cs-fixer/shim": "^3.8", "phpstan/extension-installer": "^1.1", + "phpstan/phpstan": "^1.8", + "phpstan/phpstan-phpunit": "^1.1", "phpunit/phpunit": "^8.5 || ^9.5", "symfony/console": "^5.4.7 || ^6.0.7", "symfony/finder": "^5.4.7 || ^6.0.7", From 254b81e84543bee600f0fbe0d84b8a127d4107e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= Date: Sun, 10 Jul 2022 12:35:44 +0200 Subject: [PATCH 37/57] Add infection (#110) --- .gitignore | 13 ++++++++----- composer.json | 4 ++-- infection.json | 14 ++++++++++++++ 3 files changed, 24 insertions(+), 7 deletions(-) create mode 100644 infection.json diff --git a/.gitignore b/.gitignore index 1919fa1..102d2c3 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,8 @@ -composer.lock -vendor -vendor-bin/*/vendor -.phpunit.result.cache -.php-cs-fixer.cache +/composer.lock +/dist/ +/tools/ +/vendor/ +/vendor-bin/*/vendor/ +/.phive/ +/.phpunit.result.cache +/.php-cs-fixer.cache diff --git a/composer.json b/composer.json index 57d84f8..6228052 100644 --- a/composer.json +++ b/composer.json @@ -18,7 +18,6 @@ "require-dev": { "ext-json": "*", "composer/composer": "^2.0", - "ergebnis/composer-normalize": "~2.9 || ^2.28", "php-cs-fixer/shim": "^3.8", "phpstan/extension-installer": "^1.1", "phpstan/phpstan": "^1.8", @@ -41,7 +40,8 @@ "config": { "allow-plugins": { "phpstan/extension-installer": true, - "ergebnis/composer-normalize": true + "ergebnis/composer-normalize": true, + "infection/extension-installer": true }, "sort-packages": true }, diff --git a/infection.json b/infection.json new file mode 100644 index 0000000..4db6121 --- /dev/null +++ b/infection.json @@ -0,0 +1,14 @@ +{ + "$schema": "vendor/infection/infection/resources/schema.json", + "source": { + "directories": [ + "src" + ] + }, + "logs": { + "text": "dist/infection.txt" + }, + "mutators": { + "@default": true + } +} From 1b224526b0da016851705b8556529de43a865a16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= Date: Sun, 10 Jul 2022 12:56:17 +0200 Subject: [PATCH 38/57] Introduce Makefile (#107) --- .gitignore | 1 - .makefile/touch.sh | 61 ++++++++++++++++ .phive/phars.xml | 5 ++ Makefile | 120 +++++++++++++++++++++++++++++++ README.md | 10 +-- phpunit.xml.dist | 10 ++- tests/Fixtures/MyTestCommand.php | 4 +- 7 files changed, 204 insertions(+), 7 deletions(-) create mode 100755 .makefile/touch.sh create mode 100644 .phive/phars.xml create mode 100644 Makefile diff --git a/.gitignore b/.gitignore index 102d2c3..42e182a 100644 --- a/.gitignore +++ b/.gitignore @@ -3,6 +3,5 @@ /tools/ /vendor/ /vendor-bin/*/vendor/ -/.phive/ /.phpunit.result.cache /.php-cs-fixer.cache diff --git a/.makefile/touch.sh b/.makefile/touch.sh new file mode 100755 index 0000000..5c0a775 --- /dev/null +++ b/.makefile/touch.sh @@ -0,0 +1,61 @@ +#!/usr/bin/env bash +# +# Takes a given string, e.g. 'bin/console' or 'docker-compose exec php bin/console' +# and split it by words. For each words, if the target is a file, it is touched. +# +# This allows to implement a similar rule to: +# +# ```Makefile +# bin/php-cs-fixer: vendor +# touch $@ +# ``` +# +# Indeed when the rule `bin/php-cs-fixer` is replaced with a docker-compose +# equivalent, it will not play out as nicely. +# +# Arguments: +# $1 - {string} Command potentially containing a file +# + +set -Eeuo pipefail; + + +readonly ERROR_COLOR="\e[41m"; +readonly NO_COLOR="\e[0m"; + + +if [ $# -ne 1 ]; then + printf "${ERROR_COLOR}Illegal number of parameters.${NO_COLOR}\n"; + + exit 1; +fi + + +readonly FILES="$1"; + + +####################################### +# Touch the given file path if the target is a file and do not create the file +# if does not exist. +# +# Globals: +# None +# +# Arguments: +# $1 - {string} File path +# +# Returns: +# None +####################################### +touch_file() { + local file="$1"; + + if [ -e ${file} ]; then + touch -c ${file}; + fi +} + +for file in ${FILES} +do + touch_file ${file}; +done diff --git a/.phive/phars.xml b/.phive/phars.xml new file mode 100644 index 0000000..b2e72ce --- /dev/null +++ b/.phive/phars.xml @@ -0,0 +1,5 @@ + + + + + diff --git a/Makefile b/Makefile new file mode 100644 index 0000000..c0f4f5b --- /dev/null +++ b/Makefile @@ -0,0 +1,120 @@ +# See https://tech.davis-hansson.com/p/make/ +MAKEFLAGS += --warn-undefined-variables +MAKEFLAGS += --no-builtin-rules + +# General variables +TOUCH = bash .makefile/touch.sh + +# PHP variables +COMPOSER=composer +COVERAGE_DIR = dist/coverage +INFECTION_BIN = tools/infection +INFECTION = php -d zend.enable_gc=0 $(INFECTION_BIN) --skip-initial-tests --coverage=$(COVERAGE_DIR) --only-covered --threads=4 --min-msi=100 --min-covered-msi=100 --ansi +PHPUNIT_BIN = vendor/bin/phpunit +PHPUNIT = php -d zend.enable_gc=0 $(PHPUNIT_BIN) +PHPUNIT_COVERAGE = XDEBUG_MODE=coverage $(PHPUNIT) --group default --coverage-xml=$(COVERAGE_DIR)/coverage-xml --log-junit=$(COVERAGE_DIR)/phpunit.junit.xml +PHPSTAN_BIN = vendor/bin/phpstan +PHPSTAN = $(PHPSTAN_BIN) analyse src tests +PHP_CS_FIXER_BIN = vendor/bin/php-cs-fixer +PHP_CS_FIXER = $(PHP_CS_FIXER_BIN) fix --ansi --verbose --config=.php-cs-fixer.php +COMPOSER_NORMALIZE_BIN=tools/composer-normalize +COMPOSER_NORMALIZE = ./$(COMPOSER_NORMALIZE_BIN) + + +.DEFAULT_GOAL := default + + +# +# Command +#--------------------------------------------------------------------------- + +.PHONY: help +help: ## Shows the help +help: + @printf "\033[33mUsage:\033[0m\n make TARGET\n\n\033[32m#\n# Commands\n#---------------------------------------------------------------------------\033[0m\n" + @fgrep -h "##" $(MAKEFILE_LIST) | fgrep -v fgrep | sed -e 's/\\$$//' | sed -e 's/##//' | awk 'BEGIN {FS = ":"}; {printf "\033[33m%s:\033[0m%s\n", $$1, $$2}' + + +.PHONY: default +default: ## Runs the default task: CS fix and all the tests +default: cs test + + +.PHONY: cs +cs: ## Runs PHP-CS-Fixer +cs: $(PHP_CS_FIXER_BIN) $(COMPOSER_NORMALIZE_BIN) + $(PHP_CS_FIXER) + $(COMPOSER_NORMALIZE) + + +.PHONY: phpstan +phpstan: ## Runs PHPStan +phpstan: + $(PHPSTAN) + + +.PHONY: infection +infection: ## Runs infection +infection: $(INFECTION_BIN) $(COVERAGE_DIR) vendor + if [ -d $(COVERAGE_DIR)/coverage-xml ]; then $(INFECTION); fi + + +.PHONY: test +test: ## Runs all the tests +test: validate-package phpstan $(COVERAGE_DIR) e2e #infection include infection later + + +.PHONY: validate-package +validate-package: ## Validates the Composer package +validate-package: vendor + $(COMPOSER) validate --strict + + +.PHONY: coverage +coverage: ## Runs PHPUnit with code coverage +coverage: $(PHPUNIT_BIN) vendor + $(PHPUNIT_COVERAGE) + + +.PHONY: unit-test +unit-test: ## Runs PHPUnit (default group) +unit-test: $(PHPUNIT_BIN) vendor + $(PHPUNIT) --group default + + +.PHONY: e2e +e2e: ## Runs PHPUnit end-to-end tests +e2e: $(PHPUNIT_BIN) vendor + $(PHPUNIT) --group e2e + + +# +# Rules +#--------------------------------------------------------------------------- + +# Vendor does not depend on the composer.lock since the later is not tracked +# or committed. +vendor: composer.json + $(COMPOSER) update + $(TOUCH) "$@" + +$(PHPUNIT_BIN): vendor + $(TOUCH) "$@" + +$(INFECTION_BIN): ./.phive/phars.xml + phive install infection + $(TOUCH) "$@" + +$(COMPOSER_NORMALIZE_BIN): ./.phive/phars.xml + phive install composer-normalize + $(TOUCH) "$@" + +$(COVERAGE_DIR): $(PHPUNIT_BIN) src tests phpunit.xml.dist + $(PHPUNIT_COVERAGE) + $(TOUCH) "$@" + +$(PHP_CS_FIXER_BIN): vendor + $(TOUCH) "$@" + +$(PHPSTAN_BIN): vendor + $(TOUCH) "$@" diff --git a/README.md b/README.md index 8a64ecb..1719511 100644 --- a/README.md +++ b/README.md @@ -236,14 +236,15 @@ vendor-bin/**/composer.lock binary ## Contributing -Before getting started, you need to install the plugin globally: +A makefile is available to help out: ```bash -$ composer install -$ vendor/bin/phpunit # run the tests -$ vendor/bin/php-cs-fixer # run PHP-CS-Fixer +$ make # Runs all checks +$ make help # List all available commands ``` +**Note:** you do need to install [phive][phive] first. + [1]: https://github.com/etsy/phan [2]: https://github.com/phpmetrics/PhpMetrics @@ -253,3 +254,4 @@ $ vendor/bin/php-cs-fixer # run PHP-CS-Fixer [6]: https://github.com/nikic/PHP-Parser [7]: https://github.com/theofidry/composer-inheritance-plugin [8]: https://github.com/wikimedia/composer-merge-plugin +[phive]: https://phar.io/ diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 866c96b..6d57a8f 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -1,5 +1,7 @@ - + + + src + + + diff --git a/tests/Fixtures/MyTestCommand.php b/tests/Fixtures/MyTestCommand.php index 19fb77d..8eff0b0 100644 --- a/tests/Fixtures/MyTestCommand.php +++ b/tests/Fixtures/MyTestCommand.php @@ -37,7 +37,7 @@ public function __construct() ]); } - public function execute(InputInterface $input, OutputInterface $output) + public function execute(InputInterface $input, OutputInterface $output): int { $this->composer = $this->tryComposer(); @@ -52,5 +52,7 @@ public function execute(InputInterface $input, OutputInterface $output) $this->resetComposer(); $this->getApplication()->resetComposer(); + + return self::SUCCESS; } } From 917e1b87e93a7ba88b9127fba3bbcdabcb6fd7ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= Date: Sun, 10 Jul 2022 15:07:10 +0200 Subject: [PATCH 39/57] Move PHP-CS-Fixer to CI (#111) --- .github/workflows/tests.yaml | 10 ---------- .phive/phars.xml | 1 + Makefile | 3 ++- composer.json | 1 - 4 files changed, 3 insertions(+), 12 deletions(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 99dcccc..b2c4219 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -31,11 +31,6 @@ jobs: php-version: "${{ matrix.php }}" tools: "composer" - # It is not used in the CI and the PHP constraints may not match - # the CI. - - name: "Remove PHP-CS-Fixer" - run: composer remove --dev --no-update php-cs-fixer/shim - - name: "Install Composer dependencies" uses: "ramsey/composer-install@v2" with: @@ -66,11 +61,6 @@ jobs: php-version: "${{ matrix.php }}" tools: "composer" - # It is not used in the CI and the PHP constraints may not match - # the CI. - - name: "Remove PHP-CS-Fixer" - run: composer remove --dev --no-update php-cs-fixer/shim - - name: "Correct bin plugin version for e2e scenarios (PR-only)" if: github.event_name == 'pull_request' run: find e2e -maxdepth 1 -mindepth 1 -type d -exec bash -c "cd {} && composer require --dev bamarni/composer-bin-plugin:dev-${GITHUB_SHA} --no-update" \; diff --git a/.phive/phars.xml b/.phive/phars.xml index b2e72ce..335086e 100644 --- a/.phive/phars.xml +++ b/.phive/phars.xml @@ -2,4 +2,5 @@ + diff --git a/Makefile b/Makefile index c0f4f5b..8b3dd2f 100644 --- a/Makefile +++ b/Makefile @@ -15,7 +15,7 @@ PHPUNIT = php -d zend.enable_gc=0 $(PHPUNIT_BIN) PHPUNIT_COVERAGE = XDEBUG_MODE=coverage $(PHPUNIT) --group default --coverage-xml=$(COVERAGE_DIR)/coverage-xml --log-junit=$(COVERAGE_DIR)/phpunit.junit.xml PHPSTAN_BIN = vendor/bin/phpstan PHPSTAN = $(PHPSTAN_BIN) analyse src tests -PHP_CS_FIXER_BIN = vendor/bin/php-cs-fixer +PHP_CS_FIXER_BIN = tools/php-cs-fixer PHP_CS_FIXER = $(PHP_CS_FIXER_BIN) fix --ansi --verbose --config=.php-cs-fixer.php COMPOSER_NORMALIZE_BIN=tools/composer-normalize COMPOSER_NORMALIZE = ./$(COMPOSER_NORMALIZE_BIN) @@ -114,6 +114,7 @@ $(COVERAGE_DIR): $(PHPUNIT_BIN) src tests phpunit.xml.dist $(TOUCH) "$@" $(PHP_CS_FIXER_BIN): vendor + phive install php-cs-fixer $(TOUCH) "$@" $(PHPSTAN_BIN): vendor diff --git a/composer.json b/composer.json index 6228052..617ffb6 100644 --- a/composer.json +++ b/composer.json @@ -18,7 +18,6 @@ "require-dev": { "ext-json": "*", "composer/composer": "^2.0", - "php-cs-fixer/shim": "^3.8", "phpstan/extension-installer": "^1.1", "phpstan/phpstan": "^1.8", "phpstan/phpstan-phpunit": "^1.1", From a0b2a84c8b0072350af870985f554a7ce4bc1780 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= Date: Sun, 10 Jul 2022 15:38:43 +0200 Subject: [PATCH 40/57] Add extra logging on normal verbosity (#112) --- e2e/scenario0/expected.txt | 2 ++ e2e/scenario2/expected.txt | 2 ++ src/BinCommand.php | 2 +- 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/e2e/scenario0/expected.txt b/e2e/scenario0/expected.txt index e1a8e3f..a192945 100644 --- a/e2e/scenario0/expected.txt +++ b/e2e/scenario0/expected.txt @@ -1,3 +1,4 @@ +[bamarni-bin] Checking namespace vendor-bin/ns1 Loading composer repositories with package information Updating dependencies Nothing to modify in lock file @@ -5,6 +6,7 @@ Writing lock file Installing dependencies from lock file (including require-dev) Nothing to install, update or remove Generating autoload files +[bamarni-bin] Checking namespace vendor-bin/ns2 Loading composer repositories with package information Updating dependencies Nothing to modify in lock file diff --git a/e2e/scenario2/expected.txt b/e2e/scenario2/expected.txt index 195d503..4fed774 100644 --- a/e2e/scenario2/expected.txt +++ b/e2e/scenario2/expected.txt @@ -1,4 +1,6 @@ +[bamarni-bin] Checking namespace vendor-bin/ns1 > echo ns1 ns1 +[bamarni-bin] Checking namespace vendor-bin/ns2 > echo ns2 ns2 diff --git a/src/BinCommand.php b/src/BinCommand.php index 73c5da3..fb4e98c 100644 --- a/src/BinCommand.php +++ b/src/BinCommand.php @@ -174,7 +174,7 @@ private function executeInNamespace( OutputInterface $output, ReflectionProperty $commandsReflection ): int { - $this->logger->logDebug( + $this->logger->logStandard( sprintf( 'Checking namespace %s', $namespace From aa8fa4498c5b4d7cec8253b4844860168ec4b25d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= Date: Sun, 10 Jul 2022 15:48:46 +0200 Subject: [PATCH 41/57] Capture buggy behaviour with forwarded command (#113) Related to #106 --- e2e/scenario7/.gitignore | 5 +++++ e2e/scenario7/README.md | 1 + e2e/scenario7/composer.json | 16 +++++++++++++++ e2e/scenario7/expected.txt | 9 ++++++++ e2e/scenario7/script.sh | 24 ++++++++++++++++++++++ e2e/scenario7/vendor-bin/ns1/composer.json | 8 ++++++++ e2e/scenario8/.gitignore | 5 +++++ e2e/scenario8/README.md | 3 +++ e2e/scenario8/composer.json | 21 +++++++++++++++++++ e2e/scenario8/expected.txt | 22 ++++++++++++++++++++ e2e/scenario8/script.sh | 22 ++++++++++++++++++++ e2e/scenario8/vendor-bin/ns1/composer.json | 8 ++++++++ 12 files changed, 144 insertions(+) create mode 100644 e2e/scenario7/.gitignore create mode 100644 e2e/scenario7/README.md create mode 100644 e2e/scenario7/composer.json create mode 100644 e2e/scenario7/expected.txt create mode 100755 e2e/scenario7/script.sh create mode 100644 e2e/scenario7/vendor-bin/ns1/composer.json create mode 100644 e2e/scenario8/.gitignore create mode 100644 e2e/scenario8/README.md create mode 100644 e2e/scenario8/composer.json create mode 100644 e2e/scenario8/expected.txt create mode 100755 e2e/scenario8/script.sh create mode 100644 e2e/scenario8/vendor-bin/ns1/composer.json diff --git a/e2e/scenario7/.gitignore b/e2e/scenario7/.gitignore new file mode 100644 index 0000000..cac9c03 --- /dev/null +++ b/e2e/scenario7/.gitignore @@ -0,0 +1,5 @@ +/actual.txt +/composer.lock +/vendor/ +/vendor-bin/*/composer.lock +/vendor-bin/*/vendor/ diff --git a/e2e/scenario7/README.md b/e2e/scenario7/README.md new file mode 100644 index 0000000..87140b4 --- /dev/null +++ b/e2e/scenario7/README.md @@ -0,0 +1 @@ +Tests that dev dependencies are not installed if no-dev is passed. diff --git a/e2e/scenario7/composer.json b/e2e/scenario7/composer.json new file mode 100644 index 0000000..bff880f --- /dev/null +++ b/e2e/scenario7/composer.json @@ -0,0 +1,16 @@ +{ + "repositories": [ + { + "type": "path", + "url": "../../" + } + ], + "require": { + "bamarni/composer-bin-plugin": "dev-master" + }, + "config": { + "allow-plugins": { + "bamarni/composer-bin-plugin": true + } + } +} diff --git a/e2e/scenario7/expected.txt b/e2e/scenario7/expected.txt new file mode 100644 index 0000000..f6bd9e9 --- /dev/null +++ b/e2e/scenario7/expected.txt @@ -0,0 +1,9 @@ +[bamarni-bin] Current working directory: /path/to/project/e2e/scenario7 +[bamarni-bin] Configuring bin directory to /path/to/project/e2e/scenario7/vendor/bin. +[bamarni-bin] Checking namespace vendor-bin/ns1 +[bamarni-bin] Changed current directory to vendor-bin/ns1. +[bamarni-bin] Running `@composer update --no-dev --verbose --working-dir='.' -- foo`. +Cannot update only a partial set of packages without a lock file present. Run `composer update` to generate a lock file. +[bamarni-bin] Changed current directory to /path/to/project/e2e/scenario7. +––––––––––––––––––––– +No dependencies installed. Try running composer install or update. diff --git a/e2e/scenario7/script.sh b/e2e/scenario7/script.sh new file mode 100755 index 0000000..ea402a7 --- /dev/null +++ b/e2e/scenario7/script.sh @@ -0,0 +1,24 @@ +#!/usr/bin/env bash + +set -Eeuo pipefail + +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 ns1 update --no-dev --verbose -- foo 2>&1 | tee > actual.txt || true +echo "–––––––––––––––––––––" >> actual.txt +composer bin ns1 show --direct --name-only 2>&1 | tee >> actual.txt || true diff --git a/e2e/scenario7/vendor-bin/ns1/composer.json b/e2e/scenario7/vendor-bin/ns1/composer.json new file mode 100644 index 0000000..7b72340 --- /dev/null +++ b/e2e/scenario7/vendor-bin/ns1/composer.json @@ -0,0 +1,8 @@ +{ + "require": { + "nikic/iter": "v1.6.0" + }, + "require-dev": { + "phpstan/phpstan": "1.8.0" + } +} diff --git a/e2e/scenario8/.gitignore b/e2e/scenario8/.gitignore new file mode 100644 index 0000000..cac9c03 --- /dev/null +++ b/e2e/scenario8/.gitignore @@ -0,0 +1,5 @@ +/actual.txt +/composer.lock +/vendor/ +/vendor-bin/*/composer.lock +/vendor-bin/*/vendor/ diff --git a/e2e/scenario8/README.md b/e2e/scenario8/README.md new file mode 100644 index 0000000..9cdf3ed --- /dev/null +++ b/e2e/scenario8/README.md @@ -0,0 +1,3 @@ +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/scenario8/composer.json b/e2e/scenario8/composer.json new file mode 100644 index 0000000..3061d0c --- /dev/null +++ b/e2e/scenario8/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/scenario8/expected.txt b/e2e/scenario8/expected.txt new file mode 100644 index 0000000..0bfcdd3 --- /dev/null +++ b/e2e/scenario8/expected.txt @@ -0,0 +1,22 @@ +Loading composer repositories with package information +Updating dependencies +Lock file operations: 1 install, 0 updates, 0 removals + - Locking bamarni/composer-bin-plugin (dev-hash) +Writing lock file +Installing dependencies from lock file +Package operations: 1 install, 0 updates, 0 removals + - Installing bamarni/composer-bin-plugin (dev-hash): Symlinking from ../.. +Generating autoload files +[bamarni-bin] The command is being forwarded. +Loading composer repositories with package information +Updating dependencies +Lock file operations: 2 installs, 0 updates, 0 removals + - Locking nikic/iter (v1.6.0) + - Locking phpstan/phpstan (1.8.0) +Writing lock file +Installing dependencies from lock file +Package operations: 1 install, 0 updates, 0 removals + - Installing nikic/iter (v1.6.0): Extracting archive +Generating autoload files +–––––––––––––– +nikic/iter diff --git a/e2e/scenario8/script.sh b/e2e/scenario8/script.sh new file mode 100755 index 0000000..11786d5 --- /dev/null +++ b/e2e/scenario8/script.sh @@ -0,0 +1,22 @@ +#!/usr/bin/env bash + +set -Eeuo pipefail + +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 + +# Actual command to execute the test itself +composer update --no-dev 2>&1 | tee > actual.txt || true +echo "––––––––––––––" >> actual.txt +composer bin ns1 show --direct --name-only 2>&1 | tee >> actual.txt || true diff --git a/e2e/scenario8/vendor-bin/ns1/composer.json b/e2e/scenario8/vendor-bin/ns1/composer.json new file mode 100644 index 0000000..7b72340 --- /dev/null +++ b/e2e/scenario8/vendor-bin/ns1/composer.json @@ -0,0 +1,8 @@ +{ + "require": { + "nikic/iter": "v1.6.0" + }, + "require-dev": { + "phpstan/phpstan": "1.8.0" + } +} From 468af9dbe17bd5566ebf1888438c8312d6ec11fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= Date: Sun, 10 Jul 2022 15:55:58 +0200 Subject: [PATCH 42/57] Add env variables to e2e tests to make the tests more portable (#114) --- e2e/scenario0/script.sh | 6 ++++++ e2e/scenario1/script.sh | 6 ++++++ e2e/scenario2/script.sh | 6 ++++++ e2e/scenario3/script.sh | 6 ++++++ e2e/scenario5/script.sh | 6 ++++++ e2e/scenario6/script.sh | 6 ++++++ e2e/scenario7/script.sh | 6 ++++++ e2e/scenario8/script.sh | 6 ++++++ 8 files changed, 48 insertions(+) diff --git a/e2e/scenario0/script.sh b/e2e/scenario0/script.sh index 3b03638..3557486 100755 --- a/e2e/scenario0/script.sh +++ b/e2e/scenario0/script.sh @@ -2,6 +2,12 @@ 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 diff --git a/e2e/scenario1/script.sh b/e2e/scenario1/script.sh index 3917ea0..575b36b 100755 --- a/e2e/scenario1/script.sh +++ b/e2e/scenario1/script.sh @@ -2,6 +2,12 @@ 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 diff --git a/e2e/scenario2/script.sh b/e2e/scenario2/script.sh index 7894dab..8b9f863 100755 --- a/e2e/scenario2/script.sh +++ b/e2e/scenario2/script.sh @@ -2,6 +2,12 @@ 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 diff --git a/e2e/scenario3/script.sh b/e2e/scenario3/script.sh index 2b31397..fa19247 100755 --- a/e2e/scenario3/script.sh +++ b/e2e/scenario3/script.sh @@ -2,6 +2,12 @@ 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 diff --git a/e2e/scenario5/script.sh b/e2e/scenario5/script.sh index 27d9f0f..b570d5d 100755 --- a/e2e/scenario5/script.sh +++ b/e2e/scenario5/script.sh @@ -2,6 +2,12 @@ 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 diff --git a/e2e/scenario6/script.sh b/e2e/scenario6/script.sh index b253a7d..4c2e74d 100755 --- a/e2e/scenario6/script.sh +++ b/e2e/scenario6/script.sh @@ -2,6 +2,12 @@ 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 diff --git a/e2e/scenario7/script.sh b/e2e/scenario7/script.sh index ea402a7..ac11060 100755 --- a/e2e/scenario7/script.sh +++ b/e2e/scenario7/script.sh @@ -2,6 +2,12 @@ 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 diff --git a/e2e/scenario8/script.sh b/e2e/scenario8/script.sh index 11786d5..c030a0a 100755 --- a/e2e/scenario8/script.sh +++ b/e2e/scenario8/script.sh @@ -2,6 +2,12 @@ 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 From a522fc2b4144ca63ac9ef0eb3db0ec6d88dab1e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= Date: Sun, 10 Jul 2022 16:33:46 +0200 Subject: [PATCH 43/57] Ensure additional flags & options are forwarded (#106) Closes #65 --- e2e/scenario7/expected.txt | 1 + e2e/scenario8/expected.txt | 31 +++-- e2e/scenario8/script.sh | 4 +- e2e/scenario8/vendor-bin/ns1/composer.json | 9 +- src/BinCommand.php | 8 +- src/BinInputFactory.php | 71 ++++++++-- src/InvalidBinInput.php | 32 +++++ tests/BinInputFactoryTest.php | 154 ++++++++++++++++++--- tests/EndToEndTest.php | 12 +- 9 files changed, 262 insertions(+), 60 deletions(-) create mode 100644 src/InvalidBinInput.php diff --git a/e2e/scenario7/expected.txt b/e2e/scenario7/expected.txt index f6bd9e9..c00cece 100644 --- a/e2e/scenario7/expected.txt +++ b/e2e/scenario7/expected.txt @@ -6,4 +6,5 @@ Cannot update only a partial set of packages without a lock file present. Run `composer update` to generate a lock file. [bamarni-bin] Changed current directory to /path/to/project/e2e/scenario7. ––––––––––––––––––––– +[bamarni-bin] Checking namespace vendor-bin/ns1 No dependencies installed. Try running composer install or update. diff --git a/e2e/scenario8/expected.txt b/e2e/scenario8/expected.txt index 0bfcdd3..dbca4c5 100644 --- a/e2e/scenario8/expected.txt +++ b/e2e/scenario8/expected.txt @@ -1,22 +1,35 @@ Loading composer repositories with package information Updating dependencies +Dependency resolution completed in 0.000 seconds +Analyzed 90 packages to resolve dependencies +Analyzed 90 rules to resolve dependencies +Dependency resolution completed in 0.000 seconds Lock file operations: 1 install, 0 updates, 0 removals +Installs: bamarni/composer-bin-plugin:dev-hash - Locking bamarni/composer-bin-plugin (dev-hash) Writing lock file -Installing dependencies from lock file +Installing dependencies from lock file (including require-dev) Package operations: 1 install, 0 updates, 0 removals +Installs: bamarni/composer-bin-plugin:dev-hash - Installing bamarni/composer-bin-plugin (dev-hash): Symlinking from ../.. Generating autoload files +> post-autoload-dump: Bamarni\Composer\Bin\Plugin->onPostAutoloadDump +[bamarni-bin] Calling onPostAutoloadDump(). [bamarni-bin] The command is being forwarded. +[bamarni-bin] Original input: update --prefer-lowest --verbose. +[bamarni-bin] Current working directory: /path/to/project/e2e/scenario8 +[bamarni-bin] Configuring bin directory to /path/to/project/e2e/scenario8/vendor/bin. +[bamarni-bin] Checking namespace vendor-bin/ns1 +[bamarni-bin] Changed current directory to vendor-bin/ns1. +[bamarni-bin] Running `@composer update --prefer-lowest --verbose --working-dir='.'`. Loading composer repositories with package information Updating dependencies -Lock file operations: 2 installs, 0 updates, 0 removals - - Locking nikic/iter (v1.6.0) - - Locking phpstan/phpstan (1.8.0) +Dependency resolution completed in 0.000 seconds +Analyzed 90 packages to resolve dependencies +Analyzed 90 rules to resolve dependencies +Nothing to modify in lock file Writing lock file -Installing dependencies from lock file -Package operations: 1 install, 0 updates, 0 removals - - Installing nikic/iter (v1.6.0): Extracting archive +Installing dependencies from lock file (including require-dev) +Nothing to install, update or remove Generating autoload files -–––––––––––––– -nikic/iter +[bamarni-bin] Changed current directory to /path/to/project/e2e/scenario8. diff --git a/e2e/scenario8/script.sh b/e2e/scenario8/script.sh index c030a0a..93a8c2a 100755 --- a/e2e/scenario8/script.sh +++ b/e2e/scenario8/script.sh @@ -23,6 +23,4 @@ rm -rf vendor-bin/*/composer.lock || true rm -rf vendor-bin/*/vendor || true # Actual command to execute the test itself -composer update --no-dev 2>&1 | tee > actual.txt || true -echo "––––––––––––––" >> actual.txt -composer bin ns1 show --direct --name-only 2>&1 | tee >> actual.txt || true +composer update --prefer-lowest --verbose 2>&1 | tee >> actual.txt || true diff --git a/e2e/scenario8/vendor-bin/ns1/composer.json b/e2e/scenario8/vendor-bin/ns1/composer.json index 7b72340..0967ef4 100644 --- a/e2e/scenario8/vendor-bin/ns1/composer.json +++ b/e2e/scenario8/vendor-bin/ns1/composer.json @@ -1,8 +1 @@ -{ - "require": { - "nikic/iter": "v1.6.0" - }, - "require-dev": { - "phpstan/phpstan": "1.8.0" - } -} +{} diff --git a/src/BinCommand.php b/src/BinCommand.php index fb4e98c..9d7e9cc 100644 --- a/src/BinCommand.php +++ b/src/BinCommand.php @@ -52,10 +52,10 @@ protected function configure(): void { $this ->setDescription('Run a command inside a bin namespace') - ->setDefinition([ - new InputArgument(self::NAMESPACE_ARG, InputArgument::REQUIRED), - new InputArgument('args', InputArgument::REQUIRED | InputArgument::IS_ARRAY), - ]) + ->addArgument( + self::NAMESPACE_ARG, + InputArgument::REQUIRED + ) ->ignoreValidationErrors(); } diff --git a/src/BinInputFactory.php b/src/BinInputFactory.php index 2faac72..90fab1b 100644 --- a/src/BinInputFactory.php +++ b/src/BinInputFactory.php @@ -6,29 +6,84 @@ use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\StringInput; +use function array_filter; +use function array_map; +use function implode; +use function preg_match; use function preg_quote; -use function preg_replace; use function sprintf; final class BinInputFactory { + /** + * Extracts the input to execute in the bin namespace. + * + * For example: `bin namespace-name update --prefer-lowest` => `update --prefer-lowest` + * + * Note that no input definition is bound in the resulting input. + */ public static function createInput( string $namespace, InputInterface $previousInput ): InputInterface { - return new StringInput( - preg_replace( - sprintf('/bin\s+(--ansi\s)?%s(\s.+)/', preg_quote($namespace, '/')), - '$1$2', - (string) $previousInput, - 1 + $matchResult = preg_match( + sprintf( + '/^(?.+)?bin (?:(?.+?) )?(?:%1$s|\'%1$s\') (?.+?)(? -- .*)?$/', + preg_quote($namespace, '/') + ), + (string) $previousInput, + $matches + ); + + if (1 !== $matchResult) { + throw InvalidBinInput::forBinInput($previousInput); + } + + $inputParts = array_filter( + array_map( + 'trim', + [ + $matches['binCommand'], + $matches['preBinOptions2'] ?? '', + $matches['preBinOptions'] ?? '', + $matches['extraInput'] ?? '', + ] ) ); + + // Move the options present _before_ bin namespaceName to after, but + // before the end of option marker (--) if present. + $reorderedInput = implode(' ', $inputParts); + + return new StringInput($reorderedInput); } public static function createNamespaceInput(InputInterface $previousInput): InputInterface { - return new StringInput((string) $previousInput . ' --working-dir=.'); + $matchResult = preg_match( + '/^(.+?\s?)(--(?: .+)?)?$/', + (string) $previousInput, + $matches + ); + + if (1 !== $matchResult) { + throw InvalidBinInput::forNamespaceInput($previousInput); + } + + $inputParts = array_filter( + array_map( + 'trim', + [ + $matches[1], + '--working-dir=.', + $matches[2] ?? '', + ] + ) + ); + + $newInput = implode(' ', $inputParts); + + return new StringInput($newInput); } public static function createForwardedCommandInput(InputInterface $input): InputInterface diff --git a/src/InvalidBinInput.php b/src/InvalidBinInput.php new file mode 100644 index 0000000..a966b60 --- /dev/null +++ b/src/InvalidBinInput.php @@ -0,0 +1,32 @@ + ", for example "bin all update --prefer-lowest".', + $input + ) + ); + } + + public static function forNamespaceInput(InputInterface $input): self + { + return new self( + sprintf( + 'Could not parse the input (executed within the namespace) "%s".', + $input + ) + ); + } +} diff --git a/tests/BinInputFactoryTest.php b/tests/BinInputFactoryTest.php index 8bf6b97..2eca750 100644 --- a/tests/BinInputFactoryTest.php +++ b/tests/BinInputFactoryTest.php @@ -8,6 +8,7 @@ use PHPUnit\Framework\TestCase; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\StringInput; +use function sprintf; /** * @covers \Bamarni\Composer\Bin\BinInputFactory @@ -29,30 +30,110 @@ public function test_it_can_create_a_new_input( public static function inputProvider(): iterable { - yield [ - 'foo-namespace', - new StringInput('bin foo-namespace flex:update --prefer-lowest'), - new StringInput('flex:update --prefer-lowest'), + $namespaceNames = [ + 'simpleNamespaceName', + 'composed-namespaceName', + 'regexLimiter/namespaceName', + 'all', ]; - yield [ - 'foo-namespace', - new StringInput('bin foo-namespace flex:update --prefer-lowest --ansi'), - new StringInput('flex:update --prefer-lowest --ansi'), - ]; + foreach ($namespaceNames as $namespaceName) { + $labelPrefix = sprintf('[%s]', $namespaceName); + + yield $labelPrefix.'simple command' => [ + $namespaceName, + new StringInput( + sprintf( + 'bin %s show', + $namespaceName + ) + ), + new StringInput('show'), + ]; + + yield $labelPrefix.' namespaced command' => [ + $namespaceName, + new StringInput( + sprintf( + 'bin %s check:platform', + $namespaceName + ) + ), + new StringInput('check:platform'), + ]; + + yield $labelPrefix.'command with options' => [ + $namespaceName, + new StringInput( + sprintf( + 'bin %s show --tree -i', + $namespaceName + ) + ), + new StringInput('show --tree -i'), + ]; + + yield $labelPrefix.'command with annoyingly placed options' => [ + $namespaceName, + new StringInput( + sprintf( + '--ansi bin %s -o --quiet show --tree -i', + $namespaceName + ) + ), + new StringInput('-o --quiet show --tree -i --ansi'), + ]; + + yield $labelPrefix.'command with options with option separator' => [ + $namespaceName, + new StringInput( + sprintf( + 'bin %s show --tree -i --', + $namespaceName + ) + ), + new StringInput('show --tree -i --'), + ]; + + yield $labelPrefix.'command with options with option separator and follow up argument' => [ + $namespaceName, + new StringInput( + sprintf( + 'bin %s show --tree -i -- foo', + $namespaceName + ) + ), + new StringInput('show --tree -i -- foo'), + ]; + + yield $labelPrefix.'command with options with option separator and follow up option' => [ + $namespaceName, + new StringInput( + sprintf( + 'bin %s show --tree -i -- --foo', + $namespaceName + ) + ), + new StringInput('show --tree -i -- --foo'), + ]; + + yield $labelPrefix.'command with annoyingly placed options and option separator and follow up option' => [ + $namespaceName, + new StringInput( + sprintf( + '--ansi bin %s -o --quiet show --tree -i -- --foo', + $namespaceName + ) + ), + new StringInput('-o --quiet show --tree -i --ansi -- --foo'), + ]; + } // See https://github.com/bamarni/composer-bin-plugin/pull/23 yield [ 'foo-namespace', new StringInput('bin --ansi foo-namespace flex:update --prefer-lowest'), - new StringInput('--ansi flex:update --prefer-lowest'), - ]; - - // See https://github.com/bamarni/composer-bin-plugin/pull/73 - yield [ - 'irrelevant', - new StringInput('update --dry-run --no-plugins roave/security-advisories'), - new StringInput('update --dry-run --no-plugins roave/security-advisories'), + new StringInput('flex:update --prefer-lowest --ansi'), ]; } @@ -70,14 +151,41 @@ public function test_it_can_create_a_new_input_for_a_namespace( public static function namespaceInputProvider(): iterable { - yield [ - new StringInput('flex:update --prefer-lowest'), - new StringInput('flex:update --prefer-lowest --working-dir=.'), + $namespaceNames = [ + 'simpleNamespaceName', + 'composed-namespaceName', + 'regexLimiter/namespaceName', + 'all', ]; - yield [ - new StringInput('flex:update --prefer-lowest --ansi'), - new StringInput('flex:update --prefer-lowest --ansi --working-dir=.'), + yield 'simple command' => [ + new StringInput('flex:update'), + new StringInput('flex:update --working-dir=.'), + ]; + + yield 'command with options' => [ + new StringInput('flex:update --prefer-lowest -i'), + new StringInput('flex:update --prefer-lowest -i --working-dir=.'), + ]; + + yield 'command with annoyingly placed options' => [ + new StringInput('-o --quiet flex:update --prefer-lowest -i'), + new StringInput('-o --quiet flex:update --prefer-lowest -i --working-dir=.'), + ]; + + yield 'command with options with option separator' => [ + new StringInput('flex:update --prefer-lowest -i --'), + new StringInput('flex:update --prefer-lowest -i --working-dir=. --'), + ]; + + yield 'command with options with option separator and follow up argument' => [ + new StringInput('flex:update --prefer-lowest -i -- foo'), + new StringInput('flex:update --prefer-lowest -i --working-dir=. -- foo'), + ]; + + yield 'command with annoyingly placed options and option separator and follow up option' => [ + new StringInput('-o --quiet flex:update --prefer-lowest -i -- --foo'), + new StringInput('-o --quiet flex:update --prefer-lowest -i --working-dir=. -- --foo'), ]; } diff --git a/tests/EndToEndTest.php b/tests/EndToEndTest.php index f4940ba..81ed11e 100644 --- a/tests/EndToEndTest.php +++ b/tests/EndToEndTest.php @@ -54,9 +54,7 @@ public function test_it_passes_the_e2e_test(string $scenarioPath): void $originalContent = 'File was not created.'; } - self::assertTrue( - $scenarioProcess->isSuccessful(), - <<isSuccessful(), + $errorMessage ); $actual = self::retrieveActualOutput( @@ -73,7 +75,7 @@ public function test_it_passes_the_e2e_test(string $scenarioPath): void $originalContent ); - self::assertSame($expected, $actual); + self::assertSame($expected, $actual, $errorMessage); } public static function scenarioProvider(): iterable From 3ccc33d28631ebb5e26b203993cf1119acbf73a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= Date: Sun, 10 Jul 2022 19:11:22 +0200 Subject: [PATCH 44/57] Fix executing plugin in a namespace (#115) Closes #62 Re-using the existing application to run the command in the sub-namespace is having another side-effect (besides potentially registering commands which we already are cleaning up since #96) which is that plugins are not re-loaded. As a result if a plugin is registered in a namespace, it will not be loaded at all hence all the commands it registers will not be available --- e2e/scenario8/README.md | 2 - e2e/scenario9/.gitignore | 5 ++ e2e/scenario9/README.md | 1 + e2e/scenario9/composer.json | 21 +++++++ e2e/scenario9/expected.txt | 9 +++ e2e/scenario9/script.sh | 28 +++++++++ .../vendor-bin/composer-unused/.gitignore | 2 + .../vendor-bin/composer-unused/composer.json | 11 ++++ src/BinCommand.php | 59 ++++++++----------- src/FreshInstanceApplicationFactory.php | 15 +++++ src/NamespaceApplicationFactory.php | 12 ++++ tests/BinCommandTest.php | 3 +- tests/EndToEndTest.php | 18 +++++- tests/Fixtures/ReuseApplicationFactory.php | 16 +++++ 14 files changed, 164 insertions(+), 38 deletions(-) create mode 100644 e2e/scenario9/.gitignore create mode 100644 e2e/scenario9/README.md create mode 100644 e2e/scenario9/composer.json create mode 100644 e2e/scenario9/expected.txt create mode 100755 e2e/scenario9/script.sh create mode 100644 e2e/scenario9/vendor-bin/composer-unused/.gitignore create mode 100644 e2e/scenario9/vendor-bin/composer-unused/composer.json create mode 100644 src/FreshInstanceApplicationFactory.php create mode 100644 src/NamespaceApplicationFactory.php create mode 100644 tests/Fixtures/ReuseApplicationFactory.php 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 @@ + Date: Sun, 10 Jul 2022 19:28:16 +0200 Subject: [PATCH 45/57] Add tests to ensure environment variables are correctly forwarded (#116) Closes #63 --- e2e/scenario10/.gitignore | 6 ++++ e2e/scenario10/README.md | 1 + e2e/scenario10/composer.json | 21 ++++++++++++++ e2e/scenario10/expected.txt | 2 ++ e2e/scenario10/script.sh | 31 +++++++++++++++++++++ e2e/scenario10/vendor-bin/ns1/composer.json | 5 ++++ e2e/scenario9/README.md | 2 +- 7 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 e2e/scenario10/.gitignore create mode 100644 e2e/scenario10/README.md create mode 100644 e2e/scenario10/composer.json create mode 100644 e2e/scenario10/expected.txt create mode 100755 e2e/scenario10/script.sh create mode 100644 e2e/scenario10/vendor-bin/ns1/composer.json diff --git a/e2e/scenario10/.gitignore b/e2e/scenario10/.gitignore new file mode 100644 index 0000000..1fced8e --- /dev/null +++ b/e2e/scenario10/.gitignore @@ -0,0 +1,6 @@ +/actual.txt +/.composer/ +/composer.lock +/vendor/ +/vendor-bin/*/composer.lock +/vendor-bin/*/vendor/ diff --git a/e2e/scenario10/README.md b/e2e/scenario10/README.md new file mode 100644 index 0000000..0ffbf03 --- /dev/null +++ b/e2e/scenario10/README.md @@ -0,0 +1 @@ +Check that the composer environment variables are well respected when commands are forwarded to the namespaces. diff --git a/e2e/scenario10/composer.json b/e2e/scenario10/composer.json new file mode 100644 index 0000000..3061d0c --- /dev/null +++ b/e2e/scenario10/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/scenario10/expected.txt b/e2e/scenario10/expected.txt new file mode 100644 index 0000000..033cf0b --- /dev/null +++ b/e2e/scenario10/expected.txt @@ -0,0 +1,2 @@ +./.composer +.composer diff --git a/e2e/scenario10/script.sh b/e2e/scenario10/script.sh new file mode 100755 index 0000000..9f012a3 --- /dev/null +++ b/e2e/scenario10/script.sh @@ -0,0 +1,31 @@ +#!/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 || true +rm -rf composer.lock || true +rm -rf vendor || true +rm -rf vendor-bin/*/composer.lock || true +rm -rf vendor-bin/*/vendor || true +rm -rf vendor-bin/*/.composer || true + +readonly CUSTOM_COMPOSER_DIR=$(pwd)/.composer +COMPOSER_CACHE_DIR=$CUSTOM_COMPOSER_DIR composer update + +# Actual command to execute the test itself +find . ".composer" -name ".composer" -type d 2>&1 | tee > actual.txt || true diff --git a/e2e/scenario10/vendor-bin/ns1/composer.json b/e2e/scenario10/vendor-bin/ns1/composer.json new file mode 100644 index 0000000..9871ea3 --- /dev/null +++ b/e2e/scenario10/vendor-bin/ns1/composer.json @@ -0,0 +1,5 @@ +{ + "require": { + "nikic/iter": "v1.6.0" + } +} diff --git a/e2e/scenario9/README.md b/e2e/scenario9/README.md index 9af1b6d..80f120b 100644 --- a/e2e/scenario9/README.md +++ b/e2e/scenario9/README.md @@ -1 +1 @@ -Tests that extra arguments and options are not lost when forwarding the command to a bin namespace. +Tests that plugins installed in a namespace are loaded when a command is executed in the namespace. From 5cc3b4f8fcabcf180a9bd60ba87a679072bd3be5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Viktor=20Sz=C3=A9pe?= Date: Sun, 10 Jul 2022 22:17:54 +0000 Subject: [PATCH 46/57] Add PHPStan to CI (#117) Add a CI job for running PHPStan --- .github/workflows/phpstan.yml | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 .github/workflows/phpstan.yml diff --git a/.github/workflows/phpstan.yml b/.github/workflows/phpstan.yml new file mode 100644 index 0000000..ebb5767 --- /dev/null +++ b/.github/workflows/phpstan.yml @@ -0,0 +1,35 @@ +name: "Static analysis" + +on: + push: + branches: + - "main" + - "master" + pull_request: null + +jobs: + static-analysis: + runs-on: "ubuntu-latest" + name: "PHPStan on PHP ${{ matrix.php }}" + strategy: + fail-fast: false + matrix: + php: + - "8.1" + steps: + - name: "Check out repository code" + uses: "actions/checkout@v2" + + - name: "Setup PHP" + uses: "shivammathur/setup-php@v2" + with: + php-version: "${{ matrix.php }}" + tools: "composer" + + - name: "Install Composer dependencies" + uses: "ramsey/composer-install@v2" + with: + dependency-versions: "highest" + + - name: "Perform static analysis" + run: "make phpstan" From 7d6f98a74b5574f317343095fb7ff162e827a7cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Viktor=20Sz=C3=A9pe?= Date: Sun, 10 Jul 2022 22:23:38 +0000 Subject: [PATCH 47/57] Fix array shape in MyTestCommand (#119) --- tests/Fixtures/MyTestCommand.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Fixtures/MyTestCommand.php b/tests/Fixtures/MyTestCommand.php index 8eff0b0..8999511 100644 --- a/tests/Fixtures/MyTestCommand.php +++ b/tests/Fixtures/MyTestCommand.php @@ -20,7 +20,7 @@ class MyTestCommand extends BaseCommand public $composer; /** - * @var list + * @var list */ public $data = []; From 1b22c35ecde8b268d29b8ca17a8f1d28fd7ac6b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Viktor=20Sz=C3=A9pe?= Date: Sun, 10 Jul 2022 22:25:19 +0000 Subject: [PATCH 48/57] Remove extra parameter of assertDataSetRecordedIs (#118) --- tests/BinCommandTest.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/BinCommandTest.php b/tests/BinCommandTest.php index f40a74f..3cbad4e 100644 --- a/tests/BinCommandTest.php +++ b/tests/BinCommandTest.php @@ -175,8 +175,7 @@ public function test_the_bin_dir_can_be_changed(): void $this->assertDataSetRecordedIs( $this->tmpDir.'/'.$binDir, $this->tmpDir.'/'.'vendor-bin/theirspace', - $this->tmpDir.'/'.'vendor-bin/theirspace/vendor', - '' + $this->tmpDir.'/'.'vendor-bin/theirspace/vendor' ); $this->assertNoMoreDataFound(); } From f2745f0b4bef1e844f48e2c23668dd8be2c424f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Viktor=20Sz=C3=A9pe?= Date: Sun, 10 Jul 2022 22:26:34 +0000 Subject: [PATCH 49/57] Raise PHPStan level to lvl5 (#120) --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 8b3dd2f..031dfde 100644 --- a/Makefile +++ b/Makefile @@ -14,7 +14,7 @@ PHPUNIT_BIN = vendor/bin/phpunit PHPUNIT = php -d zend.enable_gc=0 $(PHPUNIT_BIN) PHPUNIT_COVERAGE = XDEBUG_MODE=coverage $(PHPUNIT) --group default --coverage-xml=$(COVERAGE_DIR)/coverage-xml --log-junit=$(COVERAGE_DIR)/phpunit.junit.xml PHPSTAN_BIN = vendor/bin/phpstan -PHPSTAN = $(PHPSTAN_BIN) analyse src tests +PHPSTAN = $(PHPSTAN_BIN) analyse --level=5 src tests PHP_CS_FIXER_BIN = tools/php-cs-fixer PHP_CS_FIXER = $(PHP_CS_FIXER_BIN) fix --ansi --verbose --config=.php-cs-fixer.php COMPOSER_NORMALIZE_BIN=tools/composer-normalize From 8757e203a0add98a2d0cb63501367697e75482ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= <5175937+theofidry@users.noreply.github.com> Date: Thu, 14 Jul 2022 09:54:00 +0200 Subject: [PATCH 50/57] Add a Backward Compatibility Promise section (#126) --- README.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/README.md b/README.md index 1719511..4c8021b 100644 --- a/README.md +++ b/README.md @@ -21,6 +21,7 @@ 1. [Forward mode](#forward-mode) 1. [Reduce clutter](#reduce-clutter) 1. [Related plugins](#related-plugins) +1. [Backward Compatibility Promise](#backward-compatibility-promise) 1. [Contributing](#contributing) @@ -234,6 +235,18 @@ vendor-bin/**/composer.lock binary * [theofidry/composer-inheritance-plugin][7]: Opinionated version of [Wikimedia composer-merge-plugin][8] to work in pair with this plugin. +## Backward Compatibility Promise + +The backward compatibility promise only applies to the following API: + +- The commands registered by the plugin +- The behaviour of the commands (but not their logging/output) +- The Composer configuration + +The plugin implementation is considered to be strictly internal and its code may +change at any time in a non back-ward compatible way. + + ## Contributing A makefile is available to help out: @@ -255,3 +268,4 @@ $ make help # List all available commands [7]: https://github.com/theofidry/composer-inheritance-plugin [8]: https://github.com/wikimedia/composer-merge-plugin [phive]: https://phar.io/ +[symfony-bc-policy]: https://symfony.com/doc/current/contributing/code/bc.html From cb30caef2fef64b9c82d20afeebdbb4b094f8fbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= <5175937+theofidry@users.noreply.github.com> Date: Thu, 14 Jul 2022 09:55:34 +0200 Subject: [PATCH 51/57] Add tests for the config (#127) --- src/Config.php | 46 +++++++++++++++++++++++++---------- tests/ConfigTest.php | 57 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 12 deletions(-) create mode 100644 tests/ConfigTest.php diff --git a/src/Config.php b/src/Config.php index 81cebf4..02100ba 100644 --- a/src/Config.php +++ b/src/Config.php @@ -9,10 +9,32 @@ final class Config { + public const EXTRA_CONFIG_KEY = 'bamarni-bin'; + + public const BIN_LINKS_ENABLED = 'bin-links'; + public const TARGET_DIRECTORY = 'target-directory'; + public const FORWARD_COMMAND = 'forward-command'; + + private const DEFAULT_CONFIG = [ + self::BIN_LINKS_ENABLED => true, + self::TARGET_DIRECTORY => 'vendor-bin', + self::FORWARD_COMMAND => false, + ]; + + /** + * @var bool + */ + private $binLinks; + /** - * @var array{'bin-links': bool, 'target-directory': string, 'forward-command': bool} + * @var string */ - private $config; + private $targetDirectory; + + /** + * @var bool + */ + private $forwardCommand; public static function fromComposer(Composer $composer): self { @@ -24,28 +46,28 @@ public static function fromComposer(Composer $composer): self */ public function __construct(array $extra) { - $this->config = array_merge( - [ - 'bin-links' => true, - 'target-directory' => 'vendor-bin', - 'forward-command' => false, - ], - $extra['bamarni-bin'] ?? [] + $config = array_merge( + self::DEFAULT_CONFIG, + $extra[self::EXTRA_CONFIG_KEY] ?? [] ); + + $this->binLinks = $config[self::BIN_LINKS_ENABLED]; + $this->targetDirectory = $config[self::TARGET_DIRECTORY]; + $this->forwardCommand = $config[self::FORWARD_COMMAND]; } public function binLinksAreEnabled(): bool { - return true === $this->config['bin-links']; + return $this->binLinks; } public function getTargetDirectory(): string { - return $this->config['target-directory']; + return $this->targetDirectory; } public function isCommandForwarded(): bool { - return $this->config['forward-command']; + return $this->forwardCommand; } } diff --git a/tests/ConfigTest.php b/tests/ConfigTest.php new file mode 100644 index 0000000..94c96f1 --- /dev/null +++ b/tests/ConfigTest.php @@ -0,0 +1,57 @@ +binLinksAreEnabled()); + self::assertSame($expectedTargetDirectory, $config->getTargetDirectory()); + self::assertSame($expectedForwardCommand, $config->isCommandForwarded()); + } + + public static function provideExtraConfig(): iterable + { + yield 'default values' => [ + [], + true, + 'vendor-bin', + false, + ]; + + yield 'unknown extra entry' => [ + ['unknown' => 'foo'], + true, + 'vendor-bin', + false, + ]; + + yield 'nominal' => [ + [ + Config::EXTRA_CONFIG_KEY => [ + Config::BIN_LINKS_ENABLED => false, + Config::TARGET_DIRECTORY => 'tools', + Config::FORWARD_COMMAND => true, + ], + ], + false, + 'tools', + true, + ]; + } +} From 737a9824b6bd5bf8d7c402d36750d8674e510184 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= <5175937+theofidry@users.noreply.github.com> Date: Thu, 14 Jul 2022 10:00:00 +0200 Subject: [PATCH 52/57] Fix PHPStan issues (#128) --- src/BinCommand.php | 2 +- src/BinInputFactory.php | 6 +++--- src/InvalidBinInput.php | 4 ++-- src/Plugin.php | 2 +- tests/BinCommandTest.php | 4 +++- 5 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/BinCommand.php b/src/BinCommand.php index 2e97f5d..bbfeeea 100644 --- a/src/BinCommand.php +++ b/src/BinCommand.php @@ -217,7 +217,7 @@ private function executeInNamespace( $this->logger->logDebug( sprintf( 'Running `@composer %s`.', - $namespaceInput + $namespaceInput->__toString() ) ); diff --git a/src/BinInputFactory.php b/src/BinInputFactory.php index 90fab1b..6f60d88 100644 --- a/src/BinInputFactory.php +++ b/src/BinInputFactory.php @@ -31,7 +31,7 @@ public static function createInput( '/^(?.+)?bin (?:(?.+?) )?(?:%1$s|\'%1$s\') (?.+?)(? -- .*)?$/', preg_quote($namespace, '/') ), - (string) $previousInput, + $previousInput->__toString(), $matches ); @@ -62,7 +62,7 @@ public static function createNamespaceInput(InputInterface $previousInput): Inpu { $matchResult = preg_match( '/^(.+?\s?)(--(?: .+)?)?$/', - (string) $previousInput, + $previousInput->__toString(), $matches ); @@ -91,7 +91,7 @@ public static function createForwardedCommandInput(InputInterface $input): Input return new StringInput( sprintf( 'bin all %s', - (string) $input + $input->__toString() ) ); } diff --git a/src/InvalidBinInput.php b/src/InvalidBinInput.php index a966b60..be48698 100644 --- a/src/InvalidBinInput.php +++ b/src/InvalidBinInput.php @@ -15,7 +15,7 @@ public static function forBinInput(InputInterface $input): self return new self( sprintf( 'Could not parse the input "%s". Expected the input to be in the format "bin ", for example "bin all update --prefer-lowest".', - $input + $input->__toString() ) ); } @@ -25,7 +25,7 @@ public static function forNamespaceInput(InputInterface $input): self return new self( sprintf( 'Could not parse the input (executed within the namespace) "%s".', - $input + $input->__toString() ) ); } diff --git a/src/Plugin.php b/src/Plugin.php index dc65124..3e24295 100644 --- a/src/Plugin.php +++ b/src/Plugin.php @@ -143,7 +143,7 @@ protected function onForwardedCommand( $this->logger->logDebug( sprintf( 'Original input: %s.', - $input + $input->__toString() ) ); diff --git a/tests/BinCommandTest.php b/tests/BinCommandTest.php index 3cbad4e..c226657 100644 --- a/tests/BinCommandTest.php +++ b/tests/BinCommandTest.php @@ -60,7 +60,9 @@ protected function setUp(): void chdir($tmpDir); // On OSX sys_get_temp_dir() may return a symlink - $this->tmpDir = realpath($tmpDir); + $tmpDirRealPath = realpath($tmpDir); + self::assertNotFalse($tmpDirRealPath); + $this->tmpDir = $tmpDirRealPath; file_put_contents( $this->tmpDir.'/composer.json', From 8b832b847e0ac5f9a21e43cd04a94d56e4db3b0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= <5175937+theofidry@users.noreply.github.com> Date: Thu, 14 Jul 2022 10:44:59 +0200 Subject: [PATCH 53/57] Make the e2e tests more resilient (#129) --- e2e/scenario1/expected.txt | 2 -- e2e/scenario10/script.sh | 2 +- e2e/scenario3/expected.txt | 6 ------ e2e/scenario5/script.sh | 2 +- e2e/scenario6/expected.txt | 2 +- e2e/scenario6/script.sh | 2 +- e2e/scenario8/expected.txt | 3 --- tests/EndToEndTest.php | 6 ++++-- 8 files changed, 8 insertions(+), 17 deletions(-) diff --git a/e2e/scenario1/expected.txt b/e2e/scenario1/expected.txt index 161b70a..d0dcc1c 100644 --- a/e2e/scenario1/expected.txt +++ b/e2e/scenario1/expected.txt @@ -5,7 +5,6 @@ [bamarni-bin] Running `@composer update --verbose --working-dir='.'`. Loading composer repositories with package information Updating dependencies -Dependency resolution completed in 0.000 seconds Analyzed 90 packages to resolve dependencies Analyzed 90 rules to resolve dependencies Nothing to modify in lock file @@ -19,7 +18,6 @@ Generating autoload files [bamarni-bin] Running `@composer update --verbose --working-dir='.'`. Loading composer repositories with package information Updating dependencies -Dependency resolution completed in 0.000 seconds Analyzed 90 packages to resolve dependencies Analyzed 90 rules to resolve dependencies Nothing to modify in lock file diff --git a/e2e/scenario10/script.sh b/e2e/scenario10/script.sh index 9f012a3..4177416 100755 --- a/e2e/scenario10/script.sh +++ b/e2e/scenario10/script.sh @@ -28,4 +28,4 @@ readonly CUSTOM_COMPOSER_DIR=$(pwd)/.composer COMPOSER_CACHE_DIR=$CUSTOM_COMPOSER_DIR composer update # Actual command to execute the test itself -find . ".composer" -name ".composer" -type d 2>&1 | tee > actual.txt || true +find . ".composer" -name ".composer" -type d 2>&1 | sort -n | tee > actual.txt || true diff --git a/e2e/scenario3/expected.txt b/e2e/scenario3/expected.txt index 9e29669..cadbdcb 100644 --- a/e2e/scenario3/expected.txt +++ b/e2e/scenario3/expected.txt @@ -1,9 +1,7 @@ Loading composer repositories with package information Updating dependencies -Dependency resolution completed in 0.000 seconds Analyzed 90 packages to resolve dependencies Analyzed 90 rules to resolve dependencies -Dependency resolution completed in 0.000 seconds Lock file operations: 1 install, 0 updates, 0 removals Installs: bamarni/composer-bin-plugin:dev-hash - Locking bamarni/composer-bin-plugin (dev-hash) @@ -24,7 +22,6 @@ Generating autoload files [bamarni-bin] Running `@composer update --verbose --working-dir='.'`. Loading composer repositories with package information Updating dependencies -Dependency resolution completed in 0.000 seconds Analyzed 90 packages to resolve dependencies Analyzed 90 rules to resolve dependencies Nothing to modify in lock file @@ -44,7 +41,6 @@ Generating autoload files [bamarni-bin] Running `@composer update --verbose --working-dir='.'`. Loading composer repositories with package information Updating dependencies -Dependency resolution completed in 0.000 seconds Analyzed 90 packages to resolve dependencies Analyzed 90 rules to resolve dependencies Nothing to modify in lock file @@ -54,11 +50,9 @@ Generating autoload files [bamarni-bin] Changed current directory to /path/to/project/e2e/scenario3. Loading composer repositories with package information Updating dependencies -Dependency resolution completed in 0.000 seconds Analyzed 90 packages to resolve dependencies Analyzed 90 rules to resolve dependencies Nothing to modify in lock file -Dependency resolution completed in 0.000 seconds Installing dependencies from lock file (including require-dev) Nothing to install, update or remove Generating autoload files diff --git a/e2e/scenario5/script.sh b/e2e/scenario5/script.sh index b570d5d..4e58fb6 100755 --- a/e2e/scenario5/script.sh +++ b/e2e/scenario5/script.sh @@ -26,4 +26,4 @@ composer update composer bin all update # Actual command to execute the test itself -find vendor/bin vendor-bin/*/vendor/bin -maxdepth 1 -type f 2>&1 | tee > actual.txt || true +find vendor/bin vendor-bin/*/vendor/bin -maxdepth 1 -type f 2>&1 | sort -n | tee > actual.txt || true diff --git a/e2e/scenario6/expected.txt b/e2e/scenario6/expected.txt index 623e1e1..25f159f 100644 --- a/e2e/scenario6/expected.txt +++ b/e2e/scenario6/expected.txt @@ -1,4 +1,4 @@ +find: vendor-bin/*/vendor/bin: No such file or directory vendor/bin/phpstan vendor/bin/phpstan.phar -find: vendor-bin/*/vendor/bin: No such file or directory PHPStan - PHP Static Analysis Tool 1.8.0 diff --git a/e2e/scenario6/script.sh b/e2e/scenario6/script.sh index 4c2e74d..1b972f5 100755 --- a/e2e/scenario6/script.sh +++ b/e2e/scenario6/script.sh @@ -26,5 +26,5 @@ composer update composer bin all update # Actual command to execute the test itself -find vendor/bin vendor-bin/*/vendor/bin -maxdepth 1 -type f 2>&1 | tee > actual.txt || true +find vendor/bin vendor-bin/*/vendor/bin -maxdepth 1 -type f 2>&1 | sort -n | tee > actual.txt || true vendor/bin/phpstan --version >> actual.txt diff --git a/e2e/scenario8/expected.txt b/e2e/scenario8/expected.txt index dbca4c5..cdef9f5 100644 --- a/e2e/scenario8/expected.txt +++ b/e2e/scenario8/expected.txt @@ -1,9 +1,7 @@ Loading composer repositories with package information Updating dependencies -Dependency resolution completed in 0.000 seconds Analyzed 90 packages to resolve dependencies Analyzed 90 rules to resolve dependencies -Dependency resolution completed in 0.000 seconds Lock file operations: 1 install, 0 updates, 0 removals Installs: bamarni/composer-bin-plugin:dev-hash - Locking bamarni/composer-bin-plugin (dev-hash) @@ -24,7 +22,6 @@ Generating autoload files [bamarni-bin] Running `@composer update --prefer-lowest --verbose --working-dir='.'`. Loading composer repositories with package information Updating dependencies -Dependency resolution completed in 0.000 seconds Analyzed 90 packages to resolve dependencies Analyzed 90 rules to resolve dependencies Nothing to modify in lock file diff --git a/tests/EndToEndTest.php b/tests/EndToEndTest.php index c35bf51..83ca8f0 100644 --- a/tests/EndToEndTest.php +++ b/tests/EndToEndTest.php @@ -159,9 +159,11 @@ private static function retrieveActualOutput( // We are not interested in the time taken which can vary from locally // and on the CI. + // Also since the place at which this line may change depending on where + // it is run (i.e. is not deterministic), we simply remove it. $normalizedContent = preg_replace( - '/Dependency resolution completed in \d\.\d{3} seconds/', - 'Dependency resolution completed in 0.000 seconds', + '/Dependency resolution completed in \d\.\d{3} seconds\s/', + '', $normalizedContent ); From bc9cbe64f12bf885ed021b166704e7e64ce84a1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= <5175937+theofidry@users.noreply.github.com> Date: Thu, 14 Jul 2022 11:23:29 +0200 Subject: [PATCH 54/57] Validate the config (#130) --- src/Config.php | 48 +++++++++++++++++++++++++++++++++++++++++--- tests/ConfigTest.php | 44 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+), 3 deletions(-) diff --git a/src/Config.php b/src/Config.php index 02100ba..e4c77b9 100644 --- a/src/Config.php +++ b/src/Config.php @@ -5,6 +5,7 @@ namespace Bamarni\Composer\Bin; use Composer\Composer; +use UnexpectedValueException; use function array_merge; final class Config @@ -51,9 +52,50 @@ public function __construct(array $extra) $extra[self::EXTRA_CONFIG_KEY] ?? [] ); - $this->binLinks = $config[self::BIN_LINKS_ENABLED]; - $this->targetDirectory = $config[self::TARGET_DIRECTORY]; - $this->forwardCommand = $config[self::FORWARD_COMMAND]; + $getType = function_exists('get_debug_type') ? 'get_debug_type' : 'gettype'; + + $binLinks = $config[self::BIN_LINKS_ENABLED]; + + if (!is_bool($binLinks)) { + throw new UnexpectedValueException( + sprintf( + 'Expected setting "%s.%s" to be a boolean value. Got "%s".', + self::EXTRA_CONFIG_KEY, + self::BIN_LINKS_ENABLED, + $getType($binLinks) + ) + ); + } + + $targetDirectory = $config[self::TARGET_DIRECTORY]; + + if (!is_string($targetDirectory)) { + throw new UnexpectedValueException( + sprintf( + 'Expected setting "%s.%s" to be a string. Got "%s".', + self::EXTRA_CONFIG_KEY, + self::TARGET_DIRECTORY, + $getType($targetDirectory) + ) + ); + } + + $forwardCommand = $config[self::FORWARD_COMMAND]; + + if (!is_bool($forwardCommand)) { + throw new UnexpectedValueException( + sprintf( + 'Expected setting "%s.%s" to be a boolean value. Got "%s".', + self::EXTRA_CONFIG_KEY, + self::FORWARD_COMMAND, + gettype($forwardCommand) + ) + ); + } + + $this->binLinks = $binLinks; + $this->targetDirectory = $targetDirectory; + $this->forwardCommand = $forwardCommand; } public function binLinksAreEnabled(): bool diff --git a/tests/ConfigTest.php b/tests/ConfigTest.php index 94c96f1..948d5f2 100644 --- a/tests/ConfigTest.php +++ b/tests/ConfigTest.php @@ -6,6 +6,7 @@ use Bamarni\Composer\Bin\Config; use PHPUnit\Framework\TestCase; +use UnexpectedValueException; final class ConfigTest extends TestCase { @@ -54,4 +55,47 @@ public static function provideExtraConfig(): iterable true, ]; } + + /** + * @dataProvider provideInvalidExtraConfig + */ + public function test_it_cannot_be_instantiated_with_invalid_config( + array $extra, + string $expectedMessage + ): void { + $this->expectException(UnexpectedValueException::class); + $this->expectExceptionMessage($expectedMessage); + + new Config($extra); + } + + public static function provideInvalidExtraConfig(): iterable + { + yield 'non bool bin links' => [ + [ + Config::EXTRA_CONFIG_KEY => [ + Config::BIN_LINKS_ENABLED => 'foo', + ], + ], + 'Expected setting "bamarni-bin.bin-links" to be a boolean value. Got "string".', + ]; + + yield 'non string target directory' => [ + [ + Config::EXTRA_CONFIG_KEY => [ + Config::TARGET_DIRECTORY => false, + ], + ], + 'Expected setting "bamarni-bin.target-directory" to be a string. Got "bool".', + ]; + + yield 'non bool forward command' => [ + [ + Config::EXTRA_CONFIG_KEY => [ + Config::FORWARD_COMMAND => 'foo', + ], + ], + 'Expected setting "bamarni-bin.forward-command" to be a boolean value. Got "string".', + ]; + } } From e43ab3074702714ad10c349a018a3c9e4a003204 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= <5175937+theofidry@users.noreply.github.com> Date: Thu, 14 Jul 2022 11:45:33 +0200 Subject: [PATCH 55/57] Introduce config deprecations (#123) --- e2e/scenario11/.gitignore | 6 +++ e2e/scenario11/README.md | 1 + e2e/scenario11/composer.json | 21 +++++++++++ e2e/scenario11/expected.txt | 19 ++++++++++ e2e/scenario11/script.sh | 28 ++++++++++++++ e2e/scenario11/vendor-bin/ns1/composer.json | 1 + e2e/scenario3/composer.json | 1 + e2e/scenario3/expected.txt | 2 - e2e/scenario8/composer.json | 1 + e2e/scenario8/expected.txt | 1 - src/Config.php | 41 +++++++++++++++++++-- src/Plugin.php | 9 +++++ tests/ConfigTest.php | 31 +++++++++++++++- 13 files changed, 154 insertions(+), 8 deletions(-) create mode 100644 e2e/scenario11/.gitignore create mode 100644 e2e/scenario11/README.md create mode 100644 e2e/scenario11/composer.json create mode 100644 e2e/scenario11/expected.txt create mode 100755 e2e/scenario11/script.sh create mode 100644 e2e/scenario11/vendor-bin/ns1/composer.json diff --git a/e2e/scenario11/.gitignore b/e2e/scenario11/.gitignore new file mode 100644 index 0000000..1fced8e --- /dev/null +++ b/e2e/scenario11/.gitignore @@ -0,0 +1,6 @@ +/actual.txt +/.composer/ +/composer.lock +/vendor/ +/vendor-bin/*/composer.lock +/vendor-bin/*/vendor/ diff --git a/e2e/scenario11/README.md b/e2e/scenario11/README.md new file mode 100644 index 0000000..03ab64e --- /dev/null +++ b/e2e/scenario11/README.md @@ -0,0 +1 @@ +Check that the deprecation messages are well rendered. diff --git a/e2e/scenario11/composer.json b/e2e/scenario11/composer.json new file mode 100644 index 0000000..3061d0c --- /dev/null +++ b/e2e/scenario11/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/scenario11/expected.txt b/e2e/scenario11/expected.txt new file mode 100644 index 0000000..0368aeb --- /dev/null +++ b/e2e/scenario11/expected.txt @@ -0,0 +1,19 @@ +Loading composer repositories with package information +Updating dependencies +Lock file operations: 1 install, 0 updates, 0 removals + - Locking bamarni/composer-bin-plugin (dev-hash) +Writing lock file +Installing dependencies from lock file (including require-dev) +Package operations: 1 install, 0 updates, 0 removals + - Installing bamarni/composer-bin-plugin (dev-hash): Symlinking from ../.. +Generating autoload files +[bamarni-bin] The setting "bamarni-bin.bin-links" will be set to "false" from 2.x onwards. If you wish to keep it to "true", you need to set it explicitly. +[bamarni-bin] The command is being forwarded. +[bamarni-bin] Checking namespace vendor-bin/ns1 +Loading composer repositories with package information +Updating dependencies +Nothing to modify in lock file +Writing lock file +Installing dependencies from lock file (including require-dev) +Nothing to install, update or remove +Generating autoload files diff --git a/e2e/scenario11/script.sh b/e2e/scenario11/script.sh new file mode 100755 index 0000000..e73e86f --- /dev/null +++ b/e2e/scenario11/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 || true +rm -rf composer.lock || true +rm -rf vendor || true +rm -rf vendor-bin/*/composer.lock || true +rm -rf vendor-bin/*/vendor || true +rm -rf vendor-bin/*/.composer || true + +# Actual command to execute the test itself +composer update 2>&1 | tee > actual.txt diff --git a/e2e/scenario11/vendor-bin/ns1/composer.json b/e2e/scenario11/vendor-bin/ns1/composer.json new file mode 100644 index 0000000..0967ef4 --- /dev/null +++ b/e2e/scenario11/vendor-bin/ns1/composer.json @@ -0,0 +1 @@ +{} diff --git a/e2e/scenario3/composer.json b/e2e/scenario3/composer.json index d449a2e..a1cc737 100644 --- a/e2e/scenario3/composer.json +++ b/e2e/scenario3/composer.json @@ -15,6 +15,7 @@ }, "extra": { "bamarni-bin": { + "bin-links": false, "forward-command": true } } diff --git a/e2e/scenario3/expected.txt b/e2e/scenario3/expected.txt index cadbdcb..af2a499 100644 --- a/e2e/scenario3/expected.txt +++ b/e2e/scenario3/expected.txt @@ -16,7 +16,6 @@ Generating autoload files [bamarni-bin] The command is being forwarded. [bamarni-bin] Original input: update --verbose. [bamarni-bin] Current working directory: /path/to/project/e2e/scenario3 -[bamarni-bin] Configuring bin directory to /path/to/project/e2e/scenario3/vendor/bin. [bamarni-bin] Checking namespace vendor-bin/ns1 [bamarni-bin] Changed current directory to vendor-bin/ns1. [bamarni-bin] Running `@composer update --verbose --working-dir='.'`. @@ -35,7 +34,6 @@ Generating autoload files [bamarni-bin] The command is being forwarded. [bamarni-bin] Original input: update --verbose. [bamarni-bin] Current working directory: /path/to/project/e2e/scenario3 -[bamarni-bin] Configuring bin directory to /path/to/project/e2e/scenario3/vendor/bin. [bamarni-bin] Checking namespace vendor-bin/ns1 [bamarni-bin] Changed current directory to vendor-bin/ns1. [bamarni-bin] Running `@composer update --verbose --working-dir='.'`. diff --git a/e2e/scenario8/composer.json b/e2e/scenario8/composer.json index 3061d0c..7681c07 100644 --- a/e2e/scenario8/composer.json +++ b/e2e/scenario8/composer.json @@ -15,6 +15,7 @@ }, "extra": { "bamarni-bin": { + "bin-links": false, "forward-command": true } } diff --git a/e2e/scenario8/expected.txt b/e2e/scenario8/expected.txt index cdef9f5..82a28d3 100644 --- a/e2e/scenario8/expected.txt +++ b/e2e/scenario8/expected.txt @@ -16,7 +16,6 @@ Generating autoload files [bamarni-bin] The command is being forwarded. [bamarni-bin] Original input: update --prefer-lowest --verbose. [bamarni-bin] Current working directory: /path/to/project/e2e/scenario8 -[bamarni-bin] Configuring bin directory to /path/to/project/e2e/scenario8/vendor/bin. [bamarni-bin] Checking namespace vendor-bin/ns1 [bamarni-bin] Changed current directory to vendor-bin/ns1. [bamarni-bin] Running `@composer update --prefer-lowest --verbose --working-dir='.'`. diff --git a/src/Config.php b/src/Config.php index e4c77b9..05ea7ac 100644 --- a/src/Config.php +++ b/src/Config.php @@ -6,6 +6,7 @@ use Composer\Composer; use UnexpectedValueException; +use function array_key_exists; use function array_merge; final class Config @@ -37,6 +38,11 @@ final class Config */ private $forwardCommand; + /** + * @var list + */ + private $deprecations = []; + public static function fromComposer(Composer $composer): self { return new self($composer->getPackage()->getExtra()); @@ -47,10 +53,9 @@ public static function fromComposer(Composer $composer): self */ public function __construct(array $extra) { - $config = array_merge( - self::DEFAULT_CONFIG, - $extra[self::EXTRA_CONFIG_KEY] ?? [] - ); + $userExtra = $extra[self::EXTRA_CONFIG_KEY] ?? []; + + $config = array_merge(self::DEFAULT_CONFIG, $userExtra); $getType = function_exists('get_debug_type') ? 'get_debug_type' : 'gettype'; @@ -67,6 +72,16 @@ public function __construct(array $extra) ); } + $binLinksSetExplicitly = array_key_exists(self::BIN_LINKS_ENABLED, $userExtra); + + if ($binLinks && !$binLinksSetExplicitly) { + $this->deprecations[] = sprintf( + 'The setting "%s.%s" will be set to "false" from 2.x onwards. If you wish to keep it to "true", you need to set it explicitly.', + self::EXTRA_CONFIG_KEY, + self::BIN_LINKS_ENABLED + ); + } + $targetDirectory = $config[self::TARGET_DIRECTORY]; if (!is_string($targetDirectory)) { @@ -93,6 +108,16 @@ public function __construct(array $extra) ); } + $forwardCommandSetExplicitly = array_key_exists(self::FORWARD_COMMAND, $userExtra); + + if (!$forwardCommand && !$forwardCommandSetExplicitly) { + $this->deprecations[] = sprintf( + 'The setting "%s.%s" will be set to "true" from 2.x onwards. If you wish to keep it to "false", you need to set it explicitly.', + self::EXTRA_CONFIG_KEY, + self::FORWARD_COMMAND + ); + } + $this->binLinks = $binLinks; $this->targetDirectory = $targetDirectory; $this->forwardCommand = $forwardCommand; @@ -112,4 +137,12 @@ public function isCommandForwarded(): bool { return $this->forwardCommand; } + + /** + * @return list + */ + public function getDeprecations(): array + { + return $this->deprecations; + } } diff --git a/src/Plugin.php b/src/Plugin.php index 3e24295..da1ca13 100644 --- a/src/Plugin.php +++ b/src/Plugin.php @@ -21,6 +21,7 @@ use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; use Throwable; +use function count; use function in_array; use function sprintf; @@ -118,6 +119,14 @@ private function onEvent( ): bool { $config = Config::fromComposer($this->composer); + $deprecations = $config->getDeprecations(); + + if (count($deprecations) > 0) { + foreach ($deprecations as $deprecation) { + $this->logger->logStandard($deprecation); + } + } + if ($config->isCommandForwarded() && in_array($commandName, self::FORWARDED_COMMANDS, true) ) { diff --git a/tests/ConfigTest.php b/tests/ConfigTest.php index 948d5f2..14e4256 100644 --- a/tests/ConfigTest.php +++ b/tests/ConfigTest.php @@ -12,27 +12,38 @@ final class ConfigTest extends TestCase { /** * @dataProvider provideExtraConfig + * + * @param list $expectedDeprecations */ public function test_it_can_be_instantiated( array $extra, bool $expectedBinLinksEnabled, string $expectedTargetDirectory, - bool $expectedForwardCommand + bool $expectedForwardCommand, + array $expectedDeprecations ): void { $config = new Config($extra); self::assertSame($expectedBinLinksEnabled, $config->binLinksAreEnabled()); self::assertSame($expectedTargetDirectory, $config->getTargetDirectory()); self::assertSame($expectedForwardCommand, $config->isCommandForwarded()); + self::assertSame($expectedDeprecations, $config->getDeprecations()); } public static function provideExtraConfig(): iterable { + $binLinksEnabledDeprecationMessage = 'The setting "bamarni-bin.bin-links" will be set to "false" from 2.x onwards. If you wish to keep it to "true", you need to set it explicitly.'; + $forwardCommandDeprecationMessage = 'The setting "bamarni-bin.forward-command" will be set to "true" from 2.x onwards. If you wish to keep it to "false", you need to set it explicitly.'; + yield 'default values' => [ [], true, 'vendor-bin', false, + [ + $binLinksEnabledDeprecationMessage, + $forwardCommandDeprecationMessage, + ], ]; yield 'unknown extra entry' => [ @@ -40,6 +51,23 @@ public static function provideExtraConfig(): iterable true, 'vendor-bin', false, + [ + $binLinksEnabledDeprecationMessage, + $forwardCommandDeprecationMessage, + ], + ]; + + yield 'same as default but explicit' => [ + [ + Config::EXTRA_CONFIG_KEY => [ + Config::BIN_LINKS_ENABLED => true, + Config::FORWARD_COMMAND => false, + ], + ], + true, + 'vendor-bin', + false, + [], ]; yield 'nominal' => [ @@ -53,6 +81,7 @@ public static function provideExtraConfig(): iterable false, 'tools', true, + [], ]; } From c4cefbcc17c25f31387a12a3d266893b6a16b624 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= <5175937+theofidry@users.noreply.github.com> Date: Thu, 14 Jul 2022 12:13:11 +0200 Subject: [PATCH 56/57] Re-organise the code (#131) --- composer.json | 2 +- e2e/scenario3/expected.txt | 4 ++-- e2e/scenario8/expected.txt | 2 +- .../FreshInstanceApplicationFactory.php | 2 +- .../NamespaceApplicationFactory.php | 2 +- src/{Plugin.php => BamarniBinPlugin.php} | 5 ++++- src/{ => Command}/BinCommand.php | 10 ++++++++-- .../CouldNotCreateNamespaceDir.php | 2 +- src/CommandProvider.php | 1 + src/{ => Config}/Config.php | 18 +++++++++++++----- src/{ => Config}/ConfigFactory.php | 2 +- .../InvalidBamarniComposerExtraConfig.php | 11 +++++++++++ src/{ => Input}/BinInputFactory.php | 2 +- src/{ => Input}/InvalidBinInput.php | 2 +- tests/{ => Command}/BinCommandTest.php | 7 ++++--- tests/{ => Config}/ConfigTest.php | 11 +++++++---- tests/Fixtures/ReuseApplicationFactory.php | 2 +- tests/{ => Input}/BinInputFactoryTest.php | 6 +++--- 18 files changed, 62 insertions(+), 29 deletions(-) rename src/{ => ApplicationFactory}/FreshInstanceApplicationFactory.php (84%) rename src/{ => ApplicationFactory}/NamespaceApplicationFactory.php (78%) rename src/{Plugin.php => BamarniBinPlugin.php} (95%) rename src/{ => Command}/BinCommand.php (95%) rename src/{ => Command}/CouldNotCreateNamespaceDir.php (90%) rename src/{ => Config}/Config.php (89%) rename src/{ => Config}/ConfigFactory.php (94%) create mode 100644 src/Config/InvalidBamarniComposerExtraConfig.php rename src/{ => Input}/BinInputFactory.php (98%) rename src/{ => Input}/InvalidBinInput.php (95%) rename tests/{ => Command}/BinCommandTest.php (97%) rename tests/{ => Config}/ConfigTest.php (93%) rename tests/{ => Input}/BinInputFactoryTest.php (97%) diff --git a/composer.json b/composer.json index 617ffb6..06a4b5e 100644 --- a/composer.json +++ b/composer.json @@ -45,6 +45,6 @@ "sort-packages": true }, "extra": { - "class": "Bamarni\\Composer\\Bin\\Plugin" + "class": "Bamarni\\Composer\\Bin\\BamarniBinPlugin" } } diff --git a/e2e/scenario3/expected.txt b/e2e/scenario3/expected.txt index af2a499..133ffe7 100644 --- a/e2e/scenario3/expected.txt +++ b/e2e/scenario3/expected.txt @@ -11,7 +11,7 @@ Package operations: 1 install, 0 updates, 0 removals Installs: bamarni/composer-bin-plugin:dev-hash - Installing bamarni/composer-bin-plugin (dev-hash): Symlinking from ../.. Generating autoload files -> post-autoload-dump: Bamarni\Composer\Bin\Plugin->onPostAutoloadDump +> post-autoload-dump: Bamarni\Composer\Bin\BamarniBinPlugin->onPostAutoloadDump [bamarni-bin] Calling onPostAutoloadDump(). [bamarni-bin] The command is being forwarded. [bamarni-bin] Original input: update --verbose. @@ -54,6 +54,6 @@ Nothing to modify in lock file Installing dependencies from lock file (including require-dev) Nothing to install, update or remove Generating autoload files -> post-autoload-dump: Bamarni\Composer\Bin\Plugin->onPostAutoloadDump +> post-autoload-dump: Bamarni\Composer\Bin\BamarniBinPlugin->onPostAutoloadDump [bamarni-bin] Calling onPostAutoloadDump(). [bamarni-bin] Command already forwarded within the process: skipping. diff --git a/e2e/scenario8/expected.txt b/e2e/scenario8/expected.txt index 82a28d3..625e41f 100644 --- a/e2e/scenario8/expected.txt +++ b/e2e/scenario8/expected.txt @@ -11,7 +11,7 @@ Package operations: 1 install, 0 updates, 0 removals Installs: bamarni/composer-bin-plugin:dev-hash - Installing bamarni/composer-bin-plugin (dev-hash): Symlinking from ../.. Generating autoload files -> post-autoload-dump: Bamarni\Composer\Bin\Plugin->onPostAutoloadDump +> post-autoload-dump: Bamarni\Composer\Bin\BamarniBinPlugin->onPostAutoloadDump [bamarni-bin] Calling onPostAutoloadDump(). [bamarni-bin] The command is being forwarded. [bamarni-bin] Original input: update --prefer-lowest --verbose. diff --git a/src/FreshInstanceApplicationFactory.php b/src/ApplicationFactory/FreshInstanceApplicationFactory.php similarity index 84% rename from src/FreshInstanceApplicationFactory.php rename to src/ApplicationFactory/FreshInstanceApplicationFactory.php index d791340..a4dcd58 100644 --- a/src/FreshInstanceApplicationFactory.php +++ b/src/ApplicationFactory/FreshInstanceApplicationFactory.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace Bamarni\Composer\Bin; +namespace Bamarni\Composer\Bin\ApplicationFactory; use Composer\Console\Application; diff --git a/src/NamespaceApplicationFactory.php b/src/ApplicationFactory/NamespaceApplicationFactory.php similarity index 78% rename from src/NamespaceApplicationFactory.php rename to src/ApplicationFactory/NamespaceApplicationFactory.php index 78bc01a..ff58a4d 100644 --- a/src/NamespaceApplicationFactory.php +++ b/src/ApplicationFactory/NamespaceApplicationFactory.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace Bamarni\Composer\Bin; +namespace Bamarni\Composer\Bin\ApplicationFactory; use Composer\Console\Application; diff --git a/src/Plugin.php b/src/BamarniBinPlugin.php similarity index 95% rename from src/Plugin.php rename to src/BamarniBinPlugin.php index da1ca13..1d21c93 100644 --- a/src/Plugin.php +++ b/src/BamarniBinPlugin.php @@ -4,7 +4,10 @@ namespace Bamarni\Composer\Bin; +use Bamarni\Composer\Bin\Command\BinCommand; use Bamarni\Composer\Bin\CommandProvider as BamarniCommandProvider; +use Bamarni\Composer\Bin\Config\Config; +use Bamarni\Composer\Bin\Input\BinInputFactory; use Composer\Composer; use Composer\Console\Application; use Composer\EventDispatcher\EventSubscriberInterface; @@ -28,7 +31,7 @@ /** * @final Will be final in 2.x. */ -class Plugin implements PluginInterface, Capable, EventSubscriberInterface +class BamarniBinPlugin implements PluginInterface, Capable, EventSubscriberInterface { private const FORWARDED_COMMANDS = ['install', 'update']; diff --git a/src/BinCommand.php b/src/Command/BinCommand.php similarity index 95% rename from src/BinCommand.php rename to src/Command/BinCommand.php index bbfeeea..bc28875 100644 --- a/src/BinCommand.php +++ b/src/Command/BinCommand.php @@ -2,8 +2,14 @@ declare(strict_types=1); -namespace Bamarni\Composer\Bin; - +namespace Bamarni\Composer\Bin\Command; + +use Bamarni\Composer\Bin\Config\Config; +use Bamarni\Composer\Bin\Config\ConfigFactory; +use Bamarni\Composer\Bin\ApplicationFactory\FreshInstanceApplicationFactory; +use Bamarni\Composer\Bin\Input\BinInputFactory; +use Bamarni\Composer\Bin\Logger; +use Bamarni\Composer\Bin\ApplicationFactory\NamespaceApplicationFactory; use Composer\Command\BaseCommand; use Composer\Factory; use Composer\IO\IOInterface; diff --git a/src/CouldNotCreateNamespaceDir.php b/src/Command/CouldNotCreateNamespaceDir.php similarity index 90% rename from src/CouldNotCreateNamespaceDir.php rename to src/Command/CouldNotCreateNamespaceDir.php index 17683e2..3314507 100644 --- a/src/CouldNotCreateNamespaceDir.php +++ b/src/Command/CouldNotCreateNamespaceDir.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace Bamarni\Composer\Bin; +namespace Bamarni\Composer\Bin\Command; use RuntimeException; use function sprintf; diff --git a/src/CommandProvider.php b/src/CommandProvider.php index 842f5ad..cc47092 100644 --- a/src/CommandProvider.php +++ b/src/CommandProvider.php @@ -4,6 +4,7 @@ namespace Bamarni\Composer\Bin; +use Bamarni\Composer\Bin\Command\BinCommand; use Composer\Plugin\Capability\CommandProvider as CommandProviderCapability; /** diff --git a/src/Config.php b/src/Config/Config.php similarity index 89% rename from src/Config.php rename to src/Config/Config.php index 05ea7ac..bf672cf 100644 --- a/src/Config.php +++ b/src/Config/Config.php @@ -2,12 +2,15 @@ declare(strict_types=1); -namespace Bamarni\Composer\Bin; +namespace Bamarni\Composer\Bin\Config; use Composer\Composer; -use UnexpectedValueException; use function array_key_exists; use function array_merge; +use function function_exists; +use function is_bool; +use function is_string; +use function sprintf; final class Config { @@ -43,6 +46,9 @@ final class Config */ private $deprecations = []; + /** + * @throws InvalidBamarniComposerExtraConfig + */ public static function fromComposer(Composer $composer): self { return new self($composer->getPackage()->getExtra()); @@ -50,6 +56,8 @@ public static function fromComposer(Composer $composer): self /** * @param mixed[] $extra + * + * @throws InvalidBamarniComposerExtraConfig */ public function __construct(array $extra) { @@ -62,7 +70,7 @@ public function __construct(array $extra) $binLinks = $config[self::BIN_LINKS_ENABLED]; if (!is_bool($binLinks)) { - throw new UnexpectedValueException( + throw new InvalidBamarniComposerExtraConfig( sprintf( 'Expected setting "%s.%s" to be a boolean value. Got "%s".', self::EXTRA_CONFIG_KEY, @@ -85,7 +93,7 @@ public function __construct(array $extra) $targetDirectory = $config[self::TARGET_DIRECTORY]; if (!is_string($targetDirectory)) { - throw new UnexpectedValueException( + throw new InvalidBamarniComposerExtraConfig( sprintf( 'Expected setting "%s.%s" to be a string. Got "%s".', self::EXTRA_CONFIG_KEY, @@ -98,7 +106,7 @@ public function __construct(array $extra) $forwardCommand = $config[self::FORWARD_COMMAND]; if (!is_bool($forwardCommand)) { - throw new UnexpectedValueException( + throw new InvalidBamarniComposerExtraConfig( sprintf( 'Expected setting "%s.%s" to be a boolean value. Got "%s".', self::EXTRA_CONFIG_KEY, diff --git a/src/ConfigFactory.php b/src/Config/ConfigFactory.php similarity index 94% rename from src/ConfigFactory.php rename to src/Config/ConfigFactory.php index 2243934..b2fc1d4 100644 --- a/src/ConfigFactory.php +++ b/src/Config/ConfigFactory.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace Bamarni\Composer\Bin; +namespace Bamarni\Composer\Bin\Config; use Composer\Config as ComposerConfig; use Composer\Factory; diff --git a/src/Config/InvalidBamarniComposerExtraConfig.php b/src/Config/InvalidBamarniComposerExtraConfig.php new file mode 100644 index 0000000..105f9ea --- /dev/null +++ b/src/Config/InvalidBamarniComposerExtraConfig.php @@ -0,0 +1,11 @@ +expectException(UnexpectedValueException::class); + $this->expectException(InvalidBamarniComposerExtraConfig::class); $this->expectExceptionMessage($expectedMessage); new Config($extra); diff --git a/tests/Fixtures/ReuseApplicationFactory.php b/tests/Fixtures/ReuseApplicationFactory.php index eeec010..98cf03d 100644 --- a/tests/Fixtures/ReuseApplicationFactory.php +++ b/tests/Fixtures/ReuseApplicationFactory.php @@ -4,7 +4,7 @@ namespace Bamarni\Composer\Bin\Tests\Fixtures; -use Bamarni\Composer\Bin\NamespaceApplicationFactory; +use Bamarni\Composer\Bin\ApplicationFactory\NamespaceApplicationFactory; use Composer\Console\Application; final class ReuseApplicationFactory implements NamespaceApplicationFactory diff --git a/tests/BinInputFactoryTest.php b/tests/Input/BinInputFactoryTest.php similarity index 97% rename from tests/BinInputFactoryTest.php rename to tests/Input/BinInputFactoryTest.php index 2eca750..75684a6 100644 --- a/tests/BinInputFactoryTest.php +++ b/tests/Input/BinInputFactoryTest.php @@ -2,16 +2,16 @@ declare(strict_types=1); -namespace Bamarni\Composer\Bin\Tests; +namespace Bamarni\Composer\Bin\Tests\Input; -use Bamarni\Composer\Bin\BinInputFactory; +use Bamarni\Composer\Bin\Input\BinInputFactory; use PHPUnit\Framework\TestCase; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\StringInput; use function sprintf; /** - * @covers \Bamarni\Composer\Bin\BinInputFactory + * @covers \Bamarni\Composer\Bin\Input\BinInputFactory */ final class BinInputFactoryTest extends TestCase { From 24764700027bcd3cb072e5f8005d4a150fe714fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= <5175937+theofidry@users.noreply.github.com> Date: Thu, 14 Jul 2022 12:29:51 +0200 Subject: [PATCH 57/57] Fix usage with Composer <2.3 (#132) Closes #124 --- .github/workflows/tests.yaml | 8 ++++++-- src/Command/BinCommand.php | 3 ++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index b2c4219..1dce3a3 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -10,7 +10,7 @@ on: jobs: unit-tests: runs-on: "ubuntu-latest" - name: "Unit Tests on PHP ${{ matrix.php }}" + name: "Unit Tests on PHP ${{ matrix.php }} and ${{ matrix.tools }}" strategy: fail-fast: false matrix: @@ -20,6 +20,10 @@ jobs: - "7.4" - "8.0" - "8.1" + tools: [ "composer" ] + include: + - php: "7.2" + tools: "composer:v2.0" steps: - name: "Check out repository code" @@ -29,7 +33,7 @@ jobs: uses: "shivammathur/setup-php@v2" with: php-version: "${{ matrix.php }}" - tools: "composer" + tools: "${{ matrix.tools }}" - name: "Install Composer dependencies" uses: "ramsey/composer-install@v2" diff --git a/src/Command/BinCommand.php b/src/Command/BinCommand.php index bc28875..348a04a 100644 --- a/src/Command/BinCommand.php +++ b/src/Command/BinCommand.php @@ -94,7 +94,8 @@ public function isProxyCommand(): bool public function execute(InputInterface $input, OutputInterface $output): int { - $config = Config::fromComposer($this->requireComposer()); + // Switch to requireComposer() once Composer 2.3 is set as the minimum + $config = Config::fromComposer($this->getComposer()); $currentWorkingDir = getcwd(); $this->logger->logDebug(