Skip to content

[rush] rush-pnpm accepts patch and patch-commit #3554

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.

Sign up for GitHub

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

Merged
octogonz merged 7 commits into from
Nov 28, 2022
Merged

[rush] rush-pnpm accepts patch and patch-commit #3554

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b887c6c
feat(rush-lib): rush-pnpm accepts patch & patch-commit
chengcyber Jul 27, 2022
881926d
chore: rush change
chengcyber Jul 27, 2022
99a095c
feat: rush-pnpm patch & patch-commit
chengcyber Sep 25, 2022
b82f425
chore: use common/pnpm-patches instead of common/pnpm/patches
chengcyber Nov 28, 2022
b49169a
Update pnpm-config.json template file
octogonz Nov 28, 2022
9a1c050
Merge remote-tracking branch 'remotes/origin/main' into feat-pnpm-patch
octogonz Nov 28, 2022
673e584
Prepare for a MINOR release of Rush
octogonz Nov 28, 2022
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
10 changes: 10 additions & 0 deletions common/changes/@microsoft/rush/feat-pnpm-patch_2022-07-27-03-39.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@microsoft/rush",
"comment": "Add new commands \"rush-pnpm patch\" and \"rush-pnpm patch-commit\" for patching NPM packages when using the PNPM package manager (GitHub #3554)",
"type": "none"
}
Copy link
Collaborator

@octogonz octogonz Nov 28, 2022

Choose a reason for hiding this comment

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

The rush-pnpm-patch-demo README.md mentions:

NOTE: patched dependency has the exactly same integrity of the raw dependency. It confuses rush build cache, so it's essential to invoke rebuild here to see the correct log output.

Could we fix this somehow? I don't think it should block this PR, but it seems like it could cause trouble and would be worth trying to fix.

],
Copy link
Collaborator

@octogonz octogonz Nov 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@elliot-nelson wrote:

The only test that did not work was this one:

  1. Can you target any package? - No. For example, from the root of the monorepo, you can rush-pnpm patch any transitive dependency at any level of the lockfile, but when you try to apply that patch with rush-pnpm patch-commit, it will fail. Similarly, you can rush-pnpm patch any package from a project folder (even one that isn't in that package.json), but trying to apply the patch will fail.

(I don't believe this is a blocker, and maybe it's something that can be fixed in future. The result here is that if you want to "target" a package deep in your lockfile, you first need to add it to a package.json file somewhere in your monorepo to make it visible, then you can patch and apply the patch, and then remove it from that project.)

I encountered this same problem. It's a bit confusing -- since I'm patching is-odd for the entire monorepo, why do I need to invoke it under a project folder?

The error message itself isn't super intuitive:

 ERR_PNPM_NO_PKG_MANIFEST  No package.json found in C:\rush-pnpm-patch-demo

How would we improve this though? Is it actually a design problem of PNPM?

Copy link
Contributor Author

@chengcyber chengcyber Nov 29, 2022

Choose a reason for hiding this comment

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

I believe this PR #3791 can fix this issue 🤞

"packageName": "@microsoft/rush"
}
Copy link
Collaborator

@octogonz octogonz Nov 28, 2022

Choose a reason for hiding this comment

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

FYI when I tried the test repo #3554 , the rush install failed because your main branch is using pnpm-config.json whereas rush.json specifies an old release that doesn't support that config file.

