-
Notifications
You must be signed in to change notification settings - Fork 663
Description
Summary
dependsOnAdditionalFiles with multiple globs does not give consistent caching behavior
Repro steps
Putting 8 lines into dependsOnAdditionalFiles in our rush-project.common.json, our various projects would randomly get cache hits or not.
Expected result:
Consistent caching results.
Actual result:
We'd often see 30-70% caching rather than 100% as expected.
Details
Combining the 8 lines into one glob entry that matches the same files does not have the issue.
So the issue appears to be that the globs are processed (and the results inserted) in effectively random order.
The getAdditionalFilesFromRushProjectConfigurationAsync function processing the globs with Async.forEachAsync and inserting in somewhat random order seems like the root cause.
One simple fix (that gets rid of the symptom at least) is to sort the files before hashing.
index 8eeaa5ed22..0568558d01 100644
--- a/libraries/rush-lib/src/logic/incremental/InputsSnapshot.ts
+++ b/libraries/rush-lib/src/logic/incremental/InputsSnapshot.ts
@@ -318,7 +318,9 @@ export class InputsSnapshot implements IInputsSnapshot {
const additionalFilesForOperation: ReadonlySet<string> | undefined =
record.additionalFilesByOperationName?.get(operationName);
if (additionalFilesForOperation) {
- for (const [filePath, hash] of this._resolveHashes(additionalFilesForOperation)) {
+ const sortedAdditionalFiles: string[] = Array.from(additionalFilesForOperation).sort();
+ for (const [filePath, hash] of this._resolveHashes(sortedAdditionalFiles)) {
hashes.set(filePath, hash);
}
}This doesn't fix the fact that insert order is non-deterministic, it just works around it.
Also it changes the existing hash order unnecessarily. That feels wrong so I didn't just submit the above as a PR.
Making the inserts into the list of files be deterministically ordered would be a more complex change.
Standard questions
Please answer these questions to help us investigate your issue more quickly:
| Question | Answer |
|---|---|
@microsoft/rush globally installed version? |
5.153.1 |
rushVersion from rush.json? |
5.153.1 |
pnpmVersion, npmVersion, or yarnVersion from rush.json? |
[email protected] |
(if pnpm) useWorkspaces from pnpm-config.json? |
true |
| Operating system? | Mac |
| Would you consider contributing a PR? | Yes |
Node.js version (node -v)? |
22.17.0 |
Metadata
Metadata
Assignees
Labels
Type
Projects
Status