feat: Incremental Build#1267
Draft
RandomByte wants to merge 272 commits into
Draft
Conversation
5224cd2 to
4904c84
Compare
950fc6d to
41eed91
Compare
bb39565 to
2a21507
Compare
6233816 to
f858659
Compare
71db1d0 to
a2c371f
Compare
7364b4b to
cf43f0c
Compare
252b966 to
874a943
Compare
RandomByte
commented
Feb 18, 2026
df275e5 to
0345502
Compare
RandomByte
commented
Mar 5, 2026
9fc2509 to
20ba653
Compare
20ba653 to
9fc2509
Compare
9fc2509 to
940376d
Compare
9197670 to
44d1107
Compare
44d1107 to
d7c402c
Compare
Member
|
Rebased onto origin/main |
… handles On Windows, SQLite kept the cache.db file handle open after close, preventing subsequent test runs and concurrent builds from cleaning their output directories (fs.rm fails with EBUSY). The root cause is twofold: WAL-mode SQLite retains file mappings until a checkpoint, and the CacheManager singleton was not being released by all consumers. Resolve the issue with: - Add `PRAGMA wal_checkpoint(TRUNCATE)` before close in BuildCacheStorage to flush WAL content back to the main database and release file handles cleanly. This single change resolves the underlying handle leak across cache tests and integration tests without per-test workarounds. - Close the CacheManager at the end of `buildToTarget()`, after all deferred disk writes have completed. - Expose a public `ProjectBuilder#closeCacheManager()` method and call it from `BuildServer.destroy()` so long-running consumers release the database handle on shutdown. - Add a `#destroyed` flag to BuildServer to prevent new builds from being triggered after destroy(). Clear pending build queue and timeout before awaiting active build to avoid races with setTimeout-based queue processing. - Document the lifecycle on `ProjectBuilder#build`, `#buildToTarget`, and `#closeCacheManager`: `buildToTarget` closes the CacheManager itself; consumers of `build` must call `closeCacheManager` when done. - Close singleton instances in CacheManager `create()` test, make `after.always` rimraf best-effort in cache tests (test/tmp is already cleaned at start of each run), and add maxRetries to rimraf in integration tests for any residual Windows handle-release delays.
Add tests for previously untested error-handling paths: - lib/server.js: HTTP server error event handler, buildServer error event forwarding, and the close() rejection handler when buildServer.destroy() fails. - middleware/serveIndex: byGlob rejection. - sslUtil: chmod failure callbacks when fixing outdated file permissions.
…e during index re-init After a SourceChangedDuringBuildError, the source index is nulled and state reset to RESTORING_PROJECT_INDICES. If projectSourcesChanged or dependencyResourcesChanged was called before initSourceIndex ran again, the state was incorrectly transitioned to REQUIRES_UPDATE, causing initSourceIndex to skip initialization and a subsequent NPE on the null source index.
… and missing error handler Close CacheManager in a finally block within buildToTarget() so the SQLite database is released even when the build throws. Previously, builds that threw (e.g. SourceChangedDuringBuildError) leaked a refcount, keeping the DB memory-mapped on Windows and preventing cleanup by subsequent tests. Add an error event handler to the JSDoc child process spawn. Without it, a failed spawn on Windows would never emit close, hanging the promise indefinitely.
…esource.setStream Streams from older userland packages like readable-stream@2 (used by replacestream) have .pipe but no Symbol.asyncIterator, causing "stream is not async iterable" errors when the resource content is later consumed by node:stream/consumers. Also handle WHATWG TransformStream objects by unwrapping to their readable side.
The custom task reads dependencies via taskUtil.getProject().getReader() which bypasses the monitored dependencies reader. These reads are not tracked by ResourceRequestManager, so dependency changes don't invalidate the application's result cache.
Now only used in @ui5/fs
…c-generate
Replace type expressions unsupported by JSDoc's Catharsis parser:
- Union types inside Record literals in generics ({string|number|...})
- TypeScript-style import() type syntax
- Tuple types ([T, U]) inside generics
- Optional property syntax (?) inside Record generics
…ware configuration (#1367) JIRA: CPOUI5FOUNDATION-1206 --------- Co-authored-by: Copilot <copilot@github.com>
matchResourceMetadataStrict and the duplicated fast path in HashTree#upsertResources Phase 1 returned "unchanged" whenever a file's mtime matched the cached value, without consulting size or content. External operations that preserve mtime - cp -p, tar -x, rsync -t, and atomic rename of a temp file with copied stat - therefore passed silently as cache hits and produced stale build output. Verify size before the mtime-based short-circuit can return true. This matches the cheap-path semantics used by rsync's -t quick-check and git's stat-bundle compare, and closes the gap without forcing an integrity hash on every comparison. For FS-loaded resources getSize() resolves synchronously from statInfo, so the only added cost is one microtask per comparison. Remove the duplicated mtime fast path from HashTree#upsertResources and route every existing resource through Phase 2, which delegates to the now-corrected matchResourceMetadataStrict and already handles tag-only updates.
Extend matchResourceMetadataStrict to compare the resource inode against the cached inode. When both sides expose an inode and they disagree, the file has been replaced (atomic rename, cp, tar extraction, ...) — the mtime+size short-circuit is suppressed and the comparison falls through to the integrity check. Inode is now always populated by createResourceIndex; the optional includeInode flag is removed. When either side has no inode (virtual resources, caches written before this change), the inode check is skipped and behavior matches the previous mtime+size path. Falling through to integrity rather than returning false avoids unnecessary task re-execution when the replacement preserves content (e.g. an idempotent rsync). When the replacement also alters content, the integrity check correctly classifies the resource as changed, closing a stale-cache blind spot that mtime+size alone could not detect.
…changed Drop the unused matchResourceMetadata variant (no longer used) and rename matchResourceMetadataStrict to isResourceUnchanged.
Remove unused firstTruthy method, drop the stale local ResourceMetadata typedef (use existing type from HashTree.js), and strip the UI5_CACHE_PERF instrumentation.
createStat() previously returned a synthetic statInfo with ino: 0, which Resource.clone() forwarded as statInfo into the new instance. The constructor's `this.#inode ??= statInfo.ino` then set the clone's inode to 0, so memory-only resources read through Memory.byPath / Memory.byGlob advertised a fabricated inode. That defeated the inode short-circuit in the build cache's isResourceUnchanged whenever cached metadata was later compared against an FS-backed read of the same path. Set ino to undefined in createStat() so memory-only resources and their clones report getInode() === undefined, leaving the cache to fall back on size + lastModified + integrity. Add Resource tests covering inode propagation through construction, clone, and setBuffer for both memory-only and FS-backed resources.
Expose ui5DataDir as a programmatic option on ProjectGraph.build(), plumbed through ProjectBuilder, BuildContext, and CacheManager.create(). Previously the build cache location could only be configured via the UI5_DATA_DIR env var or the ~/.ui5rc config file, both of which are process-global and unsuitable for parallel test isolation. Builder integration tests now route the SQLite-backed build cache into a per-test directory via a new isolatedUi5DataDir helper. This fixes intermittent ERR_SQLITE_ERROR "database is locked" failures on Windows CI caused by parallel AVA workers contending on the shared ~/.ui5/buildCache database, and removes the UI5_DATA_DIR fallback from the builder ava.config.js.
Addresses the documentation comments from here: #1367
Port collided with port used in caching.js test
7c59782 to
444977d
Compare
Add direct yargs builder unit tests for the --cache, --cache-mode and --output-style coerce callbacks in the build and serve commands. Drop the equivalent subprocess-based tests from base.js that exercised the same coerce logic via the real ui5 binary. The unit tests run in-process so nyc captures their coverage, restoring the cli package above its thresholds
The BuildServer constructor used to fire `watchHandler.watch(...)` without awaiting it, so `graph.serve()` could return before chokidar had fully initialised. Source-file changes made immediately afterwards were silently dropped, which surfaced as flaky cache-mode integration tests on Windows where chokidar's ReadDirectoryChangesW backend has noticeably higher startup latency than inotify/FSEvents. Move watcher setup into a `BuildServer.create()` static factory that awaits readiness, and update `ProjectGraph.serve()` to use it.
The active build promise can reject (e.g. when Cache=Force detects a stale cache and throws). When destroy() awaited that promise, the throw skipped the closeCacheManager() call below, leaving the SQLite cache.db handle open. On Windows this caused subsequent tests to fail with EBUSY when fs.rm tried to remove the buildCache directory. Wrap the active-build await in try/finally so the cache manager is released even when the build rejected.
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
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.
Implementation of RFC 0017 Incremental Build
This PR supersedes previous PoC: #1238
JIRA: CPOUI5FOUNDATION-1174