From 1ebd23ca802096f357a3817a36fc8bdd607623c1 Mon Sep 17 00:00:00 2001 From: Ralf Lang Date: Wed, 24 Jun 2026 17:01:22 +0200 Subject: [PATCH 1/5] fix(release): use a 60s HTTP timeout for release asset uploads --- src/Components.php | 8 +++++++- src/Helper/GitHubReleaseCreator.php | 18 ++++++++++++++---- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/Components.php b/src/Components.php index 5093eb20..ead17826 100644 --- a/src/Components.php +++ b/src/Components.php @@ -285,7 +285,13 @@ protected static function _prepareModular( $injector->setInstance(StreamFactoryInterface::class, $streamFactory); $injector->setInstance(RequestFactoryInterface::class, $requestFactory); - $injector->setInstance(ClientInterface::class, new CurlClient($responseFactory, $streamFactory, new Options())); + // Build the HTTP Options as a shared instance so subsystems that + // need a longer per-call timeout (release-asset upload of a 4 MB + // phar in particular) can mutate it via setOption() rather than + // having to construct a parallel CurlClient. + $httpOptions = new Options(); + $injector->setInstance(Options::class, $httpOptions); + $injector->setInstance(ClientInterface::class, new CurlClient($responseFactory, $streamFactory, $httpOptions)); // Get GitHub token from ConfigProvider hierarchy // Precedence: CLI args > GITHUB_TOKEN env var > github.token config key diff --git a/src/Helper/GitHubReleaseCreator.php b/src/Helper/GitHubReleaseCreator.php index b4a7b695..cbbd0539 100644 --- a/src/Helper/GitHubReleaseCreator.php +++ b/src/Helper/GitHubReleaseCreator.php @@ -133,8 +133,15 @@ public function uploadPharAsset( } try { - // Initialize GitHub API client - $httpClient = new CurlClient(new ResponseFactory(), new StreamFactory(), new Options()); + // Initialize GitHub API client with a longer timeout. The + // default 5s in Options is fine for most API calls but too + // short for release-asset uploads: a 4 MB phar over + // residential broadband easily blows past 5s on the response + // read even when the upload itself completes. Bump to 60s + // for the asset upload path only. + $uploadOptions = new Options(); + $uploadOptions->setOption('timeout', 60); + $httpClient = new CurlClient(new ResponseFactory(), new StreamFactory(), $uploadOptions); $requestFactory = new RequestFactory(); $streamFactory = new StreamFactory(); $config = new GithubApiConfig(accessToken: $githubToken); @@ -342,8 +349,11 @@ public function uploadAsset( throw new Exception('GitHub token not configured'); } - // Initialize GitHub API client - $httpClient = new CurlClient(new ResponseFactory(), new StreamFactory(), new Options()); + // Initialize GitHub API client with a longer timeout (see the + // PHAR-upload path above for the rationale). + $uploadOptions = new Options(); + $uploadOptions->setOption('timeout', 60); + $httpClient = new CurlClient(new ResponseFactory(), new StreamFactory(), $uploadOptions); $requestFactory = new RequestFactory(); $streamFactory = new StreamFactory(); $config = new GithubApiConfig(accessToken: $githubToken); From e248ac1d80dc7f75a234d24961870e0d03d2081c Mon Sep 17 00:00:00 2001 From: Ralf Lang Date: Wed, 24 Jun 2026 17:01:22 +0200 Subject: [PATCH 2/5] fix(ci): ci init exits 0 when files exist and --force is not given --- src/Ci/Init/InitCommand.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Ci/Init/InitCommand.php b/src/Ci/Init/InitCommand.php index 89a9e734..bb28ddee 100644 --- a/src/Ci/Init/InitCommand.php +++ b/src/Ci/Init/InitCommand.php @@ -93,7 +93,11 @@ public function execute( } $this->output->plain(''); $this->output->warn('Use --force to overwrite existing files.'); - return false; + // The command ran and decided "no-op, hint to use --force". + // That is a user-recoverable outcome, not a failure: return + // true so Module/Ci doesn't translate it into a fatal-error + // exception via the W1 convention. + return true; } // Generate files From 907c3eb23fbd3de817dbd727ad9a4349e7b4a636 Mon Sep 17 00:00:00 2001 From: Ralf Lang Date: Wed, 24 Jun 2026 19:28:49 +0200 Subject: [PATCH 3/5] feat: Implement CI bootstrap file rewrite during release --- src/Release/HordeRelease.php | 8 + src/Task/Release/RefreshCiBootstrapTask.php | 289 +++++++++++++++++ .../Release/RefreshCiBootstrapTaskTest.php | 290 ++++++++++++++++++ 3 files changed, 587 insertions(+) create mode 100644 src/Task/Release/RefreshCiBootstrapTask.php create mode 100644 test/unit/Task/Release/RefreshCiBootstrapTaskTest.php diff --git a/src/Release/HordeRelease.php b/src/Release/HordeRelease.php index 91f9b9d9..82561e97 100644 --- a/src/Release/HordeRelease.php +++ b/src/Release/HordeRelease.php @@ -20,6 +20,7 @@ use Horde\Components\Task\Release\UpdateComposerJsonTask; use Horde\Components\Task\Release\UpdateApplicationSentinelTask; use Horde\Components\Task\Release\CleanupLegacyFilesTask; +use Horde\Components\Task\Release\RefreshCiBootstrapTask; use Horde\Components\Task\Composer\ComposerValidateTask; use Horde\Components\Task\Git\CommitTask; use Horde\Components\Task\Git\TagTask; @@ -107,6 +108,13 @@ public function run(Component $component, array $options = []) // Clean up legacy files $this->runTask(new CleanupLegacyFilesTask($this->output, $this->gitHelper, $this->pretend), $context); + // Refresh CI bootstrap files if their template version is outdated. + // The bootstrap script and workflow embed a generation timestamp, + // so they always textually diff against the templates; the + // embedded `# Template version:` marker is the only meaningful + // signal of whether they need rewriting. + $this->runTask(new RefreshCiBootstrapTask($this->output, null, $this->pretend), $context); + // Validate composer.json $this->runTask(new ComposerValidateTask($this->output, $this->shellHelper, $this->pretend), $context); diff --git a/src/Task/Release/RefreshCiBootstrapTask.php b/src/Task/Release/RefreshCiBootstrapTask.php new file mode 100644 index 00000000..ba2749b9 --- /dev/null +++ b/src/Task/Release/RefreshCiBootstrapTask.php @@ -0,0 +1,289 @@ + + * @category Horde + * @license http://www.horde.org/licenses/lgpl21 LGPL 2.1 + * @package Components + */ + +declare(strict_types=1); + +namespace Horde\Components\Task\Release; + +use Horde\Components\Ci\Template\TemplateLocator; +use Horde\Components\Ci\Template\TemplateRenderer; +use Horde\Components\Ci\Template\TemplateVersion; +use Horde\Components\Output; +use Horde\Components\Task\AbstractTask; +use Horde\Components\Task\Context; +use Horde\Components\Task\Result; + +/** + * Refresh CI bootstrap script and GitHub Actions workflow when outdated. + * + * The generated CI files (bin/ci-bootstrap.sh and .github/workflows/ci.yml) + * are produced from versioned templates that embed a `# Template version:` + * marker. They also embed a generation timestamp, so a plain diff is + * never empty even when the substantive content has not changed. To + * decide whether to refresh, this task ignores the file contents and + * compares the embedded template version against the version baked into + * {@see TemplateRenderer}. + * + * Files are only refreshed when they already exist locally — bootstrapping + * a component for the first time is the job of `horde-components ci init`, + * not the release pipeline. Refreshed files are appended to the `files` + * commit list so they ride along in the release commit. + * + * Emitted Facts: + * - ci.refreshed (array) — relative paths of files that were refreshed + * - ci.refreshed_count (int) — number of files refreshed + * + * @author Ralf Lang + * @category Horde + * @copyright 2024-2026 Horde LLC + * @license http://www.horde.org/licenses/lgpl21 LGPL 2.1 + * @package Components + */ +class RefreshCiBootstrapTask extends AbstractTask +{ + /** + * Default targets. The bootstrap script's template is resolved at run + * time per file because GitHub and local modes use different scripts + * but share an outcome path; the workflow only exists in GitHub mode. + */ + private const TARGETS = [ + 'bin/ci-bootstrap.sh' => null, + '.github/workflows/ci.yml' => 'workflow.yml', + ]; + + public function __construct( + Output $output, + private readonly ?TemplateRenderer $renderer = null, + bool $pretend = false, + ) { + parent::__construct($output, $pretend); + } + + public function getName(): string + { + return 'Refresh CI bootstrap'; + } + + public function run(Context $context): Result + { + $componentPath = $context->getComponentPath(); + $renderer = $this->renderer ?? new TemplateRenderer(TemplateLocator::getTemplateDir()); + + $refreshed = []; + $skippedMissing = []; + $skippedCurrent = []; + + foreach (self::TARGETS as $relativePath => $templateName) { + $absolutePath = $componentPath . '/' . $relativePath; + + // Only refresh files that are already present. The release + // pipeline must not silently introduce CI configuration to + // a component that never had any — that is `ci init`'s job. + if (!file_exists($absolutePath)) { + $skippedMissing[] = $relativePath; + continue; + } + + // The bootstrap script has two flavours; resolve the template + // name from the existing file's mode marker so a local-mode + // component does not get a GitHub-mode rewrite. + if ($templateName === null) { + $templateName = $this->detectBootstrapTemplate($absolutePath); + if ($templateName === null) { + $this->output->warn(sprintf( + 'Cannot determine CI mode for %s; skipping refresh', + $relativePath, + )); + $skippedMissing[] = $relativePath; + continue; + } + } + + if (!TemplateVersion::isOutdated($absolutePath)) { + $comparison = TemplateVersion::compare($absolutePath); + $skippedCurrent[] = sprintf( + '%s (v%s)', + $relativePath, + $comparison['file'] ?? 'unknown', + ); + continue; + } + + $comparison = TemplateVersion::compare($absolutePath); + $fileVersion = $comparison['file'] ?? 'unknown'; + $currentVersion = $comparison['current']; + + if ($this->pretend) { + $this->output->info(sprintf( + '[PRETEND] Would refresh %s (v%s -> v%s)', + $relativePath, + $fileVersion, + $currentVersion, + )); + $refreshed[] = $relativePath; + continue; + } + + $config = $this->buildConfig($relativePath, $componentPath, $templateName); + $content = $renderer->render($templateName, $config); + + $dir = dirname($absolutePath); + if (!is_dir($dir) && !mkdir($dir, 0o755, true) && !is_dir($dir)) { + return Result::failure(sprintf( + 'Failed to create directory for %s', + $relativePath, + )); + } + + if (file_put_contents($absolutePath, $content) === false) { + return Result::failure(sprintf( + 'Failed to write %s', + $relativePath, + )); + } + + // Bootstrap script must remain executable. + if (str_ends_with($relativePath, '.sh')) { + @chmod($absolutePath, 0o755); + } + + $this->output->info(sprintf( + 'Refreshed %s (v%s -> v%s)', + $relativePath, + $fileVersion, + $currentVersion, + )); + + $refreshed[] = $relativePath; + } + + // Make refreshed files part of the release commit. CommitTask reads + // the 'files' option as the staging list — see Task/Git/CommitTask. + if (!empty($refreshed) && !$this->pretend) { + $existing = $context->getOption('files'); + $files = is_array($existing) ? $existing : []; + foreach ($refreshed as $path) { + if (!in_array($path, $files, true)) { + $files[] = $path; + } + } + $context->setOption('files', $files); + } + + $context->setFact('ci.refreshed', $refreshed); + $context->setFact('ci.refreshed_count', count($refreshed)); + + if (empty($refreshed)) { + if (!empty($skippedCurrent)) { + $message = 'CI bootstrap up to date: ' . implode(', ', $skippedCurrent); + } elseif (!empty($skippedMissing)) { + $message = 'No CI bootstrap files present; nothing to refresh'; + } else { + $message = 'No CI bootstrap targets configured'; + } + + return Result::success($message, [ + 'refreshed' => [], + 'skipped_current' => $skippedCurrent, + 'skipped_missing' => $skippedMissing, + ]); + } + + $action = $this->pretend ? 'Would refresh' : 'Refreshed'; + + return Result::success( + sprintf('%s CI bootstrap files: %s', $action, implode(', ', $refreshed)), + [ + 'refreshed' => $refreshed, + 'skipped_current' => $skippedCurrent, + 'skipped_missing' => $skippedMissing, + ], + ); + } + + /** + * Inspect an existing bootstrap script to determine which template + * generated it. + * + * The first two lines of each bootstrap template carry a distinctive + * "(GitHub Actions)" or "(Local Development)" marker in their header + * comment. We use that rather than guessing from the presence of the + * workflow file because a refresh must produce a file of the same + * flavour, even mid-flight. + */ + private function detectBootstrapTemplate(string $absolutePath): ?string + { + $content = file_get_contents($absolutePath); + if ($content === false) { + return null; + } + + // Read only the header — the marker is on line 2 of every template. + $head = substr($content, 0, 512); + + if (str_contains($head, '(GitHub Actions)')) { + return 'bootstrap-github.sh'; + } + + if (str_contains($head, '(Local Development)')) { + return 'bootstrap-local.sh'; + } + + return null; + } + + /** + * Build the template variable map for a target. + * + * Mirrors the substitutions used by `ci init`'s InitCommand so that a + * refresh produces a file equivalent to a fresh init (modulo the + * timestamp). Component-specific values are derived from the component + * directory; the PHAR URL falls back to the same env-var-or-latest + * default that InitCommand uses. + * + * @return array + */ + private function buildConfig(string $relativePath, string $componentPath, string $templateName): array + { + $componentName = basename(realpath($componentPath) ?: $componentPath); + + $config = [ + '{{COMPONENT_NAME}}' => $componentName, + '{{WORK_DIR}}' => '/tmp/horde-ci', + ]; + + // Local-mode bootstrap references LOCAL_COMPONENTS_PATH / + // LOCAL_COMPONENT_PATH which are normally provided by the + // developer's environment. Preserve the env-or-placeholder + // approach InitCommand uses so a refresh stays equivalent. + if ($templateName === 'bootstrap-local.sh') { + $config['{{LOCAL_COMPONENTS_PATH}}'] = getenv('LOCAL_COMPONENTS_PATH') ?: '${LOCAL_COMPONENTS_PATH}'; + $config['{{LOCAL_COMPONENT_PATH}}'] = getenv('LOCAL_COMPONENT_PATH') ?: '${LOCAL_COMPONENT_PATH}'; + } else { + $config['{{COMPONENTS_PHAR_URL}}'] = $this->getComponentsPharUrl(); + } + + return $config; + } + + private function getComponentsPharUrl(): string + { + $url = getenv('COMPONENTS_PHAR_URL'); + if ($url !== false && $url !== '') { + return $url; + } + + return 'https://github.com/horde/components/releases/latest/download/horde-components.phar'; + } +} diff --git a/test/unit/Task/Release/RefreshCiBootstrapTaskTest.php b/test/unit/Task/Release/RefreshCiBootstrapTaskTest.php new file mode 100644 index 00000000..7dea4db0 --- /dev/null +++ b/test/unit/Task/Release/RefreshCiBootstrapTaskTest.php @@ -0,0 +1,290 @@ +tmpDir = sys_get_temp_dir() . '/horde-refresh-ci-' . uniqid(); + mkdir($this->tmpDir, 0o755, true); + } + + protected function tearDown(): void + { + $this->rmrf($this->tmpDir); + } + + public function testSkipsWhenNoBootstrapFilesPresent(): void + { + $task = new RefreshCiBootstrapTask($this->makeOutput()); + $result = $task->run($this->makeContext()); + + $this->assertTrue($result->isSuccess()); + $this->assertStringContainsString('nothing to refresh', $result->message); + } + + public function testSkipsWhenBootstrapFileIsCurrent(): void + { + $currentVersion = TemplateRenderer::getTemplateVersion(); + $this->seedBootstrap('github', $currentVersion); + + $task = new RefreshCiBootstrapTask($this->makeOutput()); + $context = $this->makeContext(); + $result = $task->run($context); + + $this->assertTrue($result->isSuccess()); + $this->assertSame([], $context->getFact('ci.refreshed')); + $this->assertSame(0, $context->getFact('ci.refreshed_count')); + $this->assertStringContainsString('up to date', $result->message); + } + + public function testRefreshesOutdatedGithubBootstrap(): void + { + $this->seedBootstrap('github', '0.0.1'); + + $task = new RefreshCiBootstrapTask($this->makeOutput()); + $context = $this->makeContext(); + $result = $task->run($context); + + $this->assertTrue($result->isSuccess()); + $this->assertSame(['bin/ci-bootstrap.sh'], $context->getFact('ci.refreshed')); + + $refreshed = file_get_contents($this->tmpDir . '/bin/ci-bootstrap.sh'); + $this->assertNotFalse($refreshed); + // Refreshed content still identifies as the GitHub flavour. + $this->assertStringContainsString('(GitHub Actions)', $refreshed); + // And carries the current template version, not the old one. + $this->assertStringContainsString( + '# Template version: ' . TemplateRenderer::getTemplateVersion(), + $refreshed, + ); + // The file is committed alongside the release. + $this->assertSame(['bin/ci-bootstrap.sh'], $context->getOption('files')); + } + + public function testRefreshesOutdatedLocalBootstrap(): void + { + $this->seedBootstrap('local', '0.0.1'); + + $task = new RefreshCiBootstrapTask($this->makeOutput()); + $context = $this->makeContext(); + $result = $task->run($context); + + $this->assertTrue($result->isSuccess()); + $this->assertSame(['bin/ci-bootstrap.sh'], $context->getFact('ci.refreshed')); + + $refreshed = file_get_contents($this->tmpDir . '/bin/ci-bootstrap.sh'); + $this->assertNotFalse($refreshed); + // Local-mode bootstrap stays local even on refresh. + $this->assertStringContainsString('(Local Development)', $refreshed); + $this->assertStringNotContainsString('(GitHub Actions)', $refreshed); + } + + public function testRefreshesOutdatedWorkflow(): void + { + $this->seedWorkflow('0.0.1'); + + $task = new RefreshCiBootstrapTask($this->makeOutput()); + $context = $this->makeContext(); + $result = $task->run($context); + + $this->assertTrue($result->isSuccess()); + $this->assertSame(['.github/workflows/ci.yml'], $context->getFact('ci.refreshed')); + + $refreshed = file_get_contents($this->tmpDir . '/.github/workflows/ci.yml'); + $this->assertNotFalse($refreshed); + $this->assertStringContainsString( + '# Template version: ' . TemplateRenderer::getTemplateVersion(), + $refreshed, + ); + } + + public function testPretendModeReportsButDoesNotWrite(): void + { + $this->seedBootstrap('github', '0.0.1'); + $original = file_get_contents($this->tmpDir . '/bin/ci-bootstrap.sh'); + + $task = new RefreshCiBootstrapTask($this->makeOutput(), null, true); + $context = $this->makeContext(); + $result = $task->run($context); + + $this->assertTrue($result->isSuccess()); + $this->assertSame(['bin/ci-bootstrap.sh'], $context->getFact('ci.refreshed')); + + // File on disk is unchanged. + $this->assertSame($original, file_get_contents($this->tmpDir . '/bin/ci-bootstrap.sh')); + // Files commit list was not appended to either, because pretend mode + // must not affect downstream task behaviour. + $this->assertNull($context->getOption('files')); + } + + public function testWarnsAndSkipsWhenBootstrapModeUndetectable(): void + { + // A bootstrap file with no recognizable mode marker. The version + // marker is older than current to make sure mode detection — not + // version comparison — is what gates the skip. + $this->writeFile('bin/ci-bootstrap.sh', "#!/bin/bash\n# Some bespoke script\n# Template version: 0.0.1\necho hi\n"); + + $task = new RefreshCiBootstrapTask($this->makeOutput()); + $context = $this->makeContext(); + $result = $task->run($context); + + $this->assertTrue($result->isSuccess()); + $this->assertSame([], $context->getFact('ci.refreshed')); + } + + public function testMissingTemplateVersionMarkerIsTreatedAsOutdated(): void + { + // A bootstrap file with the mode marker but no version line at all. + // TemplateVersion::isOutdated returns true when no marker is found, + // so the file must be refreshed. + $this->writeFile( + 'bin/ci-bootstrap.sh', + "#!/bin/bash\n# Horde CI Bootstrap Script (GitHub Actions)\necho hi\n", + ); + + $task = new RefreshCiBootstrapTask($this->makeOutput()); + $context = $this->makeContext(); + $result = $task->run($context); + + $this->assertTrue($result->isSuccess()); + $this->assertSame(['bin/ci-bootstrap.sh'], $context->getFact('ci.refreshed')); + } + + // --- Helpers --- + + private function seedBootstrap(string $mode, string $version): void + { + $banner = $mode === 'github' ? '(GitHub Actions)' : '(Local Development)'; + $this->writeFile( + 'bin/ci-bootstrap.sh', + "#!/bin/bash\n# Horde CI Bootstrap Script {$banner}\n# Template version: {$version}\necho hi\n", + ); + } + + private function seedWorkflow(string $version): void + { + $this->writeFile( + '.github/workflows/ci.yml', + "# Horde CI Workflow\n# Template version: {$version}\nname: ci\n", + ); + } + + private function writeFile(string $relativePath, string $content): void + { + $abs = $this->tmpDir . '/' . $relativePath; + $dir = dirname($abs); + if (!is_dir($dir)) { + mkdir($dir, 0o755, true); + } + file_put_contents($abs, $content); + } + + private function makeOutput(): Output + { + $output = $this->createMock(Output::class); + + return $output; + } + + private function makeContext(): Context + { + // Component is an interface that does not declare + // getComponentDirectory(), but Context::getComponentPath() + // calls it via duck typing on the concrete implementations. + // For the test we wrap the interface in an anonymous class + // that adds the method. + $tmpDir = $this->tmpDir; + $component = new class ($tmpDir) implements Component { + public function __construct(private readonly string $dir) {} + public function getComponentDirectory(): string { return $this->dir; } + public function getName(): string { return 'test'; } + public function getSummary(): string { return ''; } + public function getDescription(): string { return ''; } + public function getVersion(): string { return '0.0.0'; } + public function getPreviousVersion(): string { return '0.0.0'; } + public function getDate(): string { return ''; } + public function getChannel(): string { return ''; } + public function getDependencies(): array { return []; } + public function getState($key = 'release'): string { return 'stable'; } + public function getLeads() { return []; } + public function getLicense() { return 'LGPL'; } + public function getLicenseLocation(): string { return ''; } + public function hasLocalPackageXml(): bool { return false; } + public function getChangelogLink(): string { return ''; } + public function getReleaseNotesPath(): string|bool { return false; } + public function getDependencyList() { return null; } + public function getData(): \stdClass { return new \stdClass(); } + public function getDocumentOrigin(): ?string { return null; } + public function updatePackage($action, $options): string { return ''; } + public function changed($log, $options): array { return []; } + public function timestamp($options): string { return ''; } + public function nextVersion($version, $initial_note, $stability_api = null, $stability_release = null, $options = []) {} + public function currentSentinel($changes, $app, $options): array { return []; } + public function tag(string $tag, string $message, \Horde\Components\Helper\Commit $commit): string { return ''; } + public function placeArchive(string $destination, $options = []): array { return ['']; } + public function repositoryRoot(\Horde\Components\Helper\Root $helper): string { return ''; } + public function installChannel(\Horde\Components\Pear\Environment $env, $options = []): void {} + public function install(\Horde\Components\Pear\Environment $env, $options = [], $reason = ''): void {} + }; + + return new Context($component); + } + + private function rmrf(string $path): void + { + if (!file_exists($path)) { + return; + } + if (is_file($path) || is_link($path)) { + @unlink($path); + return; + } + foreach (scandir($path) ?: [] as $child) { + if ($child === '.' || $child === '..') { + continue; + } + $this->rmrf($path . '/' . $child); + } + @rmdir($path); + } +} From 2ed28b6a518581178beb08898d7d650ae1b5659f Mon Sep 17 00:00:00 2001 From: Ralf Lang Date: Wed, 24 Jun 2026 19:53:16 +0200 Subject: [PATCH 4/5] feat(deps): add --platform resolver writing ci-platform to .horde.yml --- src/Helper/PlatformResolver.php | 523 ++++++++++++++++++++++ src/Module/Dependencies.php | 12 + src/Runner/Dependencies.php | 93 ++++ test/unit/Helper/PlatformResolverTest.php | 252 +++++++++++ 4 files changed, 880 insertions(+) create mode 100644 src/Helper/PlatformResolver.php create mode 100644 test/unit/Helper/PlatformResolverTest.php diff --git a/src/Helper/PlatformResolver.php b/src/Helper/PlatformResolver.php new file mode 100644 index 00000000..6eeda195 --- /dev/null +++ b/src/Helper/PlatformResolver.php @@ -0,0 +1,523 @@ +|string> + * Keyed by minor version (e.g. "8.2"). Each value is either a + * list of [name, constraint] pairs or {@see self::NOT_RESOLVABLE}. + * Constraint is null for ext-* and lib-* entries. + */ + public function resolveRangeFromHordeYml(HordeYmlFile $hordeYml): array + { + $phpConstraint = $hordeYml->getRequiredPhp(); + $packageName = $hordeYml->getComposerName(); + $versionConstraint = $this->deriveVersionConstraint($hordeYml->getReleaseVersion()); + $stability = $hordeYml->getReleaseState() ?: 'stable'; + + return $this->resolveRangeFor( + $phpConstraint, + $packageName, + $versionConstraint, + $stability, + ); + } + + /** + * Resolve platform requirements across a component's full + * supported PHP version range, reading data from the legacy + * Component interface. Prefer + * {@see self::resolveRangeFromHordeYml()} when a parsed + * .horde.yml is available; this path is for callers that only + * hold the Component abstraction. + * + * @return array|string> + */ + public function resolveRange(Component $component): array + { + $phpConstraint = $this->getComponentPhpConstraint($component); + $packageName = $this->getComponentPackageName($component); + $versionConstraint = $this->deriveVersionConstraint($component->getVersion()); + $stability = $component->getState('release') ?: 'stable'; + + return $this->resolveRangeFor( + $phpConstraint, + $packageName, + $versionConstraint, + $stability, + ); + } + + /** + * @return array|string> + */ + private function resolveRangeFor( + string $phpConstraint, + string $packageName, + string $versionConstraint, + string $stability, + ): array { + $range = $this->phpVersionRange($phpConstraint); + + $out = []; + foreach ($range as $minor) { + $this->output?->info(sprintf('Resolving platform deps for PHP %s', $minor)); + $out[$minor] = $this->resolveSingleFor( + $packageName, + $versionConstraint, + $stability, + $minor, + ); + } + + return $out; + } + + /** + * Resolve platform requirements for a single pinned PHP minor + * version. + * + * @return list|string Either the parsed + * platform list or {@see self::NOT_RESOLVABLE}. + */ + public function resolveSingle(Component $component, string $minorVersion): array|string + { + return $this->resolveSingleFor( + $this->getComponentPackageName($component), + $this->deriveVersionConstraint($component->getVersion()), + $component->getState('release') ?: 'stable', + $minorVersion, + ); + } + + /** + * @return list|string + */ + private function resolveSingleFor( + string $packageName, + string $versionConstraint, + string $stability, + string $minorVersion, + ): array|string { + // $stability comes from the component's own release state but + // is intentionally not used as the composer minimum-stability: + // the transitive dep graph routinely contains alpha and dev + // packages from sibling horde/* libraries while the component + // being resolved is RC or stable. We always set + // minimum-stability=dev + prefer-stable=true so composer picks + // stable when available and falls through to dev when nothing + // else exists. Without this, the require step fails on any + // ActiveSync-shaped tree where a transitive sibling has not + // released a stable yet. + unset($stability); + + $tmpDir = $this->makeTempDir(); + try { + // 1. composer init -n + $r = $this->shell->exec( + sprintf('composer init -n --name=%s --no-interaction 2>&1', escapeshellarg('horde-tmp/platform-resolver')), + $tmpDir, + ); + if ($r->getReturnValue() !== 0) { + return self::NOT_RESOLVABLE; + } + + // 2. composer config platform.php X.Y.0 + $r = $this->shell->exec( + sprintf('composer config platform.php %s 2>&1', escapeshellarg($minorVersion . '.0')), + $tmpDir, + ); + if ($r->getReturnValue() !== 0) { + return self::NOT_RESOLVABLE; + } + + // 3. Stability: always dev + prefer-stable. See note above. + $this->shell->exec('composer config minimum-stability dev 2>&1', $tmpDir); + $this->shell->exec('composer config prefer-stable true 2>&1', $tmpDir); + + // 4. composer require : --no-install + $requireSpec = $packageName . ($versionConstraint !== '' ? ':' . $versionConstraint : ''); + $r = $this->shell->exec( + sprintf('composer require %s --no-install --no-interaction 2>&1', escapeshellarg($requireSpec)), + $tmpDir, + ); + if ($r->getReturnValue() !== 0) { + return self::NOT_RESOLVABLE; + } + + // 5. composer update --ignore-platform-reqs --no-install + // The require step in step 4 already writes a composer.lock, + // so this step is mostly redundant. We still run it so the + // sequence matches the documented manual recipe and so any + // plugin-level resolution post-processing happens before we + // read the lock. + $r = $this->shell->exec( + 'composer update --ignore-platform-reqs --no-install --no-interaction 2>&1', + $tmpDir, + ); + if ($r->getReturnValue() !== 0) { + return self::NOT_RESOLVABLE; + } + + // 6. Parse composer.lock + $lockPath = $tmpDir . '/composer.lock'; + if (!is_file($lockPath)) { + return self::NOT_RESOLVABLE; + } + + $extracted = $this->extractFromLock(file_get_contents($lockPath) ?: ''); + if ($extracted === []) { + // A successful resolve produces at least the platform + // requirements of the requested package itself. An empty + // list almost always means the require step rolled itself + // back without surfacing an error code we caught. Treat + // as not resolvable rather than emitting a misleading + // empty list. + return self::NOT_RESOLVABLE; + } + + return $extracted; + } finally { + $this->rmrf($tmpDir); + } + } + + /** + * Pull the [name, constraint] pairs for every platform requirement + * found across every package in a composer.lock document. + * + * The list is deduplicated by name (first occurrence wins for + * constraint) and sorted with `php` first, then the composer-* + * meta packages, then ext-* and lib-* alphabetically. This is a + * deterministic order so the resulting .horde.yml diff is stable. + * + * @return list + */ + public function extractFromLock(string $lockJson): array + { + try { + $lock = json_decode($lockJson, true, flags: JSON_THROW_ON_ERROR); + } catch (JsonException) { + return []; + } + + if (!is_array($lock)) { + return []; + } + + /** @var array $found */ + $found = []; + + $packages = array_merge( + is_array($lock['packages'] ?? null) ? $lock['packages'] : [], + // packages-dev exists in synthesized projects but we never + // require any dev deps from the throwaway root, so this + // section is normally empty. Walked defensively in case a + // transitive plugin pulls dev things in. + is_array($lock['packages-dev'] ?? null) ? $lock['packages-dev'] : [], + ); + + foreach ($packages as $package) { + if (!is_array($package)) { + continue; + } + $require = $package['require'] ?? null; + if (!is_array($require)) { + continue; + } + foreach ($require as $name => $constraint) { + if (!is_string($name)) { + continue; + } + if (!self::isPlatformKey($name)) { + continue; + } + if (!array_key_exists($name, $found)) { + $found[$name] = is_string($constraint) ? $constraint : null; + } + } + } + + // Stable, opinionated ordering. + return self::sortPlatformEntries($found); + } + + /** + * Decide whether a composer require key is a platform package as + * defined by composer itself: php, composer meta, ext-*, lib-*. + */ + public static function isPlatformKey(string $name): bool + { + if ($name === 'php' || $name === 'php-64bit' || $name === 'php-ipv6' || $name === 'php-zts' || $name === 'php-debug') { + return true; + } + if ($name === 'composer' || $name === 'composer-plugin-api' || $name === 'composer-runtime-api') { + return true; + } + return str_starts_with($name, 'ext-') || str_starts_with($name, 'lib-'); + } + + /** + * Convert the {name => constraint} map into the public list shape + * with a stable order. + * + * Order: php-family first, then composer-* meta, then ext-* / + * lib-* alphabetically. Within php-family we sort the variants + * (php, php-64bit, ...) alphabetically too; in practice only + * `php` shows up. + * + * @param array $found + * @return list + */ + private static function sortPlatformEntries(array $found): array + { + $phpFamily = []; + $composerFamily = []; + $rest = []; + foreach ($found as $name => $constraint) { + if ($name === 'php' || str_starts_with($name, 'php-')) { + $phpFamily[$name] = $constraint; + } elseif ($name === 'composer' || str_starts_with($name, 'composer-')) { + $composerFamily[$name] = $constraint; + } else { + $rest[$name] = $constraint; + } + } + ksort($phpFamily); + ksort($composerFamily); + ksort($rest); + + $out = []; + foreach ([$phpFamily, $composerFamily, $rest] as $group) { + foreach ($group as $name => $constraint) { + $out[] = [$name, $constraint]; + } + } + return $out; + } + + /** + * Compute the list of PHP minor versions to resolve for, given a + * composer-style PHP version constraint. + * + * The constraint is parsed by {@see ConstraintParser}, which already + * handles every shape composer accepts (caret, tilde, ranges, OR, + * AND, wildcards, comparisons). We then probe each entry in + * {@see self::CANDIDATE_PHP_MINORS} as the major.minor.0 + * representative and keep the matches in order. + * + * The candidate list doubles as the upper-bound cap: a constraint + * like `^8.5` that allows up to PHP 9 will only return the candidate + * minors we actually know about, capped at + * {@see self::CURRENT_MAX_PHP}. + * + * Unparseable constraints fall back to a single-entry list at + * CURRENT_MAX_PHP so the caller has at least one PHP version to + * try; better than failing the whole command on an unfamiliar + * constraint string. + * + * @return list e.g. ['8.2', '8.3', '8.4'] + */ + public function phpVersionRange(string $constraint): array + { + $constraint = trim($constraint); + if ($constraint === '') { + return [self::CURRENT_MAX_PHP]; + } + + // Composer accepts both `,` and ` ` as AND separators (per the + // package versions documentation); Horde\Version\ConstraintParser + // only recognises space-separated AND, so normalise commas to + // spaces before parsing. OR (`||`) stays untouched. + $normalised = preg_replace('/\s*,\s*/', ' ', $constraint) ?? $constraint; + + try { + $parsed = (new ConstraintParser())->parse($normalised); + } catch (InvalidVersionException) { + return [self::CURRENT_MAX_PHP]; + } + + $out = []; + foreach (self::CANDIDATE_PHP_MINORS as $minor) { + $probe = new RelaxedSemanticVersion($minor . '.0'); + if ($parsed->isSatisfiedBy($probe)) { + $out[] = $minor; + } + } + + // Defensive fallback: if no candidate satisfies the constraint + // (e.g. constraint is for an unsupported PHP major), return + // CURRENT_MAX_PHP rather than an empty range. Callers iterate + // over the result and an empty list would skip the component + // entirely. + if ($out === []) { + return [self::CURRENT_MAX_PHP]; + } + + return $out; + } + + private function getComponentPhpConstraint(Component $component): string + { + $deps = $component->getDependencies(); + if (!is_array($deps)) { + return ''; + } + $required = $deps['required'] ?? null; + if (!is_array($required)) { + return ''; + } + $php = $required['php'] ?? ''; + return is_string($php) ? $php : ''; + } + + private function getComponentPackageName(Component $component): string + { + // Fall back to "horde/" if the component doesn't expose + // a richer composer name. Every modern Horde component uses + // the horde/ shape on Packagist. + $name = $component->getName(); + if ($name === '') { + return 'horde/unknown'; + } + if (str_contains($name, '/')) { + return $name; + } + return 'horde/' . $name; + } + + private function getComponentVersionConstraint(Component $component): string + { + return $this->deriveVersionConstraint($component->getVersion()); + } + + /** + * Build a caret-range constraint usable in `composer require` from + * a raw release version. `3.1.4-RC1` → `^3.1`, `3.0` → `^3.0`, + * empty → `*` (latest). + */ + private function deriveVersionConstraint(string $version): string + { + if ($version === '') { + return '*'; + } + $normalized = preg_replace('/[-+].*/', '', $version) ?? $version; + $parts = explode('.', $normalized); + if (count($parts) < 2) { + return '^' . $normalized; + } + return '^' . $parts[0] . '.' . $parts[1]; + } + + private function makeTempDir(): string + { + $base = sys_get_temp_dir() . '/horde-platform-resolver-' . uniqid('', true); + if (!mkdir($base, 0o755, true) && !is_dir($base)) { + throw new \RuntimeException('Failed to create temp dir: ' . $base); + } + return $base; + } + + private function rmrf(string $path): void + { + if (!file_exists($path)) { + return; + } + if (is_file($path) || is_link($path)) { + @unlink($path); + return; + } + foreach (scandir($path) ?: [] as $child) { + if ($child === '.' || $child === '..') { + continue; + } + $this->rmrf($path . '/' . $child); + } + @rmdir($path); + } +} diff --git a/src/Module/Dependencies.php b/src/Module/Dependencies.php index 4f20eed9..0efafa43 100644 --- a/src/Module/Dependencies.php +++ b/src/Module/Dependencies.php @@ -94,6 +94,17 @@ public function getOptionGroupOptions(): array '--allow-network-requests', ['action' => 'store_true', 'help' => 'Allow network requests to resolve dependencies from remote sources (Packagist, etc.).'] ), + new Option( + '--platform', + [ + 'action' => 'store_true', + 'help' => 'Resolve platform requirements (PHP version, ext-*, lib-*, composer-*) ' + . 'per supported PHP minor version and write them into .horde.yml under the ' + . 'ci-platform key. Uses synthesized throwaway composer projects to resolve ' + . 'against Packagist. Combine with --pretend to print the resulting YAML ' + . 'fragment without writing.', + ] + ), ]; } @@ -154,6 +165,7 @@ public function getContextOptionHelp(): array '--detect-plugins' => 'Automatically detect Composer plugins in the dependency tree.', '--allow-network-requests' => 'Allow network requests to resolve dependencies from remote sources (Packagist, PEAR channels, etc.). By default, only local git checkout is used.', '--allow-remote' => 'Legacy option: use --allow-network-requests instead.', + '--platform' => 'Resolve transitive platform requirements and update .horde.yml ci-platform.', ]; } diff --git a/src/Runner/Dependencies.php b/src/Runner/Dependencies.php index b5b0d382..777a42c2 100644 --- a/src/Runner/Dependencies.php +++ b/src/Runner/Dependencies.php @@ -20,7 +20,9 @@ use Horde\Components\Component\Factory as ComponentFactory; use Horde\Components\Helper\Dependencies as HelperDependencies; use Horde\Components\Helper\DependencyTreeBuilder; +use Horde\Components\Helper\PlatformResolver; use Horde\Components\Helper\PluginDetector; +use Horde\Components\Helper\Shell; use Horde\Components\Output; use Horde\Components\Util\YamlLoader; use Horde\HordeYmlFile\HordeYmlFile; @@ -57,6 +59,14 @@ public function __construct( public function run(): void { + // Resolve platform requirements per supported PHP minor version + // and update .horde.yml. Must come before the tree-output branches + // because --platform composes with neither. + if (!empty($this->options['platform'])) { + $this->runPlatformResolve(); + return; + } + // Check if we should export to YAML file if (!empty($this->options['export_yaml'])) { $this->runGraphExport(); @@ -73,6 +83,89 @@ public function run(): void $this->runTreeOutput(); } + /** + * Resolve transitive platform requirements per supported PHP minor + * version and write the result into .horde.yml under `ci-platform`. + * + * In `--dry-run` mode the resolved YAML fragment is printed to + * stdout instead of being written. + */ + private function runPlatformResolve(): void + { + $output = $this->getOutput(); + + if (!method_exists($this->component, 'getComponentDirectory')) { + $output->fail('Cannot resolve platform deps: component has no on-disk directory.'); + return; + } + + $componentDir = $this->component->getComponentDirectory(); + $hordeYmlPath = $componentDir . '/.horde.yml'; + + if (!file_exists($hordeYmlPath)) { + $output->fail("No .horde.yml at {$hordeYmlPath}"); + return; + } + + $hordeYml = new HordeYmlFile($hordeYmlPath); + $resolver = new PlatformResolver(new Shell($output), $output); + + $resolved = $resolver->resolveRangeFromHordeYml($hordeYml); + + // Build the YAML-friendly representation. Per the agreed shape: + // - ext-* / lib-* render as bare list items (string scalars). + // - php / composer-* render as single-key maps so the version + // constraint is preserved alongside the name. + // - 'not resolvable' renders as a scalar string under the key. + $ciPlatform = []; + foreach ($resolved as $minor => $entries) { + if (is_string($entries)) { + $ciPlatform[$minor] = $entries; + continue; + } + $list = []; + foreach ($entries as [$name, $constraint]) { + if (str_starts_with($name, 'ext-') || str_starts_with($name, 'lib-')) { + $list[] = $name; + } else { + $list[] = [$name => $constraint ?? '*']; + } + } + $ciPlatform[$minor] = $list; + } + + if (!empty($this->options['pretend'])) { + // Preview mode: surface the fragment that would be written. + $output->plain('--- ci-platform (preview, not written) ---'); + $output->plain(YamlLoader::dump(['ci-platform' => $ciPlatform])); + return; + } + + $hordeYml->set('ci-platform', $ciPlatform); + $hordeYml->save(); + + $output->ok(sprintf( + 'Wrote ci-platform for %d PHP version(s) to %s', + count($ciPlatform), + $hordeYmlPath, + )); + } + + /** + * Pull the Output helper out of HelperDependencies via the same + * reflection trick the rest of this runner uses. The helper holds + * the configured output sink for the current run; we cannot inject + * it through the constructor because Module/Dependencies wires the + * runner through a generic factory. + */ + private function getOutput(): Output + { + $reflection = new \ReflectionClass($this->dependenciesHelper); + $outputProperty = $reflection->getProperty('_output'); + $outputProperty->setAccessible(true); + return $outputProperty->getValue($this->dependenciesHelper); + } + /** * Run tree output to stdout. */ diff --git a/test/unit/Helper/PlatformResolverTest.php b/test/unit/Helper/PlatformResolverTest.php new file mode 100644 index 00000000..ca86f87c --- /dev/null +++ b/test/unit/Helper/PlatformResolverTest.php @@ -0,0 +1,252 @@ +resolver = new PlatformResolver(new Shell()); + } + + public static function constraintRangeProvider(): array + { + $max = PlatformResolver::CURRENT_MAX_PHP; + return [ + // Lower bound from the constraint, upper from CURRENT_MAX_PHP. + 'caret two-part' => ['^8.2', ['8.2', '8.3', '8.4', '8.5', '8.6']], + 'caret three-part' => ['^8.2.0', ['8.2', '8.3', '8.4', '8.5', '8.6']], + 'tilde two-part' => ['~8.2', ['8.2', '8.3', '8.4', '8.5', '8.6']], + // Three-part tilde locks the minor. + 'tilde three-part' => ['~8.2.0', ['8.2']], + 'gte two-part' => ['>=8.1', ['8.1', '8.2', '8.3', '8.4', '8.5', '8.6']], + 'gte three-part' => ['>=8.1.0', ['8.1', '8.2', '8.3', '8.4', '8.5', '8.6']], + 'lt' => ['<8.5', ['8.0', '8.1', '8.2', '8.3', '8.4']], + 'lte' => ['<=8.5', ['8.0', '8.1', '8.2', '8.3', '8.4', '8.5']], + 'compound comma' => ['>=8.2,<8.5', ['8.2', '8.3', '8.4']], + 'compound space' => ['>=8.2 <8.5', ['8.2', '8.3', '8.4']], + 'compound comma sp' => ['>=8.2, <8.5', ['8.2', '8.3', '8.4']], + 'wildcard' => ['8.2.*', ['8.2']], + 'exact two-part' => ['8.2', ['8.2']], + 'exact three-part' => ['8.2.0', ['8.2']], + 'cap kicks in' => ['^8.5', ['8.5', $max]], + // Empty constraint and unparseable input fall back to CURRENT_MAX_PHP. + 'empty falls back' => ['', [$max]], + 'garbage falls back' => ['nonsense-input', [$max]], + // OR composition: ^8.2 || ^9.0 — only the 8.x leg matches + // current candidates because we don't list 9.x yet. + 'or' => ['^8.2 || ^9.0', ['8.2', '8.3', '8.4', '8.5', '8.6']], + // Range syntax (composer "1.0.0 - 2.0.0"). + 'range syntax' => ['8.2.0 - 8.4.99', ['8.2', '8.3', '8.4']], + ]; + } + + #[DataProvider('constraintRangeProvider')] + public function testPhpVersionRange(string $constraint, array $expected): void + { + $this->assertSame($expected, $this->resolver->phpVersionRange($constraint)); + } + + public function testPhpVersionRangeUpperCappedAtCurrentMax(): void + { + // ^8.6 (i.e. at-or-above CURRENT_MAX_PHP) yields just the cap. + $max = PlatformResolver::CURRENT_MAX_PHP; + $this->assertSame([$max], $this->resolver->phpVersionRange('^' . $max)); + } + + public function testPhpVersionRangeUnsupportedMajorFallsBack(): void + { + // A constraint pinned to a major we don't enumerate (^7.4) + // returns CURRENT_MAX_PHP rather than an empty list so the + // caller has at least one PHP version to attempt. + $this->assertSame( + [PlatformResolver::CURRENT_MAX_PHP], + $this->resolver->phpVersionRange('^7.4'), + ); + } + + public static function platformKeyProvider(): array + { + return [ + 'php' => ['php', true], + 'php-64bit' => ['php-64bit', true], + 'composer-plugin-api' => ['composer-plugin-api', true], + 'composer-runtime-api' => ['composer-runtime-api', true], + 'composer bare' => ['composer', true], + 'ext-curl' => ['ext-curl', true], + 'ext-something-long' => ['ext-some-long-name', true], + 'lib-curl' => ['lib-curl', true], + 'horde package' => ['horde/exception', false], + 'vendor package' => ['psr/log', false], + 'extension-like name' => ['extension-stuff', false], + ]; + } + + #[DataProvider('platformKeyProvider')] + public function testIsPlatformKey(string $name, bool $expected): void + { + $this->assertSame($expected, PlatformResolver::isPlatformKey($name)); + } + + public function testExtractFromLockCollectsAcrossPackages(): void + { + $lock = json_encode([ + 'packages' => [ + [ + 'name' => 'horde/example', + 'require' => [ + 'php' => '^8.2', + 'ext-curl' => '*', + 'ext-json' => '*', + 'horde/util' => '^3', + ], + ], + [ + 'name' => 'horde/util', + 'require' => [ + 'php' => '^8.2', + 'ext-mbstring' => '*', + 'composer-plugin-api' => '^2.0', + ], + ], + ], + 'packages-dev' => [], + ]); + + $entries = $this->resolver->extractFromLock($lock); + + // Order is deterministic: php-family, then composer-*, then ext-* alpha. + $this->assertSame([ + ['php', '^8.2'], + ['composer-plugin-api', '^2.0'], + ['ext-curl', '*'], + ['ext-json', '*'], + ['ext-mbstring', '*'], + ], $entries); + } + + public function testExtractFromLockDeduplicatesByName(): void + { + // Two packages requiring the same extension with differing + // constraints. The resolver keeps the first occurrence. + $lock = json_encode([ + 'packages' => [ + ['name' => 'a', 'require' => ['ext-curl' => '*']], + ['name' => 'b', 'require' => ['ext-curl' => '>=1.0']], + ], + ]); + + $entries = $this->resolver->extractFromLock($lock); + + $this->assertSame([['ext-curl', '*']], $entries); + } + + public function testExtractFromLockIgnoresNonPlatformRequires(): void + { + $lock = json_encode([ + 'packages' => [ + [ + 'name' => 'a', + 'require' => [ + 'horde/foo' => '^3', + 'psr/log' => '^1.0', + 'symfony/yaml' => '^6.0', + ], + ], + ], + ]); + + $this->assertSame([], $this->resolver->extractFromLock($lock)); + } + + public function testExtractFromLockHandlesEmptyAndMalformedJson(): void + { + $this->assertSame([], $this->resolver->extractFromLock('')); + $this->assertSame([], $this->resolver->extractFromLock('not json')); + $this->assertSame([], $this->resolver->extractFromLock('{"unexpected": "shape"}')); + } + + public function testExtractFromLockTrueLeafPackagePhpOnly(): void + { + // A leaf package that declares only a PHP constraint — no + // extensions, no composer meta, no transitive deps. The + // resolver must emit exactly the php entry and nothing else; + // this guards against accidental "no extensions => empty list" + // shortcuts in the lock walker. + $lock = json_encode([ + 'packages' => [ + [ + 'name' => 'horde/leaf-only', + 'require' => [ + 'php' => '^8.2', + ], + ], + ], + 'packages-dev' => [], + ]); + + $this->assertSame( + [['php', '^8.2']], + $this->resolver->extractFromLock($lock), + ); + } + + public function testExtractFromLockLeafWithComposerMetaButNoExtensions(): void + { + // Slightly less leaf-y: only php + composer-plugin-api. Confirms + // composer-* meta entries make it through and order is stable + // even without any ext-* in the mix. + $lock = json_encode([ + 'packages' => [ + [ + 'name' => 'horde/no-extensions', + 'require' => [ + 'php' => '^8.2', + 'composer-runtime-api' => '^2.0', + ], + ], + ], + ]); + + $this->assertSame( + [ + ['php', '^8.2'], + ['composer-runtime-api', '^2.0'], + ], + $this->resolver->extractFromLock($lock), + ); + } +} From 8fe8d7462f3d89d10482dfb728f8bd232967335c Mon Sep 17 00:00:00 2001 From: Ralf Lang Date: Wed, 24 Jun 2026 20:38:05 +0200 Subject: [PATCH 5/5] feat: Support platform requirements of dependencies in CI --- src/Ci/Run/PrCommentReporter.php | 17 ++ src/Ci/Run/ResultCollector.php | 16 +- src/Ci/Run/RunCommand.php | 36 +++ src/Ci/Setup/ComposerInstaller.php | 56 +++++ src/Ci/Setup/ExtensionInstaller.php | 236 +++++++++++++++++- src/Ci/Setup/SetupCommand.php | 130 +++++++++- test/Unit/Ci/Run/ResultCollectorTest.php | 18 ++ test/Unit/Ci/Setup/ComposerInstallerTest.php | 78 ++++++ test/Unit/Ci/Setup/ExtensionInstallerTest.php | 201 +++++++++++++++ test/Unit/Ci/Setup/SetupCommandTest.php | 133 ++++++++-- 10 files changed, 893 insertions(+), 28 deletions(-) create mode 100644 test/Unit/Ci/Setup/ComposerInstallerTest.php create mode 100644 test/Unit/Ci/Setup/ExtensionInstallerTest.php diff --git a/src/Ci/Run/PrCommentReporter.php b/src/Ci/Run/PrCommentReporter.php index 5b6636de..ed0729ea 100644 --- a/src/Ci/Run/PrCommentReporter.php +++ b/src/Ci/Run/PrCommentReporter.php @@ -495,6 +495,23 @@ private function formatToolFailure(string $toolName, array $result): string return "{$toolDisplay}: Error - {$result['error']}"; } + // Setup-failed lanes (F30) arrive here as `missing: true` with + // a reason prefixed by "Setup failed: " by RunCommand. The + // ResultCollector::addMissing contract carries that reason in + // the `reason` key. We classify the user-facing rendering by + // category if available; otherwise fall back to the raw reason. + if (!empty($result['missing'])) { + $reason = (string) ($result['reason'] ?? 'Missing result file'); + $category = (string) ($result['category'] ?? ''); + $tag = match ($category) { + 'stability_gate' => '⚠️ Stability-gated', + 'platform_missing' => '❌ Platform requirement missing', + 'php_version' => '❌ PHP version conflict', + default => '❌ Setup failed', + }; + return "{$toolDisplay}: {$tag} ({$reason})"; + } + return match ($toolName) { 'phpunit' => sprintf( '%s: %d failure%s, %d error%s', diff --git a/src/Ci/Run/ResultCollector.php b/src/Ci/Run/ResultCollector.php index 48fde410..d056ea7a 100644 --- a/src/Ci/Run/ResultCollector.php +++ b/src/Ci/Run/ResultCollector.php @@ -104,17 +104,29 @@ public function addResultFromFile(string $laneName, string $tool, string $jsonFi * on the lane's PHP version) belong in {@see addSkipped()} with * `success: true`; they are not modeled here. * + * `$category` is the F29 classifier output for setup-failed lanes + * (`stability_gate`, `platform_missing`, `php_version`, `unknown`). + * PrCommentReporter uses it to render `stability_gate` failures as + * "⚠️ Stability-gated (working as designed)" rather than the generic + * "❌ Setup failed". + * * @param string $laneName Lane name * @param string $tool Tool name * @param string $reason Reason the result is missing + * @param string $category Classifier output; empty string when N/A */ - public function addMissing(string $laneName, string $tool, string $reason): void - { + public function addMissing( + string $laneName, + string $tool, + string $reason, + string $category = '' + ): void { $this->results[$laneName][$tool] = [ 'success' => false, 'exit_code' => 1, 'missing' => true, 'reason' => $reason, + 'category' => $category, ]; } diff --git a/src/Ci/Run/RunCommand.php b/src/Ci/Run/RunCommand.php index e839074f..7ceba0f1 100644 --- a/src/Ci/Run/RunCommand.php +++ b/src/Ci/Run/RunCommand.php @@ -185,6 +185,15 @@ private function runLaneScript(array $lane): void return; } + // F30: lane with build/setup-failed.json never had a run-lane.sh + // generated. Stay silent here; aggregateResults emits the ❌ with + // the failure reason. + $setupFailFile = $lane['component_dir'] . '/build/setup-failed.json'; + if (file_exists($setupFailFile)) { + $this->output->error("[{$lane['name']}] Setup failed; no lane script to run"); + return; + } + $this->output->info("[{$lane['name']}] Executing lane script..."); // Check if script exists @@ -260,6 +269,33 @@ private function aggregateResults(array $lanes): void } } + // F30: surface setup-failed lanes as ❌ for every tool that + // would have run. The reason from setup-failed.json lands in + // the PR comment so the maintainer sees *why* the lane never + // executed without having to scroll through composer logs. + // + // F29: the category (stability_gate / platform_missing / + // php_version / unknown) lets PrCommentReporter render + // "ecosystem not yet at this stability" failures distinctly + // from real-bug failures. + $setupFailFile = $buildDir . '/setup-failed.json'; + if (file_exists($setupFailFile)) { + $failData = json_decode((string) file_get_contents($setupFailFile), true); + if (is_array($failData) && ($failData['setup_failed'] ?? false)) { + $reason = (string) ($failData['reason'] ?? 'Lane setup failed'); + $category = (string) ($failData['category'] ?? 'unknown'); + foreach ((array) ($failData['tools'] ?? ['phpunit', 'phpstan']) as $tool) { + $this->collector->addMissing( + $lane['name'], + (string) $tool, + 'Setup failed: ' . $reason, + $category + ); + } + continue; + } + } + // Read PHPUnit results $phpunitFile = $buildDir . '/phpunit-results-summary.json'; if (file_exists($phpunitFile)) { diff --git a/src/Ci/Setup/ComposerInstaller.php b/src/Ci/Setup/ComposerInstaller.php index 11c8715c..8152f5c7 100644 --- a/src/Ci/Setup/ComposerInstaller.php +++ b/src/Ci/Setup/ComposerInstaller.php @@ -245,6 +245,62 @@ private function extractError(string $output): string return trim(end($nonEmpty)); } + /** + * Classify a composer install failure message into a coarse category + * so downstream reporting can present "stability-gated" failures + * differently from "ext-* missing" and from "no idea". + * + * The classification is best-effort: composer's text is a moving + * target. Categories returned: + * + * - `stability_gate`: a transitive dependency is only available at a + * stability lower than the lane's `minimum-stability`. Composer + * prints `does not match your minimum-stability`. Working as + * designed — the ecosystem is not yet ready for that lane's + * stability level. Maintainer action: wait for the upstream package + * to release at the required stability or accept the failure. + * + * - `platform_missing`: a `ext-*` (or `lib-*`) requirement is not + * installed on the runner. Composer prints `is missing from your + * system. Install or enable PHP's extension`. The fix lives + * in `.horde.yml`'s `ci-platform` (rerun + * `horde-components dependencies --platform`) or in the bootstrap + * if the resolver caught it but the apt-get install path missed + * the package. + * + * - `php_version`: the resolver couldn't satisfy a PHP version + * constraint. Composer prints `your php version (X.Y.Z) does not + * satisfy that requirement`. Usually a stale `^7` constraint + * surviving on a transitive horde/* package. + * + * - `unknown`: anything else. UI falls back to the generic + * "Setup failed" rendering with the raw message. + */ + public static function classifyError(string $output): string + { + // The composer error messages contain literal newlines and the + // workflow-command-escaped %0A variant; normalise both before + // pattern matching so the classifier works whether the caller + // hands us live output or the lane-prefixed copy from the + // CI log. + $normalised = str_replace(['%0A', '\\n'], "\n", $output); + + if (stripos($normalised, 'does not match your minimum-stability') !== false) { + return 'stability_gate'; + } + if (preg_match('/is missing from your system\\. Install or enable PHP/i', $normalised) === 1) { + return 'platform_missing'; + } + // Composer's pre-resolution platform-requirement rejection. + if (preg_match('/require ext-\\S+ \\* -> it is missing from your system/i', $normalised) === 1) { + return 'platform_missing'; + } + if (preg_match('/your php version \\(\\S+\\) does not satisfy/i', $normalised) === 1) { + return 'php_version'; + } + return 'unknown'; + } + /** * Verify composer installation succeeded. * diff --git a/src/Ci/Setup/ExtensionInstaller.php b/src/Ci/Setup/ExtensionInstaller.php index 0fd143eb..0c22a8c3 100644 --- a/src/Ci/Setup/ExtensionInstaller.php +++ b/src/Ci/Setup/ExtensionInstaller.php @@ -18,12 +18,25 @@ use Horde\Components\Exception; use Horde\Components\Output; +use Horde\HordeYmlFile\HordeYmlFile; /** * Installs PHP extensions for multiple PHP versions. * * Uses apt-get to install PHP extensions from ondrej PPA. - * Employs a hybrid detection strategy: baseline + composer.json + static mapping. + * + * Resolution strategy (in order): + * + * 1. **Preferred**: read `.horde.yml`'s `ci-platform.` section. + * Each minor entry is a list produced by + * `horde-components dependencies --platform`; this is the only data + * source that captures *transitive* extension requirements + * (`horde/mapi` → `ext-bcmath`, etc). When present, every lane gets + * exactly the extensions resolved for its PHP minor. + * + * 2. **Fallback** (no ci-platform yet, or marked `not resolvable`): the + * legacy hybrid detection — baseline extensions + composer.json + * `require`/`require-dev` walk + static per-component map. * * @category Horde * @package Components @@ -77,6 +90,12 @@ public function __construct( * 2. Extensions from composer.json require (ext-*) * 3. Static mapping based on component name * + * Prefer {@see detectExtensionsPerVersion()} when the component has + * a resolved `ci-platform` block in `.horde.yml`; this flat method + * cannot represent per-PHP-minor differences and is kept as the + * fallback for components that have not yet run + * `horde-components dependencies --platform`. + * * @param string $componentPath Path to component * @param string $componentName Component name * @return array Required extension names @@ -102,6 +121,221 @@ public function detectExtensions(string $componentPath, string $componentName): return $extensions; } + /** + * Detect required extensions per PHP minor version. + * + * Returns a map of ` => ` so each lane + * only installs the extensions actually needed for its resolved + * dependency tree under that PHP version. + * + * Source of truth, in order of preference: + * + * 1. `.horde.yml`'s `ci-platform` section, written by + * `horde-components dependencies --platform`. This is the only + * place where *transitive* requirements (e.g. `horde/mapi` → + * `ext-bcmath`) are visible without running composer. + * - A list under a minor key means "install exactly these". + * - The literal string {@see PlatformResolver::NOT_RESOLVABLE} + * under a minor key means "couldn't resolve a lock for this + * PHP version, fall back to flat detection". + * + * 2. Flat detection via {@see detectExtensions()} when the ci-platform + * map has no entry for that minor (or the whole file is missing + * the ci-platform section). This is the same set the legacy + * bootstrap installed; it will miss transitive extensions but + * will not be empty. + * + * The returned map preserves the order of `$phpVersions`. Every + * version in `$phpVersions` is represented in the map even if its + * value is just the baseline set. + * + * @param string $componentPath Path to component on disk + * @param string $componentName Component identifier (e.g. "ActiveSync") + * @param array $phpVersions PHP minors to compute sets for + * (e.g. ["8.2","8.3","8.4","8.5"]) + * @return array> Map of minor → ext names + */ + public function detectExtensionsPerVersion( + string $componentPath, + string $componentName, + array $phpVersions + ): array { + $ciPlatform = $this->loadCiPlatform($componentPath); + $fallback = $this->detectExtensions($componentPath, $componentName); + + $out = []; + foreach ($phpVersions as $minor) { + $resolved = $this->extractExtensionsForMinor($ciPlatform, $minor); + if ($resolved === null) { + // Either ci-platform missing entirely or this minor has + // no usable entry. Use flat detection. + $out[$minor] = $fallback; + continue; + } + + // ci-platform lists ext-* per minor. Always merge with the + // baseline because the resolver only surfaces what packages + // *declare*: PHP itself rarely requires you to install + // ext-json (it's built in) but `dom`/`intl` etc. would be + // missed if they weren't pulled in transitively. Baseline + // covers the runtime essentials Horde components implicitly + // rely on. + $merged = array_values(array_unique(array_merge(self::BASELINE_EXTENSIONS, $resolved))); + sort($merged); + $out[$minor] = $merged; + } + return $out; + } + + /** + * Read the `ci-platform` section from a component's `.horde.yml`. + * + * Returns an empty array when the file is missing, malformed, or + * has no ci-platform key. Callers must be prepared for any subset + * of PHP minors to be absent — components only ran the resolver + * for the range their constraint allowed. + * + * We pull from `toArray()` rather than `get('ci-platform')` because + * `HordeYmlFile::get` returns nested associative data as stdClass + * objects, but our extractor walks lists and string-keyed maps; a + * plain array round-trips through json/var_export as the parser + * sees it. The keys are stringified PHP minors ("8.3" etc.); cast + * defensively below in case Symfony YAML produces ints on a + * future version. + * + * @return array The ci-platform map keyed by minor. + */ + private function loadCiPlatform(string $componentPath): array + { + $hordeYmlPath = $componentPath . '/.horde.yml'; + if (!is_file($hordeYmlPath)) { + return []; + } + try { + $hordeYml = new HordeYmlFile($hordeYmlPath); + } catch (\Throwable) { + return []; + } + $data = $hordeYml->toArray(); + $section = $data['ci-platform'] ?? []; + if (!is_array($section)) { + return []; + } + // Normalise keys to strings so caller's array_key_exists(string) works + // regardless of whether the YAML parser produced ints (8.3 is a float + // in YAML 1.2 unless quoted, but every consumer expects "8.3"). + $normalised = []; + foreach ($section as $key => $value) { + $normalised[(string) $key] = $value; + } + return $normalised; + } + + /** + * Pull the ext-* list for a single PHP minor out of a ci-platform + * map. Returns null when the entry is missing, not resolvable, or + * malformed; returns a list (possibly empty) when the entry exists + * and is a list. + * + * The shape we accept matches what + * {@see \Horde\Components\Runner\Dependencies::runPlatformResolve()} + * writes: + * + * ci-platform: + * "8.3": + * - php: "^8.3" # single-key map (php / composer-*) + * - composer-runtime-api: "^2" + * - ext-bcmath # bare string (ext-* / lib-*) + * - ext-curl + * "8.4": "not resolvable" + * + * @param array $ciPlatform Result of {@see loadCiPlatform()} + * @return list|null Extension names (without ext- prefix) + * or null when no usable entry. + */ + private function extractExtensionsForMinor(array $ciPlatform, string $minor): ?array + { + if (!array_key_exists($minor, $ciPlatform)) { + return null; + } + $entry = $ciPlatform[$minor]; + if (is_string($entry)) { + // 'not resolvable' or any other scalar — caller falls back. + return null; + } + if (!is_array($entry)) { + return null; + } + + $exts = []; + foreach ($entry as $item) { + if (is_string($item) && str_starts_with($item, 'ext-')) { + $exts[] = substr($item, 4); + continue; + } + // Single-key maps (php / composer-*) are intentionally + // ignored here: PhpInstaller handles the PHP itself, and + // composer-meta packages are not installable extensions. + } + return array_values(array_unique($exts)); + } + + /** + * Install extensions per PHP version, given a per-version map. + * + * Each lane gets exactly the extension set computed by + * {@see detectExtensionsPerVersion()}; sibling PHP minors do not + * affect each other. Falls through to the existing per-extension + * install path, so partial failures (one extension unavailable for + * one minor) are still tolerated rather than aborting the run. + * + * @param array> $extensionsPerVersion Map of + * php minor → list of ext names (no `ext-` prefix). + * @return bool True if the loop completed (individual extension + * failures are warned, not thrown). + */ + public function installPerVersion(array $extensionsPerVersion): bool + { + if (empty($extensionsPerVersion)) { + $this->output->info('No extensions to install'); + return true; + } + + $failed = []; + $succeeded = []; + + foreach ($extensionsPerVersion as $phpVersion => $extensions) { + if (!is_array($extensions) || $extensions === []) { + continue; + } + $this->output->info(sprintf( + 'PHP %s: %s', + $phpVersion, + implode(', ', $extensions) + )); + foreach ($extensions as $extension) { + $result = $this->installExtension($extension, $phpVersion); + + if ($result === true) { + $succeeded[] = "php{$phpVersion}-{$extension}"; + } elseif ($result === false) { + $failed[] = "php{$phpVersion}-{$extension}"; + } + // null means skipped (already installed or not available) + } + } + + if (!empty($succeeded)) { + $this->output->ok('Installed: ' . implode(', ', $succeeded)); + } + if (!empty($failed)) { + // Per-extension failure is non-fatal; the relevant lane will + // surface a real error at composer install or test time. + $this->output->warn('Failed to install: ' . implode(', ', $failed)); + } + return true; + } + /** * Extract extensions from composer.json. * diff --git a/src/Ci/Setup/SetupCommand.php b/src/Ci/Setup/SetupCommand.php index 01c3925d..ae2478cf 100644 --- a/src/Ci/Setup/SetupCommand.php +++ b/src/Ci/Setup/SetupCommand.php @@ -130,13 +130,30 @@ public function execute(CiConfig $config): bool // Install extensions $this->output->info('[4/5] Installing PHP extensions...'); - $extensions = $this->extensionInstaller->detectExtensions( + // Per-PHP-minor resolution: when the component has a resolved + // ci-platform block in .horde.yml (written by + // `horde-components dependencies --platform`), each lane installs + // exactly the transitively-required extensions for its PHP minor. + // Otherwise the helper falls back to flat baseline+composer.json + // detection per version. + $extensionsPerVersion = $this->extensionInstaller->detectExtensionsPerVersion( $config->componentPath, - $config->componentName + $config->componentName, + $testableVersions ); - $this->output->info('Required extensions: ' . implode(', ', $extensions)); - $this->extensionInstaller->install($extensions, $testableVersions); + // Log the resolved sets so a failed-extension story in the + // composer install step downstream has a paper trail right + // here. + foreach ($extensionsPerVersion as $minor => $exts) { + $this->output->info(sprintf( + 'PHP %s extensions: %s', + $minor, + $exts === [] ? '(none)' : implode(', ', $exts) + )); + } + + $this->extensionInstaller->installPerVersion($extensionsPerVersion); $this->output->plain(''); // Copy to lanes @@ -160,6 +177,10 @@ public function execute(CiConfig $config): bool $this->output->bold('=== Running composer install ==='); $successful = 0; $failed = 0; + // Indexes of lanes whose composer-install failed. RunCommand will + // pick the build/setup-failed.json sidecar up and surface the lane + // as ❌ rather than aborting the whole run. + $setupFailedLanes = []; foreach ($lanes as $index => $lane) { $num = $index + 1; @@ -188,10 +209,23 @@ public function execute(CiConfig $config): bool $successful++; } else { $this->output->warn(" ⚠ Installation verification failed"); + $this->writeSetupFailureMarker( + $lane['dir'], + 'composer install verification failed', + 'unknown' + ); + $setupFailedLanes[] = $index; $failed++; } } catch (Exception $e) { - $this->output->error(" ✗ Failed: " . $e->getMessage()); + $category = ComposerInstaller::classifyError($e->getMessage()); + $this->output->error(" ✗ Failed [{$category}]: " . $e->getMessage()); + $this->writeSetupFailureMarker( + $lane['dir'], + $e->getMessage(), + $category + ); + $setupFailedLanes[] = $index; $failed++; } @@ -217,6 +251,15 @@ public function execute(CiConfig $config): bool continue; } + if (in_array($index, $setupFailedLanes, true)) { + // Lane's composer install failed; no run-lane.sh is written. + // The sidecar setup-failed.json (from + // writeSetupFailureMarker above) tells RunCommand to mark + // the lane as ❌ in the PR summary without trying to run it. + $this->output->skip('Skipped (setup failed, no script generated)'); + continue; + } + $laneConfig = [ 'lane_name' => $laneName, 'php_version' => $lane['php'], @@ -274,7 +317,29 @@ public function execute(CiConfig $config): bool $this->output->info("Tools cache: {$config->workDir}/tools"); $this->output->info("Next step: horde-components ci run --work-dir={$config->workDir}"); - return $failed === 0; + // F30: tolerate partial setup. + // + // The CI value of this run is the per-lane breakdown, not "did + // every lane install cleanly". Lanes that failed composer install + // carry a build/setup-failed.json marker; RunCommand surfaces them + // as ❌ in the PR comment alongside any lanes that ran. We only + // signal global setup failure (which Module\Ci translates into a + // throw + non-zero exit) when no non-skipped lane is usable — + // there is then nothing for ci run to do. + $usableLanes = $successful; + $totalNonSkipped = count($lanes) - count($skippedLanes); + if ($usableLanes === 0 && $totalNonSkipped > 0) { + return false; + } + if ($failed > 0) { + $this->output->warn(sprintf( + 'Continuing with %d/%d usable lanes; %d lane(s) marked as setup-failed.', + $usableLanes, + $totalNonSkipped, + $failed + )); + } + return true; } /** @@ -334,6 +399,59 @@ private function readComponentInfo(string $componentPath): array ]; } + /** + * Persist a setup-failure marker for a lane. + * + * Writes `/build/setup-failed.json` so RunCommand can pick + * the lane up, register it as ❌ for both phpunit and phpstan, and + * surface the reason in the PR comment. Distinct from + * `build/skip.json` which means "deliberately not run". A + * setup-failed lane is a real failure that the maintainer needs to + * fix; we just don't let it prevent other lanes from running. + * + * The optional `$category` is one of the strings returned by + * {@see ComposerInstaller::classifyError()}; downstream reporting + * uses it to render `stability_gate` failures as ⚠️ (working as + * designed) rather than ❌ (real bug). + * + * @param string $laneDir Lane component directory (where build/ lives) + * @param string $reason Human-readable failure summary + * @param string $category Classifier output: stability_gate / + * platform_missing / php_version / unknown + */ + private function writeSetupFailureMarker( + string $laneDir, + string $reason, + string $category = 'unknown' + ): void { + $buildDir = $laneDir . '/build'; + if (!is_dir($buildDir) && !mkdir($buildDir, 0o755, true) && !is_dir($buildDir)) { + // The lane dir may not exist if copy-to-lane failed earlier; + // in that case we cannot persist a marker. RunCommand's + // lane-discovery will not see the lane at all and the warn() + // above is the only surface for the failure. + return; + } + try { + $payload = json_encode( + [ + 'setup_failed' => true, + // PHPUnit and PHPStan are the two tools that would + // have run; both inherit the failure so the + // aggregator and PR comment present a coherent + // story. + 'tools' => ['phpunit', 'phpstan'], + 'reason' => $reason, + 'category' => $category, + ], + JSON_PRETTY_PRINT | JSON_THROW_ON_ERROR + ); + } catch (\JsonException) { + return; + } + @file_put_contents($buildDir . '/setup-failed.json', $payload); + } + /** * Detect which lanes are deliberately incompatible with the component's * PHPUnit constraint and write a sidecar build/skip.json so RunCommand diff --git a/test/Unit/Ci/Run/ResultCollectorTest.php b/test/Unit/Ci/Run/ResultCollectorTest.php index d473c51c..1922f01e 100644 --- a/test/Unit/Ci/Run/ResultCollectorTest.php +++ b/test/Unit/Ci/Run/ResultCollectorTest.php @@ -56,11 +56,29 @@ public function testMissingResultRecordsExpectedShape(): void 'exit_code' => 1, 'missing' => true, 'reason' => 'Script not found', + // F29: category defaults to '' when the caller doesn't + // supply one. PrCommentReporter falls back to the + // generic "Setup failed" rendering in that case. + 'category' => '', ], $results['php8.4-dev']['phpunit'] ); } + public function testMissingResultCarriesCategoryWhenSupplied(): void + { + $collector = new ResultCollector($this->mockOutput()); + $collector->addMissing( + 'php8.4-RC', + 'phpunit', + 'Setup failed: minimum-stability mismatch', + 'stability_gate' + ); + + $results = $collector->getResults(); + $this->assertSame('stability_gate', $results['php8.4-RC']['phpunit']['category']); + } + public function testMissingFromAddResultFromFileWhenJsonAbsent(): void { $collector = new ResultCollector($this->mockOutput()); diff --git a/test/Unit/Ci/Setup/ComposerInstallerTest.php b/test/Unit/Ci/Setup/ComposerInstallerTest.php new file mode 100644 index 00000000..7a807f23 --- /dev/null +++ b/test/Unit/Ci/Setup/ComposerInstallerTest.php @@ -0,0 +1,78 @@ + + * @license http://www.horde.org/licenses/lgpl21 LGPL 2.1 + */ + +declare(strict_types=1); + +namespace Horde\Components\Test\Unit\Ci\Setup; + +use Horde\Components\Ci\Setup\ComposerInstaller; +use PHPUnit\Framework\Attributes\CoversClass; +use PHPUnit\Framework\TestCase; + +/** + * Tests for the F29 composer-error classifier. + * + * The install path itself shells out to composer and is not unit-tested + * here; the classifier is the only part of ComposerInstaller that's + * worth pinning down to specific composer error strings, because + * downstream PR-comment rendering hinges on these category labels. + */ +#[CoversClass(ComposerInstaller::class)] +class ComposerInstallerTest extends TestCase +{ + public function testClassifyStabilityGate(): void + { + $msg = 'horde/mime[v3.0.0alpha4] require horde/listheaders ^2 ' + . '-> found horde/listheaders[2.0.0alpha1] but it does not ' + . 'match your minimum-stability.'; + $this->assertSame('stability_gate', ComposerInstaller::classifyError($msg)); + } + + public function testClassifyPlatformMissingPreResolution(): void + { + // Composer's pre-resolution rejection: a require key flagged + // missing before the dependency tree is even examined. + $msg = 'horde/mapi require ext-bcmath * -> it is missing from your system.'; + $this->assertSame('platform_missing', ComposerInstaller::classifyError($msg)); + } + + public function testClassifyPlatformMissingPostResolution(): void + { + // The longer post-resolution variant that suggests polyfills. + $msg = "PHP's bcmath extension is missing from your system. " + . "Install or enable PHP's bcmath extension."; + $this->assertSame('platform_missing', ComposerInstaller::classifyError($msg)); + } + + public function testClassifyPhpVersionConflict(): void + { + $msg = 'horde/mapi[2.0.0alpha1] require php ^7 -> ' + . 'your php version (8.2.31) does not satisfy that requirement.'; + $this->assertSame('php_version', ComposerInstaller::classifyError($msg)); + } + + public function testClassifyUnknownFallsThrough(): void + { + $msg = 'No idea what this error means; some random failure.'; + $this->assertSame('unknown', ComposerInstaller::classifyError($msg)); + } + + public function testClassifyHandlesWorkflowCommandEscapedNewlines(): void + { + // GitHub-Actions-escaped newlines arrive as %0A. + $msg = 'horde/listheaders -> found at alpha%0Abut it does not ' + . 'match your minimum-stability.'; + $this->assertSame('stability_gate', ComposerInstaller::classifyError($msg)); + } +} diff --git a/test/Unit/Ci/Setup/ExtensionInstallerTest.php b/test/Unit/Ci/Setup/ExtensionInstallerTest.php new file mode 100644 index 00000000..b8bb1ac4 --- /dev/null +++ b/test/Unit/Ci/Setup/ExtensionInstallerTest.php @@ -0,0 +1,201 @@ + + * @license http://www.horde.org/licenses/lgpl21 LGPL 2.1 + */ + +declare(strict_types=1); + +namespace Horde\Components\Test\Unit\Ci\Setup; + +use Horde\Components\Ci\Setup\ExtensionInstaller; +use Horde\Components\Output; +use PHPUnit\Framework\Attributes\CoversClass; +use PHPUnit\Framework\TestCase; + +/** + * Tests for the F28 per-PHP-minor extension resolution path. + * + * Covers the read-side: how ExtensionInstaller turns a `.horde.yml` + * ci-platform block into per-lane extension sets. The actual apt-get + * install path (installExtension / isExtensionInstalled / SudoHelper) + * shells out and is intentionally not covered here — those paths need + * an integration test with a real PHP install. + */ +#[CoversClass(ExtensionInstaller::class)] +class ExtensionInstallerTest extends TestCase +{ + private string $tmpDir = ''; + private Output $output; + private ExtensionInstaller $installer; + + protected function setUp(): void + { + $this->tmpDir = sys_get_temp_dir() . '/horde-ext-installer-test-' . uniqid(); + mkdir($this->tmpDir, 0o755, true); + $this->output = $this->createMock(Output::class); + $this->installer = new ExtensionInstaller($this->output); + } + + protected function tearDown(): void + { + if (is_dir($this->tmpDir)) { + exec('rm -rf ' . escapeshellarg($this->tmpDir)); + } + } + + public function testDetectExtensionsPerVersionFallsBackWhenNoHordeYml(): void + { + // No .horde.yml — every version gets the baseline (plus + // composer.json if present, which it isn't either). + $result = $this->installer->detectExtensionsPerVersion( + $this->tmpDir, + 'AnyComponent', + ['8.3', '8.4'] + ); + $this->assertSame(['8.3', '8.4'], array_keys($result)); + // Baseline contains intl, mbstring, etc. — the sentinel we check + // for is one of the always-present values. + $this->assertContains('intl', $result['8.3']); + $this->assertContains('intl', $result['8.4']); + // Both versions get the same flat fallback set. + $this->assertSame($result['8.3'], $result['8.4']); + } + + public function testDetectExtensionsPerVersionReadsCiPlatformList(): void + { + $this->writeHordeYml(<<installer->detectExtensionsPerVersion( + $this->tmpDir, + 'Sample', + ['8.3', '8.4'] + ); + + // Each minor must contain the ci-platform-resolved exts ON TOP + // of the baseline — the baseline covers what the resolver does + // not surface (php built-ins consumers implicitly rely on). + $this->assertContains('bcmath', $result['8.3']); + $this->assertContains('curl', $result['8.3']); + $this->assertNotContains('sodium', $result['8.3'], '8.3 must not pick up 8.4-only extensions'); + + $this->assertContains('bcmath', $result['8.4']); + $this->assertContains('sodium', $result['8.4']); + // curl is in BASELINE_EXTENSIONS, so both lanes carry it + // regardless of what ci-platform listed for that lane. We only + // care that 8.4 got its specific ci-platform addition (sodium) + // and 8.3 did not. + $this->assertContains('curl', $result['8.4'], 'curl comes from baseline'); + $this->assertNotContains('sodium', $result['8.3'], '8.3 must not pick up 8.4-only ci-platform exts'); + + // Composer-meta and php keys must NOT appear as installable + // extensions — they are not ext-* and don't apt-get install. + $this->assertNotContains('composer-runtime-api', $result['8.3']); + $this->assertNotContains('php', $result['8.3']); + } + + public function testDetectExtensionsPerVersionTreatsNotResolvableAsFallback(): void + { + $this->writeHordeYml(<<installer->detectExtensionsPerVersion( + $this->tmpDir, + 'Sample', + ['8.3', '8.4'] + ); + + // 8.3 uses ci-platform: bcmath plus baseline + $this->assertContains('bcmath', $result['8.3']); + + // 8.4 falls back to flat detection because the resolver couldn't + // produce a lock. The fallback set equals the baseline (plus any + // composer.json hits). The maintainer fix is to chase down the + // resolver failure; CI keeps running on baseline meanwhile. + $this->assertNotContains('bcmath', $result['8.4']); + $this->assertContains('intl', $result['8.4']); + } + + public function testDetectExtensionsPerVersionRespectsMinorAbsentFromCiPlatform(): void + { + $this->writeHordeYml(<<installer->detectExtensionsPerVersion( + $this->tmpDir, + 'Sample', + ['8.3', '8.4'] + ); + + $this->assertContains('bcmath', $result['8.3']); + $this->assertNotContains('bcmath', $result['8.4']); + } + + public function testInstallPerVersionWithEmptyMapIsNoop(): void + { + // No exception, no install attempt. info() is allowed. + $this->assertTrue($this->installer->installPerVersion([])); + } + + private function writeHordeYml(string $contents): void + { + file_put_contents($this->tmpDir . '/.horde.yml', $contents); + } +} diff --git a/test/Unit/Ci/Setup/SetupCommandTest.php b/test/Unit/Ci/Setup/SetupCommandTest.php index 4bfb3bf5..fd6ea9a3 100644 --- a/test/Unit/Ci/Setup/SetupCommandTest.php +++ b/test/Unit/Ci/Setup/SetupCommandTest.php @@ -134,15 +134,15 @@ public function testGeneratesLaneScriptsAfterComposerInstall(): void return "/usr/bin/php{$version}"; }); - // Mock extension installer + // Mock extension installer (F28: per-version resolution) $this->extensionInstaller ->expects($this->once()) - ->method('detectExtensions') - ->willReturn([]); + ->method('detectExtensionsPerVersion') + ->willReturn(['8.4' => []]); $this->extensionInstaller ->expects($this->once()) - ->method('install'); + ->method('installPerVersion'); // Mock lane copier $this->laneCopier @@ -211,8 +211,8 @@ public function testLaneScriptGenerationFailureIsReported(): void $this->phpInstaller->method('getPhpBinary')->willReturnCallback(function ($v) { return "/usr/bin/php{$v}"; }); - $this->extensionInstaller->method('detectExtensions')->willReturn([]); - $this->extensionInstaller->method('install'); + $this->extensionInstaller->method('detectExtensionsPerVersion')->willReturn([]); + $this->extensionInstaller->method('installPerVersion'); $this->laneCopier->method('copyToLanes'); $this->composerInstaller->method('install'); $this->composerInstaller->method('verifyInstallation')->willReturn(true); @@ -261,8 +261,8 @@ public function testLaneScriptConfigContainsCorrectValues(): void $this->phpInstaller->method('getPhpBinary')->willReturnCallback(function ($v) { return "/usr/bin/php{$v}"; }); - $this->extensionInstaller->method('detectExtensions')->willReturn([]); - $this->extensionInstaller->method('install'); + $this->extensionInstaller->method('detectExtensionsPerVersion')->willReturn([]); + $this->extensionInstaller->method('installPerVersion'); $this->laneCopier->method('copyToLanes'); $this->composerInstaller->method('install'); $this->composerInstaller->method('verifyInstallation')->willReturn(true); @@ -338,8 +338,8 @@ public function testLaneScriptsGeneratedAfterComposerNotBefore(): void $this->phpInstaller->method('getPhpBinary')->willReturnCallback(function ($v) { return "/usr/bin/php{$v}"; }); - $this->extensionInstaller->method('detectExtensions')->willReturn([]); - $this->extensionInstaller->method('install'); + $this->extensionInstaller->method('detectExtensionsPerVersion')->willReturn([]); + $this->extensionInstaller->method('installPerVersion'); $this->laneCopier->method('copyToLanes'); $this->composerInstaller->method('install')->willReturnCallback(function () use (&$callOrder) { @@ -389,8 +389,8 @@ public function testNoLaneScriptsGeneratedIfComposerFails(): void $this->phpInstaller->method('getPhpBinary')->willReturnCallback(function ($v) { return "/usr/bin/php{$v}"; }); - $this->extensionInstaller->method('detectExtensions')->willReturn([]); - $this->extensionInstaller->method('install'); + $this->extensionInstaller->method('detectExtensionsPerVersion')->willReturn([]); + $this->extensionInstaller->method('installPerVersion'); $this->laneCopier->method('copyToLanes'); // Composer install succeeds but verification fails @@ -399,17 +399,112 @@ public function testNoLaneScriptsGeneratedIfComposerFails(): void $this->toolCache->method('ensureAllTools'); - // Lane script generator should still be called (we generate scripts even if composer has issues) - // This allows debugging of composer issues by inspecting the lane + // F30: a lane whose composer install or verification failed is + // recorded as setup-failed and gets no run-lane.sh. RunCommand + // surfaces it as ❌ via the build/setup-failed.json sidecar + // instead of running a broken script. $this->laneScriptGenerator - ->expects($this->atLeastOnce()) - ->method('generate') - ->willReturn(true); + ->expects($this->never()) + ->method('generate'); // Execute setup $result = $this->setupCommand->execute($config); - // Setup reports failure due to composer issues - $this->assertFalse($result, 'Setup should report failure when composer verification fails'); + // Both lanes fail setup → no usable lanes → return false so + // Module/Ci raises the fatal-error exception (every lane dead). + $this->assertFalse($result, 'Setup should report failure when every lane fails composer verification'); + } + + /** + * F30: partial setup must not abort. Half the lanes fail composer + * install, the other half succeed; setup returns true (some lanes + * are still runnable) and the failing lanes carry a build/setup-failed.json + * marker so RunCommand reports them as ❌ in the PR comment instead + * of dying before any tests run. + */ + public function testPartialSetupSucceedsAndMarksFailedLanes(): void + { + $workDir = $this->tempDir . '/work'; + $config = new CiConfig([ + 'mode' => 'local', + 'component_name' => 'TestComponent', + 'component_path' => $this->testComponentPath, + 'work_dir' => $workDir, + 'php_versions' => ['8.4'], + 'min_php_version' => '8.2', + 'component_stability' => 'stable', + 'local_components_path' => '/usr/bin/horde-components', + 'components_path' => '/usr/bin/horde-components', + ]); + + $this->phpInstaller->method('install'); + $this->phpInstaller->method('getPhpBinary')->willReturnCallback(function ($v) { + return "/usr/bin/php{$v}"; + }); + $this->extensionInstaller->method('detectExtensionsPerVersion')->willReturn([]); + $this->extensionInstaller->method('installPerVersion'); + + // LaneCopier is mocked so the lane directories don't actually + // get created by the test infrastructure. Create them by hand + // so writeSetupFailureMarker has somewhere to land its sidecar. + $devLane = $workDir . '/lanes/php8.4-dev/TestComponent'; + $stableLane = $workDir . '/lanes/php8.4-stable/TestComponent'; + $this->laneCopier + ->method('copyToLanes') + ->willReturnCallback(function () use ($devLane, $stableLane): bool { + mkdir($devLane, 0o755, true); + mkdir($stableLane, 0o755, true); + return true; + }); + + // Dev lane installs cleanly; stable lane verification fails. + // The matching call to writeSetupFailureMarker must produce a + // sidecar JSON at $stableLane/build/setup-failed.json. + $this->composerInstaller->method('install'); + $this->composerInstaller + ->method('verifyInstallation') + ->willReturnCallback(function (string $dir) use ($devLane): bool { + return $dir === $devLane; + }); + $this->composerInstaller + ->method('getInstalledPackageCount') + ->willReturn(42); + + $this->toolCache->method('ensureAllTools'); + + // Only the dev lane should get a run-lane.sh; the failed lane is + // skipped at script-generation time. + $generatedFor = []; + $this->laneScriptGenerator + ->method('generate') + ->willReturnCallback(function (string $scriptPath, array $cfg) use (&$generatedFor): bool { + $generatedFor[] = $cfg['lane_name']; + return true; + }); + + $result = $this->setupCommand->execute($config); + + $this->assertTrue( + $result, + 'Setup should return true when at least one lane is usable' + ); + $this->assertSame( + ['php8.4-dev'], + $generatedFor, + 'Only successful lanes should get run-lane.sh' + ); + $this->assertFileExists( + $stableLane . '/build/setup-failed.json', + 'Failed lane must carry a setup-failed.json sidecar' + ); + + $marker = json_decode( + (string) file_get_contents($stableLane . '/build/setup-failed.json'), + true + ); + $this->assertIsArray($marker); + $this->assertTrue($marker['setup_failed'] ?? false); + $this->assertSame(['phpunit', 'phpstan'], $marker['tools'] ?? null); + $this->assertNotEmpty($marker['reason'] ?? ''); } }