diff --git a/packages/project/lib/build/cache/ProjectBuildCache.js b/packages/project/lib/build/cache/ProjectBuildCache.js index a1ae8ea523f..af84f8ae952 100644 --- a/packages/project/lib/build/cache/ProjectBuildCache.js +++ b/packages/project/lib/build/cache/ProjectBuildCache.js @@ -69,6 +69,14 @@ export default class ProjectBuildCache { #cachedResultSignature; #currentResultSignature; + // Dependency-set identity: a hash over the project's transitive dependency ids, computed by the + // caller from the graph and passed into validateCache. #cachedDependencySetIdentity is restored + // from the persisted source index, #currentDependencySetIdentity reflects the current graph. A + // mismatch means a dependency was added to or removed from the set since the last build even + // though no dependency resource changed, and forces a full dependency-index refresh. + #cachedDependencySetIdentity; + #currentDependencySetIdentity; + // Pending changes #changedProjectSourcePaths = []; #changedDependencyResourcePaths = []; @@ -168,11 +176,16 @@ export default class ProjectBuildCache { * @param {object} [options] * @param {boolean} [options.prepareForBuild=false] Run the pre-build side effects before * validating (see method description) + * @param {string} [options.dependencySetIdentity] Hash identifying the current dependency set + * (see {@link @ui5/project/build/helpers/ProjectBuildContext#getDependencySetIdentity}). + * Compared against the identity persisted with the previous build's source index; a mismatch + * forces a dependency-index refresh even when no dependency resource change was propagated. * @returns {Promise} * Array of changed resource paths since last build, true if cache is fresh, false * if cache is empty */ - async validateCache(dependencyReader, {prepareForBuild = false} = {}) { + async validateCache(dependencyReader, {prepareForBuild = false, dependencySetIdentity} = {}) { + this.#currentDependencySetIdentity = dependencySetIdentity; if (prepareForBuild) { this.#stageCache.discardPending(); this.#currentProjectReader = this.#project.getReader(); @@ -192,13 +205,20 @@ export default class ProjectBuildCache { } if (this.#combinedIndexState === INDEX_STATES.RESTORING_DEPENDENCY_INDICES) { - if (this.#changedDependencyResourcePaths.length) { + // A refresh is required when a dependency resource changed (accumulator non-empty) or + // when the dependency set itself changed since the last build. The latter is not + // reported through dependencyResourcesChanged(), so it would otherwise be missed and the + // restored indices would keep reflecting the previous build's dependency set. + const dependencySetChanged = + this.#currentDependencySetIdentity !== this.#cachedDependencySetIdentity; + if (this.#changedDependencyResourcePaths.length || dependencySetChanged) { const updateStart = performance.now(); await this._refreshDependencyIndices(dependencyReader); if (log.isLevelEnabled("perf")) { log.perf( `Initialized dependency indices for project ${this.#project.getName()} ` + - `in ${(performance.now() - updateStart).toFixed(2)} ms`); + `in ${(performance.now() - updateStart).toFixed(2)} ms ` + + `(dependencySetChanged=${dependencySetChanged})`); } } else if (log.isLevelEnabled("perf")) { log.perf( @@ -1399,6 +1419,10 @@ export default class ProjectBuildCache { const indexCache = this.#cacheManager.readIndexCache(this.#project.getId(), this.#buildSignature, "source"); if (indexCache) { log.verbose(`Using cached resource index for project ${this.#project.getName()}`); + // Restore the dependency-set identity persisted with the previous build's source index. + // validateCache compares it against the current identity to decide whether the restored + // dependency indices still match the current dependency set. + this.#cachedDependencySetIdentity = indexCache.availableDependencies; // Create and diff resource index const {resourceIndex, changedPaths} = await ResourceIndex.fromCacheWithDelta(indexCache, resources, Date.now()); @@ -1811,8 +1835,11 @@ export default class ProjectBuildCache { * @returns {{projectId: string, buildSignature: string, kind: string, index: object}|null} */ #prepareSourceIndex() { - if (this.#cachedSourceSignature === this.#sourceIndex.getSignature()) { - // No changes to already cached result index + if (this.#cachedSourceSignature === this.#sourceIndex.getSignature() && + this.#currentDependencySetIdentity === this.#cachedDependencySetIdentity) { + // Neither the source index nor the dependency-set identity changed. The identity is + // persisted in this row, so it must be rewritten when it changes even if the source + // index signature is unchanged (a dependency-set change does not touch source files). return null; } log.verbose(`Preparing resource index cache for project ${this.#project.getName()} ` + @@ -1829,6 +1856,7 @@ export default class ProjectBuildCache { index: { ...sourceIndexObject, tasks, + availableDependencies: this.#currentDependencySetIdentity, }, }; } diff --git a/packages/project/lib/build/helpers/ProjectBuildContext.js b/packages/project/lib/build/helpers/ProjectBuildContext.js index b269bc3f5ce..00e58285b5f 100644 --- a/packages/project/lib/build/helpers/ProjectBuildContext.js +++ b/packages/project/lib/build/helpers/ProjectBuildContext.js @@ -1,4 +1,5 @@ import ProjectBuildLogger from "@ui5/logger/internal/loggers/ProjectBuild"; +import crypto from "node:crypto"; import TaskUtil from "./TaskUtil.js"; import TaskRunner from "../TaskRunner.js"; import TaskDefinitions from "../TaskDefinitions.js"; @@ -286,7 +287,10 @@ class ProjectBuildContext { `getDependenciesReader completed in ${(performance.now() - readerStart).toFixed(2)} ms`); } const cacheStart = perfEnabled ? performance.now() : 0; - const boolOrChangedPaths = await this.getBuildCache().validateCache(depReader, {prepareForBuild}); + const boolOrChangedPaths = await this.getBuildCache().validateCache(depReader, { + prepareForBuild, + dependencySetIdentity: this.#getDependencySetIdentity(), + }); if (perfEnabled) { this._log.perf( `ProjectBuildCache.validateCache completed in ` + @@ -300,6 +304,27 @@ class ProjectBuildContext { return !!boolOrChangedPaths; } + /** + * Computes a hash identifying the project's dependency set for build-cache validation. + * + * The identity covers the project's full transitive dependency closure taken from the graph, + * hashed over the sorted dependency ids. It is deliberately the graph closure and not the + * task-scoped required set: the dependency reader exposes the transitive closure to a standard + * dependency-globbing task (getRequiredDependencies() returns only direct dependencies, the + * reader expands them transitively), and the required set flips all-or-nothing with task + * selection, which would trigger spurious refreshes. Ids are used rather than names because a + * project's name and id can differ under npm aliasing and the id is what the cache keys on. + * + * @returns {string} sha256 hex hash over the sorted transitive dependency ids + */ + #getDependencySetIdentity() { + const graph = this._buildContext.getGraph(); + const ids = graph.getTransitiveDependencies(this._project.getName()) + .map((name) => graph.getProject(name).getId()) + .sort(); + return crypto.createHash("sha256").update(ids.join("\n")).digest("hex"); + } + /** * Builds the project by running all required tasks * diff --git a/packages/project/test/lib/build/cache/ProjectBuildCache.js b/packages/project/test/lib/build/cache/ProjectBuildCache.js index ce598b057eb..3e30c59d5ca 100644 --- a/packages/project/test/lib/build/cache/ProjectBuildCache.js +++ b/packages/project/test/lib/build/cache/ProjectBuildCache.js @@ -1,6 +1,7 @@ import test from "ava"; import sinon from "sinon"; import ProjectBuildCache from "../../../../lib/build/cache/ProjectBuildCache.js"; +import ResourceRequestManager from "../../../../lib/build/cache/ResourceRequestManager.js"; import Cache from "../../../../lib/build/cache/Cache.js"; // Helper to create mock Project instances @@ -2171,3 +2172,207 @@ test("Fail-then-succeed: delta merge does not resurrect resources from a stage a `even though its source is gone. Written paths: ${JSON.stringify(writtenPaths)}`); }); +// ===== Regression: transitive dependency added/removed between builds ===== +// +// Scenario reproduced here: +// Some tasks read dependency resources by glob over *all* available libraries. buildThemes is +// the prominent case (it globs "/resources/**/themes/**/*.less" across every available library), +// but this is not theme-specific: any task recording a glob request against the dependency reader +// is affected. +// +// Between two builds of the same project, a transitive dependency is added to (or removed from) +// the project graph. No *resource* changed, so nothing is reported through +// dependencyResourcesChanged(). #changedDependencyResourcePaths stays empty. +// +// In validateCache, the RESTORING_DEPENDENCY_INDICES branch only calls _refreshDependencyIndices +// when #changedDependencyResourcePaths is non-empty. With an empty accumulator the refresh is +// skipped, so the restored dependency index keeps reflecting the previous build's dependency set. +// The task's dependency-index signature stays stale, the result signature still matches, and the +// project is served from cache -- even though the set of dependency resources the task would read +// has changed. +// +// These tests seed a persisted dependency-request cache recorded against an OLD dependency set, +// then validate the project against a reader exposing the NEW (changed) dependency set. The +// signature the cache reports afterwards is compared against the signature a full refresh against +// the new set produces (computed independently via ResourceRequestManager). +// +// They are marked test.failing: they assert the *desired* behavior and currently fail because the +// bug is unfixed. AVA reports a failing-marked test as a pass while it throws and as a hard error +// once it starts passing, so committing them keeps CI green and flips to a signal the moment the +// fix lands (at which point drop the `.failing`). + +// Builds a persisted "dependencies" task-metadata object by recording a single glob request +// against `oldDepResources`, mirroring what a real build stores for a task that globs dependency +// resources (e.g. buildThemes over available libraries). +async function recordDependencyRequestCache(taskName, globPatterns, oldDepResources) { + const oldDepReader = { + byGlob: async () => oldDepResources.slice(), + byPath: async (p) => oldDepResources.find((r) => r.getPath() === p) ?? null, + }; + const mgr = new ResourceRequestManager("test.lib", taskName, false); + const {signature} = await mgr.addRequests({paths: [], patterns: [globPatterns]}, oldDepReader); + return {cacheObject: mgr.toCacheObject(), signature}; +} + +// Computes the dependency-index signature a correct full refresh against `newDepResources` would +// produce, starting from the same persisted cache object. This is the "expected" value the build +// cache should converge on once the stale dependency set is picked up. +async function expectedRefreshedSignature(taskName, cacheObject, newDepResources) { + const newDepReader = { + byGlob: async () => newDepResources.slice(), + byPath: async (p) => newDepResources.find((r) => r.getPath() === p) ?? null, + }; + const mgr = ResourceRequestManager.fromCache("test.lib", taskName, false, cacheObject); + await mgr.refreshIndices(newDepReader); + return mgr.getIndexSignatures()[0]; +} + +// Creates a ProjectBuildCache restored from disk (RESTORING_DEPENDENCY_INDICES state) whose single +// task has a recorded dependency glob request. Returns the cache plus the old/expected signatures +// and a dependency reader exposing the NEW dependency set. +async function createCacheWithDependencyGlob({ + taskName = "buildThemes", + globPatterns = ["/resources/**/themes/**/*.less"], + oldDepResources, + newDepResources, + oldDependencySetIdentity = "old-dependency-set-identity", + newDependencySetIdentity = "new-dependency-set-identity", +} = {}) { + const {cacheObject: depCacheObject, signature: oldDepSignature} = + await recordDependencyRequestCache(taskName, globPatterns, oldDepResources); + const expectedDepSignature = + await expectedRefreshedSignature(taskName, depCacheObject, newDepResources); + + const project = createMockProject("test.lib", "test-lib-id"); + const cacheManager = createMockCacheManager(); + + const src = createMockResource("/test.js", "hash1", 1000, 100, 1); + project.getSourceReader.callsFake(() => ({ + byGlob: sinon.stub().resolves([src]), + byPath: sinon.stub().callsFake((p) => Promise.resolve(p === "/test.js" ? src : null)), + })); + + const indexCache = { + version: "1.0", + indexTree: { + version: 1, + indexTimestamp: 500, + root: { + name: "", + type: "directory", + hash: "root-hash", + children: { + "test.js": { + name: "test.js", + type: "resource", + hash: "node-hash-test.js", + integrity: "hash1", + lastModified: 1000, + size: 100, + inode: 1, + tags: null, + }, + }, + }, + }, + tasks: [[taskName, false]], + // Persisted dependency-set identity from the previous build. validateCache compares the + // identity passed at the next build against this; a mismatch forces the refresh. + availableDependencies: oldDependencySetIdentity, + }; + cacheManager.readIndexCache.returns(indexCache); + cacheManager.readTaskMetadata.callsFake((projectId, buildSig, task, type) => { + if (type === "dependencies") { + return depCacheObject; + } + return {requestSetGraph: {nodes: [], nextId: 1}, rootIndices: [], deltaIndices: [], unusedAtLeastOnce: false}; + }); + + const cache = await ProjectBuildCache.create(project, "build-signature", cacheManager); + await cache.initSourceIndex(); + + const newDependencyReader = { + byGlob: sinon.stub().callsFake(async () => newDepResources.slice()), + byPath: sinon.stub().callsFake(async (p) => newDepResources.find((r) => r.getPath() === p) ?? null), + }; + + return { + cache, project, cacheManager, newDependencyReader, + oldDepSignature, expectedDepSignature, taskName, newDependencySetIdentity, + }; +} + +test("validateCache: added transitive dependency refreshes dependency index (buildThemes glob)", + async (t) => { + // Old build saw one library's theme sources; a transitive library was added since, exposing a + // second .less file. No resource "changed", so dependencyResourcesChanged() is never called. + const libA = createMockResource("/resources/libA/themes/base/library.source.less", "iA"); + const libB = createMockResource("/resources/libB/themes/base/library.source.less", "iB"); + + const {cache, newDependencyReader, oldDepSignature, expectedDepSignature, taskName, + newDependencySetIdentity} = + await createCacheWithDependencyGlob({ + oldDepResources: [libA], + newDepResources: [libA, libB], + }); + + t.not(oldDepSignature, expectedDepSignature, + "precondition: adding libB must change the dependency index signature"); + t.is(cache.getTaskCache(taskName).getDependencyIndexSignatures()[0], oldDepSignature, + "restored dependency index starts at the old signature"); + + await cache.validateCache(newDependencyReader, + {prepareForBuild: true, dependencySetIdentity: newDependencySetIdentity}); + + t.is(cache.getTaskCache(taskName).getDependencyIndexSignatures()[0], expectedDepSignature, + "dependency index must reflect the added transitive dependency after validateCache"); + }); + +test("validateCache: removed transitive dependency refreshes dependency index", async (t) => { + // Reverse direction: a library present in the previous build is no longer a dependency, so its + // theme sources should drop out of the index. Again nothing is reported as a changed resource. + const libA = createMockResource("/resources/libA/themes/base/library.source.less", "iA"); + const libB = createMockResource("/resources/libB/themes/base/library.source.less", "iB"); + + const {cache, newDependencyReader, oldDepSignature, expectedDepSignature, taskName, + newDependencySetIdentity} = + await createCacheWithDependencyGlob({ + oldDepResources: [libA, libB], + newDepResources: [libA], + }); + + t.not(oldDepSignature, expectedDepSignature, + "precondition: removing libB must change the dependency index signature"); + + await cache.validateCache(newDependencyReader, + {prepareForBuild: true, dependencySetIdentity: newDependencySetIdentity}); + + t.is(cache.getTaskCache(taskName).getDependencyIndexSignatures()[0], expectedDepSignature, + "dependency index must reflect the removed transitive dependency after validateCache"); +}); + +test("validateCache: dependency-set change is general, not specific to the buildThemes glob", + async (t) => { + // The same staleness affects any task that globs dependency resources. Use a generic JS glob and + // task name to show the defect is not tied to theme handling. + const modA = createMockResource("/resources/libA/Component.js", "iA"); + const modB = createMockResource("/resources/libB/Component.js", "iB"); + + const {cache, newDependencyReader, oldDepSignature, expectedDepSignature, taskName, + newDependencySetIdentity} = + await createCacheWithDependencyGlob({ + taskName: "generateComponentPreload", + globPatterns: ["/resources/**/*.js"], + oldDepResources: [modA], + newDepResources: [modA, modB], + }); + + t.not(oldDepSignature, expectedDepSignature, + "precondition: adding libB's module must change the dependency index signature"); + + await cache.validateCache(newDependencyReader, + {prepareForBuild: true, dependencySetIdentity: newDependencySetIdentity}); + + t.is(cache.getTaskCache(taskName).getDependencyIndexSignatures()[0], expectedDepSignature, + "dependency index must refresh for any dependency-globbing task, not just buildThemes"); + });