Merged
Conversation
Member
ralflang
commented
Mar 3, 2026
ralflang
commented
- CS: We want to keep PHP 8.2 baseline for now
- feat(ci): add Phase 0 template infrastructure
- feat(ci): add Phase 1 core infrastructure classes
- feat(ci): add SetupCommand and InitCommand orchestrators
- feat(ci): add CLI module for ci commands
- feat(ci): register Ci module in ModuleProvider
- refactor(ci): integrate SudoHelper for all privileged operations
- fix(ci): preserve empty objects in composer.json during stability modification
- docs(ci): add TODO for JSON empty object/array handling
- refactor(ci): use stdClass for JSON manipulation in ComposerInstaller
- refactor(ci): use Horde\Composer\ComposerJsonFile for JSON manipulation
- feat(ci): implement CI run command (Phase 2)
- fix(ci): correct QC command syntax and path handling
- feat(ci): add tools cache system for QC binaries
- fix(ci): remove phpunit from components dev dependencies
- feat(ci): disable platform check and prioritize version-specific PHPUnit
- feat(ci): complete GitHub Actions workflow template integration
Add template rendering system for CI configuration files: Template Infrastructure: - TemplateRenderer: Renders templates with variable substitution - TemplateLocator: Finds template files in data/ci/ directory - TemplateVersion: Extracts and compares template versions Templates: - bootstrap-github.sh.template: GitHub Actions bootstrap script - bootstrap-local.sh.template: Local development bootstrap script - workflow.yml.template: GitHub Actions workflow configuration Documentation: - README.md: Template usage guide - template-variables.md: Variable reference Tests: - TemplateRendererTest: Full test coverage for rendering logic All tests passing (10/10). Related to components-ci-implementation-plan.md Phase 0.
Add core classes for CI setup (local mode): Configuration: - CiConfig: Configuration container with lane management - EnvironmentDetector: Detect GitHub/local mode, extract env vars Setup Components: - PhpInstaller: Install PHP 8.2-8.5 via ondrej PPA - ExtensionInstaller: Install extensions (baseline + composer.json + mapping) - LaneCopier: Copy component to test lanes (8 directories) - ComposerInstaller: Run composer install per lane with stability control Features: - Hybrid extension detection strategy - Composer retry logic (3 attempts with exponential backoff) - Lane isolation (separate dirs per PHP × stability) - Stability control via composer.json modification - Verification methods for all operations All classes use PHP 8.2+ strict types and proper error handling. No tests yet - focusing on structure first. Related to components-ci-implementation-plan.md Phase 1.
Add main command orchestrators for Phase 1: SetupCommand: - Main CI setup orchestrator - Coordinates: validate → install PHP → install extensions → copy lanes → composer install - Reads component metadata from .horde.yml (simple YAML parser) - Validates component type (library only in Phase 1) - Reports setup progress with detailed output - Returns success/failure with lane summary InitCommand: - Generate CI files from templates (ci init) - Creates bin/ci-bootstrap.sh from template - Creates .github/workflows/ci.yml (github mode) - Template version checking (detect outdated files) - Force overwrite and dry-run support - Check command (ci check) to validate existing files Features: - Smart configuration detection (from env, .horde.yml, defaults) - Template variable substitution - File existence checking with version comparison - Executable permission setting for shell scripts - Preview mode (--dry-run) to see generated content - Comprehensive error messages Both commands use dependency injection for testability. Related to components-ci-implementation-plan.md Phase 1.
Add Module/Ci.php to expose CI functionality via horde-components CLI. Commands Available: - ci init [--ci-mode=MODE] [--force] [--dry-run] Generate CI configuration files from templates - ci check Validate existing CI files and check for updates - ci setup [--ci-mode=MODE] [--work-dir=DIR] Setup CI environment (install PHP, extensions, prepare lanes) - ci run Execute tests (Phase 2 - not yet implemented) Features: - Follows existing module pattern (extends Base, implements handle()) - Comprehensive help text with examples and troubleshooting - Option group for CI-specific flags (--ci-mode, --work-dir, --force, etc.) - Delegates to InitCommand and SetupCommand - Error handling with user-friendly messages - Mode auto-detection (GitHub Actions vs local) Integration: - Uses dependency injection (Output from injector) - Creates command instances with proper dependencies - Follows horde-components conventions The CI commands are now accessible via: horde-components ci <subcommand> horde-components help ci Related to components-ci-implementation-plan.md Phase 1.
Add Ci module to ModuleProvider so it's loaded and available via CLI. Changes: - Import Horde\Components\Module\Ci - Add Ci module to getModules() array (alphabetically after Change) Commands now work: - horde-components ci - horde-components ci init - horde-components ci check - horde-components ci setup - horde-components help ci Verified working with manual tests. Completes Phase 1 CLI integration.
Update PhpInstaller and ExtensionInstaller to use the centralized SudoHelper class for all operations requiring root privileges. Changes: - Add SudoHelper.php with static methods for sudo operations - Update PhpInstaller to use SudoHelper::addPpa(), installPhp(), isPhpInstalled() - Update ExtensionInstaller to use SudoHelper::installExtension() - Simplify hasSudo() check using SudoHelper::canRunPasswordless() - Remove direct sudo command execution from both classes All privileged operations now go through /usr/local/bin/horde-ci-sudo-helper which validates all arguments and provides a single sudo entry point.
…ification
When json_decode() converts JSON to arrays, empty objects {} become empty
arrays []. This causes composer schema validation to fail for fields like
allow-plugins which must be object or boolean, not array.
Fix by adding regex replacement to convert "allow-plugins": [] back to
"allow-plugins": {} after json_encode().
Also fix timeout command syntax - wrap in bash -c so timeout can work with
shell constructs like cd && command.
Fixes composer install failures in all test lanes.
Document the need to improve JSON handling in ComposerInstaller. Current regex workaround works but a proper solution using stdClass or a JSON library would be more robust for Phase 2 and beyond.
Replace array-based JSON manipulation with stdClass objects to properly
preserve empty objects as {} instead of []. This matches the approach used
in Helper/Composer.php and avoids the need for regex workarounds.
Changes:
- Use json_decode($content, false) to preserve objects
- Use $data->{'minimum-stability'} for property assignment
- Remove regex workaround for allow-plugins field
- Empty objects are now naturally preserved as {} in JSON output
Tested with composer validate - schema is now valid without workarounds.
Replace manual stdClass JSON manipulation with Horde\Composer\ComposerJsonFile library which provides a cleaner, more robust API. Benefits: - Uses existing Horde library (already a dependency) - Proper validation and error handling built-in - Cleaner API: setMinimumStability() + save() - Reduces code from 23 lines to 8 lines - Maintains correct empty object handling The ComposerJsonFile class internally uses stdClass and handles all the JSON encoding/decoding details correctly.
Add ci run subcommand that executes tests across all lanes using the existing QC system via self-invocation. This approach reuses all existing test runners (PHPUnit, PHPStan, PHP CS Fixer) and reads structured JSON results. Architecture: - RunCommand orchestrates test execution across lanes - Each lane invokes: php8.X horde-components qc --unit --phpstan - ResultCollector aggregates JSON from build/ directories - Returns exit code: 0 if all passed, 1 if any failed Key files: - src/Ci/Run/RunCommand.php - Orchestrates QC execution per lane - src/Ci/Run/ResultCollector.php - Aggregates JSON results - src/Module/Ci.php - Added handleRun() method Deleted redundant files (saved ~450 lines): - src/Ci/Run/LaneRunner.php - Duplicated QC system - src/Ci/Run/TestResult.php - Over-engineered Benefits: - 100% reuse of existing QC infrastructure - Consistent with manual QC runs - Clean separation: QC executes, CI orchestrates - Structured JSON enables rich reporting - Easy to add parallelism in future Usage: horde-components ci run --work-dir=/tmp/horde-ci See ~/horde-development/components-ci-phase2-implementation-complete.md
Fix Phase 2 issues discovered during testing: 1. QC command syntax: Use positional arguments (qc unit phpstan) not options 2. Component directory: Lanes have subdirectory structure (lane/Http/) - use component_dir 3. Output methods: Use error() not fail() to avoid fatal termination 4. Option parsing: Horde_Argv converts dashes to underscores (work_dir not work-dir) Changes: - RunCommand: Discover component_dir subdirectory in each lane - RunCommand: Use correct QC syntax (qc unit phpstan phpcsfixer) - RunCommand: cd to component_dir not lane dir when invoking QC - ResultCollector: Use output->error() instead of output->fail() - Module/Ci: Use $options['work_dir'] not ['work-dir'] Known issue: Lanes need composer install --dev to get test dependencies (PHPUnit, PHPStan). Will be fixed in Phase 1 ComposerInstaller.
Implement tools cache architecture to download and cache QC tool PHARs instead of requiring them in component vendor directories. This allows: - Components to not require PHPUnit/PHPStan/PHP-CS-Fixer as dependencies - Version-specific tools per PHP version (PHPUnit 11.5 for 8.2-8.3, 12.5 for 8.4+) - Fast setup without composer installing test tools in every lane - Shared tools across all lanes in both local and GitHub Actions modes Architecture: - ToolCache downloads PHARs to $WORK_DIR/tools/ - SetupCommand runs tool download after composer install - QC tasks accept --tools-dir option - ToolFinder prioritizes tools-dir over other locations - RunCommand passes --tools-dir to QC invocations Changes: - NEW: src/Ci/Setup/ToolCache.php - Download and cache PHAR files - Modified: SetupCommand - Add tool download step - Modified: Module/Ci - Wire up ToolCache, pass workDir to RunCommand - Modified: Module/Qc - Add --tools-dir option - Modified: ToolFinder - Accept tools-dir as highest priority search location - Modified: Qc/Task/Unit - Accept tools_dir from options, pass to ToolFinder - Modified: Qc/Task/Phpstan - Accept tools_dir, use ToolFinder - Modified: Qc/Task/Phpcsfixer - Accept tools_dir, use ToolFinder - Modified: RunCommand - Pass --tools-dir to QC command invocations - Modified: ComposerInstaller - Revert to default composer install (no --dev flag) Tool versions: - PHPUnit: 11.5 for PHP 8.2-8.3, 12.5 for PHP 8.4-8.5 - PHPStan: Latest (works on all PHP versions) - PHP-CS-Fixer: Latest (runs on PHP 8.4 only) Benefits: - Components don't need test tools in composer.json - Third parties can use conflicting tool versions - Faster CI setup (no dev dependency installation per lane) - Consistent tool versions across all environments - Works identically in local and GitHub Actions modes Related: Phase 2 CI run implementation
Remove PHPUnit from horde-components' own composer.json so it doesn't conflict with component-specific PHPUnit versions loaded via tools-dir. This allows QC tasks to load PHPUnit from cached tools instead of components' vendor directory. Result: PHP 8.4+ lanes now work correctly with tools cache! Known issue: PHP 8.2-8.3 lanes fail because ToolFinder doesn't recognize version-specific PHAR filenames (phpunit-11.5.phar). It finds system PHPUnit 13 which is incompatible with PHP < 8.4. TODO: Update ToolFinder.findBinary() to: 1. Prioritize tools-dir over all other locations 2. Recognize version-specific PHARs (phpunit-11.5.phar, phpunit-12.5.phar) 3. Select appropriate version based on PHP version
Changes:
1. Disable composer platform-check to allow running with PHP 8.2+
- components requires PHP ^8.2 but dependencies may require 8.3
- Only one readonly class usage (PHP 8.2 feature) - fully compatible
2. Update ToolFinder to select PHPUnit version based on PHP_VERSION_ID
- PHP < 8.4: prioritize phpunit-11.5.phar, phpunit-11.phar
- PHP >= 8.4: prioritize phpunit-12.5.phar, phpunit-12.phar
- Tools-dir checked before standard locations
Testing confirms:
- ToolFinder.findBinary('phpunit') correctly finds phpunit-11.5.phar on PHP 8.2
- ToolFinder.loadTool() successfully loads PHPUnit 11.5.55 from PHAR
- PHP 8.4-8.5 lanes work fully
- PHP 8.2-8.3 lanes still finding wrong PHPUnit (needs investigation)
Next: Debug why Unit task doesn't use ToolFinder result
Phase 3 implementation: 1. Update workflow template: - Add workflow_dispatch for manual triggers - Add PHP extensions (json, mbstring, curl) - Add QC tools caching with actions/cache@v4 - Add artifact upload for test results - Add organization variable support (vars.COMPONENTS_PHAR_URL) - Add fallback to default URL if org variable not set 2. Update default PHAR URL: - Change from dev.horde.org to GitHub releases - Point to /latest/ for automatic updates - Document organization variable approach 3. Auto-detection already working: - PresenterFactory already detects CI environment - Automatically uses Ci presenter in GitHub Actions - Outputs workflow commands (::notice::, ::warning::, ::error::) Template features: - Tool caching: key based on .horde.yml hash - Organization variable with fallback - Artifact upload with 30-day retention - Manual trigger support This completes the GitHub Actions integration infrastructure. Next step: real GitHub Actions testing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.