Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[rush] Pass --frozen-lockfile to rush install #1850

Merged
merged 9 commits into from
May 13, 2020
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@
*/
/*[LINE "HYPOTHETICAL"]*/ "legacyIncrementalBuildDependencyDetection": true,

/**
* By default, rush passes --no-prefer-frozen-lockfile to 'pnpm install'.
* Set this option to true to pass '--frozen-lockfile' instead.
*/
/*[LINE "HYPOTHETICAL"]*/ "usePnpmFrozenLockfileForRushInstall": true,

/**
* If true, the chmod field in temporary project tar headers will not be normalized.
* This normalization can help ensure consistent tarball integrity across platforms.
Expand Down
6 changes: 6 additions & 0 deletions apps/rush-lib/src/api/ExperimentsConfiguration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@ export interface IExperimentsJson {
*/
legacyIncrementalBuildDependencyDetection?: boolean;

/**
* By default, rush passes --no-prefer-frozen-lockfile to 'pnpm install'.
* Set this option to true to pass '--frozen-lockfile' instead.
*/
usePnpmFrozenLockfileForRushInstall?: boolean;

/**
* If true, the chmod field in temporary project tar headers will not be normalized.
* This normalization can help ensure consistent tarball integrity across platforms.
Expand Down
39 changes: 29 additions & 10 deletions apps/rush-lib/src/logic/InstallManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -682,8 +682,17 @@ export class InstallManager {
// We will NOT locally link this package; add it as a regular dependency.
tempPackageJson.dependencies![packageName] = packageVersion;

let tryReusingPackageVersionsFromShrinkwrap: boolean = true;

if (this._rushConfiguration.packageManager === 'pnpm') {
// Shrinkwrap churn optimization doesn't make sense when --frozen-lockfile is true
tryReusingPackageVersionsFromShrinkwrap =
!this._rushConfiguration.experimentsConfiguration.configuration.usePnpmFrozenLockfileForRushInstall;
}

if (shrinkwrapFile) {
if (!shrinkwrapFile.tryEnsureCompatibleDependency(dependencySpecifier, rushProject.tempProjectName)) {
if (!shrinkwrapFile.tryEnsureCompatibleDependency(dependencySpecifier, rushProject.tempProjectName,
tryReusingPackageVersionsFromShrinkwrap)) {
shrinkwrapWarnings.push(`"${packageName}" (${packageVersion}) required by`
+ ` "${rushProject.packageName}"`);
shrinkwrapIsUpToDate = false;
Expand Down Expand Up @@ -823,7 +832,7 @@ export class InstallManager {
}

/**
* Runs "npm install" in the common folder.
* Runs "npm/pnpm/yarn install" in the "common/temp" folder.
*/
private _installCommonModules(options: {
shrinkwrapIsUpToDate: boolean;
Expand All @@ -834,6 +843,8 @@ export class InstallManager {
variantIsUpToDate
} = options;

const usePnpmFrozenLockfile: boolean = this._rushConfiguration.packageManager === 'pnpm' &&
this._rushConfiguration.experimentsConfiguration.configuration.usePnpmFrozenLockfileForRushInstall === true;
return Promise.resolve().then(() => {
console.log(os.EOL + colors.bold('Checking node_modules in ' + this._rushConfiguration.commonTempFolder)
+ os.EOL);
Expand Down Expand Up @@ -861,7 +872,7 @@ export class InstallManager {
// then we can't skip this install
potentiallyChangedFiles.push(this._rushConfiguration.getCommittedShrinkwrapFilename(options.variant));

// Add common-versions.json file in potentially changed file list.
// Add common-versions.json file to the potentially changed files list.
potentiallyChangedFiles.push(this._rushConfiguration.getCommonVersionsFilePath(options.variant));

if (this._rushConfiguration.packageManager === 'pnpm') {
Expand Down Expand Up @@ -1070,7 +1081,7 @@ export class InstallManager {
this._fixupNpm5Regression();
}

if (options.allowShrinkwrapUpdates && !shrinkwrapIsUpToDate) {
if (options.allowShrinkwrapUpdates && (usePnpmFrozenLockfile || !shrinkwrapIsUpToDate)) {
// Shrinkwrap files may need to be post processed after install, so load and save it
const tempShrinkwrapFile: BaseShrinkwrapFile | undefined = ShrinkwrapFileFactory.getShrinkwrapFile(
this._rushConfiguration.packageManager,
Expand Down Expand Up @@ -1300,13 +1311,21 @@ export class InstallManager {
// last install flag, which encapsulates the entire installation
args.push('--no-lock');

// Ensure that Rush's tarball dependencies get synchronized properly with the pnpm-lock.yaml file.
// See this GitHub issue: https://github.com/pnpm/pnpm/issues/1342

if (semver.gte(this._rushConfiguration.packageManagerToolVersion, '3.0.0')) {
args.push('--no-prefer-frozen-lockfile');
if (this._rushConfiguration.experimentsConfiguration.configuration.usePnpmFrozenLockfileForRushInstall &&
!this._options.allowShrinkwrapUpdates) {
if (semver.gte(this._rushConfiguration.packageManagerToolVersion, '3.0.0')) {
args.push('--frozen-lockfile');
} else {
args.push('--frozen-shrinkwrap');
}
} else {
args.push('--no-prefer-frozen-shrinkwrap');
// Ensure that Rush's tarball dependencies get synchronized properly with the pnpm-lock.yaml file.
// See this GitHub issue: https://github.com/pnpm/pnpm/issues/1342
if (semver.gte(this._rushConfiguration.packageManagerToolVersion, '3.0.0')) {
args.push('--no-prefer-frozen-lockfile');
} else {
args.push('--no-prefer-frozen-shrinkwrap');
}
}

if (options.collectLogFile) {
Expand Down
9 changes: 5 additions & 4 deletions apps/rush-lib/src/logic/base/BaseShrinkwrapFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,10 @@ export abstract class BaseShrinkwrapFile {
*
* @virtual
*/
public tryEnsureCompatibleDependency(dependencySpecifier: DependencySpecifier, tempProjectName: string): boolean {
public tryEnsureCompatibleDependency(dependencySpecifier: DependencySpecifier,
tempProjectName: string, tryReusingPackageVersionsFromShrinkwrap: boolean = true): boolean {
const shrinkwrapDependency: DependencySpecifier | undefined =
this.tryEnsureDependencyVersion(dependencySpecifier, tempProjectName);
this.tryEnsureDependencyVersion(dependencySpecifier, tempProjectName, tryReusingPackageVersionsFromShrinkwrap);
if (!shrinkwrapDependency) {
return false;
}
Expand All @@ -105,15 +106,15 @@ export abstract class BaseShrinkwrapFile {

/** @virtual */
protected abstract tryEnsureDependencyVersion(dependencySpecifier: DependencySpecifier,
tempProjectName: string): DependencySpecifier | undefined;
tempProjectName: string, tryReusingPackageVersionsFromShrinkwrap: boolean): DependencySpecifier | undefined;

/** @virtual */
protected abstract getTopLevelDependencyVersion(dependencyName: string): DependencySpecifier | undefined;

/** @virtual */
protected abstract serialize(): string;

protected _getTempProjectNames(dependencies: { [key: string]: {} } ): ReadonlyArray<string> {
protected _getTempProjectNames(dependencies: { [key: string]: {} }): ReadonlyArray<string> {
const result: string[] = [];
for (const key of Object.keys(dependencies)) {
// If it starts with @rush-temp, then include it:
Expand Down
7 changes: 4 additions & 3 deletions apps/rush-lib/src/logic/pnpm/PnpmShrinkwrapFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -417,12 +417,12 @@ export class PnpmShrinkwrapFile extends BaseShrinkwrapFile {
/**
* Gets the resolved version number of a dependency for a specific temp project.
* For PNPM, we can reuse the version that another project is using.
* Note that this function modifies the shrinkwrap data.
* Note that this function modifies the shrinkwrap data if tryReusingPackageVersionsFromShrinkwrap is set to true.
*
* @override
*/
protected tryEnsureDependencyVersion(dependencySpecifier: DependencySpecifier,
tempProjectName: string): DependencySpecifier | undefined {
tempProjectName: string, tryReusingPackageVersionsFromShrinkwrap: boolean): DependencySpecifier | undefined {

// PNPM doesn't have the same advantage of NPM, where we can skip generate as long as the
// shrinkwrap file puts our dependency in either the top of the node_modules folder
Expand All @@ -446,7 +446,8 @@ export class PnpmShrinkwrapFile extends BaseShrinkwrapFile {
}

if (!packageDescription.dependencies.hasOwnProperty(packageName)) {
if (dependencySpecifier.versionSpecifier) {

if (tryReusingPackageVersionsFromShrinkwrap && dependencySpecifier.versionSpecifier) {
// this means the current temp project doesn't provide this dependency,
// however, we may be able to use a different version. we prefer the latest version
let latestVersion: string | undefined = undefined;
Expand Down
5 changes: 4 additions & 1 deletion apps/rush-lib/src/schemas/experiments.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@
"description": "Rush 5.14.0 improved incremental builds to ignore spurious changes in the pnpm-lock.json file. This optimization is enabled by default. If you encounter a problem where \"rush build\" is neglecting to build some projects, please open a GitHub issue. As a workaround you can uncomment this line to temporarily restore the old behavior where everything must be rebuilt whenever pnpm-lock.json is modified.",
"type": "boolean"
},

"usePnpmFrozenLockfileForRushInstall": {
"description": "By default, rush passes --no-prefer-frozen-lockfile to 'pnpm install'. Set this option to true to pass '--frozen-lockfile' instead.",
"type": "boolean"
},
"noChmodFieldInTarHeaderNormalization": {
"description": "If true, the chmod field in temporary project tar headers will not be normalized. This normalization can help ensure consistent tarball integrity across platforms.",
"type": "boolean"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"changes": [
{
"packageName": "@microsoft/rush",
"comment": "Provide an option to pass --frozen-lockfile to pnpm for rush install",
"type": "none"
}
],
"packageName": "@microsoft/rush",
"email": "[email protected]"
}
1 change: 1 addition & 0 deletions common/reviews/api/rush-lib.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ export interface IConfigurationEnvironmentVariable {
export interface IExperimentsJson {
legacyIncrementalBuildDependencyDetection?: boolean;
noChmodFieldInTarHeaderNormalization?: boolean;
usePnpmFrozenLockfileForRushInstall?: boolean;
}

// @public
Expand Down