Skip to content

Commit 1429229

Browse files
[rush] Changes prior to #5496 (#5532)
* Remove deprecated shell: true usage. * Stop quoting args. * TEMPORARY - run retest fromt he locally-built copy of Rush. * Fix an issue where where the Windows CMD shell's arguments aren't passed as individual arguments. * Don't escape individual args in RushX, as they're passed to cmd/sh. * fixup! Remove deprecated shell: true usage. * Remove an unused function. * Use destructuring in a few places. * Fix an issue with detecting if a package isn't published. * Wrap some command invocations in a shell. * Fix an issue with detection of non-published packages. * fixup! Wrap some command invocations in a shell. * fixup! Fix an issue with detection of non-published packages. * Fix issues with install-run on Windows. * TEMPORARY - ensure install-run works as expected. * Fix an issue with commands and arguments containing spaces. * fixup! Fix an issue with commands and arguments containing spaces. * fixup! TEMPORARY - ensure install-run works as expected. * fixup! TEMPORARY - ensure install-run works as expected. * TEMPORARY - install-run debugging. * fixup! TEMPORARY - install-run debugging. * Fix issues with git calls in stageAndCommitGitChangesAsync Co-authored-by: David Michon <dmichon@microsoft.com> * Deduplicate instances of the Windows platform check. * Include unit tests for convertCommandAndArgsToShell. * Remove a redundant condition. * Clean up an error message. * Fix an issue with double-quoting in install-run. * TEMPORARY - only build to rush-lib. * fixup! Fix an issue with double-quoting in install-run. * Don't run as a shell command on Windows. * fixup! TEMPORARY - only build to rush-lib. * Revert "Don't run as a shell command on Windows." This reverts commit 1fa9d41. * Pass 'process.env' to the 'npm view' command. * Handle null status code too. * fixup! Fix an issue with double-quoting in install-run. * Set windowsVerbatimArguments to true in places where spawn is called with a shell wrapper. * Fix an issue with quoting of arguments. * fixup! Set windowsVerbatimArguments to true in places where spawn is called with a shell wrapper. * fixup! Fix an issue with quoting of arguments. * Revert "fixup! TEMPORARY - install-run debugging." This reverts commit 10ff217. * Revert "TEMPORARY - install-run debugging." This reverts commit 2640932. * Pull back shell: true changes. * Rush change. * fixup! Pull back shell: true changes. * Put the CI config back. * Include signals in install-run npm command logging. * Minor cleanup. * Remove an unused function. * Move convertCommandAndArgsToShell into Utilities. --------- Co-authored-by: David Michon <dmichon@microsoft.com>
1 parent a91ccd5 commit 1429229

30 files changed

Lines changed: 569 additions & 334 deletions

apps/heft/src/utilities/GitUtilities.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { default as ignore, type Ignore as IIgnoreMatcher } from 'ignore';
99

1010
import { Executable, FileSystem, InternalError, Path, Text } from '@rushstack/node-core-library';
1111

