From 27ec4765bb57e4f3c57ba0adda7ab064855ad0bc Mon Sep 17 00:00:00 2001 From: Sachin Joseph Date: Mon, 8 Jul 2019 19:20:05 -0700 Subject: [PATCH 1/6] Fix 1347 --- .../src/logic/pnpm/PnpmLinkManager.ts | 57 ++++++++++++++++--- .../src/logic/pnpm/PnpmShrinkwrapFile.ts | 22 ++++--- 2 files changed, 63 insertions(+), 16 deletions(-) diff --git a/apps/rush-lib/src/logic/pnpm/PnpmLinkManager.ts b/apps/rush-lib/src/logic/pnpm/PnpmLinkManager.ts index 1f00212c0dd..d12c3bdb95a 100644 --- a/apps/rush-lib/src/logic/pnpm/PnpmLinkManager.ts +++ b/apps/rush-lib/src/logic/pnpm/PnpmLinkManager.ts @@ -23,6 +23,7 @@ import { BasePackage } from '../base/BasePackage'; import { RushConstants } from '../../logic/RushConstants'; import { IRushLinkJson } from '../../api/RushConfiguration'; import { RushConfigurationProject } from '../../api/RushConfigurationProject'; +import { PnpmShrinkwrapFile } from './PnpmShrinkwrapFile'; // special flag for debugging, will print extra diagnostic information, // but comes with performance cost @@ -35,12 +36,24 @@ export class PnpmLinkManager extends BaseLinkManager { localLinks: {} }; + const pnpmShrinkwrapFileName: string = this._rushConfiguration.getCommittedShrinkwrapFilename( + this._rushConfiguration.currentInstalledVariant + ); + + const pnpmShrinkwrapFile: PnpmShrinkwrapFile | undefined = PnpmShrinkwrapFile.loadFromFile( + pnpmShrinkwrapFileName + ); + + if (!pnpmShrinkwrapFile) { + throw new InternalError(`Cannot load shrinkwrap at "${pnpmShrinkwrapFileName}"`); + } + let promise: Promise = Promise.resolve(); for (const rushProject of this._rushConfiguration.projects) { promise = promise.then(() => { console.log(os.EOL + 'LINKING: ' + rushProject.packageName); - return this._linkProject(rushProject, rushLinkJson); + return this._linkProject(rushProject, rushLinkJson, pnpmShrinkwrapFile); }); } @@ -60,7 +73,8 @@ export class PnpmLinkManager extends BaseLinkManager { */ private _linkProject( project: RushConfigurationProject, - rushLinkJson: IRushLinkJson): Promise { + rushLinkJson: IRushLinkJson, + pnpmShrinkwrapFile: PnpmShrinkwrapFile | undefined): Promise { // first, read the temp package.json information @@ -139,14 +153,39 @@ export class PnpmLinkManager extends BaseLinkManager { // appropriate. This folder is usually something like: // C:\{uri-encoed-path-to-tgz}\node_modules\{package-name} + // e.g.: + // file:projects/bentleyjs-core.tgz + // file:projects/build-tools.tgz_dc21d88642e18a947127a751e00b020a + // file:projects/imodel-from-geojson.tgz_request@2.88.0 + const topLevelDependencyVersion: string | undefined = + pnpmShrinkwrapFile!.getTopLevelDependencyVersion(project.tempProjectName); + + if (!topLevelDependencyVersion) { + throw new InternalError(`Cannot find top level dependency for "${project.tempProjectName}"` + + ` in shrinkwrap.`); + } + + const packageId: string | undefined = pnpmShrinkwrapFile!.getPackageId(topLevelDependencyVersion); + + // e.g.: projects\api-documenter.tgz + let relativePathToTgzFile: string; + + if (packageId) { + relativePathToTgzFile = packageId.slice('file:'.length); + } else { + relativePathToTgzFile = topLevelDependencyVersion.slice('file:'.length); + } + // e.g.: C:\wbt\common\temp\projects\api-documenter.tgz - const pathToTgzFile: string = path.join( - this._rushConfiguration.commonTempFolder, - 'projects', - `${unscopedTempProjectName}.tgz`); + const absolutePathToTgzFile: string = path.join(this._rushConfiguration.commonTempFolder, relativePathToTgzFile); - // e.g.: C%3A%2Fwbt%2Fcommon%2Ftemp%2Fprojects%2Fapi-documenter.tgz - const escapedPathToTgzFile: string = uriEncode(Text.replaceAll(pathToTgzFile, path.sep, '/')); + // e.g.: + // C%3A%2Fwbt%2Fcommon%2Ftemp%2Fprojects%2Fapi-documenter.tgz + // C%3A%2Fdev%2Fimodeljs%2Fimodeljs%2Fcommon%2Ftemp%2Fprojects%2Fpresentation-integration-tests.tgz_jsdom@11.12.0 + // C%3A%2Fdev%2Fimodeljs%2Fimodeljs%2Fcommon%2Ftemp%2Fprojects%2Fbuild-tools.tgz_2a665c89609864b4e75bc5365d7f8f56 + const dirNameInLocal: string = uriEncode(Text.replaceAll(absolutePathToTgzFile, path.sep, '/')) + + (packageId && packageId.length < topLevelDependencyVersion.length ? + topLevelDependencyVersion.slice(packageId.length) : ''); // tslint:disable-next-line:max-line-length // e.g.: C:\wbt\common\temp\node_modules\.local\C%3A%2Fwbt%2Fcommon%2Ftemp%2Fprojects%2Fapi-documenter.tgz\node_modules @@ -154,7 +193,7 @@ export class PnpmLinkManager extends BaseLinkManager { this._rushConfiguration.commonTempFolder, RushConstants.nodeModulesFolderName, '.local', - escapedPathToTgzFile, + dirNameInLocal, RushConstants.nodeModulesFolderName); for (const dependencyName of Object.keys(commonPackage.packageJson!.dependencies || {})) { diff --git a/apps/rush-lib/src/logic/pnpm/PnpmShrinkwrapFile.ts b/apps/rush-lib/src/logic/pnpm/PnpmShrinkwrapFile.ts index c2cd09f0fd9..014e9ee65e2 100644 --- a/apps/rush-lib/src/logic/pnpm/PnpmShrinkwrapFile.ts +++ b/apps/rush-lib/src/logic/pnpm/PnpmShrinkwrapFile.ts @@ -24,6 +24,7 @@ interface IPnpmShrinkwrapDependencyYaml { }; /** The list of dependencies and the resolved version */ dependencies: { [dependency: string]: string }; + id: string; } /** @@ -146,18 +147,25 @@ export class PnpmShrinkwrapFile extends BaseShrinkwrapFile { } /** - * Serializes the PNPM Shrinkwrap file + * Gets the version number from the list of top-level dependencies in the "dependencies" section + * of the shrinkwrap file */ - protected serialize(): string { - return yaml.safeDump(this._shrinkwrapJson, SHRINKWRAP_YAML_FORMAT); + public getTopLevelDependencyVersion(dependencyName: string): string | undefined { + return BaseShrinkwrapFile.tryGetValue(this._shrinkwrapJson.dependencies, dependencyName); } /** - * Gets the version number from the list of top-level dependencies in the "dependencies" section - * of the shrinkwrap file + * Gets the id of the specified package */ - protected getTopLevelDependencyVersion(dependencyName: string): string | undefined { - return BaseShrinkwrapFile.tryGetValue(this._shrinkwrapJson.dependencies, dependencyName); + public getPackageId(packageName: string): string | undefined { + return this._shrinkwrapJson.packages[packageName].id; + } + + /** + * Serializes the PNPM Shrinkwrap file + */ + protected serialize(): string { + return yaml.safeDump(this._shrinkwrapJson, SHRINKWRAP_YAML_FORMAT); } /** From 78a6df9ffcaf78c5955c4842b8c8ce81036f5d95 Mon Sep 17 00:00:00 2001 From: Sachin Joseph Date: Tue, 9 Jul 2019 18:05:05 -0700 Subject: [PATCH 2/6] Run rush change --- .../rush/sachinjoseph-fix1347_2019-07-10-01-04.json | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 common/changes/@microsoft/rush/sachinjoseph-fix1347_2019-07-10-01-04.json diff --git a/common/changes/@microsoft/rush/sachinjoseph-fix1347_2019-07-10-01-04.json b/common/changes/@microsoft/rush/sachinjoseph-fix1347_2019-07-10-01-04.json new file mode 100644 index 00000000000..019a90ab244 --- /dev/null +++ b/common/changes/@microsoft/rush/sachinjoseph-fix1347_2019-07-10-01-04.json @@ -0,0 +1,11 @@ +{ + "changes": [ + { + "comment": "Fix https://github.com/microsoft/web-build-tools/issues/1347: rush link was failing on pnpm 3+ with the changes in shrinkwrap format with regard to peer dependencies. Rush now resolves the path to the local project accurately by referring to the shrinkwrap rather than figuring out the path on its own.", + "packageName": "@microsoft/rush", + "type": "none" + } + ], + "packageName": "@microsoft/rush", + "email": "sachinjoseph@users.noreply.github.com" +} \ No newline at end of file From 7465e3c17c8d3750783f431eb7e42ab3cf5cb4ba Mon Sep 17 00:00:00 2001 From: Sachin Joseph Date: Tue, 9 Jul 2019 23:35:44 -0700 Subject: [PATCH 3/6] Update apps/rush-lib/src/logic/pnpm/PnpmLinkManager.ts Co-Authored-By: Ian Clanton-Thuon --- apps/rush-lib/src/logic/pnpm/PnpmLinkManager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/rush-lib/src/logic/pnpm/PnpmLinkManager.ts b/apps/rush-lib/src/logic/pnpm/PnpmLinkManager.ts index d12c3bdb95a..b9af4891cfa 100644 --- a/apps/rush-lib/src/logic/pnpm/PnpmLinkManager.ts +++ b/apps/rush-lib/src/logic/pnpm/PnpmLinkManager.ts @@ -177,7 +177,7 @@ export class PnpmLinkManager extends BaseLinkManager { } // e.g.: C:\wbt\common\temp\projects\api-documenter.tgz - const absolutePathToTgzFile: string = path.join(this._rushConfiguration.commonTempFolder, relativePathToTgzFile); + const absolutePathToTgzFile: string = path.resolve(this._rushConfiguration.commonTempFolder, relativePathToTgzFile); // e.g.: // C%3A%2Fwbt%2Fcommon%2Ftemp%2Fprojects%2Fapi-documenter.tgz From b545a6b944676dd12662d49d6f47913d09f32a36 Mon Sep 17 00:00:00 2001 From: Sachin Joseph Date: Tue, 9 Jul 2019 23:36:12 -0700 Subject: [PATCH 4/6] Update apps/rush-lib/src/logic/pnpm/PnpmLinkManager.ts Co-Authored-By: Ian Clanton-Thuon --- apps/rush-lib/src/logic/pnpm/PnpmLinkManager.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/apps/rush-lib/src/logic/pnpm/PnpmLinkManager.ts b/apps/rush-lib/src/logic/pnpm/PnpmLinkManager.ts index b9af4891cfa..35b4ab44e09 100644 --- a/apps/rush-lib/src/logic/pnpm/PnpmLinkManager.ts +++ b/apps/rush-lib/src/logic/pnpm/PnpmLinkManager.ts @@ -168,7 +168,9 @@ export class PnpmLinkManager extends BaseLinkManager { const packageId: string | undefined = pnpmShrinkwrapFile!.getPackageId(topLevelDependencyVersion); // e.g.: projects\api-documenter.tgz - let relativePathToTgzFile: string; + const relativePathToTgzFile: string = packageId + ? packageId.slice('file:'.length) + : topLevelDependencyVersion.slice('file:'.length); if (packageId) { relativePathToTgzFile = packageId.slice('file:'.length); From b3900a9f6d21355ec1acc281144726eba63f0148 Mon Sep 17 00:00:00 2001 From: Sachin Joseph Date: Wed, 10 Jul 2019 01:16:37 -0700 Subject: [PATCH 5/6] Use tarball path instead of id for determining the path in .local, add more comments --- .../src/logic/pnpm/PnpmLinkManager.ts | 48 +++++++++++++------ .../src/logic/pnpm/PnpmShrinkwrapFile.ts | 16 +++---- 2 files changed, 41 insertions(+), 23 deletions(-) diff --git a/apps/rush-lib/src/logic/pnpm/PnpmLinkManager.ts b/apps/rush-lib/src/logic/pnpm/PnpmLinkManager.ts index 35b4ab44e09..4d850c6bd39 100644 --- a/apps/rush-lib/src/logic/pnpm/PnpmLinkManager.ts +++ b/apps/rush-lib/src/logic/pnpm/PnpmLinkManager.ts @@ -76,6 +76,10 @@ export class PnpmLinkManager extends BaseLinkManager { rushLinkJson: IRushLinkJson, pnpmShrinkwrapFile: PnpmShrinkwrapFile | undefined): Promise { + if (!pnpmShrinkwrapFile) { + throw new InternalError(`Shrinkwrap is undefined."`); + } + // first, read the temp package.json information // Example: "project1" @@ -158,36 +162,50 @@ export class PnpmLinkManager extends BaseLinkManager { // file:projects/build-tools.tgz_dc21d88642e18a947127a751e00b020a // file:projects/imodel-from-geojson.tgz_request@2.88.0 const topLevelDependencyVersion: string | undefined = - pnpmShrinkwrapFile!.getTopLevelDependencyVersion(project.tempProjectName); + pnpmShrinkwrapFile.getTopLevelDependencyVersion(project.tempProjectName); if (!topLevelDependencyVersion) { throw new InternalError(`Cannot find top level dependency for "${project.tempProjectName}"` + ` in shrinkwrap.`); } - const packageId: string | undefined = pnpmShrinkwrapFile!.getPackageId(topLevelDependencyVersion); + // e.g.: file:projects/project-name.tgz + const tarballEntry: string | undefined = pnpmShrinkwrapFile.getTarballPath(topLevelDependencyVersion); - // e.g.: projects\api-documenter.tgz - const relativePathToTgzFile: string = packageId - ? packageId.slice('file:'.length) - : topLevelDependencyVersion.slice('file:'.length); - - if (packageId) { - relativePathToTgzFile = packageId.slice('file:'.length); - } else { - relativePathToTgzFile = topLevelDependencyVersion.slice('file:'.length); + if (!tarballEntry) { + throw new InternalError(`Cannot find tarball path for "${topLevelDependencyVersion}"` + + ` in shrinkwrap.`); } + // e.g.: projects\api-documenter.tgz + const relativePathToTgzFile: string | undefined = tarballEntry.slice(`file:`.length); + // e.g.: C:\wbt\common\temp\projects\api-documenter.tgz const absolutePathToTgzFile: string = path.resolve(this._rushConfiguration.commonTempFolder, relativePathToTgzFile); + // The folder name in `.local` is constructed as: + // UriEncode(absolutePathToTgzFile) + _suffix + // + // Note that _suffix is not encoded. The tarball attribute of the package 'file:projects/project-name.tgz_suffix' + // holds the tarball path 'file:projects/project-name.tgz', which can be used for the constructing the folder name. + // + // '_suffix' is extracted by stripping the tarball path from top level dependency value. + // tarball path = 'file:projects/project-name.tgz' + // top level dependency = 'file:projects/project-name.tgz_suffix' + + // e.g.: + // '' [empty string] + // _jsdom@11.12.0 + // _2a665c89609864b4e75bc5365d7f8f56 + const folderNameSuffix: string = (tarballEntry && tarballEntry.length < topLevelDependencyVersion.length ? + topLevelDependencyVersion.slice(tarballEntry.length) : ''); + // e.g.: // C%3A%2Fwbt%2Fcommon%2Ftemp%2Fprojects%2Fapi-documenter.tgz // C%3A%2Fdev%2Fimodeljs%2Fimodeljs%2Fcommon%2Ftemp%2Fprojects%2Fpresentation-integration-tests.tgz_jsdom@11.12.0 // C%3A%2Fdev%2Fimodeljs%2Fimodeljs%2Fcommon%2Ftemp%2Fprojects%2Fbuild-tools.tgz_2a665c89609864b4e75bc5365d7f8f56 - const dirNameInLocal: string = uriEncode(Text.replaceAll(absolutePathToTgzFile, path.sep, '/')) + - (packageId && packageId.length < topLevelDependencyVersion.length ? - topLevelDependencyVersion.slice(packageId.length) : ''); + const folderNameInLocalInstallationRoot: string = uriEncode(Text.replaceAll(absolutePathToTgzFile, path.sep, '/')) + + folderNameSuffix; // tslint:disable-next-line:max-line-length // e.g.: C:\wbt\common\temp\node_modules\.local\C%3A%2Fwbt%2Fcommon%2Ftemp%2Fprojects%2Fapi-documenter.tgz\node_modules @@ -195,7 +213,7 @@ export class PnpmLinkManager extends BaseLinkManager { this._rushConfiguration.commonTempFolder, RushConstants.nodeModulesFolderName, '.local', - dirNameInLocal, + folderNameInLocalInstallationRoot, RushConstants.nodeModulesFolderName); for (const dependencyName of Object.keys(commonPackage.packageJson!.dependencies || {})) { diff --git a/apps/rush-lib/src/logic/pnpm/PnpmShrinkwrapFile.ts b/apps/rush-lib/src/logic/pnpm/PnpmShrinkwrapFile.ts index 014e9ee65e2..2d5b65e3365 100644 --- a/apps/rush-lib/src/logic/pnpm/PnpmShrinkwrapFile.ts +++ b/apps/rush-lib/src/logic/pnpm/PnpmShrinkwrapFile.ts @@ -24,7 +24,6 @@ interface IPnpmShrinkwrapDependencyYaml { }; /** The list of dependencies and the resolved version */ dependencies: { [dependency: string]: string }; - id: string; } /** @@ -147,18 +146,19 @@ export class PnpmShrinkwrapFile extends BaseShrinkwrapFile { } /** - * Gets the version number from the list of top-level dependencies in the "dependencies" section - * of the shrinkwrap file + * Gets the path to the tarball file if the package is a tarball, + * otherwise returns undefined. Example: file:projects/build-tools.tgz */ - public getTopLevelDependencyVersion(dependencyName: string): string | undefined { - return BaseShrinkwrapFile.tryGetValue(this._shrinkwrapJson.dependencies, dependencyName); + public getTarballPath(packageName: string): string | undefined { + return this._shrinkwrapJson.packages[packageName].resolution.tarball; } /** - * Gets the id of the specified package + * Gets the version number from the list of top-level dependencies in the "dependencies" section + * of the shrinkwrap file */ - public getPackageId(packageName: string): string | undefined { - return this._shrinkwrapJson.packages[packageName].id; + public getTopLevelDependencyVersion(dependencyName: string): string | undefined { + return BaseShrinkwrapFile.tryGetValue(this._shrinkwrapJson.dependencies, dependencyName); } /** From 18c6d4629b6b8d5f6b9c31a76fef4b90a26f5657 Mon Sep 17 00:00:00 2001 From: Sachin Joseph Date: Wed, 10 Jul 2019 12:44:52 -0700 Subject: [PATCH 6/6] Address comments --- apps/rush-lib/src/logic/pnpm/PnpmLinkManager.ts | 6 +----- apps/rush-lib/src/logic/pnpm/PnpmShrinkwrapFile.ts | 13 ++++++++++--- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/apps/rush-lib/src/logic/pnpm/PnpmLinkManager.ts b/apps/rush-lib/src/logic/pnpm/PnpmLinkManager.ts index 4d850c6bd39..52379ce45c2 100644 --- a/apps/rush-lib/src/logic/pnpm/PnpmLinkManager.ts +++ b/apps/rush-lib/src/logic/pnpm/PnpmLinkManager.ts @@ -74,11 +74,7 @@ export class PnpmLinkManager extends BaseLinkManager { private _linkProject( project: RushConfigurationProject, rushLinkJson: IRushLinkJson, - pnpmShrinkwrapFile: PnpmShrinkwrapFile | undefined): Promise { - - if (!pnpmShrinkwrapFile) { - throw new InternalError(`Shrinkwrap is undefined."`); - } + pnpmShrinkwrapFile: PnpmShrinkwrapFile): Promise { // first, read the temp package.json information diff --git a/apps/rush-lib/src/logic/pnpm/PnpmShrinkwrapFile.ts b/apps/rush-lib/src/logic/pnpm/PnpmShrinkwrapFile.ts index 2d5b65e3365..e16ef76b90a 100644 --- a/apps/rush-lib/src/logic/pnpm/PnpmShrinkwrapFile.ts +++ b/apps/rush-lib/src/logic/pnpm/PnpmShrinkwrapFile.ts @@ -146,11 +146,18 @@ export class PnpmShrinkwrapFile extends BaseShrinkwrapFile { } /** - * Gets the path to the tarball file if the package is a tarball, - * otherwise returns undefined. Example: file:projects/build-tools.tgz + * Gets the path to the tarball file if the package is a tarball. + * Returns undefined if the package entry doesn't exist or the package isn't a tarball. + * Example of return value: file:projects/build-tools.tgz */ public getTarballPath(packageName: string): string | undefined { - return this._shrinkwrapJson.packages[packageName].resolution.tarball; + const dependency: IPnpmShrinkwrapDependencyYaml = this._shrinkwrapJson.packages[packageName]; + + if (!dependency) { + return undefined; + } + + return dependency.resolution.tarball; } /**