feat(build): phpup build php|ext subcommand (PR 2/6)#56
Merged
Conversation
Push now accepts an Annotations value carrying manifest-level metadata (BundleName, SpecHash). LookupBySpec walks the layout index for a manifest whose annotations match a given (name, spec-hash) pair — the foundation for phpup build's cache-probe short-circuit (arrives in PR 2 Tasks 4-5). Remote backend's LookupBySpec is a no-op returning (zero, false, nil); anonymous OCI registries don't expose index walks, so build callers should use an oci-layout registry as their cache store and publish to remote only after a fresh build produces an artifact. Breaking change: Store.Push signature gains an Annotations value. Only internal callers (layout_test.go, remote_test.go) were affected and have been updated in the same commit.
- layout.LookupBySpec now rejects empty name or specHash explicitly
so a Ref{} zero-value probe can't false-positive on un-annotated
manifests. Add regression test.
- remote.LookupBySpec now returns ErrUnsupported so callers who
wire only a remote store see the structural capability gap
instead of a silent miss every call. Build callers can convert
to soft-miss via errors.Is(err, ErrUnsupported).
- New test exercises the empty-Annotations back-compat fallback
where Push derives BundleName from ref.Name.
- Godoc clarifications on asMap's never-nil contract and on how
to call LookupBySpec from an Annotations value.
…r helpers Extract HashFiles / PerVersionYAMLFromFile / ExtensionYAMLFromFile / ParseEnvValue from cmd/planner/main.go inlining into exported internal/planner helpers so phpup build (PR 2 Task 4/5) can compute spec-hashes from the same inputs CI planning uses. Values are byte-identical across the refactor — verified against the full catalog (10 PHP cells, 172 ext cells) before/after: zero spec_hash diffs. internal/build.ComputeSpecHash accepts CLI-friendly SpecHashInputs (kind/name/version/os/arch/ts/repo-root) and wraps planner.ComputeSpecHash with repo-relative path resolution. Tests use tempdir fixtures; no real catalog or builders touched. The wrapper also includes a cross-check test that builds the inputs by hand the planner's way and asserts the wrapper's output matches, so any future drift between the two code paths surfaces at CI time.
Thin wrapper over `docker run --rm` that PR 2 Tasks 4-5 drive. A package-level RunnerFunc var is swappable via SetRunner(fn) (ret a restore callback) so unit tests don't need real docker to assert the argv phpup would build. The realDockerRun default goes through exec.CommandContext; env keys are sorted for deterministic argv. A gated smoke test exercises real docker with `alpine:3 echo hello`, skipped under -short or when docker is absent.
… false positive gocritic:hugeParam on DockerRun wasn't a false positive — the ~136B DockerRunOpts is a real large-struct copy. Switch the public API to *DockerRunOpts (and RunnerFunc accordingly), eliminating the //nolint:gocritic suppression entirely. gosec:G204 on exec.CommandContext IS a genuine false positive: CommandContext passes argv directly to execve(2) without shell interpretation, and all argv strings come from typed DockerRunOpts fields. Keep the //nolint:gosec with a tightened justification invoking the false-positive criterion rather than prior-art config exclusions.
phpup build php [--php 8.4] [--os jammy] [--arch x86_64] [--ts nts] [--registry oci-layout:./out/oci-layout] [--repo .] Computes the spec-hash from the same builder + catalog inputs the planner uses, probes the target registry with LookupBySpec, and short-circuits on hit. On miss: spins up a bare ubuntu:22.04 (jammy) or ubuntu:24.04 (noble) container under the requested platform (linux/amd64 or linux/arm64), mounts the repo read-only + a tempdir output mount, runs builders/linux/build-php.sh unchanged, reads the resulting bundle.tar.zst + meta.json, and Pushes to the registry with bundle-name + spec-hash annotations. Callers with a remote-only registry (no oci-layout cache) are tolerated: LookupBySpec's ErrUnsupported is treated as a soft miss. BuildExt is declared as a stub returning an explanatory error until PR 2 Task 5 implements it.
- Container mount for bundle output was /tmp/out; builders write to /tmp via pack-bundle.sh's hardcoded OUTPUT_PATH. Real docker runs would have lost the bundle on container exit. Mount outDir at /tmp directly; keep OUTPUT_DIR=/tmp/out for make install INSTALL_ROOT staging inside the mount. Unit tests bypassed this via fake runners. - Add TestBuildPHP_RealDockerSmoke that swaps the repo fixture's build-php.sh for a 3-line synthetic and exercises real docker to catch mount-contract regressions in seconds. - Reject --ts zts at parse time: the flag was cache-key-only with no builder support, so accepting it silently cached an NTS artifact under a ZTS key. Fail loudly until builder support lands. - Normalize --arch aliases (amd64 -> x86_64, arm64 -> aarch64) at parse time so spec-hashes are stable regardless of caller convention; dockerPlatform simplified. - Stream apt output through to the user's stderr instead of silencing it; extract linuxAptPreamble constant so Task 5 can share. - Drop dead SchemaVersion default and linter-theater filepath.Clean calls. Revise BuildExt stub wording for end-user readability.
…nored) phpup build php's mounted output directory was a tempdir under /tmp. Per user redirect, switch the default to <repo>/build/php/<version>- <os>-<arch>-<ts>/ so artifacts persist next to the source tree for easy inspection, keyed deterministically on the spec tuple (repeated runs overwrite the same dir instead of piling up tempdirs). Add --out-dir flag for explicit override; tests pass t.TempDir() via the flag to keep the worktree clean. .gitignore gains /build/ (docker output) and /out/ (default oci-layout registry target). Leading slashes anchor the patterns to the repo root so they don't shadow internal/build/ (the Go package).
phpup build ext --ext redis --ext-version 6.2.0 --php-abi 8.4-nts \ --php-core-digest sha256:... [--arch x86_64] [--os jammy] \ [--registry oci-layout:./out/oci-layout] [--out-dir ./build/ext/...] Spins up an ephemeral distribution:3 registry on a fresh docker network, copies the prerequisite php-core from the source registry.Store to the sidecar via remote.Write (name.Insecure — distribution speaks HTTP by default), then runs builders/linux/build-ext.sh inside a container on the same network with REGISTRY overridden to the sidecar. fetch-core.sh pulls from the sidecar without any script change. Strict spec compliance: builders/** stays unchanged. After the build exits, phpup reads the ext bundle + meta.json from the output mount and pushes back to the destination Store with bundle-name + spec-hash annotations. Cache probe via LookupBySpec short-circuits redundant rebuilds. Default output dir mirrors Task 4's pattern: <repo>/build/ext/<name>-<version>-<php_abi>-<os>-<arch>/ (gitignored). SidecarLifecycle is a package-level var swappable via SetSidecarLifecycle for unit tests; a fakeSidecar exercises BuildExt's orchestration without real docker. A gated TestSidecar_LifecycleAndSeed_Real covers the real distribution:3 path (skipped under -short or when docker is absent).
Makefile's bundle-php / bundle-ext targets now invoke bin/phpup build instead of raw `docker run ubuntu:22.04 bash -c "... build-*.sh"`. Defaults match the hermetic dev loop (--registry oci-layout:./out/oci-layout); users override with REGISTRY=... for remote targets. A new bin/phpup file target depends on the embedded bundles.lock so make auto-rebuilds the binary when the lockfile changes. build-php-core.yml now runs the core build via `bin/phpup build php` (docker-wrapped builders/linux/build-php.sh, unchanged), writes into a project-relative output dir, then a thin "Stage bundle for publish" step copies bundle.tar.zst + .sha256 + meta.json to /tmp/ so the existing Push to GHCR, Sign bundle, Smoke test, and Upload bundle artifact steps read from the same paths they did before — preserving the `digest` job-output contract (sha256 of the bundle layer) byte- for-byte. build-extension.yml is intentionally not rewired in this commit: phpup build ext requires a workflow-supplied --php-core-digest and works against a store whose Push supports remote backends. Both gaps land in follow-up PRs (planner surfacing core-digest to the ext matrix, and internal/registry.remoteStore.Push). Left as a separate task so this wiring stays scoped.
TestBuildPHP_RealDockerSmoke failed on GitHub Actions linux runners with apt-key unable to create temporary config files. The outDir mounted into the build container at /tmp was mode 0o750, which the container-internal _apt user (dropped-privilege, different uid from host) couldn't write to. Docker Desktop on macOS transparently maps uids so the test passed locally; GHA enforces real mount semantics. Fix: prepareOutDir now chmods the directory to 1777 (world-writable + sticky) matching traditional /tmp semantics. This is exactly the intended use (the dir IS a /tmp replacement for a docker container); justification comment on the //nolint:gosec accompanies the chmod.
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.
Summary
PR 2 of 6 in the local+CI unification rollout (spec).
internal/registry.Storeextended withAnnotations{BundleName, SpecHash}onPushand a newLookupBySpec(ctx, name, specHash)probe for the cache short-circuit. Remote backend's lookup returnsErrUnsupported(anonymous OCI registries don't expose index walks); build callers soft-miss it.internal/plannerhelpers exported (HashFiles,PerVersionYAMLFromFile,ExtensionYAMLFromFile,ParseEnvValue) sointernal/build.ComputeSpecHashwrapsplanner.ComputeSpecHashfrom the same inputs. Zero spec-hash drift verified across 182 production cells (10 PHP + 172 ext).internal/build.DockerRun— swappableRunnerFuncviaSetRunner(fn)so unit tests don't need real docker. Gated real-docker smoke test exercises the default runner.phpup build phpsubcommand — docker-wrapsbuilders/linux/build-php.shunchanged, cache-probes via spec-hash, pushes toregistry.Storewith bundle-name + spec-hash annotations. Real-docker integration test passes end-to-end.phpup build extsubcommand — ephemeraldistribution/distribution:3sidecar registry on an isolated docker network; prerequisitephp-corecopied from source Store to sidecar viaremote.Writewithname.Insecure; build container runs on the same network withREGISTRYoverridden sofetch-core.shpulls from the sidecar transparently. Builder scripts stay unchanged. Real-docker sidecar test passes../build/php/…/and./build/ext/…/(gitignored) so artifacts persist for inspection;--out-diroverrides.make bundle-php/make bundle-extnow callphpup buildwith oci-layout defaults..github/workflows/build-php-core.ymlrewired to invokephpup build php; digest job-output contract preserved byte-for-byte via a thin stagingcpstep before the existing GHCR push.Backward compatibility: existing consumers of
buildrush/setup-php@v1see zero change —phpup installis untouched.Test plan
go test ./internal/build/... ./internal/registry/... ./internal/planner/...— 30 internal/build tests (83.5% coverage w/ integration), 28 internal/planner tests (90.5%), 28 internal/registry tests (76.9%).TestBuildPHP_RealDockerSmokeexercises the mount contract end-to-end with a synthetic builder.TestSidecar_LifecycleAndSeed_Realspins up distribution:3 and verifies bundle round-trip viaremote.Get.make checkgreen end-to-end including docker-backedlocal-cismoke (ubuntu:22.04 + 24.04 × amd64 + arm64 × hard4 extensions).cmd/planneroutput ona853877(main) vs.c603093(post-refactor) shows zero drift across all 182 cells.Explicitly deferred to follow-ups
.github/workflows/build-extension.ymlis unchanged. Rewiring cleanly depends on (a) implementingremoteStore.Push(~100 LOC; was intentionallyErrUnsupportedin PR 1) and (b) threadingphp_core_digestthroughplanner.MatrixCell+plan-and-build.ymlinputs. Both exceed the "wiring only" scope of Task 6 and will land in a PR 2.5 or PR 3.defaultSidecarLifecycle.Startcurrently does not sweep stalephpup-sidecar-*containers orphpup-build-*networks. Flagged in Task 5 code review; can be added via adocker ps --filter label=buildrush.phpup.sidecar=1sweep atStart.loadExtBuildDepsrefactor — current implementation parsescatalog/extensions/<name>.yamlviamap[string]any; should switch to the existing typedcatalog.LoadExtensionSpecfor DRY.cmd/lockfile-updatehash-inputs divergence (pre-existing, flagged in PR 2 Task 2 review) — hashes onlybuild-<kind>.sh + bundle-schema-version.env; doesn't includecapture-hermetic-libs.sh+pack-bundle.sh. Not a regression from this PR; port toplanner.HashFilesin a follow-up to centralize.Non-goals (land in subsequent PRs per the spec's 6-PR rollout)
phpup testsubcommand — PR 3.ci.ymlwith thin OS×ARCH×PHP matrix — PR 4.planner,lockfile-update, etc. →phpupsubcommands) — PR 6.Notes for reviewers
featcommits introduce capabilities behind tests;fixcommits address prior-review findings.//nolint:gosecis introduced (onexecDockerininternal/build/sidecar.go) — G204 genuine false positive (execve without shell; argv from typed struct fields). Matches Task 3's docker.go pattern.