Skip to content

Commit 12cfdfb

Browse files
WendellAdrielWendellAdriel
committed
Harden the package runner and stream execution output
- Run subprocesses over reopened stdio so exit codes survive any test harness - Stream Composer output live instead of buffering and discarding it - Thread console output through Package and drop the printColor buffering - Validate package targets against Composer's official name grammar
1 parent 3809add commit 12cfdfb

13 files changed

Lines changed: 141 additions & 134 deletions

‎src/Cache/Metadata.php‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99

1010
class Metadata
1111
{
12+
private const FILE = '.cpx_metadata.json';
13+
1214
/**
1315
* @param array<string, PackageMetadata> $packages
1416
* @param array<string, array{packages?: list<string>, last_updated?: int, last_run?: int}> $execCache
@@ -20,7 +22,7 @@ protected function __construct(
2022

2123
public static function open(): self
2224
{
23-
$metadataFile = cpx_path('.cpx_metadata.json');
25+
$metadataFile = cpx_path(self::FILE);
2426

2527
if (! file_exists($metadataFile)) {
2628
return new self;
@@ -64,7 +66,7 @@ public function recordUpdate(Package $package): self
6466

6567
public function save(): void
6668
{
67-
$metadataFile = cpx_path('.cpx_metadata.json');
69+
$metadataFile = cpx_path(self::FILE);
6870

6971
if (! is_dir(dirname($metadataFile))) {
7072
mkdir(dirname($metadataFile), 0755, true);

‎src/Commands/RunPackageCommand.php‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -44,20 +44,12 @@ protected function execute(InputInterface $input, OutputInterface $output): int
4444
array_shift($tokens);
4545
}
4646

47-
ob_start();
48-
4947
try {
5048
return $this->packageCommandRunner->run(PackageInvocation::fromRawTokens($tokens), $output);
5149
} catch (InvalidArgumentException $e) {
5250
$output->writeln("<error>{$e->getMessage()}</error>");
5351

5452
return SymfonyCommand::FAILURE;
55-
} finally {
56-
$contents = ob_get_clean();
57-
58-
if ($contents !== false) {
59-
$output->write($contents);
60-
}
6153
}
6254
}
6355
}

‎src/Commands/TinkerCommand.php‎

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -27,17 +27,9 @@ protected function execute(InputInterface $input, OutputInterface $output): int
2727
return self::FAILURE;
2828
}
2929

30-
ob_start();
31-
32-
try {
33-
return Package::parse('psy/psysh')->runCommand(PackageInvocation::fromRawTokens(['psysh', '--config', $psyshConfig]));
34-
} finally {
35-
$contents = ob_get_clean();
36-
37-
if ($contents !== false) {
38-
$output->write($contents);
39-
}
40-
}
41-
30+
return Package::parse('psy/psysh')->runCommand(
31+
PackageInvocation::fromRawTokens(['psysh', '--config', $psyshConfig]),
32+
$output,
33+
);
4234
}
4335
}

‎src/Composer/ComposerRunner.php‎

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,27 +14,22 @@ class ComposerRunner
1414

1515
/**
1616
* @param list<string> $arguments
17-
* @return list<string>
1817
*/
19-
public static function run(array $arguments, ?string $directory = null): array
18+
public static function run(array $arguments, ?string $directory = null): int
2019
{
21-
$command = ['composer', ...$arguments, '--no-interaction', '--quiet'];
20+
$command = ['composer', ...$arguments, '--no-interaction'];
2221

2322
if ($directory !== null) {
2423
$command[] = "--working-dir={$directory}";
2524
}
2625

27-
$result = (new ProcessRunner)->capture($command);
26+
$exitCode = (new ProcessRunner)->run($command);
2827

29-
if ($result['exitCode'] !== Command::SUCCESS) {
30-
$message = trim($result['stderr']) ?: 'Composer command failed: '.implode(' ', $arguments);
31-
32-
throw new Exception($message);
28+
if ($exitCode !== Command::SUCCESS) {
29+
throw new Exception('Composer command failed: '.implode(' ', $arguments));
3330
}
3431

35-
$output = trim($result['stdout']);
36-
37-
return $output === '' ? [] : explode(PHP_EOL, $output);
32+
return $exitCode;
3833
}
3934

4035
/** @return list<string> */

‎src/Input/PackageInvocation.php‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@
88

99
class PackageInvocation
1010
{
11+
/** @var array<string, string|null|list<string|null>>|null */
12+
private ?array $parsedOptions = null;
13+
1114
/**
1215
* @param list<string> $forwardedTokens
1316
*/
@@ -51,6 +54,10 @@ public function hasOption(string $option): bool
5154
return array_key_exists($option, $this->options());
5255
}
5356

57+
/**
58+
* Returns a single value for the option. When an option is repeated
59+
* (e.g. --filter=one --filter=two) the first non-null value wins.
60+
*/
5461
public function option(string $option): ?string
5562
{
5663
$value = $this->options()[$option] ?? null;
@@ -71,6 +78,10 @@ public function option(string $option): ?string
7178
/** @return array<string, string|null|list<string|null>> */
7279
private function options(): array
7380
{
81+
if ($this->parsedOptions !== null) {
82+
return $this->parsedOptions;
83+
}
84+
7485
$options = [];
7586
$tokens = $this->forwardedTokens;
7687

@@ -95,7 +106,7 @@ private function options(): array
95106
$this->addOption($options, $matches['name'], $value);
96107
}
97108

98-
return $options;
109+
return $this->parsedOptions = $options;
99110
}
100111

101112
/**

‎src/Packages/Package.php‎

Lines changed: 49 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,17 @@
1212
use Cpx\Support\Filesystem;
1313
use InvalidArgumentException;
1414
use Symfony\Component\Console\Command\Command;
15+
use Symfony\Component\Console\Output\OutputInterface;
1516

1617
class Package
1718
{
19+
/**
20+
* Composer's official package-name grammar (vendor/name), extended with an optional ":version" constraint.
21+
*/
22+
private const PACKAGE_PATTERN = '/\A(?<vendor>[a-z0-9](?:[_.-]?[a-z0-9]+)*)\/(?<name>[a-z0-9](?:(?:[_.]?|-{0,2})[a-z0-9]+)*)(?::(?<version>(?![.]+\z)[a-zA-Z0-9_.@~^*<>!=|,-]+))?\z/';
23+
24+
private const SCAFFOLD_VERSION = '1.0.0';
25+
1826
protected function __construct(
1927
public string $vendor,
2028
public string $name,
@@ -32,7 +40,7 @@ public static function parse(string $str): self
3240
throw new InvalidArgumentException('A package name must be provided.');
3341
}
3442

35-
if (preg_match('/\A(?<vendor>[a-z0-9](?:[a-z0-9_.-]*[a-z0-9])?)\/(?<name>[a-z0-9](?:[a-z0-9_.-]*[a-z0-9])?)(?::(?<version>(?![.]+\z)[a-zA-Z0-9_.@~^*<>!=|,-]+))?\z/', $str, $matches) !== 1) {
43+
if (preg_match(self::PACKAGE_PATTERN, $str, $matches) !== 1) {
3644
throw new InvalidArgumentException('A package name should be in the format "<vendor>/<package>[:version]".');
3745
}
3846

@@ -64,14 +72,14 @@ public function delete(): void
6472
Filesystem::deleteDirectory(cpx_path($this->folder()));
6573
}
6674

67-
public function runCommand(PackageInvocation $invocation, bool $autoUpdate = true): int
75+
public function runCommand(PackageInvocation $invocation, OutputInterface $output, bool $autoUpdate = true): int
6876
{
69-
$installDir = $this->installOrUpdatePackage($autoUpdate);
77+
$installDir = $this->installOrUpdatePackage($output, $autoUpdate);
7078
$packageDir = "{$installDir}/vendor/{$this->vendor}/{$this->name}";
7179
$binScripts = ComposerRunner::detectBinFromComposer($packageDir);
7280

7381
if (empty($binScripts)) {
74-
echo "No bin command found in {$this}.".PHP_EOL;
82+
$output->writeln("<error>No bin command found in {$this}.</error>");
7583

7684
return Command::FAILURE;
7785
}
@@ -80,27 +88,26 @@ public function runCommand(PackageInvocation $invocation, bool $autoUpdate = tru
8088
$resolved = $this->resolveBinCommand($binScripts, $invocation);
8189

8290
if ($resolved === null) {
83-
echo "More than 1 bin command found for {$this}: ".implode(', ', array_keys($binScripts)).'.'.PHP_EOL;
91+
$output->writeln("<error>More than 1 bin command found for {$this}: ".implode(', ', array_keys($binScripts)).'.</error>');
8492

8593
return Command::FAILURE;
8694
}
8795

88-
[$command, $invocation] = $resolved;
89-
$binPath = "{$packageDir}/{$command}";
96+
$binPath = "{$packageDir}/{$resolved->command}";
9097

9198
if (! file_exists($binPath)) {
92-
echo 'Command '.basename($command)." not found in {$this}.".PHP_EOL;
99+
$output->writeln('<error>Command '.basename($resolved->command)." not found in {$this}.</error>");
93100

94101
return Command::FAILURE;
95102
}
96103

97104
Metadata::open()->recordRun($this)->save();
98-
printColor('Running '.basename($command)." from {$this}");
105+
$output->writeln('<info>Running '.basename($resolved->command)." from {$this}</info>");
99106

100-
return (new ProcessRunner)->run([$binPath, ...$invocation->forwardedTokens()]);
107+
return (new ProcessRunner)->run([$binPath, ...$resolved->invocation->forwardedTokens()]);
101108
}
102109

103-
public function installOrUpdatePackage(bool $updateCheck = true): string
110+
public function installOrUpdatePackage(OutputInterface $output, bool $updateCheck = true): string
104111
{
105112
$installDir = cpx_path($this->folder());
106113

@@ -109,9 +116,9 @@ public function installOrUpdatePackage(bool $updateCheck = true): string
109116
}
110117

111118
match (true) {
112-
! is_dir("{$installDir}/vendor") => $this->installPackage($installDir),
113-
$updateCheck && $this->shouldCheckForUpdates() => $this->updatePackage($installDir),
114-
default => printColor("{$this} is already installed and doesn't need updating."),
119+
! is_dir("{$installDir}/vendor") => $this->installPackage($output, $installDir),
120+
$updateCheck && $this->shouldCheckForUpdates() => $this->updatePackage($output, $installDir),
121+
default => $output->writeln("<info>{$this} is already installed and doesn't need updating.</info>"),
115122
};
116123

117124
return $installDir;
@@ -139,46 +146,54 @@ public function shouldCheckForUpdates(): bool
139146

140147
/**
141148
* @param array<string, string> $binScripts
142-
* @return array{0: string, 1: PackageInvocation}|null
143149
*/
144-
private function resolveBinCommand(array $binScripts, PackageInvocation $invocation): ?array
150+
private function resolveBinCommand(array $binScripts, PackageInvocation $invocation): ?ResolvedBin
145151
{
146152
if (count($binScripts) === 1) {
147-
return [$binScripts[array_key_first($binScripts)], $invocation];
153+
return new ResolvedBin($binScripts[array_key_first($binScripts)], $invocation);
148154
}
149155

150-
$possibleCommands = array_values(array_unique(array_filter([
156+
$candidates = array_values(array_unique(array_filter([
151157
$invocation->target,
152158
$invocation->firstForwardedToken(),
153159
$this->name,
154160
])));
155161

156-
foreach ($possibleCommands as $possibleCommand) {
157-
$command = $binScripts[$possibleCommand] ?? null;
158-
159-
if ($command === null && in_array($possibleCommand, $binScripts, true)) {
160-
$command = $possibleCommand;
161-
}
162+
foreach ($candidates as $candidate) {
163+
$command = $this->matchBin($binScripts, $candidate);
162164

163165
if ($command === null) {
164166
continue;
165167
}
166168

167-
return $invocation->firstForwardedToken() === $possibleCommand
168-
? [$command, $invocation->withoutFirstForwardedToken()]
169-
: [$command, $invocation];
169+
return $invocation->firstForwardedToken() === $candidate
170+
? new ResolvedBin($command, $invocation->withoutFirstForwardedToken())
171+
: new ResolvedBin($command, $invocation);
170172
}
171173

172174
return null;
173175
}
174176

175-
private function installPackage(string $installDir): void
177+
/**
178+
* @param array<string, string> $binScripts
179+
*/
180+
private function matchBin(array $binScripts, string $candidate): ?string
181+
{
182+
if (array_key_exists($candidate, $binScripts)) {
183+
return $binScripts[$candidate];
184+
}
185+
186+
return in_array($candidate, $binScripts, true) ? $candidate : null;
187+
}
188+
189+
private function installPackage(OutputInterface $output, string $installDir): void
176190
{
177-
printColor("Installing {$this}...");
191+
$output->writeln("<info>Installing {$this}...</info>");
178192
file_put_contents("{$installDir}/composer.json", json_encode([
179193
'name' => "cpx-{$this->vendor}/cpx-{$this->name}",
180-
'version' => '1.0.0',
194+
'version' => self::SCAFFOLD_VERSION,
181195
'config' => [
196+
// Requested packages may ship Composer plugins (binaries, installers).
182197
'allow-plugins' => true,
183198
],
184199
]));
@@ -187,17 +202,17 @@ private function installPackage(string $installDir): void
187202
Metadata::open()->recordUpdate($this)->save();
188203
}
189204

190-
private function updatePackage(string $installDir): void
205+
private function updatePackage(OutputInterface $output, string $installDir): void
191206
{
192-
printColor("Checking for updates for {$this}...");
207+
$output->writeln("<info>Checking for updates for {$this}...</info>");
193208
$previousVersion = ComposerRunner::getCurrentVersion($installDir);
194209
ComposerRunner::run(['update'], $installDir);
195210
$newVersion = ComposerRunner::getCurrentVersion($installDir);
196211

197212
if ($previousVersion !== $newVersion) {
198-
printColor("{$this} was upgraded from {$previousVersion} to {$newVersion}.");
213+
$output->writeln("<info>{$this} was upgraded from {$previousVersion} to {$newVersion}.</info>");
199214
} else {
200-
printColor("{$this} is already up-to-date.");
215+
$output->writeln("<info>{$this} is already up-to-date.</info>");
201216
}
202217

203218
Metadata::open()->recordUpdate($this)->save();

‎src/Packages/PackageCommandRunner.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,12 @@ public function run(PackageInvocation $invocation, OutputInterface $output): int
2323
$aliases = PackageAliases::all();
2424

2525
if (array_key_exists($invocation->target, $aliases)) {
26-
return Package::parse($aliases[$invocation->target]->package)->runCommand($invocation);
26+
return Package::parse($aliases[$invocation->target]->package)->runCommand($invocation, $output);
2727
}
2828

2929
if (str_contains($invocation->target, '/')) {
3030
try {
31-
return Package::parse($invocation->target)->runCommand($invocation);
31+
return Package::parse($invocation->target)->runCommand($invocation, $output);
3232
} catch (InvalidArgumentException) {
3333
HelpCommand::render($output, $invocation->target);
3434

‎src/Packages/ResolvedBin.php‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Cpx\Packages;
6+
7+
use Cpx\Input\PackageInvocation;
8+
9+
readonly class ResolvedBin
10+
{
11+
public function __construct(
12+
public string $command,
13+
public PackageInvocation $invocation,
14+
) {}
15+
}

0 commit comments

Comments
 (0)