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] child_process.spawnSync on windows must set shell: true when targeting a .cmd file #4637

Merged
merged 7 commits into from
Apr 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
Loading