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
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@microsoft/rush",
"comment": "childprocess.spawnSync on windows must set shell: true",
"type": "none"
}
],
"packageName": "@microsoft/rush"
}
27 changes: 18 additions & 9 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(

const spawnSyncOptions: childProcess.SpawnSyncOptions = {
cwd: rushTempFolder,
stdio: [],
shell: isWindows()
};

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

if (npmVersionSpawnResult.status !== 0) {
Expand Down Expand Up @@ -357,7 +361,8 @@ function _installPackage(
const result: childProcess.SpawnSyncReturns<Buffer> = childProcess.spawnSync(npmPath, [command], {
stdio: 'inherit',
cwd: packageInstallFolder,
env: process.env
env: process.env,
shell: isWindows()
});

if (result.status !== 0) {
Expand All @@ -375,10 +380,14 @@ 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);
}

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 @@ -439,7 +448,7 @@ export function installAndRun(
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 shouldUseShell: boolean = isWindows();
const platformBinPath: string = shouldUseShell ? `"${binPath}"` : binPath;

process.env.PATH = [binFolderPath, originalEnvPath].join(path.delimiter);
Expand Down
Loading