12-
// Matches lines starting with "#" and whitepace lines
12+
// Matches lines starting with "#" and whitespace lines
1313
const GITIGNORE_IGNORABLE_LINE_REGEX: RegExp = /^(?:(?:#.*)|(?:\s+))$/;
1414
const UNINITIALIZED: 'UNINITIALIZED' = 'UNINITIALIZED';
1515

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
{
2+
"changes": [
3+
{
4+
"comment": "",
5+
"type": "none",
6+
"packageName": "@microsoft/rush"
7+
}
8+
],
9+
"packageName": "@microsoft/rush",
10+
"email": "iclanton@users.noreply.github.com"
11+
}
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"changes": [
3+
{
4+
"packageName": "@rushstack/heft",
5+
"comment": "",
6+
"type": "none"
7+
}
8+
],
9+
"packageName": "@rushstack/heft"
10+
}

libraries/rush-lib/src/api/EnvironmentConfiguration.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
22
// See LICENSE in the project root for license information.
33

4-
import * as os from 'node:os';
54
import * as path from 'node:path';
65

76
import { trueCasePathSync } from 'true-case-path';
87

98
import type { IEnvironment } from '../utilities/Utilities';
9+
import { IS_WINDOWS } from '../utilities/executionUtilities';
1010

1111
/**
1212
* @beta
@@ -482,8 +482,7 @@ export class EnvironmentConfiguration {
482482
if (process.env.hasOwnProperty(envVarName) && envVarName.match(/^RUSH_/i)) {
483483
const value: string | undefined = process.env[envVarName];
484484
// Environment variables are only case-insensitive on Windows
485-
const normalizedEnvVarName: string =
486-
os.platform() === 'win32' ? envVarName.toUpperCase() : envVarName;
485+
const normalizedEnvVarName: string = IS_WINDOWS ? envVarName.toUpperCase() : envVarName;
487486
switch (normalizedEnvVarName) {
488487
case EnvironmentVariableNames.RUSH_TEMP_FOLDER: {
489488
EnvironmentConfiguration._rushTempFolderOverride =

libraries/rush-lib/src/cli/RushXCommandLine.ts

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import { EnvironmentVariableNames } from '../api/EnvironmentConfiguration';
2828
import { RushConstants } from '../logic/RushConstants';
2929
import { PnpmSyncUtilities } from '../utilities/PnpmSyncUtilities';
3030
import { initializeDotEnv } from '../logic/dotenv';
31+
import { escapeArgumentIfNeeded } from '../utilities/executionUtilities';
3132

3233
interface IRushXCommandLineArguments {
3334
/**
@@ -143,7 +144,9 @@ export class RushXCommandLine {
143144
rushConfiguration: RushConfiguration | undefined,
144145
options: ILaunchOptions
145146
): Promise<void> {
146-
if (!rushxArguments.quiet) {
147+
const { quiet, help, commandName, commandArgs } = rushxArguments;
148+
149+
if (!quiet) {
147150
RushStartupBanner.logStreamlinedBanner(Rush.version, options.isManaged);
148151
}
149152
// Are we in a Rush repo?
@@ -182,15 +185,15 @@ export class RushXCommandLine {
182185

183186
const projectCommandSet: ProjectCommandSet = new ProjectCommandSet(packageJson);
184187

185-
if (rushxArguments.help) {
188+
if (help) {
186189
RushXCommandLine._showUsage(packageJson, projectCommandSet);
187190
return;
188191
}
189192

190-
const scriptBody: string | undefined = projectCommandSet.tryGetScriptBody(rushxArguments.commandName);
193+
const scriptBody: string | undefined = projectCommandSet.tryGetScriptBody(commandName);
191194

192195
if (scriptBody === undefined) {
193-
let errorMessage: string = `The command "${rushxArguments.commandName}" is not defined in the package.json file for this project.`;
196+
let errorMessage: string = `The command "${commandName}" is not defined in the package.json file for this project.`;
194197

195198
if (projectCommandSet.commandNames.length > 0) {
196199
errorMessage +=
@@ -203,20 +206,15 @@ export class RushXCommandLine {
203206

204207
let commandWithArgs: string = scriptBody;
205208
let commandWithArgsForDisplay: string = scriptBody;
206-
if (rushxArguments.commandArgs.length > 0) {
207-
// This approach is based on what NPM 7 now does:
208-
// https://github.com/npm/run-script/blob/47a4d539fb07220e7215cc0e482683b76407ef9b/lib/run-script-pkg.js#L34
209-
const escapedRemainingArgs: string[] = rushxArguments.commandArgs.map((x) =>
210-
Utilities.escapeShellParameter(x)
211-
);
212-
209+
if (commandArgs.length > 0) {
210+
const escapedRemainingArgs: string[] = commandArgs.map((x) => escapeArgumentIfNeeded(x));
213211
commandWithArgs += ' ' + escapedRemainingArgs.join(' ');
214212

215213
// Display it nicely without the extra quotes
216-
commandWithArgsForDisplay += ' ' + rushxArguments.commandArgs.join(' ');
214+
commandWithArgsForDisplay += ' ' + commandArgs.join(' ');
217215
}
218216

219-
if (!rushxArguments.quiet) {
217+
if (!quiet) {
220218
// eslint-disable-next-line no-console
221219
console.log(`> ${JSON.stringify(commandWithArgsForDisplay)}\n`);
222220
}
@@ -258,10 +256,7 @@ export class RushXCommandLine {
258256
}
259257

260258
if (exitCode > 0) {
261-
throw new ProcessError(
262-
`Failed calling ${commandWithArgsForDisplay}. Exit code: ${exitCode}`,
263-
exitCode
264-
);
259+
throw new ProcessError(`Failed calling ${commandWithArgs}. Exit code: ${exitCode}`, exitCode);
265260
}
266261
}
267262

libraries/rush-lib/src/cli/actions/ChangeAction.ts

Lines changed: 3 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ import {
3030
import { ProjectChangeAnalyzer } from '../../logic/ProjectChangeAnalyzer';
3131
import { Git } from '../../logic/Git';
3232
import { RushConstants } from '../../logic/RushConstants';
33-
import { Utilities } from '../../utilities/Utilities';
3433

3534
const BULK_LONG_NAME: string = '--bulk';
3635
const BULK_MESSAGE_LONG_NAME: string = '--message';
@@ -310,11 +309,12 @@ export class ChangeAction extends BaseRushAction {
310309
}
311310
if (this._commitChangesFlagParameter.value || this._commitChangesMessageStringParameter.value) {
312311
if (changefiles && changefiles.length !== 0) {
313-
await this._stageAndCommitGitChangesAsync(
312+
await this._git.stageAndCommitGitChangesAsync(
314313
changefiles,
315314
this._commitChangesMessageStringParameter.value ||
316315
this.rushConfiguration.gitChangefilesCommitMessage ||
317-
'Rush change'
316+
'Rush change',
317+
this.terminal
318318
);
319319
} else {
320320
this.terminal.writeWarningLine('Warning: No change files generated, nothing to commit.');
@@ -748,21 +748,4 @@ export class ChangeAction extends BaseRushAction {
748748
// eslint-disable-next-line no-console
749749
console.log('No changes were detected to relevant packages on this branch. Nothing to do.');
750750
}
751-
752-
private async _stageAndCommitGitChangesAsync(pattern: string[], message: string): Promise<void> {
753-
try {
754-
await Utilities.executeCommandAsync({
755-
command: 'git',
756-
args: ['add', ...pattern],
757-
workingDirectory: this.rushConfiguration.changesFolder
758-
});
759-
await Utilities.executeCommandAsync({
760-
command: 'git',
761-
args: ['commit', ...pattern, '-m', message],
762-
workingDirectory: this.rushConfiguration.changesFolder
763-
});
764-
} catch (error) {
765-
this.terminal.writeErrorLine(`ERROR: Cannot stage and commit git changes ${(error as Error).message}`);
766-
}
767-
}
768751
}

libraries/rush-lib/src/cli/actions/PublishAction.ts

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import { DEFAULT_PACKAGE_UPDATE_MESSAGE } from './VersionAction';
2929
import { Utilities } from '../../utilities/Utilities';
3030
import { Git } from '../../logic/Git';
3131
import { RushConstants } from '../../logic/RushConstants';
32+
import { IS_WINDOWS } from '../../utilities/executionUtilities';
3233

3334
export class PublishAction extends BaseRushAction {
3435
private readonly _addCommitDetails: CommandLineFlagParameter;
@@ -473,14 +474,14 @@ export class PublishAction extends BaseRushAction {
473474
// If the auth token was specified via the command line, avoid printing it on the console
474475
const secretSubstring: string | undefined = this._npmAuthToken.value;
475476

476-
await PublishUtilities.execCommandAsync(
477-
!!this._publish.value,
478-
packageManagerToolFilename,
477+
await PublishUtilities.execCommandAsync({
478+
shouldExecute: this._publish.value,
479+
command: packageManagerToolFilename,
479480
args,
480-
packagePath,
481-
env,
481+
workingDirectory: packagePath,
482+
environment: env,
482483
secretSubstring
483-
);
484+
});
484485
}
485486
}
486487

@@ -522,13 +523,13 @@ export class PublishAction extends BaseRushAction {
522523
const args: string[] = ['pack'];
523524
const env: { [key: string]: string | undefined } = PublishUtilities.getEnvArgs();
524525

525-
await PublishUtilities.execCommandAsync(
526-
!!this._publish.value,
527-
this.rushConfiguration.packageManagerToolFilename,
526+
await PublishUtilities.execCommandAsync({
527+
shouldExecute: this._publish.value,
528+
command: this.rushConfiguration.packageManagerToolFilename,
528529
args,
529-
project.publishFolder,
530-
env
531-
);
530+
workingDirectory: project.publishFolder,
531+
environment: env
532+
});
532533

533534
if (this._publish.value) {
534535
// Copy the tarball the release folder
@@ -597,7 +598,7 @@ export class PublishAction extends BaseRushAction {
597598
}
598599

599600
private _addSharedNpmConfig(env: { [key: string]: string | undefined }, args: string[]): void {
600-
const userHomeEnvVariable: string = process.platform === 'win32' ? 'USERPROFILE' : 'HOME';
601+
const userHomeEnvVariable: string = IS_WINDOWS ? 'USERPROFILE' : 'HOME';
601602
let registry: string = '//registry.npmjs.org/';
602603

603604
// Check if .npmrc file exists in "common\temp\publish-home"

libraries/rush-lib/src/cli/parsing/ParseParallelism.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33

44
import * as os from 'node:os';
55

6+
import { IS_WINDOWS } from '../../utilities/executionUtilities';
7+
68
/**
79
* Parses a command line specification for desired parallelism.
810
* Factored out to enable unit tests
@@ -39,7 +41,7 @@ export function parseParallelism(
3941
} else {
4042
// If an explicit parallelism number wasn't provided, then choose a sensible
4143
// default.
42-
if (os.platform() === 'win32') {
44+
if (IS_WINDOWS) {
4345
// On desktop Windows, some people have complained that their system becomes
4446
// sluggish if Rush is using all the CPU cores. Leave one thread for
4547
// other operations. For CI environments, you can use the "max" argument to use all available cores.

libraries/rush-lib/src/cli/test/Cli.test.ts

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,11 @@ describe('CLI', () => {
2828
const startPath: string = path.resolve(__dirname, '../../../lib-commonjs/startx.js');
2929

3030
// Run "rushx show-args 1 2 -x" in the "repo/rushx-project" folder
31-
const output: string = await Utilities.executeCommandAndCaptureOutputAsync(
32-
'node',
33-
[startPath, 'show-args', '1', '2', '-x'],
34-
`${__dirname}/repo/rushx-project`
35-
);
31+
const output: string = await Utilities.executeCommandAndCaptureOutputAsync({
32+
command: 'node',
33+
args: [startPath, 'show-args', '1', '2', '-x'],
34+
workingDirectory: `${__dirname}/repo/rushx-project`
35+
});
3636
const lastLine: string =
3737
output
3838
.split(/\s*\n\s*/)
@@ -45,11 +45,11 @@ describe('CLI', () => {
4545
// Invoke "rushx"
4646
const startPath: string = path.resolve(__dirname, '../../../lib-commonjs/startx.js');
4747

48-
const output: string = await Utilities.executeCommandAndCaptureOutputAsync(
49-
'node',
50-
[startPath, 'show-args', '1', '2', '-x'],
51-
`${__dirname}/repo/rushx-not-in-rush-project`
52-
);
48+
const output: string = await Utilities.executeCommandAndCaptureOutputAsync({
49+
command: 'node',
50+
args: [startPath, 'show-args', '1', '2', '-x'],
51+
workingDirectory: `${__dirname}/repo/rushx-not-in-rush-project`
52+
});
5353

5454
expect(output).toEqual(
5555
expect.stringMatching(

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

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -523,6 +523,28 @@ export class Git {
523523
return result;
524524
}
525525

526+
public async stageAndCommitGitChangesAsync(
527+
pattern: string[],
528+
message: string,
529+
terminal: ITerminal
530+
): Promise<void> {
531+
try {
532+
const gitPath: string = this.getGitPathOrThrow();
533+
await this._executeGitCommandAndCaptureOutputAsync(
534+
gitPath,
535+
['add', '--', ...pattern],
536+
this._rushConfiguration.changesFolder
537+
);
538+
await this._executeGitCommandAndCaptureOutputAsync(
539+
gitPath,
540+
['commit', '-m', message, '--', ...pattern],
541+
this._rushConfiguration.changesFolder
542+
);
543+
} catch (error) {
544+
terminal.writeErrorLine(`ERROR: Cannot stage and commit git changes ${(error as Error).message}`);
545+
}
546+
}
547+
526548
/**
527549
* Returns an object containing either the result of the `git config user.email`
528550
* command or an error.
@@ -604,10 +626,14 @@ export class Git {
604626
public async _executeGitCommandAndCaptureOutputAsync(
605627
gitPath: string,
606628
args: string[],
607-
repositoryRoot: string = this._rushConfiguration.rushJsonFolder
629+
workingDirectory: string = this._rushConfiguration.rushJsonFolder
608630
): Promise<string> {
609631
try {
610-
return await Utilities.executeCommandAndCaptureOutputAsync(gitPath, args, repositoryRoot);
632+
return await Utilities.executeCommandAndCaptureOutputAsync({
633+
command: gitPath,
634+
args,
635+
workingDirectory
636+
});
611637
} catch (e) {
612638
ensureGitMinimumVersion(gitPath);
613639
throw e;

0 commit comments

Comments
 (0)