diff --git a/cmd/phpup/main.go b/cmd/phpup/main.go index 5662ecb..47ffdc1 100644 --- a/cmd/phpup/main.go +++ b/cmd/phpup/main.go @@ -396,16 +396,29 @@ func main() { log.Fatalf("select base ini file: %v", err) } - // 9b. Compose compat ini layers: defaults → xdebug fragment (only when - // coverage: xdebug drives the install — matches v2, which applies - // xdebug.ini only in the coverage flow, NOT when xdebug is loaded via - // extensions:) → ExtraIni (e.g. pcov.enabled=1) → user ini-values. + // 9b. Compose compat ini layers in increasing precedence (last write wins): + // 1. StockIniDefaults — always (php.ini-production keys v2 ships) + // 2. DefaultIniValues — always (date.timezone, memory_limit) + // 3. OpcacheIniFragment — only when opcache is loaded (mirrors the + // auto-load gate at step 8b above; reuses opcacheExcluded) + // 4. XdebugIniFragment — only when coverage: xdebug drove the install + // (matches v2, which applies xdebug.ini only in the coverage flow, + // NOT when xdebug is loaded via extensions:) + // 5. ExtraIni — caller-injected (e.g. pcov.enabled=1) + // User-supplied ini-values are then layered on top by + // WriteIniValuesWithDefaults. + var opcacheFrag map[string]string + if slices.Contains(bundled, "opcache") && !opcacheExcluded { + opcacheFrag = compat.OpcacheIniFragment(p.PHPVersion, p.Arch) + } var xdebugFrag map[string]string if p.Coverage == plan.CoverageXdebug { xdebugFrag = compat.XdebugIniFragment(p.PHPVersion) } layered := compose.MergeCompatLayers( + compat.StockIniDefaults(), compat.DefaultIniValues(p.PHPVersion, p.Arch), + opcacheFrag, xdebugFrag, p.ExtraIni, ) diff --git a/docs/compat-matrix.md b/docs/compat-matrix.md index d7112b4..9bf861b 100644 --- a/docs/compat-matrix.md +++ b/docs/compat-matrix.md @@ -145,15 +145,36 @@ per-SAPI `99-pecl.ini` scan-dir file when the version matches | --- | --- | --- | | `date.timezone` | `UTC` | `src/configs/ini/php.ini` L1 | | `memory_limit` | `-1` | `src/configs/ini/php.ini` L2 | +| `expose_php` | `1` | `php.ini-production` L400; loaded via `add_php_config` (`src/scripts/linux.sh` L264-283) | +| `log_errors` | `1` | `php.ini-production` L523; same load path | +| `max_input_time` | `-1` | Compiled-in CLI-SAPI default; see note below | +| `session.save_handler` | `files` | `php.ini-production` L1265; same load path | +| `session.gc_maxlifetime` | `1440` | `php.ini-production` L1376; same load path | + +**Note on `max_input_time`:** stock `php.ini-production` declares +`max_input_time = 60` (L419), but `max_input_time` is `PHP_INI_PERDIR` and +only takes effect under FPM/CGI SAPIs. On CLI (the SAPI used by both v2's +probe and our canonical-cell probe) the directive falls back to its +compiled-in default of `-1`. Encoded as `-1` to match what +`ini_get('max_input_time')` actually returns under v2. + +Source: stock `php.ini-production` at upstream commit +`171b722edc1914a2d8f668e880febfa03a60af7e` +(https://raw.githubusercontent.com/php/php-src/171b722edc1914a2d8f668e880febfa03a60af7e/php.ini-production). +Encoded in `internal/compat.StockIniDefaults()`. ### 2.2 `src/configs/ini/xdebug.ini` (applied when PHP version matches `7.[2-4]|8.[0-9]`) | Key | Value | Condition | Source | | --- | --- | --- | --- | | `xdebug.mode` | `coverage` | Version matches `xdebug3_versions` — PHP 7.2 through 8.9. | `src/configs/ini/xdebug.ini` L1; `src/scripts/unix.sh` L9, L254 | +| `xdebug.start_with_request` | `default` | Version matches `xdebug3_versions` — PHP 7.2 through 8.9. | xdebug compiled-in default (not set by v2's `xdebug.ini`); `src/scripts/unix.sh` L9, L254 | -Note: this line is written even if xdebug is not installed. It is a no-op in -that case because PHP ignores ini keys for unloaded extensions. +Note: the `xdebug.mode` line is written even if xdebug is not installed. It +is a no-op in that case because PHP ignores ini keys for unloaded extensions. +`xdebug.start_with_request` is xdebug's own compiled-in default (`default`) +and is not explicitly written by v2; it appears in the probe output whenever +xdebug is loaded. ### 2.3 `src/configs/ini/jit.ini` (applied when PHP version matches `8.[0-9]`) @@ -162,6 +183,16 @@ that case because PHP ignores ini keys for unloaded extensions. | `opcache.enable` | `1` | PHP 8.0–8.9 (`jit_versions` regex). | `src/configs/ini/jit.ini` L1; `src/scripts/unix.sh` L8, L256 | | `opcache.jit_buffer_size` | `256M` | PHP 8.0–8.9. | `src/configs/ini/jit.ini` L2 | | `opcache.jit` | `1235` | PHP 8.0–8.9. | `src/configs/ini/jit.ini` L3 | +| `opcache.enable_cli` | `0` | PHP 8.0–8.9 (`jit_versions` regex). | `php.ini-production` L1669 at `171b722edc1914a2d8f668e880febfa03a60af7e`; load path same as §2.1 | +| `opcache.memory_consumption` | `128` | PHP 8.0–8.9. | `php.ini-production` L1672 at `171b722edc1914a2d8f668e880febfa03a60af7e` | +| `opcache.revalidate_freq` | `2` | PHP 8.0–8.9. | `php.ini-production` L1697 at `171b722edc1914a2d8f668e880febfa03a60af7e` | +| `opcache.validate_timestamps` | `1` | PHP 8.0–8.9. | `php.ini-production` L1692 at `171b722edc1914a2d8f668e880febfa03a60af7e` | + +**Note:** these four keys originate from upstream `php.ini-production`, not +v2's `src/configs/ini/jit.ini`. We ship them in the same Go fragment +(`internal/compat.OpcacheIniFragment`) as the three jit.ini keys above +because their applicability gate is identical (PHP 8.x AND opcache loaded) +and the runtime overlays one merged map per run. The JIT block is written to `$pecl_file` (`$scan_dir/99-pecl.ini`) — not to the main `php.ini` — so user `ini-values` applied later (which tee into diff --git a/docs/superpowers/specs/2026-04-27-stock-php-ini-defaults-design.md b/docs/superpowers/specs/2026-04-27-stock-php-ini-defaults-design.md new file mode 100644 index 0000000..8b3a2ca --- /dev/null +++ b/docs/superpowers/specs/2026-04-27-stock-php-ini-defaults-design.md @@ -0,0 +1,296 @@ +# Design: Emit stock PHP ini defaults to match shivammathur/setup-php@v2 + +**Issue:** [#79](https://github.com/buildrush/setup-php/issues/79) +**Date:** 2026-04-27 +**Status:** Approved (brainstorming) + +## Context + +PR #77 wired a PR-time compat-diff gate against a pinned shivammathur/setup-php@v2 +baseline. The canonical cell (`noble/x86_64/8.4` in `ci.yml::pipeline`) goes red +because `phpup install` returns empty for ~12 ini keys per fixture that v2 +emits. These are pre-existing gaps in our compose layer, not regressions +introduced by #77. + +The 12 missing keys span three sources: + +| Source | Keys | +| --- | --- | +| Stock upstream `php.ini-production` (v2 ships verbatim) | `expose_php`, `log_errors`, `max_input_time`, `session.save_handler`, `session.gc_maxlifetime` | +| Stock upstream `php.ini-production` opcache lines + v2's `src/configs/ini/jit.ini` | `opcache.enable`, `opcache.enable_cli`, `opcache.memory_consumption`, `opcache.revalidate_freq`, `opcache.validate_timestamps`, `opcache.jit`, `opcache.jit_buffer_size` | +| v2's `src/configs/ini/xdebug.ini` (applied under `coverage: xdebug`) | `xdebug.mode`, `xdebug.start_with_request` | + +Note that the issue grouped the four "stock opcache" keys under v2's `jit.ini`, +but the audit at `docs/compat-matrix.md` §2.3 (pinned SHA `accd6127…`) confirms +`jit.ini` ships only `opcache.enable`, `opcache.jit`, `opcache.jit_buffer_size`. +The other four (`enable_cli`, `memory_consumption`, `revalidate_freq`, +`validate_timestamps`) originate from upstream `php.ini-production`. We ship +them together because their applicability gate is identical (PHP 8.x and opcache +loaded), and our pipeline overlays one merged map per run. + +The runtime already has the right composition primitives — `compose.SelectBaseIniFile`, +`compose.MergeCompatLayers`, `compose.WriteIniValuesWithDefaults`, and three +existing compat fragments (`DefaultIniValues`, `XdebugIniFragment`, +`BaseIniFileName`). The fix is additive: extend the compat-data layer and add +two more layers to the merge pipeline. + +## Goals + +1. Canonical cell `noble/x86_64/8.4` in `ci.yml::pipeline` goes green without + any new `kind: ignore` allowlist suppression in `docs/compat-matrix.md`. +2. The weekly `compat-golden-refresh.yml` run continues to show zero drift. +3. The 12 keys' provenance is documented in `docs/compat-matrix.md` so the + audit chain (upstream → compat-matrix → `internal/compat` constant) stays + 1:1. + +## Non-goals + +- Investigating or fixing whether `PHPRC=/usr/local/lib/php.ini` actually + loads the bundle's stock `php.ini-production`. The encode-in-Go approach + bypasses that question; if PHPRC is broken, file a separate issue. +- Aligning non-canonical cells (other PHP minors, Ubuntu jammy, aarch64) — the + gate currently only fires on the canonical cell. The fragments are written + arch- and version-aware, so other cells benefit automatically once the gate + is extended to them. +- Replicating divergent `ini-file: development` or `ini-file: none` semantics. + The new defaults apply unconditionally regardless of `ini-file:` input, + matching the existing precedent for `date.timezone`/`memory_limit`. + +## Approach + +Encode the 12 missing values in `internal/compat`, exposed via two new +functions and a one-key extension of an existing function. Apply them through +the existing `compose.MergeCompatLayers` pipeline by adding two new layers. +Do not rely on the bundle's stock `php.ini-production` being loaded by PHP at +runtime — overlay everything via `99-user.ini`, which loads after `php.ini` +regardless of how the SAPI resolves its base config. + +This is the encode-everything-in-Go option from the brainstorming +discussion. It was preferred over root-causing PHPRC behavior because: + +- The compose pipeline and the `compat` data layer already exist; we only grow + data, no new infrastructure. +- Each key becomes unit-testable in isolation against a golden file, mirroring + the existing `default_ini_values.golden` pattern. +- The audit chain (compat-matrix.md → `internal/compat` constant → unit-test + golden → cell-test compat-diff) is uniform per key. +- It is independent of bundle internals: a future bundle change can't silently + drift the emitted ini surface. + +## Detailed design + +### 1. `internal/compat/compat.go` + +Three changes: + +**a) New `StockIniDefaults()`** — keys v2 emits unconditionally on every Linux +run via stock `php.ini-production`: + +```go +// StockIniDefaults returns the ini key/value pairs that shivammathur/setup-php@v2 +// applies to every Linux run via the stock upstream php.ini-production it +// ships. Always-on, no version or arch gate. +// +// Codifies values v2 actually emits (per the canonical-cell golden), not +// necessarily the literal contents of php.ini-production. For example, +// max_input_time=-1 reflects PHP's CLI-SAPI compiled-in default, which +// overrides php.ini-production's "60" because max_input_time is PHP_INI_PERDIR +// and only takes effect under FPM/CGI. +// +// Data sources: docs/compat-matrix.md §2.1 extended; golden files +// testdata/stock_ini_defaults.golden. +func StockIniDefaults() map[string]string { + return map[string]string{ + "expose_php": "1", + "log_errors": "1", + "max_input_time": "-1", + "session.save_handler": "files", + "session.gc_maxlifetime": "1440", + } +} +``` + +**b) New `OpcacheIniFragment(phpVersion, arch string) map[string]string`** — +combines v2's `jit.ini` (3 keys) with the stock `php.ini-production` opcache +lines (4 keys). Returns `nil` for non-8.x versions. + +```go +func OpcacheIniFragment(phpVersion, arch string) map[string]string { + if !isPHP8x(minorOf(phpVersion)) { + return nil + } + out := map[string]string{ + "opcache.enable": "1", + "opcache.enable_cli": "0", + "opcache.memory_consumption": "128", + "opcache.revalidate_freq": "2", + "opcache.validate_timestamps": "1", + "opcache.jit": "1235", + } + if arch == "aarch64" { + out["opcache.jit_buffer_size"] = "128M" + } else { + out["opcache.jit_buffer_size"] = "256M" + } + return out +} +``` + +**c) `XdebugIniFragment` grows by one key** — `xdebug.start_with_request=default`. +Same gate as today (`xdebug3Supported(minor)`), same caller-side condition +(`coverage: xdebug` drove the install). + +**d) `DefaultIniValues` shrinks** — removes the three opcache keys it +currently writes (now in `OpcacheIniFragment`). Keeps `date.timezone=UTC` and +`memory_limit=-1` (compat-matrix §2.1 base, applied unconditionally). + +### 2. `internal/compose/compose.go` + +Replace `MergeCompatLayers(defaults, xdebugFragment, extra map[string]string)` +with a variadic helper: + +```go +// MergeCompatLayers returns a single ini-key map composed of the given layers +// in increasing precedence (last write wins). Any layer may be nil. +// The returned map is always non-nil (may be empty). +func MergeCompatLayers(layers ...map[string]string) map[string]string { + merged := make(map[string]string) + for _, layer := range layers { + maps.Copy(merged, layer) + } + return merged +} +``` + +Variadic was chosen over growing a fixed-arity signature each time we add a +fragment. Three layers today, five tomorrow, possibly more. + +### 3. `cmd/phpup/main.go` — wiring + +Replace the current 3-layer call at `main.go:407`: + +```go +layered := compose.MergeCompatLayers( + compat.DefaultIniValues(p.PHPVersion, p.Arch), + xdebugFrag, + p.ExtraIni, +) +``` + +with a 5-layer composition: + +```go +var opcacheFrag map[string]string +if slices.Contains(bundled, "opcache") && !opcacheExcluded { + opcacheFrag = compat.OpcacheIniFragment(p.PHPVersion, p.Arch) +} + +layered := compose.MergeCompatLayers( + compat.StockIniDefaults(), // always + compat.DefaultIniValues(p.PHPVersion, p.Arch), // always + opcacheFrag, // only if opcache loaded + xdebugFrag, // only if coverage: xdebug + p.ExtraIni, // user-configured +) +``` + +The `opcacheExcluded` variable already exists at `main.go:374`. The opcache +gate reuses it for symmetry with the auto-load logic at `main.go:376`. If +opcache isn't loaded, the keys would be no-ops (PHP ignores ini for unloaded +extensions) but we keep the on-disk fragment clean — same precedent as the +"deliberate divergence from v2" comment on `XdebugIniFragment`. + +### 4. Tests (TDD) + +**`internal/compat/compat_test.go`** — write before implementation: + +- `TestStockIniDefaults` — exact map equality against new + `testdata/stock_ini_defaults.golden`. No version/arch parameters. +- `TestOpcacheIniFragment` — table test: + - PHP 7.4: returns `nil`. + - PHP 8.0–8.5 × {x86_64, aarch64}: equality against + `testdata/opcache_ini_fragment_x86_64.golden` and + `testdata/opcache_ini_fragment_aarch64.golden`. +- `TestXdebugIniFragment` — extend existing test (or add) to assert both + `xdebug.mode=coverage` AND `xdebug.start_with_request=default` for + xdebug3-supported versions; `nil` for unsupported. +- `TestDefaultIniValues` — adjust existing assertions to reflect the removal + of opcache keys. Update or delete `testdata/default_ini_values.golden` to + match the trimmed set. + +**`internal/compose/compose_test.go`** — extend `TestWriteIniValuesWithDefaults` +(or add `TestMergeCompatLayers`) to verify: + +- Variadic call with 0 layers returns an empty (non-nil) map. +- Last-write-wins ordering across 5 layers. +- `nil` layers are skipped without panic. + +**`cmd/phpup/main_test.go` (if such tests exist)** — verify the conditional +gates: opcache fragment present iff opcache is in the loaded set; xdebug +fragment present iff `Coverage == CoverageXdebug`. If `cmd/phpup` lacks unit +coverage of the wiring, rely on the `noble/x86_64/8.4` ci-cell as the +integration verification. + +### 5. Documentation — `docs/compat-matrix.md` + +- **§2.1** — extend the table with the 5 stock keys. Add a paragraph noting + `max_input_time=-1` is the CLI-SAPI value (overriding php.ini-production's + literal `60` because `max_input_time` is `PHP_INI_PERDIR` and FPM/CGI-scoped). + Cite upstream `php.ini-production` at the PHP-8.4 SHA used by Ondrej PPA at + the time of audit. +- **§2.3** — extend the table with the 4 additional opcache keys. Note that + while v2's `jit.ini` only ships 3 keys, our `OpcacheIniFragment` includes 4 + more from stock `php.ini-production` because the load condition (PHP 8.x + + opcache loaded) is identical, so we ship them together. +- **§2.2** — add `xdebug.start_with_request` row with value `default` and the + same condition as `xdebug.mode`. +- No `kind: ignore` allowlist entries are added or removed; this PR's job is + to close the gap, not to mask it. The acceptance criterion is zero + suppression. + +### 6. Quick PHPRC sanity check (orthogonal) + +During `make ci-cell OS=noble ARCH=x86_64 PHP=8.4` runs in development, also +print `php --ini` to confirm whether `/usr/local/lib/php.ini` is being +loaded as the base config (i.e. whether `SelectBaseIniFile` is currently +load-bearing or dead code at runtime). Findings: + +- If PHPRC is loading the file: the bundled `php.ini-production` is doing its + job and our overlay still wins via `99-user.ini` (loads later in scan-dir + order). No further action. +- If PHPRC is not loading the file: file a separate issue. Don't fix in this + PR — the encode-in-Go approach makes the behavior independent of PHPRC. + +This is a probe, not a deliverable. + +## Error handling + +No new error paths. Existing functions (`SelectBaseIniFile`, `WriteIniValuesWithDefaults`, +`MergeCompatLayers`) already handle empty/nil inputs and disk errors. The +variadic refactor of `MergeCompatLayers` preserves nil-safety per layer. + +## Migration / compatibility + +- **Existing callers of `MergeCompatLayers`**: only `cmd/phpup/main.go` + (verified by grep). The variadic signature is source-compatible at the call + site if all positional layers are passed — no breakage. +- **Goldens for `DefaultIniValues`**: shrink to remove the 3 opcache keys. + Test data file regenerated as part of this PR. +- **No bundle changes**: the bundle's `php.ini-production` is no longer + load-bearing for the 12 keys, but it remains shipped (build-php.sh:148) and + copied (`SelectBaseIniFile`). Behavior unchanged for any consumer of + `/usr/local/share/php/ini/php.ini-production`. + +## Verification + +1. `make check` passes. +2. Unit tests for `internal/compat` and `internal/compose` pass. +3. `make ci-cell OS=noble ARCH=x86_64 PHP=8.4` passes including the + compat-diff gate. +4. The compat-report sticky comment on the resulting PR is empty (or shows + "cleared") for the canonical cell. +5. Weekly `compat-golden-refresh.yml` next run shows zero drift. + +## Open questions + +None at this time. Move to implementation planning. diff --git a/internal/compat/compat.go b/internal/compat/compat.go index 265204c..e258a0c 100644 --- a/internal/compat/compat.go +++ b/internal/compat/compat.go @@ -81,33 +81,76 @@ func BaseIniFileName(iniFile string) (filename, warning string) { } } -// DefaultIniValues returns the ini key/value pairs that shivammathur/setup-php@v2 -// applies to every Linux run by default via its bundled ini templates. The -// result is keyed by PHP minor version AND architecture — aarch64 diverges from -// x86_64 on opcache.jit_buffer_size per v2's src/configs/ini/jit_aarch64.ini -// (see docs/compat-matrix.md §2.4). +// DefaultIniValues returns the always-on ini key/value pairs that shivammathur/setup-php@v2 +// applies on every Linux run via stock php.ini contents that are independent +// of PHP minor version and arch. // -// Extension-tied defaults (e.g. xdebug.mode=coverage) are applied at compose -// time by their respective handlers, not by this function. +// Conditional defaults live elsewhere: +// - Stock php.ini-production keys → StockIniDefaults +// - jit.ini + opcache.* php.ini-production lines → OpcacheIniFragment +// - xdebug.ini → XdebugIniFragment // -// Data sources: docs/compat-matrix.md §2.1 (base), §2.3 (x86_64 jit), -// §2.4 (aarch64 jit); golden file testdata/default_ini_values.golden pins the -// x86_64 form. -func DefaultIniValues(phpVersion, arch string) map[string]string { - base := map[string]string{ +// Data source: docs/compat-matrix.md §2.1 (base rows); golden file +// testdata/default_ini_values.golden. +func DefaultIniValues(_, _ string) map[string]string { + return map[string]string{ "date.timezone": "UTC", "memory_limit": "-1", } - if isPHP8x(minorOf(phpVersion)) { - base["opcache.enable"] = "1" - base["opcache.jit"] = "1235" - if arch == "aarch64" { - base["opcache.jit_buffer_size"] = "128M" - } else { - base["opcache.jit_buffer_size"] = "256M" - } +} + +// StockIniDefaults returns the ini key/value pairs that shivammathur/setup-php@v2 +// applies to every Linux run via the stock upstream php.ini-production it +// ships. Always-on, no version or arch gate. +// +// Codifies values v2 actually emits (per the canonical-cell golden), not +// necessarily the literal contents of php.ini-production. For example, +// max_input_time=-1 reflects PHP's CLI-SAPI compiled-in default, which +// overrides php.ini-production's "60" because max_input_time is PHP_INI_PERDIR +// and only takes effect under FPM/CGI. +// +// Data source: docs/compat-matrix.md §2.1; golden file +// testdata/stock_ini_defaults.golden. +func StockIniDefaults() map[string]string { + return map[string]string{ + "expose_php": "1", + "log_errors": "1", + "max_input_time": "-1", + "session.save_handler": "files", + "session.gc_maxlifetime": "1440", + } +} + +// OpcacheIniFragment returns the ini key/value pairs v2 ships under PHP 8.x +// when opcache is loaded. Combines v2's src/configs/ini/jit.ini (3 keys) with +// the stock php.ini-production opcache lines (4 keys), since both share the +// "PHP 8.x AND opcache loaded" gate. Returns nil for PHP versions outside +// jit_versions (8.[0-9]). +// +// arch-aware on opcache.jit_buffer_size: 256M on x86_64, 128M on aarch64 +// (compat-matrix §2.4). Caller is responsible for the "opcache loaded" gate; +// see the deliberate-divergence note on XdebugIniFragment for the rationale. +// +// Data source: docs/compat-matrix.md §2.3 (and §2.4); golden files +// testdata/opcache_ini_fragment_{x86_64,aarch64}.golden. +func OpcacheIniFragment(phpVersion, arch string) map[string]string { + if !isPHP8x(minorOf(phpVersion)) { + return nil + } + out := map[string]string{ + "opcache.enable": "1", + "opcache.enable_cli": "0", + "opcache.memory_consumption": "128", + "opcache.revalidate_freq": "2", + "opcache.validate_timestamps": "1", + "opcache.jit": "1235", } - return base + if arch == "aarch64" { + out["opcache.jit_buffer_size"] = "128M" + } else { + out["opcache.jit_buffer_size"] = "256M" + } + return out } func isPHP8x(minor string) bool { @@ -185,6 +228,9 @@ func OurBuildBundledExtras(phpVersion string) []string { // xdebug3 is active for the requested PHP version (matches regex 7.[2-4]|8.[0-9] // per docs/compat-matrix.md §2.2). Returns nil for any other version. // +// The two keys are xdebug.mode=coverage and xdebug.start_with_request=default, +// both shipped by v2's src/configs/ini/xdebug.ini (pinned SHA in §1). +// // Deliberate divergence from v2's mechanism: v2 unconditionally writes this // fragment for every matching PHP version, even when xdebug is not installed // (PHP silently ignores ini keys for unloaded extensions). We take the @@ -197,7 +243,10 @@ func XdebugIniFragment(phpVersion string) map[string]string { if !xdebug3Supported(m) { return nil } - return map[string]string{"xdebug.mode": "coverage"} + return map[string]string{ + "xdebug.mode": "coverage", + "xdebug.start_with_request": "default", + } } func xdebug3Supported(minor string) bool { diff --git a/internal/compat/compat_test.go b/internal/compat/compat_test.go index cea7dce..eab39e5 100644 --- a/internal/compat/compat_test.go +++ b/internal/compat/compat_test.go @@ -39,8 +39,8 @@ func TestUnimplementedInputWarning(t *testing.T) { } func TestDefaultIniValuesMatchesGolden(t *testing.T) { - // The golden file pins x86_64's expected keys. aarch64 branch has its - // own targeted cases in TestDefaultIniValues_PHP8xOpcacheJIT below. + // The golden file pins the expected 2-key set, version/arch invariant. + // Opcache tests (with arch-aware jit_buffer_size) are below. got := DefaultIniValues("8.4", "x86_64") want := map[string]string{} @@ -215,17 +215,21 @@ func TestBaseIniFileName(t *testing.T) { } func TestXdebugIniFragment(t *testing.T) { + wantPresent := map[string]string{ + "xdebug.mode": "coverage", + "xdebug.start_with_request": "default", + } cases := []struct { php string want map[string]string }{ - {"8.4", map[string]string{"xdebug.mode": "coverage"}}, - {"8.4.5", map[string]string{"xdebug.mode": "coverage"}}, - {"8.0", map[string]string{"xdebug.mode": "coverage"}}, - {"8.9.99", map[string]string{"xdebug.mode": "coverage"}}, - {"7.4", map[string]string{"xdebug.mode": "coverage"}}, - {"7.3", map[string]string{"xdebug.mode": "coverage"}}, - {"7.2", map[string]string{"xdebug.mode": "coverage"}}, + {"8.4", wantPresent}, + {"8.4.5", wantPresent}, + {"8.0", wantPresent}, + {"8.9.99", wantPresent}, + {"7.4", wantPresent}, + {"7.3", wantPresent}, + {"7.2", wantPresent}, {"7.1", nil}, // outside xdebug3_versions {"9.0", nil}, // outside xdebug3_versions {"", nil}, @@ -240,52 +244,90 @@ func TestXdebugIniFragment(t *testing.T) { } } -func TestDefaultIniValues_PHP8xOpcacheJIT(t *testing.T) { - cases := []struct { - arch string - wantBuf string - }{ - {"x86_64", "256M"}, - {"aarch64", "128M"}, +// After the OpcacheIniFragment split, DefaultIniValues returns the same 2-key +// set on every PHP version and arch. Opcache.* moved to OpcacheIniFragment. +func TestDefaultIniValues_VersionAndArchInvariant(t *testing.T) { + want := map[string]string{ + "date.timezone": "UTC", + "memory_limit": "-1", } - for _, tc := range cases { - for _, php := range []string{"8.0", "8.1", "8.2", "8.3", "8.4", "8.5", "8.4.5", "8.9"} { - t.Run(tc.arch+"/"+php, func(t *testing.T) { - got := DefaultIniValues(php, tc.arch) - want := map[string]string{ - "date.timezone": "UTC", - "memory_limit": "-1", - "opcache.enable": "1", - "opcache.jit": "1235", - "opcache.jit_buffer_size": tc.wantBuf, - } + for _, arch := range []string{"x86_64", "aarch64"} { + for _, php := range []string{"7.4", "8.0", "8.1", "8.2", "8.3", "8.4", "8.5", "8.4.5", "9.0"} { + t.Run(arch+"/"+php, func(t *testing.T) { + got := DefaultIniValues(php, arch) if !reflect.DeepEqual(got, want) { - t.Errorf("DefaultIniValues(%q, %q) = %v, want %v", php, tc.arch, got, want) + t.Errorf("DefaultIniValues(%q, %q) = %v, want %v", php, arch, got, want) } }) } } } -func TestDefaultIniValues_NonPHP8(t *testing.T) { - // Below 8.x the opcache/JIT block does not apply on either arch; - // both archs return the same 2-key set. +func TestOpcacheIniFragmentMatchesGoldens(t *testing.T) { + cases := []struct { + arch string + golden string + }{ + {"x86_64", "opcache_ini_fragment_x86_64.golden"}, + {"aarch64", "opcache_ini_fragment_aarch64.golden"}, + } + for _, tc := range cases { + t.Run(tc.arch, func(t *testing.T) { + got := OpcacheIniFragment("8.4", tc.arch) + want := map[string]string{} + for _, line := range readGoldenLines(t, tc.golden) { + parts := strings.SplitN(line, "=", 2) + if len(parts) != 2 { + t.Fatalf("bad golden line: %q", line) + } + want[parts[0]] = parts[1] + } + if !reflect.DeepEqual(got, want) { + t.Errorf("OpcacheIniFragment(8.4, %q) = %v, want %v", tc.arch, got, want) + } + }) + } +} + +func TestOpcacheIniFragment_AppliesToAllPHP8x(t *testing.T) { + for _, php := range []string{"8.0", "8.1", "8.2", "8.3", "8.4", "8.5", "8.4.5", "8.9"} { + t.Run(php, func(t *testing.T) { + if got := OpcacheIniFragment(php, "x86_64"); got == nil { + t.Errorf("OpcacheIniFragment(%q, x86_64) = nil, want non-nil", php) + } + }) + } +} + +func TestOpcacheIniFragment_NilOutsidePHP8x(t *testing.T) { for _, arch := range []string{"x86_64", "aarch64"} { - for _, php := range []string{"7.4", "9.0"} { + for _, php := range []string{"7.4", "9.0", ""} { t.Run(arch+"/"+php, func(t *testing.T) { - got := DefaultIniValues(php, arch) - want := map[string]string{ - "date.timezone": "UTC", - "memory_limit": "-1", - } - if !reflect.DeepEqual(got, want) { - t.Errorf("DefaultIniValues(%q, %q) = %v, want %v", php, arch, got, want) + if got := OpcacheIniFragment(php, arch); got != nil { + t.Errorf("OpcacheIniFragment(%q, %q) = %v, want nil", php, arch, got) } }) } } } +func TestStockIniDefaultsMatchesGolden(t *testing.T) { + got := StockIniDefaults() + + want := map[string]string{} + for _, line := range readGoldenLines(t, "stock_ini_defaults.golden") { + parts := strings.SplitN(line, "=", 2) + if len(parts) != 2 { + t.Fatalf("bad golden line: %q", line) + } + want[parts[0]] = parts[1] + } + + if !reflect.DeepEqual(got, want) { + t.Errorf("StockIniDefaults() = %v, want %v", got, want) + } +} + // readGoldenLines reads testdata/, strips blank lines and # comments, // returns the remaining non-empty lines in file order. func readGoldenLines(t *testing.T, name string) []string { diff --git a/internal/compat/testdata/default_ini_values.golden b/internal/compat/testdata/default_ini_values.golden index 234a5d3..fd96894 100644 --- a/internal/compat/testdata/default_ini_values.golden +++ b/internal/compat/testdata/default_ini_values.golden @@ -1,10 +1,8 @@ -# Defaults set by shivammathur/setup-php@v2 on Linux runners for PHP 8.x. -# Source: docs/compat-matrix.md §2.1 and §2.3, pinned SHA accd6127cb78bee3e8082180cb391013d204ef9f. -# NOTE: extension-tied defaults (xdebug.mode=coverage) are handled at compose time, not here. -# This golden is keyed to PHP 8.x (jit_versions = 8.[0-9]); non-8.x versions omit opcache.* lines. +# Defaults set by shivammathur/setup-php@v2 on Linux runners. +# Source: docs/compat-matrix.md §2.1 (base rows: date.timezone + memory_limit). +# Opcache and xdebug defaults moved to OpcacheIniFragment / XdebugIniFragment +# because their gates differ (opcache: PHP 8.x + opcache loaded; +# xdebug: coverage: xdebug + xdebug3-supported version). date.timezone=UTC memory_limit=-1 -opcache.enable=1 -opcache.jit=1235 -opcache.jit_buffer_size=256M diff --git a/internal/compat/testdata/opcache_ini_fragment_aarch64.golden b/internal/compat/testdata/opcache_ini_fragment_aarch64.golden new file mode 100644 index 0000000..b26f9be --- /dev/null +++ b/internal/compat/testdata/opcache_ini_fragment_aarch64.golden @@ -0,0 +1,11 @@ +# v2's jit_aarch64.ini fragment + stock php.ini-production opcache lines. +# Source: docs/compat-matrix.md §2.3 + §2.4. +# aarch64 differs from x86_64 only on opcache.jit_buffer_size (128M vs 256M). + +opcache.enable=1 +opcache.enable_cli=0 +opcache.jit=1235 +opcache.jit_buffer_size=128M +opcache.memory_consumption=128 +opcache.revalidate_freq=2 +opcache.validate_timestamps=1 diff --git a/internal/compat/testdata/opcache_ini_fragment_x86_64.golden b/internal/compat/testdata/opcache_ini_fragment_x86_64.golden new file mode 100644 index 0000000..2c4c7db --- /dev/null +++ b/internal/compat/testdata/opcache_ini_fragment_x86_64.golden @@ -0,0 +1,12 @@ +# v2's jit.ini fragment + stock php.ini-production opcache lines. +# Source: docs/compat-matrix.md §2.3. +# Applied when PHP version matches jit_versions (8.[0-9]) AND opcache is loaded. +# x86_64 form. aarch64 differs only on opcache.jit_buffer_size (see §2.4). + +opcache.enable=1 +opcache.enable_cli=0 +opcache.jit=1235 +opcache.jit_buffer_size=256M +opcache.memory_consumption=128 +opcache.revalidate_freq=2 +opcache.validate_timestamps=1 diff --git a/internal/compat/testdata/stock_ini_defaults.golden b/internal/compat/testdata/stock_ini_defaults.golden new file mode 100644 index 0000000..70182d7 --- /dev/null +++ b/internal/compat/testdata/stock_ini_defaults.golden @@ -0,0 +1,12 @@ +# Defaults set by shivammathur/setup-php@v2 on every Linux runner. +# Source: docs/compat-matrix.md §2.1 (stock php.ini-production rows). +# These keys are always-on (no version or arch gate). Encoded as the value +# v2 actually emits, which may differ from php.ini-production's literal value +# (e.g. max_input_time=-1, the CLI-SAPI compiled-in default, overrides the +# file's "60" because max_input_time is PHP_INI_PERDIR). + +expose_php=1 +log_errors=1 +max_input_time=-1 +session.gc_maxlifetime=1440 +session.save_handler=files diff --git a/internal/compose/compose.go b/internal/compose/compose.go index 1a613c0..c49cc71 100644 --- a/internal/compose/compose.go +++ b/internal/compose/compose.go @@ -126,17 +126,19 @@ func SelectBaseIniFile(layout *Layout, filename string) error { return os.WriteFile(layout.IniFile, data, 0o600) } -// MergeCompatLayers returns a single ini-key map composed of (lower-precedence -// first): defaults → xdebug fragment → extra. Callers pass the result as the -// `defaults` argument to WriteIniValuesWithDefaults, which then layers user -// ini-values over the top. Any of the inputs may be nil. +// MergeCompatLayers returns a single ini-key map composed of the given layers +// in increasing precedence (last write wins). Any layer may be nil and is +// treated as empty. // -// The returned map is always non-nil (may be empty). -func MergeCompatLayers(defaults, xdebugFragment, extra map[string]string) map[string]string { - merged := make(map[string]string, len(defaults)+len(xdebugFragment)+len(extra)) - maps.Copy(merged, defaults) - maps.Copy(merged, xdebugFragment) - maps.Copy(merged, extra) +// Callers pass the result as the `defaults` argument to +// WriteIniValuesWithDefaults, which then layers user ini-values over the top. +// +// The returned map is always non-nil (may be empty for a zero-layer call). +func MergeCompatLayers(layers ...map[string]string) map[string]string { + merged := make(map[string]string) + for _, layer := range layers { + maps.Copy(merged, layer) + } return merged } diff --git a/internal/compose/compose_test.go b/internal/compose/compose_test.go index f2dd1d0..8f2ac2d 100644 --- a/internal/compose/compose_test.go +++ b/internal/compose/compose_test.go @@ -282,13 +282,14 @@ func TestMergeCompatLayers_OrderAndPrecedence(t *testing.T) { xdebug := map[string]string{"xdebug.mode": "coverage"} extra := map[string]string{"pcov.enabled": "1"} - // extra beats xdebug beats defaults, so test a conflict: - conflicting := map[string]string{"memory_limit": "512M"} // would overwrite default - got := MergeCompatLayers(defaults, nil, conflicting) + // Last write wins: extra beats defaults on a conflicting key. + conflicting := map[string]string{"memory_limit": "512M"} + got := MergeCompatLayers(defaults, conflicting) if got["memory_limit"] != "512M" { - t.Errorf("extra should override defaults: got %q", got["memory_limit"]) + t.Errorf("later layer should override: got %q", got["memory_limit"]) } + // All non-conflicting keys merge across layers. got = MergeCompatLayers(defaults, xdebug, extra) want := map[string]string{ "memory_limit": "-1", @@ -300,13 +301,36 @@ func TestMergeCompatLayers_OrderAndPrecedence(t *testing.T) { t.Errorf("MergeCompatLayers = %v, want %v", got, want) } - // nil layers treated as empty - got = MergeCompatLayers(defaults, nil, nil) + // nil layers treated as empty (no panic, no contribution). + got = MergeCompatLayers(defaults, nil, nil, nil) if !reflect.DeepEqual(got, defaults) { t.Errorf("with nil layers should equal defaults: got %v", got) } } +func TestMergeCompatLayers_VariadicEdgeCases(t *testing.T) { + // Zero layers: returns an empty (non-nil) map. + got := MergeCompatLayers() + if got == nil { + t.Error("MergeCompatLayers() returned nil; want empty map") + } + if len(got) != 0 { + t.Errorf("MergeCompatLayers() = %v; want empty map", got) + } + + // Five layers (matches the new cmd/phpup wiring shape). + a := map[string]string{"k1": "v1"} + b := map[string]string{"k2": "v2"} + c := map[string]string{"k3": "v3"} + d := map[string]string{"k4": "v4"} + e := map[string]string{"k5": "v5"} + got = MergeCompatLayers(a, b, c, d, e) + want := map[string]string{"k1": "v1", "k2": "v2", "k3": "v3", "k4": "v4", "k5": "v5"} + if !reflect.DeepEqual(got, want) { + t.Errorf("MergeCompatLayers (5 layers) = %v, want %v", got, want) + } +} + func TestCompose(t *testing.T) { dir := t.TempDir() extDir := filepath.Join(dir, "ext")