2 changes: 1 addition & 1 deletion common/config/rush/version-policies.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@
"policyName": "rush",
"definitionName": "lockStepVersion",
"version": "5.85.1",
"nextBump": "patch",
"nextBump": "minor",
"mainProject": "@microsoft/rush"
}
]
6 changes: 6 additions & 0 deletions common/reviews/api/rush-lib.api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,7 @@ export interface _IPnpmOptionsJson extends IPackageManagerOptionsJsonBase {
globalOverrides?: Record<string, string>;
// Warning: (ae-forgotten-export) The symbol "IPnpmPackageExtension" needs to be exported by the entry point index.d.ts
globalPackageExtensions?: Record<string, IPnpmPackageExtension>;
globalPatchedDependencies?: Record<string, string>;
// Warning: (ae-forgotten-export) The symbol "IPnpmPeerDependencyRules" needs to be exported by the entry point index.d.ts
globalPeerDependencyRules?: IPnpmPeerDependencyRules;
pnpmStore?: PnpmStoreOptions;
Expand Down Expand Up @@ -715,7 +716,10 @@ export class PnpmOptionsConfiguration extends PackageManagerOptionsConfiguration
readonly globalNeverBuiltDependencies: string[] | undefined;
readonly globalOverrides: Record<string, string> | undefined;
readonly globalPackageExtensions: Record<string, IPnpmPackageExtension> | undefined;
get globalPatchedDependencies(): Record<string, string> | undefined;
readonly globalPeerDependencyRules: IPnpmPeerDependencyRules | undefined;
// (undocumented)
get jsonFilename(): string | undefined;
// @internal (undocumented)
static loadFromJsonFileOrThrow(jsonFilename: string, commonTempFolder: string): PnpmOptionsConfiguration;
// @internal (undocumented)
Expand All @@ -725,6 +729,7 @@ export class PnpmOptionsConfiguration extends PackageManagerOptionsConfiguration
readonly preventManualShrinkwrapChanges: boolean;
readonly strictPeerDependencies: boolean;
readonly unsupportedPackageJsonSettings: unknown | undefined;
updateGlobalPatchedDependencies(patchedDependencies: Record<string, string> | undefined): void;
readonly useWorkspaces: boolean;
}

Expand Down Expand Up @@ -925,6 +930,7 @@ export class RushConstants {
static readonly pnpmConfigFilename: string;
static readonly pnpmfileV1Filename: string;
static readonly pnpmfileV6Filename: string;
static readonly pnpmPatchesFolderName: string;
static readonly pnpmV3ShrinkwrapFilename: string;
static readonly projectRushFolderName: string;
static readonly projectShrinkwrapFilename: string;
Expand Down
10 changes: 10 additions & 0 deletions libraries/rush-lib/assets/rush-init/common/config/rush/pnpm-config.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,16 @@
/*[LINE "HYPOTHETICAL"]*/ "request": "*"
},


/**
* (THIS FIELD IS MACHINE GENERATED) The "globalPatchedDependencies" field is updated automatically
* by the `rush-pnpm patch-commit` command. It is a dictionary, where the key is an NPM package name
* and exact version, and the value is a relative path to the associated patch file.
*
* PNPM documentation: https://pnpm.io/package_json#pnpmpatcheddependencies
*/
"globalPatchedDependencies": { },

