Skip to content

Commit

Permalink
Merge pull request #4637 from ben-polinsky/fix-spawnsync-windows
Browse files Browse the repository at this point in the history
[rush] child_process.spawnSync on windows must set shell: true when targeting a .cmd file
  • Loading branch information
dmichon-msft authored Apr 12, 2024
2 parents 6dcee7f + 376679d commit 34a862d
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 15 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@microsoft/rush",
"comment": "Adapt to Node.js behavior change when invoking a .CMD file via `child_process.spawn`. On Windows this requires setting `shell: true`.",
"type": "none"
}
],
"packageName": "@microsoft/rush"
}
46 changes: 31 additions & 15 deletions libraries/rush-lib/src/scripts/install-run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ let _npmPath: string | undefined = undefined;
export function getNpmPath(): string {
if (!_npmPath) {
try {
if (os.platform() === 'win32') {
if (_isWindows()) {
// We're on Windows
const whereOutput: string = childProcess.execSync('where npm', { stdio: [] }).toString();
const lines: string[] = whereOutput.split(os.EOL).filter((line) => !!line);
Expand Down Expand Up @@ -191,13 +191,17 @@ function _resolvePackageVersion(
// ```
//
// if only a single version matches.
const npmVersionSpawnResult: childProcess.SpawnSyncReturns<Buffer> = childProcess.spawnSync(
npmPath,

const spawnSyncOptions: childProcess.SpawnSyncOptions = {
cwd: rushTempFolder,
stdio: [],
shell: _isWindows()
};
const platformNpmPath: string = _getPlatformPath(npmPath);
const npmVersionSpawnResult: childProcess.SpawnSyncReturns<Buffer | string> = childProcess.spawnSync(
platformNpmPath,
['view', `${name}@${version}`, 'version', '--no-update-notifier', '--json'],
{
cwd: rushTempFolder,
stdio: []
}
spawnSyncOptions
);

if (npmVersionSpawnResult.status !== 0) {
Expand Down Expand Up @@ -354,10 +358,12 @@ function _installPackage(
try {
logger.info(`Installing ${name}...`);
const npmPath: string = getNpmPath();
const result: childProcess.SpawnSyncReturns<Buffer> = childProcess.spawnSync(npmPath, [command], {
const platformNpmPath: string = _getPlatformPath(npmPath);
const result: childProcess.SpawnSyncReturns<Buffer> = childProcess.spawnSync(platformNpmPath, [command], {
stdio: 'inherit',
cwd: packageInstallFolder,
env: process.env
env: process.env,
shell: _isWindows()
});

if (result.status !== 0) {
Expand All @@ -375,10 +381,21 @@ function _installPackage(
*/
function _getBinPath(packageInstallFolder: string, binName: string): string {
const binFolderPath: string = path.resolve(packageInstallFolder, NODE_MODULES_FOLDER_NAME, '.bin');
const resolvedBinName: string = os.platform() === 'win32' ? `${binName}.cmd` : binName;
const resolvedBinName: string = _isWindows() ? `${binName}.cmd` : binName;
return path.resolve(binFolderPath, resolvedBinName);
}

/**
* Returns a cross-platform path - windows must enclose any path containing spaces within double quotes.
*/
function _getPlatformPath(platformPath: string): string {
return _isWindows() && platformPath.includes(' ') ? `"${platformPath}"` : platformPath;
}

function _isWindows(): boolean {
return os.platform() === 'win32';
}

/**
* Write a flag file to the package's install directory, signifying that the install was successful.
*/
Expand Down Expand Up @@ -437,16 +454,15 @@ export function installAndRun(
const originalEnvPath: string = process.env.PATH || '';
let result: childProcess.SpawnSyncReturns<Buffer>;
try {
// Node.js on Windows can not spawn a file when the path has a space on it
// unless the path gets wrapped in a cmd friendly way and shell mode is used
const shouldUseShell: boolean = binPath.includes(' ') && os.platform() === 'win32';
const platformBinPath: string = shouldUseShell ? `"${binPath}"` : binPath;
// `npm` bin stubs on Windows are `.cmd` files
// Node.js will not directly invoke a `.cmd` file unless `shell` is set to `true`
const platformBinPath: string = _getPlatformPath(binPath);

process.env.PATH = [binFolderPath, originalEnvPath].join(path.delimiter);
result = childProcess.spawnSync(platformBinPath, packageBinArgs, {
stdio: 'inherit',
windowsVerbatimArguments: false,
shell: shouldUseShell,
shell: _isWindows(),
cwd: process.cwd(),
env: process.env
});
Expand Down

0 comments on commit 34a862d

Please sign in to comment.