Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"comment": "In PnpmShrinkwrapFile getIntegrityForImporter, remove the external filter and include workspace-local link: dependencies by recursing into their importer entries, so shrinkwrap-deps.json hashes cover the full dependency tree.",
"type": "patch",
"packageName": "@microsoft/rush"
}
],
"packageName": "@microsoft/rush"
}
82 changes: 56 additions & 26 deletions libraries/rush-lib/src/logic/pnpm/PnpmShrinkwrapFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -911,36 +911,57 @@ export class PnpmShrinkwrapFile extends BaseShrinkwrapFile {
if (!integrityMap) {
const importer: IPnpmShrinkwrapImporterYaml | undefined = this.getImporter(importerKey);
if (importer) {
integrityMap = new Map();
this._integrities.set(importerKey, integrityMap);
const resolvedIntegrityMap: Map<string, string> = new Map();
integrityMap = resolvedIntegrityMap;
this._integrities.set(importerKey, resolvedIntegrityMap);

const sha256Digest: string = crypto
.createHash('sha256')
.update(JSON.stringify(importer))
.digest('base64');
const selfIntegrity: string = `${importerKey}:${sha256Digest}:`;
integrityMap.set(importerKey, selfIntegrity);
resolvedIntegrityMap.set(importerKey, selfIntegrity);

const { dependencies, devDependencies, optionalDependencies } = importer;

const externalFilter: (name: string, version: IPnpmVersionSpecifier) => boolean = (
name: string,
versionSpecifier: IPnpmVersionSpecifier
): boolean => {
const version: string = normalizePnpmVersionSpecifier(versionSpecifier);
return !version.includes('link:');
const processCollection = (
collection: Record<string, IPnpmVersionSpecifier>,
optional: boolean
): void => {
const externalDeps: Record<string, IPnpmVersionSpecifier> = {};
for (const [name, versionSpecifier] of Object.entries(collection)) {
const version: string = normalizePnpmVersionSpecifier(versionSpecifier);
if (version.startsWith('link:')) {
// This is a workspace-local dependency; resolve it to an importer key and recurse.
// The link: path is relative to the project folder (which is the importer key itself),
// so we join the importer key with the link path (not dirname).
// Lockfile paths are always POSIX, so we use path.posix helpers.
const linkPath: string = version.slice('link:'.length);
const targetKey: string = path.posix.normalize(path.posix.join(importerKey, linkPath));
const linkedIntegrities: Map<string, string> | undefined =
this.getIntegrityForImporter(targetKey);
if (linkedIntegrities) {
for (const [dep, integrity] of linkedIntegrities) {
resolvedIntegrityMap.set(dep, integrity);
}
}
} else {
externalDeps[name] = versionSpecifier;
}
}
this._addIntegrities(resolvedIntegrityMap, externalDeps, optional);
};

if (dependencies) {
this._addIntegrities(integrityMap, dependencies, false, externalFilter);
processCollection(dependencies, false);
}

if (devDependencies) {
this._addIntegrities(integrityMap, devDependencies, false, externalFilter);
processCollection(devDependencies, false);
}

if (optionalDependencies) {
this._addIntegrities(integrityMap, optionalDependencies, true, externalFilter);
processCollection(optionalDependencies, true);
}
}
}
Expand Down Expand Up @@ -1269,23 +1290,32 @@ export class PnpmShrinkwrapFile extends BaseShrinkwrapFile {
private _addIntegrities(
integrityMap: Map<string, string>,
collection: Record<string, IPnpmVersionSpecifier>,
optional: boolean,
filter?: (name: string, version: IPnpmVersionSpecifier) => boolean
optional: boolean
): void {
for (const [name, version] of Object.entries(collection)) {
if (filter && !filter(name, version)) {
continue;
}

const packageId: string = this._getPackageId(name, version);
if (integrityMap.has(packageId)) {
// The entry could already have been added as a nested dependency
continue;
}
const normalizedVersion: string = normalizePnpmVersionSpecifier(version);
if (normalizedVersion.startsWith('link:')) {
// In a package snapshot, link: paths are resolved relative to the pnpm workspace root,
// which means they are already direct keys into the importer map.
const targetKey: string = normalizedVersion.slice('link:'.length);
const linkedIntegrities: Map<string, string> | undefined =
this.getIntegrityForImporter(targetKey);
if (linkedIntegrities) {
for (const [dep, integrity] of linkedIntegrities) {
integrityMap.set(dep, integrity);
}
}
} else {
const packageId: string = this._getPackageId(name, version);
if (integrityMap.has(packageId)) {
// The entry could already have been added as a nested dependency
continue;
}

const contribution: Map<string, string> = this._getIntegrityForPackage(packageId, optional);
for (const [dep, integrity] of contribution) {
integrityMap.set(dep, integrity);
const contribution: Map<string, string> = this._getIntegrityForPackage(packageId, optional);
for (const [dep, integrity] of contribution) {
integrityMap.set(dep, integrity);
}
}
}
}
Expand Down
228 changes: 228 additions & 0 deletions libraries/rush-lib/src/logic/pnpm/test/PnpmShrinkwrapFile.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,234 @@ snapshots:
// because the sub-dependency (bar) resolved to different versions
expect(fooIntegrity1).not.toEqual(fooIntegrity2);
});

it('includes workspace-local link: dependencies by recursing into their importer entries', () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot Be sure to cover the following scenarios with test cases:

  1. Workspace project 1 -> Workspace project 2 -> External Dep 1 -> External Dep 2
  2. Workspace project 1 -> External Dep 1 -> Workspace project 2 -> External Dep 2

Note that these may not necessarily be the same dependencies between examples.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added both scenarios in b37088f:

  • Scenario 1 ("workspace project 1 → workspace project 2 → external dep 1 → external dep 2"): project-1 links to project-2, project-2 depends on ext-a, ext-a transitively depends on ext-b — all four entries verified in project-1's integrity map.
  • Scenario 2 ("workspace project 1 → external dep 1 and workspace project 2 → external dep 2"): project-1 has both a direct external dep (ext-x) and a workspace link dep (project-2), project-2 depends on ext-y — all four entries verified.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot , for clarity, in Scenario 2, external dep 1 has a link: dependency that points back into the repository at workspace project 2

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated in ee7bdb3. Scenario 2 now models the correct topology: ../../project-1 depends on ext-a@1.0.0, whose snapshot has project-2: link:../../project-2, and ../../project-2 depends on ext-b@2.0.0.

Also added handling in _addIntegrities for link: deps in package snapshots — since pnpm resolves these relative to the workspace root, the path after link: is used directly as the importer key with no further manipulation.

// This test verifies that link: (workspace-local) dependencies are no longer filtered out.
// The shrinkwrap-deps.json for an importer should include hashes from its workspace
// dependencies' importer sections, all the way down the tree.
//
// In a real Rush repo (no subspaces), importer keys start with '../../' and
// link: paths start with '../'.
//
// Topology: project-1 -> (link:) project-2 -> lodash@4.17.21

const shrinkwrapContent: string = `
lockfileVersion: '9.0'
settings:
autoInstallPeers: true
excludeLinksFromLockfile: false
importers:
.:
{}
../../project-1:
dependencies:
project-2:
specifier: workspace:*
version: link:../project-2
../../project-2:
dependencies:
lodash:
specifier: ^4.17.0
version: 4.17.21
packages:
lodash@4.17.21:
resolution:
integrity: sha512-lodash==
snapshots:
lodash@4.17.21: {}
`;

const shrinkwrapFile = PnpmShrinkwrapFile.loadFromString(shrinkwrapContent, {
subspaceHasNoProjects: false
});

PnpmShrinkwrapFile.clearCache();

const proj1IntegrityMap = shrinkwrapFile.getIntegrityForImporter('../../project-1');

expect(proj1IntegrityMap).toBeDefined();

// project-1's integrity map should include project-2's importer entry
expect(proj1IntegrityMap!.has('../../project-2')).toBe(true);

// It should also include the transitive external dependency of project-2
expect(proj1IntegrityMap!.has('lodash@4.17.21')).toBe(true);

// The integrity map for project-2 itself should also be populated
const proj2IntegrityMap = shrinkwrapFile.getIntegrityForImporter('../../project-2');
expect(proj2IntegrityMap).toBeDefined();
expect(proj2IntegrityMap!.has('../../project-2')).toBe(true);
expect(proj2IntegrityMap!.has('lodash@4.17.21')).toBe(true);
});

it('produces different hashes when a workspace-local dependency changes', () => {
// This test verifies that changing the dependencies of a workspace-local package
// causes the dependent importer's integrity to differ.
//
// Topology: project-1 -> (link:) project-2 -> lodash@4.17.x (version differs between cases)

const buildContent = (lodashVersion: string): string => `
lockfileVersion: '9.0'
settings:
autoInstallPeers: true
excludeLinksFromLockfile: false
importers:
.:
{}
../../project-1:
dependencies:
project-2:
specifier: workspace:*
version: link:../project-2
../../project-2:
dependencies:
lodash:
specifier: ^4.17.0
version: ${lodashVersion}
packages:
lodash@${lodashVersion}:
resolution:
integrity: sha512-lodash-${lodashVersion}==
snapshots:
lodash@${lodashVersion}: {}
`;

const shrinkwrapFile1 = PnpmShrinkwrapFile.loadFromString(buildContent('4.17.21'), {
subspaceHasNoProjects: false
});
const shrinkwrapFile2 = PnpmShrinkwrapFile.loadFromString(buildContent('4.17.20'), {
subspaceHasNoProjects: false
});

PnpmShrinkwrapFile.clearCache();

const proj1IntegrityMap1 = shrinkwrapFile1.getIntegrityForImporter('../../project-1');
const proj1IntegrityMap2 = shrinkwrapFile2.getIntegrityForImporter('../../project-1');

expect(proj1IntegrityMap1).toBeDefined();
expect(proj1IntegrityMap2).toBeDefined();

// The self-hash of project-1 does NOT change because the root importer object itself is
// identical in both cases (it still references link:../project-2). However, project-2's
// integrity hash should differ because its lodash dependency resolved to a different version.
const proj2Integrity1 = proj1IntegrityMap1!.get('../../project-2');
const proj2Integrity2 = proj1IntegrityMap2!.get('../../project-2');

expect(proj2Integrity1).toBeDefined();
expect(proj2Integrity2).toBeDefined();
expect(proj2Integrity1).not.toEqual(proj2Integrity2);
});

it('scenario 1: workspace project 1 -> workspace project 2 -> external dep 1 -> external dep 2', () => {
// Tests the full chain: project-1 links to project-2, project-2 depends on ext-a,
// and ext-a transitively depends on ext-b. All four should appear in project-1's integrity map.

const shrinkwrapContent: string = `
lockfileVersion: '9.0'
settings:
autoInstallPeers: true
excludeLinksFromLockfile: false
importers:
.:
{}
../../project-1:
dependencies:
project-2:
specifier: workspace:*
version: link:../project-2
../../project-2:
dependencies:
ext-a:
specifier: ^1.0.0
version: 1.0.0
packages:
ext-a@1.0.0:
resolution:
integrity: sha512-ext-a==
ext-b@1.0.0:
resolution:
integrity: sha512-ext-b==
snapshots:
ext-a@1.0.0:
dependencies:
ext-b: 1.0.0
ext-b@1.0.0: {}
`;

const shrinkwrapFile = PnpmShrinkwrapFile.loadFromString(shrinkwrapContent, {
subspaceHasNoProjects: false
});

PnpmShrinkwrapFile.clearCache();

const proj1IntegrityMap = shrinkwrapFile.getIntegrityForImporter('../../project-1');

expect(proj1IntegrityMap).toBeDefined();
// project-2's importer entry
expect(proj1IntegrityMap!.has('../../project-2')).toBe(true);
// ext-a (direct dep of project-2)
expect(proj1IntegrityMap!.has('ext-a@1.0.0')).toBe(true);
// ext-b (transitive dep through ext-a)
expect(proj1IntegrityMap!.has('ext-b@1.0.0')).toBe(true);
});

it('scenario 2: workspace project 1 -> external dep 1 -> (link:) workspace project 2 -> external dep 2', () => {
// Tests that when an external package's snapshot has a link: dependency pointing back into
// the workspace, the linked workspace project's integrity is fully captured.
//
// project-1 depends on ext-a (external).
// ext-a's snapshot has a link: dep that resolves to project-2 (a workspace project).
// project-2 depends on ext-b (external).
// All four entries should appear in project-1's integrity map.

const shrinkwrapContent: string = `
lockfileVersion: '9.0'
settings:
autoInstallPeers: true
excludeLinksFromLockfile: false
importers:
.:
{}
../../project-1:
dependencies:
ext-a:
specifier: ^1.0.0
version: 1.0.0
../../project-2:
dependencies:
ext-b:
specifier: ^2.0.0
version: 2.0.0
packages:
ext-a@1.0.0:
resolution:
integrity: sha512-ext-a==
ext-b@2.0.0:
resolution:
integrity: sha512-ext-b==
snapshots:
ext-a@1.0.0:
dependencies:
project-2: link:../../project-2
ext-b@2.0.0: {}
`;

const shrinkwrapFile = PnpmShrinkwrapFile.loadFromString(shrinkwrapContent, {
subspaceHasNoProjects: false
});

PnpmShrinkwrapFile.clearCache();

const proj1IntegrityMap = shrinkwrapFile.getIntegrityForImporter('../../project-1');

expect(proj1IntegrityMap).toBeDefined();
// ext-a (direct external dep of project-1)
expect(proj1IntegrityMap!.has('ext-a@1.0.0')).toBe(true);
// project-2 (workspace project, reached via link: in ext-a's snapshot)
expect(proj1IntegrityMap!.has('../../project-2')).toBe(true);
// ext-b (external dep of project-2, reached transitively through the link:)
expect(proj1IntegrityMap!.has('ext-b@2.0.0')).toBe(true);
});
});

describe('Check is workspace project modified', () => {
Expand Down
Loading