Ship frontend build as a committed archive (extracted at cache warmup)#3779
Merged
Conversation
Replace the checked-in expanded public/build tree with a single deterministic build-dist/build-<id>.zip that is reconstructed into public/build automatically. This removes the per-build file churn and the merge conflicts caused by committing the whole build on every pipeline run. - BuildArchiveExtractor + a generic CacheWarmer + a provider trait extract the archive while vendor is still writable (composer/deploy), lazily in dev; read-only-filesystem safe and never overwrites a manual dev build - Deterministic build id (content hash of assets/) gives stable output dir names and zip filename, so identical source produces an identical archive (no git noise) - StaticResourcesResolver serves only the latest build by grouping dirs via their .build-id; package-build keeps only the newest archive and the extractor always picks the newest build-*.zip - public/build is now gitignored; CI commits build-dist/ instead of public/ See UPGRADE.md: read-only deployments must run cache:warmup while vendor is writable.
- assets/package-lock.json: add adm-zip + @types/adm-zip so `npm ci` passes - use node: protocol imports in build-id.ts and package-build.cjs (S7772) - BuildArchiveExtractor: use sha256 instead of md5 for the lock filename (S4790) - wrap lines over 120 chars in StaticResourcesResolver and the iframe provider (S103)
- BuildArchiveExtractor: drop the hash() from the lock filename (use a fixed name in the target's parent dir) to avoid the S4790 security hotspot — the lock name is not a security context - iframe provider: keep the interface list on one line under a wrapped `implements` to satisfy S103 (<=120) without the S1808 multiline alignment
- build-id.ts: use sha256 instead of sha1 (avoids the S4790 weak-hash security hotspot; the id is just a content fingerprint, but sha256 keeps Sonar happy) - BuildArchiveProviderInterface now extends WebpackEntryPointProviderInterface, so the two providers implement a single short interface — fits on one line (no S103) with no multiline implements list (no S1808), and reads truthfully: a build-archive provider is an entry-point provider
Removes the `git rev-parse` subprocess (Sonar S4036 hotspot: command resolved via PATH). The build id is now a content hash over the js/ source tree plus the dependency manifests, using normalized relative paths so it stays stable across checkout locations. No OS command, same deterministic-id guarantee, and it also works when building outside a git checkout.
…erge) Adds this PR branch to the push trigger so CI runs THIS branch's workflow definition (with the package-build step) instead of the base-branch one used by pull_request_target. Lets the pipeline generate build-dist/build-<id>.zip on the PR. Does not touch 2025.4. To be reverted once the archive is generated.
The archive (build-dist/build-4e57a40d1ec3.zip) has been generated by CI; remove the temporary branch entry from the push trigger so this PR does not alter the trigger config on merge.
The concrete build-archive classes are implementation detail, not public API; only BuildArchiveProviderInterface (the extension point, parallel to WebpackEntryPointProviderInterface) stays public.
- Inject an optional PSR logger into BuildArchiveExtractor and log extraction, skips and failures — most importantly a warning when extraction is needed but the target is not writable (previously a silent failure on a deploy-critical path). The logger is nullable so the trait's dev fallback (`new ...`) still works; the cache warmer gets the autowired service with the logger. - .gitattributes: match build-dist/build-*.zip (the id is content-derived, so the filename varies) instead of the fixed build.zip, so binary/merge=ours/generated actually apply. Update stale build.zip references in docs/comments.
Use #[Required] setter autowiring so the provider services (and their lazy fallback path) use the same DI BuildArchiveExtractor instance — with the logger — instead of constructing a logger-less one. Verified the setter is autowired in a real Symfony container.
- BuildArchiveExtractor: key the extraction decision off whether the expanded build is actually present (not just the marker), so an orphaned marker left behind when the build dirs are deleted no longer skips re-extraction. Rename the marker to a visible `extracted-archive.json` (was a hidden dotfile that survived deleting the visible dirs). Freshness now compares the archive name (content-hash id, so name == content) and archive selection is deterministic — no filemtime anywhere. TEMP: logs extraction duration (remove before merge). - StaticResourcesResolver: pick the active build group without filemtime — prefer the build the extractor recorded in extracted-archive.json, else deterministic. - build-id: fingerprint the whole assets/ tree (excluding node_modules/dist) so config/font/api-spec changes also bump the id, not just js/ source.
… generic The build-archive selection (group by .build-id, prefer the extracted marker) lived in StaticResourcesResolver, which is generic and iterates every entry-point provider — including third-party ones that ship no archive. Move it to BuildArchiveExtractor::entryPointLocations() (it already owns the marker and the on-disk build layout); the trait's getEntryPointsJsonLocations() delegates to it, so only providers that opt into the archive get the selection. The resolver is generic again.
Replace the verbose root UPGRADE.md with doc/06_Upgrade.md following the studio-backend-bundle blueprint (# Upgrade Information, concise per-version sections, > Note callouts), targeted at the next release 2025.4.6, and link it from the README documentation overview.
Benchmarking confirmed extraction is ~0.4s; the timing log is no longer needed.
- package-build: pick the build id deterministically (sorted) instead of by mtime, consistent with the runtime selection. - Drop comments that just restate the code; keep the ones explaining non-obvious behaviour (no-clobber, read-only, concurrency).
- BuildArchiveExtractor: final readonly class (stateless; only a readonly logger), consistent with the other Build value/service classes. - services.yaml: drop the explicit $extractor argument on the cache warmer — it is autowired by type; only the tagged iterator needs wiring. Fix a stale comment. Verified the container still compiles and the warmer keeps its kernel.cache_warmer tag.
Align with pimcore/ee-tinymce-bundle's CopyCacheWarmer: extraction must run during warmup (the writable deploy phase), not be skippable and fall back to the lazy provider path, which cannot write on a read-only production filesystem.
When the build must be extracted but the target is not writable and no build is present (a read-only deploy that didn't warm the cache while writable), throw BuildArchiveNotWritableException from the entry point provider instead of a vague 'entry point not found'. If a build is already present it keeps being served. The warmer logs the cause and never fails warmup (matching pimcore core/tinymce warmers).
- build id selection: sort with localeCompare instead of the default sort. - drop the redundant pairDirs.sort() (files are sorted globally afterwards).
pull_request_target runs the base 2025.4 workflow (no packaging), so the committed archive goes stale. Add this branch to the push trigger so each push rebuilds and commits build-dist/build-<id>.zip with the current build id. REMOVE BEFORE MERGE.
…ents Cover the extraction decision branches: fresh extract, up-to-date no-op, manual-build no-clobber, stale re-extract, read-only (throws), multiple-archive deterministic pick, and entryPointLocations selection (marker-preferred and deterministic fallback). Also correct two comments left over from the earlier mtime/"newest" design that no longer matched the deterministic content-hash selection.
Cover the warmer behaviour with fake providers and a real extractor on temp dirs (no container): isOptional() is false, archive providers are extracted during warmUp, non-archive and null-archive providers are skipped, and extractor exceptions are swallowed so warmup never fails.
The build id is the source-tree hash and is reproducible, but the compiled output is not byte-for-byte: Module Federation's mf-stats.json lists modules in non-deterministic order, so re-zipping produced a churned "Automatic frontend build" commit on every CI run even with no source change. Treat the id as the archive's identity: if build-<id>.zip already exists, keep it untouched (still sweeping any stray archive from another id to preserve the one-archive invariant); only a new id writes a fresh zip.
Clarify in build-id.ts that the bundler scripts themselves are part of the id fingerprint. This is an assets/ change, so the build id changes and CI should commit a new build-<id>.zip (Automatic frontend build).
Add a docblock to the extractor test class. This touches tests/ only, not assets/, so the build id is unchanged and CI should NOT produce a new build-<id>.zip / Automatic frontend build commit.
Restores build-id.ts to its pre-test state. assets/ returns to the prior content, so the id computes back to 1e1780b17a24 and CI will restore that archive.
|
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Changes in this pull request
Replaces the committed expanded
public/build/frontend tree with a single deterministic archive (build-dist/build-<id>.zip), reconstructed intopublic/build/automatically — removing the per-build file churn and merge conflicts the "Automatic frontend build" commits caused.package-buildstep zips the build intobuild-dist/build-<id>.zip(deterministic bytes, only the latest kept).public/build/is gitignored; CI commitsbuild-dist/.assets/source tree (replaces the randomv4()), so identical source ⇒ an identical archive (no git noise), while any source/config change bumps it.BuildArchiveExtractor, driven by a non-optional cache warmer atcache:warmup(whilevendor/is writable) and a writable-guarded provider fallback for local dev /git pull. Read-only-prod safe, re-extracts if the build dir is removed, never overwrites a manualnpm run dev-appbuild, and selects the active build without relying on file mtimes.BuildArchiveProviderInterface+BuildArchiveExtractionTraitlet other bundles adopt by shipping a zip anduse-ing the trait (rollout is a follow-up).See
doc/06_Upgrade.mdfor the deployment requirement.Additional info
public/build/deletion is the one-time untracking of the previously committed build;build-dist/build-<id>.zipis built and committed by the frontend-build CI workflow.MkdirCacheWarmerandpimcore/tinymce-bundle'sCopyCacheWarmerlikewise materialize files at warmup (non-optional, read-only-guarded), so read-only deployments are handled the same way Pimcore core already handles them.🤖 Generated with Claude Code