Skip to content

Commit a8cc900

Browse files
LPegasusoctogonz
andauthored
[rush-lib] ProjectChangeAnalyzer should analyze the scope of impact of pnpm-lock.yaml changes across all subspaces (#5482)
* [rush-lib] ProjectChangeAnalyzer should check non-default subspace pnpm-lock.yaml changes * Update common/changes/@microsoft/rush/fix-project-change-analyzer-nondefault-subspace-pnpmlock-check_2025-12-03-13-27.json Co-authored-by: Pete Gonzalez <[email protected]> * Update libraries/rush-lib/src/logic/test/ProjectChangeAnalyzer.test.ts Co-authored-by: Pete Gonzalez <[email protected]> * [rush-lib] ProjectChangeAnalyzer L138-L140, use array instead of Set for subspaces iteration * [rush-lib] Fix test for ProjectChangeAnalyzer 1. Fix `.pnpmfile.cjs` `readPackage` function is not defined. 2. Fix the comment of why 'e' is not in changedProjects. --------- Co-authored-by: LPegasus <[email protected]> Co-authored-by: Pete Gonzalez <[email protected]>
1 parent e049b1c commit a8cc900

File tree

18 files changed

+329
-38
lines changed

18 files changed

+329
-38
lines changed
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"changes": [
3+
{
4+
"packageName": "@microsoft/rush",
5+
"comment": "Fix an issue where ProjectChangeAnalyzer checked the pnpm-lock.yaml file in the default subspace only, when it should consider all subspaces.",
6+
"type": "none"
7+
}
8+
],
9+
"packageName": "@microsoft/rush"
10+
}

libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts

Lines changed: 61 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
import type { ITerminal } from '@rushstack/terminal';
1818

