From ed3dcadc3deaa9874511bf3eeda672d4f22a0c8c Mon Sep 17 00:00:00 2001 From: Greg Bowler Date: Wed, 7 Oct 2026 18:42:01 +0100 Subject: [PATCH] feature: much nicer handling of non-installed failures closes #232 --- README.md | 28 ++++-- src/BuildRunner.php | 58 ++++++++++--- src/ClientSidePackages.php | 89 +++++++++++++++++++ test/phpunit/BuildRunnerTest.php | 89 +++++++++++++++++++ test/phpunit/ClientSidePackagesTest.php | 109 ++++++++++++++++++++++++ 5 files changed, 355 insertions(+), 18 deletions(-) create mode 100644 src/ClientSidePackages.php create mode 100644 test/phpunit/ClientSidePackagesTest.php diff --git a/README.md b/README.md index f26c6c5..2acf10c 100644 --- a/README.md +++ b/README.md @@ -61,12 +61,28 @@ An example `build.json` below shows three different usages: } ``` -Not a dependency manager ------------------------- - -This library assumes the configuration of the system is already configured. - -The primary objective is to provide a client-side build system that is automatically configured for PHP projects, leaving the configuration of the system down to the developer's choice of client-side dependency management software. +Installing client-side packages +------------------------------- + +For projects with a `package.json`, the build runner checks direct dependencies with +`npm ls --json --depth=0 --include=dev` before checking build requirements. Missing +packages or versions that do not satisfy `package.json` trigger `npm install +--include=dev` automatically, with progress shown in the terminal. Installation +must succeed and satisfy the dependency check before the build continues. +Extraneous packages alone do not trigger installation. This checks installed +dependencies, rather than whether `npm install` has ever been run; it does not +validate every transitive dependency or compare the installation against a lockfile. + +The project's `node_modules/.bin` is added to PATH during the build, so commands +such as `webpack` and `sass` can use their locally installed binaries. + +Install [Node.js and npm](https://docs.npmjs.com/downloading-and-installing-node-js-and-npm) +first. See [npm's installation documentation](https://docs.npmjs.com/cli/commands/npm-install) +for installing the packages declared by your project. Missing build commands and +installation failures include these links in the terminal. + +Projects without a `package.json` continue to use their chosen dependency manager +and installed tools without invoking npm. Features at a glance -------------------- diff --git a/src/BuildRunner.php b/src/BuildRunner.php index 2e8f3ef..1eaa7a5 100644 --- a/src/BuildRunner.php +++ b/src/BuildRunner.php @@ -2,6 +2,7 @@ namespace GT\Build; use Gt\Cli\Stream; +use Gt\Daemon\CommandNotFoundException; /** Responsible for running all build tasks and optionally watching for changes */ class BuildRunner { @@ -36,22 +37,55 @@ public function run( $workingDirectory = $this->formatWorkingDirectory(); $jsonPath = $this->getJsonPath($workingDirectory); - $startTime = microtime(true); + $previousPath = getenv("PATH"); + $previousCwd = getcwd(); + $binPath = realpath($workingDirectory) . DIRECTORY_SEPARATOR . "node_modules" + . DIRECTORY_SEPARATOR . ".bin"; + putenv("PATH=$binPath" . PATH_SEPARATOR . ($previousPath ?: "")); -// Check that the developer has all the necessary requirements. -// $errors will be passed by reference to Build::check. Passing an array by -// reference will suppress exceptions, instead filling the array with error -// strings for output back to the terminal. - $errors = []; - $build = $this->checkRequirements($jsonPath, $workingDirectory, $errors, $mode); + try { + $this->installClientSidePackages($workingDirectory); + $startTime = microtime(true); + $errors = []; + $build = $this->checkRequirements($jsonPath, $workingDirectory, $errors, $mode); - if(!empty($errors)) { - $this->showErrors($errors); - return; + if(!empty($errors)) { + $this->showErrors($errors); + $this->showInstallationHelp(); + return; + } + + $this->build($build, $continue); + $this->logElapsedTime($startTime); + } + catch(CommandNotFoundException $exception) { + $this->logMessage("Command not found: " . $exception->getMessage(), Stream::ERROR); + $this->showInstallationHelp(); } + catch(BuildException $exception) { + $this->logMessage($exception->getMessage(), Stream::ERROR); + $this->showInstallationHelp(); + } + finally { + putenv($previousPath === false ? "PATH" : "PATH=$previousPath"); + chdir($previousCwd); + } + } + + protected function installClientSidePackages(string $workingDirectory):void { + $packages = new ClientSidePackages($workingDirectory, $this->stream); + $packages->installIfNeeded(); + } - $this->build($build, $continue); - $this->logElapsedTime($startTime); + protected function showInstallationHelp():void { + $this->stream->writeLine( + "Install Node.js and npm: https://docs.npmjs.com/downloading-and-installing-node-js-and-npm", + Stream::ERROR, + ); + $this->stream->writeLine( + "Install your project's build tools with `npm install`: https://docs.npmjs.com/cli/commands/npm-install", + Stream::ERROR, + ); } public function setDefaultPath(string $path):void { diff --git a/src/ClientSidePackages.php b/src/ClientSidePackages.php new file mode 100644 index 0000000..9580576 --- /dev/null +++ b/src/ClientSidePackages.php @@ -0,0 +1,89 @@ +workingDirectory . DIRECTORY_SEPARATOR . "package.json")) { + return; + } + + if(!$this->needsInstallation()) { + return; + } + + $this->stream->writeLine( + "Client-side packages are missing or do not match package.json, running `npm install`..." + ); + [$exitCode] = $this->runCommand(["npm", "install", "--include=dev"], true); + if($exitCode !== 0 || $this->needsInstallation()) { + throw new BuildException( + "Client-side package installation failed. Run `npm install` in $this->workingDirectory." + ); + } + } + + /** @phpstan-impure */ + protected function needsInstallation():bool { + [$exitCode, $output, $error] = $this->runCommand([ + "npm", "ls", "--json", "--depth=0", "--include=dev", + ]); + $result = json_decode($output, true); + if(!is_array($result)) { + throw new BuildException("Unable to check client-side packages: $error$output"); + } + + foreach($result["problems"] ?? [] as $problem) { + if(str_starts_with($problem, "missing:") || str_starts_with($problem, "invalid:")) { + return true; + } + } + +// Extraneous packages do not require an install. Other npm errors must be +// reported rather than assuming an install will fix them. + if($exitCode !== 0 && empty($result["problems"])) { + throw new BuildException("Unable to check client-side packages: $error$output"); + } + return false; + } + + /** + * @param array $command + * @return array{int|null, string, string} + */ + protected function runCommand(array $command, bool $showOutput = false):array { + $process = new Process(...$command); + $process->setExecCwd($this->workingDirectory); + $process->exec(); + $output = ""; + $error = ""; + do { + $stdout = $process->getOutput(); + $stderr = $process->getErrorOutput(); + $output .= $stdout; + $error .= $stderr; + if($showOutput) { + $this->stream->write($stdout); + $this->stream->write($stderr, Stream::ERROR); + } + usleep(10000); + } + while($process->isRunning()); + // Drain output written between the last read and process exit. + $stdout = $process->getOutput(); + $stderr = $process->getErrorOutput(); + if($showOutput) { + $this->stream->write($stdout); + $this->stream->write($stderr, Stream::ERROR); + } + return [$process->getExitCode(), $output . $stdout, $error . $stderr]; + } +} diff --git a/test/phpunit/BuildRunnerTest.php b/test/phpunit/BuildRunnerTest.php index 06bfc78..6790942 100644 --- a/test/phpunit/BuildRunnerTest.php +++ b/test/phpunit/BuildRunnerTest.php @@ -68,6 +68,95 @@ public function testFormatWorkingDirectoryNormalisesFilePath():void { self::assertSame($workingDirectory, $sut->exposedFormatWorkingDirectory()); } + public function testMissingCommandShowsDocumentationAndRestoresEnvironment():void { + $workingDirectory = $this->createTemporaryDirectory(); + $this->createTemporaryFile($workingDirectory . "/source.js", ""); + $this->createTemporaryFile($workingDirectory . "/build.ini", "[*.js]\nexecute=phpgt-missing-build-command unused\n"); + $stream = $this->createStream(); + $runner = new \GT\Build\BuildRunner($workingDirectory, $stream); + $previousCwd = getcwd(); + $previousPath = getenv("PATH"); + + $runner->run(false); + + self::assertSame($previousCwd, getcwd()); + self::assertSame($previousPath, getenv("PATH")); + $stream->getErrorStream()->rewind(); + $output = $stream->getErrorStream()->fread(4096); + self::assertStringContainsString("Command not found: phpgt-missing-build-command", $output); + self::assertStringContainsString("https://docs.npmjs.com/cli/commands/npm-install", $output); + } + + public function testLocalBinaryIsAvailableToRequirementsAndBuild():void { + $workingDirectory = $this->createTemporaryDirectory(); + foreach(["/node_modules", "/node_modules/.bin"] as $directory) { + mkdir($workingDirectory . $directory); + $this->temporaryPaths []= $workingDirectory . $directory; + } + $command = $workingDirectory . "/node_modules/.bin/phpgt-local-tool"; + $this->createTemporaryFile($command, "#!" . PHP_BINARY . "\ncreateTemporaryFile($workingDirectory . "/source.js", ""); + $this->createTemporaryFile($workingDirectory . "/build.ini", + "[*.js]\nexecute=phpgt-local-tool unused\nrequire=phpgt-local-tool ^1.0\n"); + $stream = $this->createStream(); + $runner = new \GT\Build\BuildRunner($workingDirectory, $stream); + $previousPath = getenv("PATH"); + + $runner->run(false); + + self::assertSame($previousPath, getenv("PATH")); + $stream->getOutStream()->rewind(); + self::assertStringContainsString("Success:", $stream->getOutStream()->fread(4096)); + $stream->getErrorStream()->rewind(); + self::assertSame("", $stream->getErrorStream()->fread(4096)); + } + + public function testInstallationFailurePreventsBuildAndShowsHelp():void { + $workingDirectory = $this->createTemporaryDirectory(); + $this->createTemporaryFile($workingDirectory . "/build.ini", ""); + $stream = $this->createStream(); + $runner = new class($workingDirectory, $stream) extends \GT\Build\BuildRunner { + protected function installClientSidePackages(string $workingDirectory):void { + throw new BuildException("Client-side package installation failed"); + } + protected function build(\GT\Build\Build $build, bool $continue = true):void { + throw new \LogicException("Build must not start after a failed install"); + } + }; + $runner->run(false); + $stream->getErrorStream()->rewind(); + $output = $stream->getErrorStream()->fread(4096); + self::assertStringContainsString("Client-side package installation failed", $output); + self::assertStringContainsString("https://docs.npmjs.com/cli/commands/npm-install", $output); + } + + public function testMissingNpmShowsNodeInstallationHelp():void { + $workingDirectory = $this->createTemporaryDirectory(); + $this->createTemporaryFile($workingDirectory . "/build.ini", ""); + $this->createTemporaryFile($workingDirectory . "/package.json", "{}"); + $stream = $this->createStream(); + $runner = new \GT\Build\BuildRunner($workingDirectory, $stream); + $previousPath = getenv("PATH"); + putenv("PATH=" . $workingDirectory); + try { + $runner->run(false); + self::assertSame($workingDirectory, getenv("PATH")); + } + finally { + putenv($previousPath === false ? "PATH" : "PATH=$previousPath"); + } + $stream->getErrorStream()->rewind(); + $output = $stream->getErrorStream()->fread(4096); + self::assertStringContainsString("Command not found: npm", $output); + self::assertStringContainsString("https://docs.npmjs.com/downloading-and-installing-node-js-and-npm", $output); + } + + private function createTemporaryFile(string $path, string $content):void { + file_put_contents($path, $content); + $this->temporaryPaths []= $path; + } + private function createTemporaryDirectory():string { $path = sys_get_temp_dir() . DIRECTORY_SEPARATOR . uniqid("phpgt-build-", true); mkdir($path); diff --git a/test/phpunit/ClientSidePackagesTest.php b/test/phpunit/ClientSidePackagesTest.php new file mode 100644 index 0000000..e541304 --- /dev/null +++ b/test/phpunit/ClientSidePackagesTest.php @@ -0,0 +1,109 @@ +directory = sys_get_temp_dir() . '/' . uniqid('phpgt-packages-', true); + mkdir($this->directory); + file_put_contents($this->directory . '/package.json', '{}'); + } + + protected function tearDown():void { + if(is_file($this->directory . '/package.json')) { + unlink($this->directory . '/package.json'); + } + rmdir($this->directory); + } + + public function testProjectWithoutPackageJsonDoesNotRunNpm():void { + unlink($this->directory . '/package.json'); + $sut = $this->createPackages([]); + $sut->installIfNeeded(); + self::assertSame([], $sut->commands); + } + + public function testMatchingDependenciesDoNotRunInstall():void { + $sut = $this->createPackages([[0, '{}', '']]); + $sut->installIfNeeded(); + self::assertCount(1, $sut->commands); + self::assertSame(['npm', 'ls', '--json', '--depth=0', '--include=dev'], $sut->commands[0]); + } + + public function testMissingDependenciesAreInstalledAndRechecked():void { + $this->assertDependenciesInstalled('missing: webpack@^5.0.0'); + } + + public function testInvalidDependenciesAreInstalledAndRechecked():void { + $this->assertDependenciesInstalled('invalid: webpack@4.0.0'); + } + + public function testExtraneousDependenciesDoNotTriggerInstallation():void { + $sut = $this->createPackages([[1, '{"problems":["extraneous: other@1.0.0"]}', '']]); + $sut->installIfNeeded(); + self::assertCount(1, $sut->commands); + } + + public function testFailedInstallStopsBeforeBuild():void { + $sut = $this->createPackages([ + [1, '{"problems":["missing: webpack@^5"]}', ''], + [1, '', 'Network error'], + ]); + $this->expectException(BuildException::class); + $this->expectExceptionMessage('Client-side package installation failed'); + $sut->installIfNeeded(); + } + + public function testUnresolvedDependenciesAfterInstallAreReported():void { + $sut = $this->createPackages([ + [1, '{"problems":["invalid: webpack@4"]}', ''], + [0, '', ''], + [1, '{"problems":["invalid: webpack@4"]}', ''], + ]); + $this->expectException(BuildException::class); + $sut->installIfNeeded(); + } + + public function testNpmErrorsAreReportedWithoutInstalling():void { + $sut = $this->createPackages([[1, '{"error":{"code":"EJSONPARSE"}}', 'Invalid package.json']]); + $this->expectException(BuildException::class); + $this->expectExceptionMessage('Invalid package.json'); + $sut->installIfNeeded(); + } + + private function assertDependenciesInstalled(string $problem):void { + $stream = new Stream('php://memory', 'php://memory', 'php://memory'); + $sut = $this->createPackages([ + [1, json_encode(['problems' => [$problem]]), ''], + [0, 'Installed', ''], + [0, '{}', ''], + ], $stream); + $sut->installIfNeeded(); + self::assertCount(3, $sut->commands); + self::assertSame(['npm', 'install', '--include=dev'], $sut->commands[1]); + $stream->getOutStream()->rewind(); + self::assertStringContainsString('running `npm install`...', $stream->getOutStream()->fread(4096)); + } + + private function createPackages(array $responses, ?Stream $stream = null):object { + $stream ??= new Stream('php://memory', 'php://memory', 'php://memory'); + return new class($this->directory, $stream, $responses) extends ClientSidePackages { + public array $commands = []; + + public function __construct(string $path, Stream $stream, private array $responses) { + parent::__construct($path, $stream); + } + + protected function runCommand(array $command, bool $showOutput = false):array { + $this->commands []= $command; + return array_shift($this->responses); + } + }; + } +}