From c7a2e5228f3db1331729db6030d2063ec67ea4cc Mon Sep 17 00:00:00 2001 From: jrfnl Date: Mon, 21 Jan 2019 16:53:34 +0100 Subject: [PATCH 1/4] Improve feedback to user on failure to set paths Related to 79. When the setting of `installed_paths` fails, show an error instead of giving the impression that the setting of the `installed_paths` succeeded. --- src/Plugin.php | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/Plugin.php b/src/Plugin.php index 3539a2c3..b707ab9f 100644 --- a/src/Plugin.php +++ b/src/Plugin.php @@ -238,9 +238,7 @@ private function saveInstalledPaths() ); } - $this->io->write($configMessage); - - $this->processExecutor->execute( + $exitCode = $this->processExecutor->execute( sprintf( 'phpcs %s', implode(' ', $arguments) @@ -249,6 +247,16 @@ private function saveInstalledPaths() $this->composer->getConfig()->get('bin-dir') ); + if ($exitCode === 0) { + $this->io->write($configMessage); + } else { + $failMessage = sprintf( + 'Failed to set PHP CodeSniffer %s Config', + self::PHPCS_CONFIG_KEY + ); + $this->io->write($failMessage); + } + if ($this->io->isVerbose() && !empty($configResult)) { $this->io->write(sprintf('%s', $configResult)); } From 7199248c9b40ea7981a837cdc0407bc54b50af32 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Mon, 21 Jan 2019 18:12:51 +0100 Subject: [PATCH 2/4] Use the PHP version used by Composer Possible not the best way to do this, but it does work, so consider this a proof of concept, if not the solution. The `getPhpExecCommand()` method in the `Composer\EventDispatcher\EventDispatcher` class basically does what is needed: find the PHP executable used by Composer and turn it into an executable command. Unfortunately, that method is `protected`, making it difficult to call that method from within the plugin. This now copies that method into the plugin and uses it to create the command to pass on to the `ProcessExecutor`. The copied method has some minor modifications to comply with the coding standards used within this project. Based on the proof of conflict referenced in 79, I can confirm that this would solve the issue. --- src/Plugin.php | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/src/Plugin.php b/src/Plugin.php index b707ab9f..2d98b951 100644 --- a/src/Plugin.php +++ b/src/Plugin.php @@ -25,6 +25,7 @@ use Symfony\Component\Process\Exception\LogicException; use Symfony\Component\Process\Exception\ProcessFailedException; use Symfony\Component\Process\Exception\RuntimeException; +use Symfony\Component\Process\PhpExecutableFinder; /** * PHP_CodeSniffer standard installation manager. @@ -240,11 +241,12 @@ private function saveInstalledPaths() $exitCode = $this->processExecutor->execute( sprintf( - 'phpcs %s', + '%s ./bin/phpcs %s', + $this->getPhpExecCommand(), implode(' ', $arguments) ), $configResult, - $this->composer->getConfig()->get('bin-dir') + $this->getPHPCodeSnifferInstallPath() ); if ($exitCode === 0) { @@ -262,6 +264,30 @@ private function saveInstalledPaths() } } + /** + * Get the path to the current PHP version being used. + * + * Duplicate of the same in the EventDispatcher class in Composer itself. + */ + protected function getPhpExecCommand() + { + $finder = new PhpExecutableFinder(); + $phpPath = $finder->find(false); + if (!$phpPath) { + throw new \RuntimeException('Failed to locate PHP binary to execute ' . $phpPath); + } + $phpArgs = $finder->findArguments(); + $phpArgs = $phpArgs ? ' ' . implode(' ', $phpArgs) : ''; + + $command = ProcessExecutor::escape($phpPath); + $command .= $phpArgs; + $command .= ' -d allow_url_fopen=' . ProcessExecutor::escape(ini_get('allow_url_fopen')); + $command .= ' -d disable_functions=' . ProcessExecutor::escape(ini_get('disable_functions')); + $command .= ' -d memory_limit=' . ProcessExecutor::escape(ini_get('memory_limit')); + + return $command; + } + /** * Iterate trough all known paths and check if they are still valid. * From 6de96f606d5ad87f94b1eaa279bf630371113532 Mon Sep 17 00:00:00 2001 From: Ben Peachey Date: Wed, 11 Sep 2019 21:24:27 +0200 Subject: [PATCH 3/4] Minor code cleanup for Plugin.php - Moves command outside of exec call, for readability - Moves error message composition to begin of method, to keep it together with other preparations and keep `if/else` statement cleaner - Adds whitespace to separate actions that do not belong together or for readability - Removes redundant variable concat --- src/Plugin.php | 45 ++++++++++++++++++++++++++++----------------- 1 file changed, 28 insertions(+), 17 deletions(-) diff --git a/src/Plugin.php b/src/Plugin.php index 2d98b951..3087a1e6 100644 --- a/src/Plugin.php +++ b/src/Plugin.php @@ -239,12 +239,20 @@ private function saveInstalledPaths() ); } + // Prepare message in case of failure + $failMessage = sprintf( + 'Failed to set PHP CodeSniffer %s Config', + self::PHPCS_CONFIG_KEY + ); + + // Okay, lets rock! 🤘 + $command = vsprintf('%s ./bin/phpcs %s', array( + 'php executable' => $this->getPhpExecCommand(), + 'arguments' => implode(' ', $arguments) + )); + $exitCode = $this->processExecutor->execute( - sprintf( - '%s ./bin/phpcs %s', - $this->getPhpExecCommand(), - implode(' ', $arguments) - ), + $command, $configResult, $this->getPHPCodeSnifferInstallPath() ); @@ -252,10 +260,6 @@ private function saveInstalledPaths() if ($exitCode === 0) { $this->io->write($configMessage); } else { - $failMessage = sprintf( - 'Failed to set PHP CodeSniffer %s Config', - self::PHPCS_CONFIG_KEY - ); $this->io->write($failMessage); } @@ -272,18 +276,25 @@ private function saveInstalledPaths() protected function getPhpExecCommand() { $finder = new PhpExecutableFinder(); + $phpPath = $finder->find(false); - if (!$phpPath) { + + if ($phpPath === false) { throw new \RuntimeException('Failed to locate PHP binary to execute ' . $phpPath); } - $phpArgs = $finder->findArguments(); - $phpArgs = $phpArgs ? ' ' . implode(' ', $phpArgs) : ''; - $command = ProcessExecutor::escape($phpPath); - $command .= $phpArgs; - $command .= ' -d allow_url_fopen=' . ProcessExecutor::escape(ini_get('allow_url_fopen')); - $command .= ' -d disable_functions=' . ProcessExecutor::escape(ini_get('disable_functions')); - $command .= ' -d memory_limit=' . ProcessExecutor::escape(ini_get('memory_limit')); + $phpArgs = $finder->findArguments(); + $phpArgs = $phpArgs + ? ' ' . implode(' ', $phpArgs) + : '' + ; + + $command = ProcessExecutor::escape($phpPath) . + $phpArgs . + ' -d allow_url_fopen=' . ProcessExecutor::escape(ini_get('allow_url_fopen')) . + ' -d disable_functions=' . ProcessExecutor::escape(ini_get('disable_functions')) . + ' -d memory_limit=' . ProcessExecutor::escape(ini_get('memory_limit')) + ; return $command; } From 22d3cdc79db37841ce8b8cf66f947f96077d32f5 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Mon, 9 Dec 2019 05:13:27 +0100 Subject: [PATCH 4/4] Fix compatibility with PHPCS 2.x for the fix --- src/Plugin.php | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/src/Plugin.php b/src/Plugin.php index 3087a1e6..e656acbb 100644 --- a/src/Plugin.php +++ b/src/Plugin.php @@ -245,18 +245,28 @@ private function saveInstalledPaths() self::PHPCS_CONFIG_KEY ); - // Okay, lets rock! 🤘 - $command = vsprintf('%s ./bin/phpcs %s', array( - 'php executable' => $this->getPhpExecCommand(), - 'arguments' => implode(' ', $arguments) - )); - - $exitCode = $this->processExecutor->execute( - $command, - $configResult, - $this->getPHPCodeSnifferInstallPath() + // Determine the path to the main PHPCS file. + $phpcsPath = $this->getPHPCodeSnifferInstallPath(); + if (file_exists($phpcsPath . '/bin/phpcs') === true) { + // PHPCS 3.x. + $phpcsExecutable = './bin/phpcs'; + } else { + // PHPCS 2.x. + $phpcsExecutable = './scripts/phpcs'; + } + + // Okay, lets rock! + $command = vsprintf( + '%s %s %s', + array( + 'php executable' => $this->getPhpExecCommand(), + 'phpcs executable' => $phpcsExecutable, + 'arguments' => implode(' ', $arguments) + ) ); + $exitCode = $this->processExecutor->execute($command, $configResult, $phpcsPath); + if ($exitCode === 0) { $this->io->write($configMessage); } else {