1919
import type { RushConfiguration } from '../api/RushConfiguration';
20+
import type { Subspace } from '../api/Subspace';
2021
import { RushProjectConfiguration } from '../api/RushProjectConfiguration';
2122
import type { RushConfigurationProject } from '../api/RushConfigurationProject';
2223
import { BaseProjectShrinkwrapFile } from './base/BaseProjectShrinkwrapFile';
@@ -134,53 +135,76 @@ export class ProjectChangeAnalyzer {
134135
// Even though changing the installed version of a nested dependency merits a change file,
135136
// ignore lockfile changes for `rush change` for the moment
136137

138+
const subspaces: Iterable<Subspace> = rushConfiguration.subspacesFeatureEnabled
139+
? rushConfiguration.subspaces
140+
: [rushConfiguration.defaultSubspace];
141+
137142
const variantToUse: string | undefined =
138143
variant ?? (await this._rushConfiguration.getCurrentlyInstalledVariantAsync());
139-
const fullShrinkwrapPath: string =
140-
rushConfiguration.defaultSubspace.getCommittedShrinkwrapFilePath(variantToUse);
141144

142-
const relativeShrinkwrapFilePath: string = Path.convertToSlashes(
143-
path.relative(repoRoot, fullShrinkwrapPath)
144-
);
145-
const shrinkwrapStatus: IFileDiffStatus | undefined = changedFiles.get(relativeShrinkwrapFilePath);
145+
await Async.forEachAsync(subspaces, async (subspace: Subspace) => {
146+
const fullShrinkwrapPath: string = subspace.getCommittedShrinkwrapFilePath(variantToUse);
146147

147-
if (shrinkwrapStatus) {
148-
if (shrinkwrapStatus.status !== 'M') {
149-
terminal.writeLine(`Lockfile was created or deleted. Assuming all projects are affected.`);
150-
return new Set(rushConfiguration.projects);
151-
}
152-
153-
if (rushConfiguration.isPnpm) {
154-
const currentShrinkwrap: PnpmShrinkwrapFile | undefined =
155-
PnpmShrinkwrapFile.loadFromFile(fullShrinkwrapPath);
148+
const relativeShrinkwrapFilePath: string = Path.convertToSlashes(
149+
path.relative(repoRoot, fullShrinkwrapPath)
150+
);
151+
const shrinkwrapStatus: IFileDiffStatus | undefined = changedFiles.get(relativeShrinkwrapFilePath);
152+
const subspaceProjects: RushConfigurationProject[] = subspace.getProjects();
156153

157-
if (!currentShrinkwrap) {
158-
throw new Error(`Unable to obtain current shrinkwrap file.`);
154+
if (shrinkwrapStatus) {
155+
if (shrinkwrapStatus.status !== 'M') {
156+
if (rushConfiguration.subspacesFeatureEnabled) {
157+
terminal.writeLine(
158+
`"${subspace.subspaceName}" subspace lockfile was created or deleted. Assuming all projects are affected.`
159+
);
160+
} else {
161+
terminal.writeLine(`Lockfile was created or deleted. Assuming all projects are affected.`);
162+
}
163+
for (const project of subspaceProjects) {
164+
changedProjects.add(project);
165+
}
166+
return;
159167
}
160168

161-
const oldShrinkwrapText: string = await this._git.getBlobContentAsync({
162-
// <ref>:<path> syntax: https://git-scm.com/docs/gitrevisions
163-
blobSpec: `${mergeCommit}:${relativeShrinkwrapFilePath}`,
164-
repositoryRoot: repoRoot
165-
});
166-
const oldShrinkWrap: PnpmShrinkwrapFile = PnpmShrinkwrapFile.loadFromString(oldShrinkwrapText);
167-
168-
for (const project of rushConfiguration.projects) {
169-
if (
170-
currentShrinkwrap
171-
.getProjectShrinkwrap(project)
172-
.hasChanges(oldShrinkWrap.getProjectShrinkwrap(project))
173-
) {
174-
changedProjects.add(project);
169+
if (rushConfiguration.isPnpm) {
170+
const currentShrinkwrap: PnpmShrinkwrapFile | undefined =
171+
PnpmShrinkwrapFile.loadFromFile(fullShrinkwrapPath);
172+
173+
if (!currentShrinkwrap) {
174+
throw new Error(`Unable to obtain current shrinkwrap file.`);
175+
}
176+
177+
const oldShrinkwrapText: string = await this._git.getBlobContentAsync({
178+
// <ref>:<path> syntax: https://git-scm.com/docs/gitrevisions
179+
blobSpec: `${mergeCommit}:${relativeShrinkwrapFilePath}`,
180+
repositoryRoot: repoRoot
181+
});
182+
const oldShrinkWrap: PnpmShrinkwrapFile = PnpmShrinkwrapFile.loadFromString(oldShrinkwrapText);
183+
184+
for (const project of subspaceProjects) {
185+
if (
186+
currentShrinkwrap
187+
.getProjectShrinkwrap(project)
188+
.hasChanges(oldShrinkWrap.getProjectShrinkwrap(project))
189+
) {
190+
changedProjects.add(project);
191+
}
175192
}
193+
} else {
194+
if (rushConfiguration.subspacesFeatureEnabled) {
195+
terminal.writeLine(
196+
`"${subspace.subspaceName}" subspace lockfile has changed and lockfile content comparison is only supported for pnpm. Assuming all projects are affected.`
197+
);
198+
} else {
199+
terminal.writeLine(
200+
`Lockfile has changed and lockfile content comparison is only supported for pnpm. Assuming all projects are affected.`
201+
);
202+
}
203+
subspace.getProjects().forEach((project) => changedProjects.add(project));
204+
return;
176205
}
177-
} else {
178-
terminal.writeLine(
179-
`Lockfile has changed and lockfile content comparison is only supported for pnpm. Assuming all projects are affected.`
180-
);
181-
return new Set(rushConfiguration.projects);
182206
}
183-
}
207+
});
184208
}
185209

186210
return changedProjects;

libraries/rush-lib/src/logic/test/ProjectChangeAnalyzer.test.ts

Lines changed: 135 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,67 @@ jest.mock(`@rushstack/package-deps-hash`, () => {
4141
},
4242
hashFilesAsync(rootDirectory: string, filePaths: Iterable<string>): ReadonlyMap<string, string> {
4343
return new Map(Array.from(filePaths, (filePath: string) => [filePath, filePath]));
44+
},
45+
getRepoChanges(): Map<string, IFileDiffStatus> {
46+
return new Map<string, IFileDiffStatus>([
47+
[
48+
// Test subspace lockfile change detection
49+
'common/config/subspaces/project-change-analyzer-test-subspace/pnpm-lock.yaml',
50+
{
51+
mode: 'modified',
52+
newhash: 'newhash',
53+
oldhash: 'oldhash',
54+
status: 'M'
55+
}
56+
],
57+
[
58+
// Test lockfile deletion detection
59+
'common/config/subspaces/default/pnpm-lock.yaml',
60+
{
61+
mode: 'deleted',
62+
newhash: '',
63+
oldhash: 'oldhash',
64+
status: 'D'
65+
}
66+
]
67+
]);
68+
}
69+
};
70+
});
71+
72+
const { Git: OriginalGit } = jest.requireActual('../Git');
73+
/** Mock Git to test `getChangedProjectsAsync` */
74+
jest.mock('../Git', () => {
75+
return {
76+
Git: class MockGit extends OriginalGit {
77+
public async determineIfRefIsACommitAsync(ref: string): Promise<boolean> {
78+
return true;
79+
}
80+
public async getMergeBaseAsync(ref1: string, ref2: string): Promise<string> {
81+
return 'merge-base-sha';
82+
}
83+
public async getBlobContentAsync(opts: { blobSpec: string; repositoryRoot: string }): Promise<string> {
84+
return '';
85+
}
86+
}
87+
};
88+
});
89+
90+
const OriginalPnpmShrinkwrapFile = jest.requireActual('../pnpm/PnpmShrinkwrapFile').PnpmShrinkwrapFile;
91+
jest.mock('../pnpm/PnpmShrinkwrapFile', () => {
92+
return {
93+
PnpmShrinkwrapFile: {
94+
loadFromFile: (fullShrinkwrapPath: string): PnpmShrinkwrapFile => {
95+
return OriginalPnpmShrinkwrapFile.loadFromString(_getMockedPnpmShrinkwrapFile());
96+
},
97+
loadFromString: (text: string): PnpmShrinkwrapFile => {
98+
return OriginalPnpmShrinkwrapFile.loadFromString(
99+
_getMockedPnpmShrinkwrapFile()
100+
// Change dependencies version
101+
.replace(/1\.0\.1/g, '1.0.0')
102+
.replace(/foo_1_0_1/g, 'foo_1_0_0')
103+
);
104+
}
44105
}
45106
};
46107
});
@@ -55,7 +116,7 @@ jest.mock('../incremental/InputsSnapshot', () => {
55116

56117
import { resolve } from 'node:path';
57118

58-
import type { IDetailedRepoState } from '@rushstack/package-deps-hash';
119+
import type { IDetailedRepoState, IFileDiffStatus } from '@rushstack/package-deps-hash';
59120
import { StringBufferTerminalProvider, Terminal } from '@rushstack/terminal';
60121

61122
import { ProjectChangeAnalyzer } from '../ProjectChangeAnalyzer';
@@ -65,6 +126,7 @@ import type {
65126
GetInputsSnapshotAsyncFn,
66127
IInputsSnapshotParameters
67128
} from '../incremental/InputsSnapshot';
129+
import type { PnpmShrinkwrapFile } from '../pnpm/PnpmShrinkwrapFile';
68130

69131
describe(ProjectChangeAnalyzer.name, () => {
70132
beforeEach(() => {
@@ -101,4 +163,76 @@ describe(ProjectChangeAnalyzer.name, () => {
101163
expect(mockInput.additionalHashes).toEqual(new Map());
102164
});
103165
});
166+
167+
describe(ProjectChangeAnalyzer.prototype.getChangedProjectsAsync.name, () => {
168+
it('Subspaces detects external changes', async () => {
169+
const rootDir: string = resolve(__dirname, 'repoWithSubspaces');
170+
const rushConfiguration: RushConfiguration = RushConfiguration.loadFromConfigurationFile(
171+
resolve(rootDir, 'rush.json')
172+
);
173+
const projectChangeAnalyzer: ProjectChangeAnalyzer = new ProjectChangeAnalyzer(rushConfiguration);
174+
175+
const terminalProvider: StringBufferTerminalProvider = new StringBufferTerminalProvider(true);
176+
const terminal: Terminal = new Terminal(terminalProvider);
177+
178+
const changedProjects = await projectChangeAnalyzer.getChangedProjectsAsync({
179+
enableFiltering: false,
180+
includeExternalDependencies: true,
181+
targetBranchName: 'main',
182+
terminal
183+
});
184+
185+
// a,b,c is included because of change modifier is not modified
186+
// d is included because its dependency foo version changed in the subspace lockfile
187+
['a', 'b', 'c', 'd'].forEach((projectName) => {
188+
expect(changedProjects.has(rushConfiguration.getProjectByName(projectName)!)).toBe(true);
189+
});
190+
191+
// e depends on d via workspace:*, but its calculated lockfile (e.g. "e/.rush/temp/shrinkwrap-deps.json") didn't change.
192+
// So it's not included. e will be included by `expandConsumers` if needed.
193+
['e', 'f'].forEach((projectName) => {
194+
expect(changedProjects.has(rushConfiguration.getProjectByName(projectName)!)).toBe(false);
195+
});
196+
});
197+
});
104198
});
199+
200+
/**
201+
* Create a fake pnpm-lock.yaml content matches "libraries/rush-lib/src/logic/test/repoWithSubspaces" test repo
202+
*/
203+
function _getMockedPnpmShrinkwrapFile(): string {
204+
return `lockfileVersion: '9.0'
205+
206+
settings:
207+
autoInstallPeers: false
208+
excludeLinksFromLockfile: false
209+
210+
importers:
211+
212+
.: {}
213+
214+
../../../d:
215+
dependencies:
216+
foo:
217+
specifier: ~1.0.0
218+
version: 1.0.1
219+
220+
../../../e:
221+
dependencies:
222+
d:
223+
specifier: workspace:*
224+
version: link:../../../d
225+
226+
../../../f:
227+
dependencies:
228+
229+
packages:
230+
231+
232+
resolution: {integrity: 'foo_1_0_1'}
233+
234+
snapshots:
235+
236+
237+
`;
238+
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"name": "a",
3+
"version": "1.0.0",
4+
"description": "Test package a"
5+
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
"name": "b",
3+
"version": "2.0.0",
4+
"description": "Test package b",
5+
"dependencies": {
6+
"foo": "~1.0.0"
7+
}
8+
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
"name": "c",
3+
"version": "3.1.1",
4+
"description": "Test package c",
5+
"dependencies": {
6+
"b": "workspace:*"
7+
}
8+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"exemptDecoupledDependenciesBetweenSubspaces": true
3+
}
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"$schema": "https://developer.microsoft.com/json-schemas/rush/v5/pnpm-config.schema.json",
3+
"useWorkspaces": true
4+
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"$schema": "https://developer.microsoft.com/json-schemas/rush/v5/subspaces.schema.json",
3+
"subspacesEnabled": true,
4+
"subspaceNames": ["project-change-analyzer-test-subspace"]
5+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
[]

0 commit comments

Comments
 (0)