Repository navigation
Conversation
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! |
…oser errors inline
fast-build command for targeted autoloader recoveryfast-build command for targeted autoloader recovery (parallel + tests)
|
@claude please review this PR |
|
Claude finished @enejb's task in 3m 47s —— View job PR ReviewThis is a well-designed and clearly motivated command. The two-signal approach (log scan + source drift), the parallelism strategy, and the error-reporting UX are all solid. A few bugs and design issues worth addressing before merging: Bug:
|
There was a problem hiding this comment.
Pull request overview
Adds a new jetpack fast-build CLI command intended to quickly recover local dev sites from stale autoloader/classmap fatals by planning minimal rebuild work from (1) docker debug.log scans and (2) per-project “last built” stamps.
Changes:
- Introduces
fast-buildcommand with planning, optional paralleldump-autoload, verification, and failure summarization. - Adds helpers for log scanning, FQN→project owner resolution, and per-project last-built SHA stamping + drift classification.
- Updates
jetpack buildto write last-built stamps and adds unit tests for the new helpers/planner.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/cli/commands/fast-build.js | Implements fast-build planner/runner (parallel dump-autoload, verify, reporting). |
| tools/cli/helpers/logScan.js | Adds docker debug.log tail + missing-symbol extraction. |
| tools/cli/helpers/classOwner.js | Adds PSR-4 map + git-grep fallback to resolve a missing symbol’s owning project. |
| tools/cli/helpers/lastBuilt.js | Adds last-built SHA stamps + JSON-aware change inspection for planning. |
| tools/cli/commands/build.js | Writes last-built stamps after successful non-mirror builds. |
| tools/cli/cliRouter.js | Registers the new fast-build command. |
| tools/cli/tests/unit/helpers/logScan.test.js | Unit tests for missing-symbol extraction. |
| tools/cli/tests/unit/helpers/lastBuilt.test.js | Unit tests for change bucketing logic. |
| tools/cli/tests/unit/commands/fast-build.test.js | Unit tests for planner helpers (action merge/rank, error summarization). |
| .gitignore | Ignores .jetpack-cli/ stamp directory. |
| function deepEqualJson( a, b ) { | ||
| const canon = v => JSON.stringify( v, Object.keys( v || {} ).sort() ); | ||
| return canon( a ) === canon( b ); | ||
| } |
| 'git', | ||
| [ | ||
| 'grep', | ||
| '--untracked', | ||
| '-l', | ||
| '--', | ||
| `\\(class\\|interface\\|trait\\)\\s\\+${ shortName }\\b`, | ||
| 'projects/', | ||
| ], |
| await execa( 'composer', [ 'run', '--timeout=0', script ], { | ||
| cwd, | ||
| stdio: [ 'ignore', argv.v ? 'inherit' : 'pipe', argv.v ? 'inherit' : 'pipe' ], | ||
| buffer: false, | ||
| } ); |
…lve in plugin classmaps
…ntainer is running
…non-docker setups
…short-name collisions
fast-build command for targeted autoloader recovery (parallel + tests)fast-build command (autoloader recovery, parallel, watch, seed, verify, log-path, skip list)
|
I'm confused by the use-case here (to be honest, the PR description is quite verbose and hard to pick out the specific problem it's trying to solve). Honest question...did you read it yourself? 😄 Glancing at the testing instructions, though, it seems perhaps the purpose is less about building things and more about fixing incomplete/outdated dependency installs. Perhaps the correct fix in these scenarios is instead using |
|
Thanks for the feedback @tbradsha! I think this approach is probably not the right one. Will close this PR for now. |
Fixes # N/A
Proposed changes
Adds a new
jetpack fast-buildCLI command that restores a broken local dev site by doing only the targeted rebuild work the diff actually needs — typicallycomposer dump-autoloadin a handful of plugins, completing in seconds rather than the ~2 minutes thatjetpack build jetpacktakes today.The classic pain point this addresses: after
git switchor rebasing ontrunk, a class added by another branch isn't in the jetpack-autoloader classmap, so localhost throws a PHP fatal. The usual fix isjetpack build <plugin>, of which ~60–90s is webpack bundling that's completely irrelevant to a stale-classmap fatal.The command combines two independent signals:
/var/www/html/wp-content/debug.logfrom the running Jetpack docker container (or a host-side path via--log-path/JETPACK_FASTBUILD_LOG), extractsClass/Interface/Trait not foundfatals, resolves each FQN to its owning project (PSR-4 prefix match first, then a namespace-aware, plausibility-filteredgit grep --untrackedfallback — since 91 of 96 monorepo projects use classmap autoload rather than PSR-4), and runscomposer dump-autoloadon every plugin that transitively depends on the owner.composer.json/package.jsonversion-bump-only commit doesn't trigger a heavy install). PHP-only changes →dump-autoload; JS/asset source changes → projectbuild;require/dependenciesblock changes → surfaced as "runjetpack buildfor these" instead of auto-installed.Two-stamp model (correctness)
Each project now has two independent stamps:
last-built/<slug>.build.sha(full build, JS + PHP) andlast-built/<slug>.autoload.sha(autoloader regen only).dump-autoloadadvances theautoloadstamp without claiming the JS bundle is fresh. Without this split, adump-autoload-only action would have marked the project as fully built, hiding any stale JS bundles from future runs.Parallelism + helpful failure output
dump-autoload. Each plugin'svendor/composer/jetpack_autoload_classmap.phpis independent, so plans consisting entirely ofdump-autoloadactions run throughpLimit.--concurrency N(default 4). Expected wall-clock for a 13-plugin regen: ~6–10s with the default vs. ~31s serial. Mixed plans withbuildactions stay serial.jetpack build …to fix root cause, or--skip …to ignore on next run).--stop-on-errorrestores fail-fast.summarizeExecError()helper extracts the actualClassMapGenerator.php line 137: Could not scan for classes …from buffered stderr, stripping composer's noisy "dump-autoload [-o|...]" usage trailer.Flag inventory
--seed--dry-run--autoload-onlydump-autoload, neverbuild.--since <ref>--log-scan/--no-log-scan--log-path <path>(or$JETPACK_FASTBUILD_LOG)--lines N--skip <project>(repeatable)--concurrency Ndump-autoload(default 4).--stop-on-error--watch/--watch-interval N--production/-pbuild-productionscript forbuildactions (mirrorsjetpack build).--verify/--no-verify--urlAND grep each plugin's regenerated classmap to confirm the targeted FQNs now resolve.--url <url>http://localhost).Plus
.jetpack-cli/skip.txt— a persistent skip list (one slug per line,#comments allowed) merged into--skip. Saves typing the same--skip plugins/mu-wpcom-plugin --skip plugins/wpcomshflags every run.Files of interest
tools/cli/commands/fast-build.js— the new command (planner + parallel runner + watch loop + reporter).tools/cli/helpers/logScan.js— debug.log tail via docker exec or host path, missing-symbol extraction.tools/cli/helpers/classOwner.js— FQN → owning-project resolution. PSR-4 match → namespace-verified grep → plausibility-filtered grep (skips test fixtures / READMEs / arbitrary "class Foo" mentions).tools/cli/helpers/lastBuilt.js— per-project two-stamp model + JSON-aware change classification.tools/cli/commands/build.js— writes abuildstamp on each successful (non-mirror) build.tools/cli/cliRouter.js— registers the new command..gitignore— ignores.jetpack-cli/.tools/cli/tests/unit/helpers/{logScan,lastBuilt}.test.js+tools/cli/tests/unit/commands/fast-build.test.js— 182 tests passing (16 new).No
projects/*are touched, so no changelogger entry is needed.tools/cliis a private workspace package.Commit layout
The PR is 11 commits:
add fast-build command— the foundation.parallelize + surface composer errors inline + planner tests— done as part of the original ship.split stamp into build/autoload— fixes the stale-JS-blindness bug.--seed— first-time bootstrap.--verify confirms targeted classes now resolve— checks plugin classmaps, not just "no new fatals".hint at jetpack docker up -d— friendlier error when no container.persistent skip list—.jetpack-cli/skip.txt.--log-path— non-docker setups.--watch— auto-fix loop.--production— mirror jetpack build.tighten classOwner git-grep fallback— defends against short-name collisions across packages.Reviewable as a sequence; each commit is self-contained.
Related product discussion/links
N/A — internal developer ergonomics improvement.
Does this pull request change what data or activity we track or use?
No.
Testing instructions
Prerequisites: Jetpack docker dev environment running (
jetpack docker up -d), or a writable host path you can use via--log-path.First-time bootstrap
trunk:node tools/cli/bin/jetpack.js fast-build --seed. Expect:Seeded N project(s) at <sha>. …and a populated.jetpack-cli/last-built/directory with*.build.shaand*.autoload.shafiles.node tools/cli/bin/jetpack.js fast-build --dry-run --no-log-scanimmediately after. Expect:Nothing to do.Autoloader fatal recovery + verify
projects/packages/connection/src/, reference it from the bottom ofprojects/packages/connection/actions.phpwithnew \Automattic\Jetpack\Connection\My_New_Class();, thencurl http://localhost. Expect HTTP 500;jp-tailshowsFatal error: Uncaught Error: Class "..." not found.node tools/cli/bin/jetpack.js fast-build. Expect:composer dump-autoloadplanned for each dependent plugin.Verify: N/N targeted class(es) now resolve in their plugin classmaps.Verify: http://localhost → HTTP 200.Continue-on-error + composer's real diagnostic
plugins/mu-wpcom-plugin's classmap references a missing vendor dir,node tools/cli/bin/jetpack.js fast-build.ClassMapGenerator.php line 137error inline, the run continues through remaining plugins, and the end-of-run summary suggestsjetpack build plugins/mu-wpcom-pluginor--skip plugins/mu-wpcom-plugin.Persistent skip list
echo plugins/mu-wpcom-plugin > .jetpack-cli/skip.txt.node tools/cli/bin/jetpack.js fast-build. Expect the "Honoring N entry(s) from .jetpack-cli/skip.txt: …" notice and mu-wpcom-plugin absent from the plan.Watch mode
node tools/cli/bin/jetpack.js fast-build --watch. Expect "Watching debug.log every 3s. Ctrl-C to exit."Non-docker fallback
node tools/cli/bin/jetpack.js fast-build --log-path /path/to/host/debug.log --dry-run. Expect "Scanned debug.log from /path/to/host/debug.log." instead of the docker variant.Stamp split: stale-JS bug fix
--seed-stamped state, switch to a branch that adds JS source files but no PHP fatals. Runnode tools/cli/bin/jetpack.js fast-build --no-log-scan. Expect Signal 2 to detectjsSourcesand plan[build]for the affected project (not silently skip it).dump-autoload-only fix completes (Signal 1 fatal recovery), inspect.jetpack-cli/last-built/: only*.autoload.shaadvanced for those plugins;*.build.shais unchanged. A subsequent run will still detect JS drift.Parallelism comparison
time node tools/cli/bin/jetpack.js fast-build(default concurrency 4) andtime node tools/cli/bin/jetpack.js fast-build --concurrency 1. Default should be ~3–4× faster.Source-drift planning
node tools/cli/bin/jetpack.js fast-build --dry-run --no-log-scan --since HEAD~5— should produce a plan withdump-autoload/buildactions for projects with PHP/JS changes and a "Skipping N project(s)…" section for projects whoserequire/dependenciesblocks changed.Unit tests
cd tools/cli && NODE_OPTIONS=--experimental-vm-modules pnpm test— all 182 tests should pass.