/**
* (USE AT YOUR OWN RISK) This is a free-form property bag that will be copied into
* the `common/temp/package.json` file that is generated by Rush during installation.
Expand Down
268 changes: 4 additions & 264 deletions libraries/rush-lib/src/cli/RushPnpmCommandLine.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,276 +1,16 @@
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
// See LICENSE in the project root for license information.

import * as path from 'path';
import {
FileSystem,
AlreadyReportedError,
Executable,
EnvironmentMap,
ITerminal,
Colors,
Terminal,
ConsoleTerminalProvider
} from '@rushstack/node-core-library';
import { PrintUtilities } from '@rushstack/terminal';

import { RushConfiguration } from '../api/RushConfiguration';
import { NodeJsCompatibility } from '../logic/NodeJsCompatibility';
import { SpawnSyncReturns } from 'child_process';
import { ILaunchOptions } from '../api/Rush';
import { RushPnpmCommandLineParser } from './RushPnpmCommandLineParser';

export interface ILaunchRushPnpmInternalOptions extends ILaunchOptions {}

const RUSH_SKIP_CHECKS_PARAMETER: string = '--rush-skip-checks';

export class RushPnpmCommandLine {
public static launch(launcherVersion: string, options: ILaunchRushPnpmInternalOptions): void {
// Node.js can sometimes accidentally terminate with a zero exit code (e.g. for an uncaught
// promise exception), so we start with the assumption that the exit code is 1
// and set it to 0 only on success.
process.exitCode = 1;

const { terminalProvider } = options;
const terminal: ITerminal = new Terminal(terminalProvider ?? new ConsoleTerminalProvider());

try {
// Are we in a Rush repo?
let rushConfiguration: RushConfiguration | undefined = undefined;
if (RushConfiguration.tryFindRushJsonLocation()) {
// showVerbose is false because the logging message may break JSON output
rushConfiguration = RushConfiguration.loadFromDefaultLocation({ showVerbose: false });
}

NodeJsCompatibility.warnAboutCompatibilityIssues({
isRushLib: true,
alreadyReportedNodeTooNewError: !!options.alreadyReportedNodeTooNewError,
rushConfiguration
});

if (!rushConfiguration) {
throw new Error(
'The "rush-pnpm" command must be executed in a folder that is under a Rush workspace folder'
);
}

if (rushConfiguration.packageManager !== 'pnpm') {
throw new Error(
'The "rush-pnpm" command requires your rush.json to be configured to use the PNPM package manager'
);
}

if (!rushConfiguration.pnpmOptions.useWorkspaces) {
throw new Error(
'The "rush-pnpm" command requires the "useWorkspaces" setting to be enabled in rush.json'
);
}

const workspaceFolder: string = rushConfiguration.commonTempFolder;
const workspaceFilePath: string = path.join(workspaceFolder, 'pnpm-workspace.yaml');

if (!FileSystem.exists(workspaceFilePath)) {
terminal.writeErrorLine('Error: The PNPM workspace file has not been generated:');
terminal.writeErrorLine(` ${workspaceFilePath}\n`);
terminal.writeLine(Colors.cyan(`Do you need to run "rush install" or "rush update"?`));
throw new AlreadyReportedError();
}

if (!FileSystem.exists(rushConfiguration.packageManagerToolFilename)) {
terminal.writeErrorLine('Error: The PNPM local binary has not been installed yet.');
terminal.writeLine('\n' + Colors.cyan(`Do you need to run "rush install" or "rush update"?`));
throw new AlreadyReportedError();
}

// 0 = node.exe
// 1 = rush-pnpm
const pnpmArgs: string[] = process.argv.slice(2);

RushPnpmCommandLine._validatePnpmUsage(pnpmArgs, terminal);

const pnpmEnvironmentMap: EnvironmentMap = new EnvironmentMap(process.env);
pnpmEnvironmentMap.set('NPM_CONFIG_WORKSPACE_DIR', workspaceFolder);

if (rushConfiguration.pnpmOptions.pnpmStorePath) {
pnpmEnvironmentMap.set('NPM_CONFIG_STORE_DIR', rushConfiguration.pnpmOptions.pnpmStorePath);
}

if (rushConfiguration.pnpmOptions.environmentVariables) {
for (const [envKey, { value: envValue, override }] of Object.entries(
rushConfiguration.pnpmOptions.environmentVariables
)) {
if (override) {
pnpmEnvironmentMap.set(envKey, envValue);
} else {
if (undefined === pnpmEnvironmentMap.get(envKey)) {
pnpmEnvironmentMap.set(envKey, envValue);
}
}
}
}

const result: SpawnSyncReturns<string> = Executable.spawnSync(
rushConfiguration.packageManagerToolFilename,
pnpmArgs,
{
environmentMap: pnpmEnvironmentMap,
stdio: 'inherit'
}
);
if (result.error) {
throw new Error('Failed to invoke PNPM: ' + result.error);
}
if (result.status === null) {
throw new Error('Failed to invoke PNPM: Spawn completed without an exit code');
}
process.exitCode = result.status;
} catch (error) {
if (!(error instanceof AlreadyReportedError)) {
const prefix: string = 'ERROR: ';
terminal.writeErrorLine('\n' + PrintUtilities.wrapWords(prefix + error.message));
}
}
}

private static _validatePnpmUsage(pnpmArgs: string[], terminal: ITerminal): void {
if (pnpmArgs[0] === RUSH_SKIP_CHECKS_PARAMETER) {
pnpmArgs.shift();
// Ignore other checks
return;
}

if (pnpmArgs.length === 0) {
return;
}
const firstArg: string = pnpmArgs[0];

// Detect common safe invocations
if (pnpmArgs.includes('-h') || pnpmArgs.includes('--help') || pnpmArgs.includes('-?')) {
return;
}

if (pnpmArgs.length === 1) {
if (firstArg === '-v' || firstArg === '--version') {
return;
}
}

const BYPASS_NOTICE: string = `To bypass this check, add "${RUSH_SKIP_CHECKS_PARAMETER}" as the very first command line option.`;

if (!/^[a-z]+([a-z0-9\-])*$/.test(firstArg)) {
// We can't parse this CLI syntax
terminal.writeErrorLine(
`Warning: The "rush-pnpm" wrapper expects a command verb before "${firstArg}"\n`
);
terminal.writeLine(Colors.cyan(BYPASS_NOTICE));
throw new AlreadyReportedError();
} else {
const commandName: string = firstArg;

// Also accept SKIP_RUSH_CHECKS_PARAMETER immediately after the command verb
if (pnpmArgs[1] === RUSH_SKIP_CHECKS_PARAMETER) {
pnpmArgs.splice(1, 1);
return;
}

if (pnpmArgs.indexOf(RUSH_SKIP_CHECKS_PARAMETER) >= 0) {
// We do not attempt to parse PNPM's complete CLI syntax, so we cannot be sure how to interpret
// strings that appear outside of the specific patterns that this parser recognizes
terminal.writeErrorLine(
PrintUtilities.wrapWords(
`Error: The "${RUSH_SKIP_CHECKS_PARAMETER}" option must be the first parameter for the "rush-pnpm" command.`
)
);
throw new AlreadyReportedError();
}

// Warn about commands known not to work
/* eslint-disable no-fallthrough */
switch (commandName) {
// Blocked
case 'import': {
terminal.writeErrorLine(
PrintUtilities.wrapWords(
`Error: The "pnpm ${commandName}" command is known to be incompatible with Rush's environment.`
) + '\n'
);
terminal.writeLine(Colors.cyan(BYPASS_NOTICE));
throw new AlreadyReportedError();
}

// Show warning for install commands
case 'add':
case 'install':
/* synonym */
case 'i':
case 'install-test':
/* synonym */
case 'it': {
terminal.writeErrorLine(
PrintUtilities.wrapWords(
`Error: The "pnpm ${commandName}" command is incompatible with Rush's environment.` +
` Use the "rush install" or "rush update" commands instead.`
) + '\n'
);
terminal.writeLine(Colors.cyan(BYPASS_NOTICE));
throw new AlreadyReportedError();
}

// Show warning
case 'link':
/* synonym */
case 'ln':
case 'remove':
/* synonym */
case 'rm':
case 'unlink':
case 'update':
/* synonym */
case 'up': {
terminal.writeWarningLine(
PrintUtilities.wrapWords(
`Warning: The "pnpm ${commandName}" command makes changes that may invalidate Rush's workspace state.`
) + '\n'
);
terminal.writeWarningLine(`==> Consider running "rush install" or "rush update" afterwards.\n`);
break;
}

// Known safe
case 'audit':
case 'exec':
case 'list':
/* synonym */
case 'ls':
case 'outdated':
case 'pack':
case 'prune':
case 'publish':
case 'rebuild':
/* synonym */
case 'rb':
case 'root':
case 'run':
case 'start':
case 'store':
case 'test':
/* synonym */
case 't':
case 'why': {
break;
}
const rushPnpmCommandLineParser: RushPnpmCommandLineParser = new RushPnpmCommandLineParser(options);

// Unknown
default: {
terminal.writeErrorLine(
PrintUtilities.wrapWords(
`Error: The "pnpm ${commandName}" command has not been tested with Rush's environment. It may be incompatible.`
) + '\n'
);
terminal.writeLine(Colors.cyan(BYPASS_NOTICE));
throw new AlreadyReportedError();
}
}
/* eslint-enable no-fallthrough */
}
// RushPnpmCommandLineParser.executeAsync should never reject the promise
rushPnpmCommandLineParser.executeAsync().catch(console.error);
}
}
Loading