Skip to content

Commit a6ab285

Browse files
committed
fix: pnpm installs hang forever - close stdin, harden the pnpm backend
pnpm 10/11 never exits while its stdin is an open pipe, so every CLI-fired install (ns run/debug --force, ns install, plugin add) finished its work and then hung the CLI on the child's "close" event. Non-interactive installs now spawn with stdin ignored. Also in the pnpm backend: - pass --shamefully-hoist only when no pnpm-workspace.yaml or .npmrc layout key (node-linker, shamefully-hoist, hoist*) governs the install dir: pnpm treats a contradicting hoist flag as a config change and rebuilds node_modules, aborting when there is no TTY - drop CLI-internal options (ignoreScripts, path, frameworkPath) before flag serialization: pnpm hard-fails on unknown options where npm silently accepts them - getCachePath: `pnpm config get cache` prints "undefined" (pnpm has no cache key), yielding a relative junk path for pacote; fall back to the parent of `pnpm store path`
1 parent 68ab7ca commit a6ab285

3 files changed

Lines changed: 312 additions & 21 deletions

File tree

lib/base-package-manager.ts

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,17 @@ export abstract class BasePackageManager implements INodePackageManager {
1717
public abstract install(
1818
packageName: string,
1919
pathToSave: string,
20-
config: INodePackageManagerInstallOptions
20+
config: INodePackageManagerInstallOptions,
2121
): Promise<INpmInstallResultInfo>;
2222
public abstract uninstall(
2323
packageName: string,
2424
config?: IDictionary<string | boolean>,
25-
path?: string
25+
path?: string,
2626
): Promise<string>;
2727
public abstract view(packageName: string, config: Object): Promise<any>;
2828
public abstract search(
2929
filter: string[],
30-
config: IDictionary<string | boolean>
30+
config: IDictionary<string | boolean>,
3131
): Promise<string>;
3232
public abstract searchNpms(keyword: string): Promise<INpmsResult>;
3333
public abstract getRegistryPackageData(packageName: string): Promise<any>;
@@ -38,7 +38,7 @@ export abstract class BasePackageManager implements INodePackageManager {
3838
protected $fs: IFileSystem,
3939
private $hostInfo: IHostInfo,
4040
private $pacoteService: IPacoteService,
41-
private packageManager: string
41+
private packageManager: string,
4242
) {}
4343

4444
public async isRegistered(packageName: string): Promise<boolean> {
@@ -65,7 +65,7 @@ export abstract class BasePackageManager implements INodePackageManager {
6565
}
6666

6767
public async getPackageNameParts(
68-
fullPackageName: string
68+
fullPackageName: string,
6969
): Promise<INpmPackageNameParts> {
7070
// support <reserved_name>@<version> syntax, for example typescript@1.0.0
7171
// support <scoped_package_name>@<version> syntax, for example @nativescript/vue-template@1.0.0
@@ -84,7 +84,7 @@ export abstract class BasePackageManager implements INodePackageManager {
8484
}
8585

8686
public async getPackageFullName(
87-
packageNameParts: INpmPackageNameParts
87+
packageNameParts: INpmPackageNameParts,
8888
): Promise<string> {
8989
return packageNameParts.version
9090
? `${packageNameParts.name}@${packageNameParts.version}`
@@ -104,10 +104,15 @@ export abstract class BasePackageManager implements INodePackageManager {
104104
protected async processPackageManagerInstall(
105105
packageName: string,
106106
params: string[],
107-
opts: { cwd: string; isInstallingAllDependencies: boolean }
107+
opts: { cwd: string; isInstallingAllDependencies: boolean },
108108
): Promise<INpmInstallResultInfo> {
109109
const npmExecutable = this.getPackageManagerExecutableName();
110-
const stdioValue = isInteractive() ? "inherit" : "pipe";
110+
// stdin must be closed, not an open pipe: pnpm keeps the process alive
111+
// listening on a piped stdin after the install completes, so waiting for
112+
// "close" would hang forever.
113+
const stdioValue: any = isInteractive()
114+
? "inherit"
115+
: ["ignore", "pipe", "pipe"];
111116
await this.$childProcess.spawnFromEvent(npmExecutable, params, "close", {
112117
cwd: opts.cwd,
113118
stdio: stdioValue,

lib/pnpm-package-manager.ts

Lines changed: 62 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ export class PnpmPackageManager extends BasePackageManager {
2626
$hostInfo: IHostInfo,
2727
private $httpClient: Server.IHttpClient,
2828
private $logger: ILogger,
29-
$pacoteService: IPacoteService
29+
$pacoteService: IPacoteService,
3030
) {
3131
super($childProcess, $fs, $hostInfo, $pacoteService, "pnpm");
3232
}
@@ -35,7 +35,7 @@ export class PnpmPackageManager extends BasePackageManager {
3535
public async install(
3636
packageName: string,
3737
pathToSave: string,
38-
config: INodePackageManagerInstallOptions
38+
config: INodePackageManagerInstallOptions,
3939
): Promise<INpmInstallResultInfo> {
4040
if (config.disableNpmInstall) {
4141
return;
@@ -44,13 +44,25 @@ export class PnpmPackageManager extends BasePackageManager {
4444
if (config.ignoreScripts) {
4545
config["ignore-scripts"] = true;
4646
}
47+
// CLI-internal options must never reach the command line: pnpm, unlike
48+
// npm, hard-fails on unknown options.
49+
delete config.ignoreScripts;
50+
delete config.path;
51+
delete config.frameworkPath;
4752

4853
const packageJsonPath = path.join(pathToSave, "package.json");
4954
const jsonContentBefore = this.$fs.readJson(packageJsonPath);
5055

5156
const flags = this.getFlagsString(config, true);
52-
// With pnpm we need to install as "flat" or some imports wont be found
53-
let params = ["i", "--shamefully-hoist"];
57+
let params = ["i"];
58+
if (!this.projectManagesOwnHoisting(pathToSave)) {
59+
// With pnpm's default isolated layout some imports won't be found, so
60+
// install "flat". Skipped when the project configures its own layout:
61+
// pnpm treats a hoisting flag that contradicts the stored install state
62+
// as a config change and rebuilds node_modules from scratch after a
63+
// prompt (aborting outright when there is no TTY).
64+
params.push("--shamefully-hoist");
65+
}
5466
const isInstallingAllDependencies = packageName === pathToSave;
5567
if (!isInstallingAllDependencies) {
5668
params.push(packageName);
@@ -63,7 +75,7 @@ export class PnpmPackageManager extends BasePackageManager {
6375
const result = await this.processPackageManagerInstall(
6476
packageName,
6577
params,
66-
{ cwd, isInstallingAllDependencies }
78+
{ cwd, isInstallingAllDependencies },
6779
);
6880
return result;
6981
} catch (e) {
@@ -76,7 +88,7 @@ export class PnpmPackageManager extends BasePackageManager {
7688
public uninstall(
7789
packageName: string,
7890
config?: IDictionary<string | boolean>,
79-
cwd?: string
91+
cwd?: string,
8092
): Promise<string> {
8193
// pnpm does not want save option in remove. It saves it by default
8294
delete config["save"];
@@ -94,7 +106,7 @@ export class PnpmPackageManager extends BasePackageManager {
94106
let viewResult: any;
95107
try {
96108
viewResult = await this.$childProcess.exec(
97-
`pnpm info ${packageName} ${flags}`
109+
`pnpm info ${packageName} ${flags}`,
98110
);
99111
} catch (e) {
100112
this.$errors.fail(e.message);
@@ -110,15 +122,15 @@ export class PnpmPackageManager extends BasePackageManager {
110122
@exported("pnpm")
111123
public search(
112124
filter: string[],
113-
config: IDictionary<string | boolean>
125+
config: IDictionary<string | boolean>,
114126
): Promise<string> {
115127
const flags = this.getFlagsString(config, false);
116128
return this.$childProcess.exec(`pnpm search ${filter.join(" ")} ${flags}`);
117129
}
118130

119131
public async searchNpms(keyword: string): Promise<INpmsResult> {
120132
const httpRequestResult = await this.$httpClient.httpRequest(
121-
`https://api.npms.io/v2/search?q=keywords:${keyword}`
133+
`https://api.npms.io/v2/search?q=keywords:${keyword}`,
122134
);
123135
const result: INpmsResult = JSON.parse(httpRequestResult.body);
124136
return result;
@@ -129,23 +141,60 @@ export class PnpmPackageManager extends BasePackageManager {
129141
const registry = await this.$childProcess.exec(`pnpm config get registry`);
130142
const url = `${registry.trim()}/${packageName}`;
131143
this.$logger.trace(
132-
`Trying to get data from pnpm registry for package ${packageName}, url is: ${url}`
144+
`Trying to get data from pnpm registry for package ${packageName}, url is: ${url}`,
133145
);
134146
const responseData = (await this.$httpClient.httpRequest(url)).body;
135147
this.$logger.trace(
136-
`Successfully received data from pnpm registry for package ${packageName}. Response data is: ${responseData}`
148+
`Successfully received data from pnpm registry for package ${packageName}. Response data is: ${responseData}`,
137149
);
138150
const jsonData = JSON.parse(responseData);
139151
this.$logger.trace(
140-
`Successfully parsed data from pnpm registry for package ${packageName}.`
152+
`Successfully parsed data from pnpm registry for package ${packageName}.`,
141153
);
142154
return jsonData;
143155
}
144156

145157
@exported("pnpm")
146158
public async getCachePath(): Promise<string> {
147159
const cachePath = await this.$childProcess.exec(`pnpm config get cache`);
148-
return path.join(cachePath.trim(), CACACHE_DIRECTORY_NAME);
160+
const cacheDir = cachePath && cachePath.trim();
161+
// pnpm has no `cache` config key of its own: modern versions print
162+
// "undefined" (older ones an empty string), which would yield a relative
163+
// garbage path. Derive a stable per-user location from the store instead.
164+
if (cacheDir && cacheDir !== "undefined" && cacheDir !== "null") {
165+
return path.join(cacheDir, CACACHE_DIRECTORY_NAME);
166+
}
167+
const storePath = await this.$childProcess.exec(`pnpm store path`);
168+
return path.join(path.dirname(storePath.trim()), CACACHE_DIRECTORY_NAME);
169+
}
170+
171+
private projectManagesOwnHoisting(installDir: string): boolean {
172+
// A pnpm-workspace.yaml (pnpm's config home since v10) or an .npmrc with
173+
// a layout key marks the node_modules layout as the project's own choice.
174+
const layoutKeyPattern =
175+
/^\s*(shamefully-hoist|node-linker|hoist|hoist-pattern|public-hoist-pattern)\s*[=:]/m;
176+
let dir = path.resolve(installDir);
177+
while (true) {
178+
if (this.$fs.exists(path.join(dir, "pnpm-workspace.yaml"))) {
179+
return true;
180+
}
181+
const npmrcPath = path.join(dir, ".npmrc");
182+
if (this.$fs.exists(npmrcPath)) {
183+
try {
184+
const npmrcContent = this.$fs.readText(npmrcPath);
185+
if (npmrcContent && layoutKeyPattern.test(npmrcContent)) {
186+
return true;
187+
}
188+
} catch (err) {
189+
this.$logger.trace(`Unable to read ${npmrcPath}. Error is: `, err);
190+
}
191+
}
192+
const parent = path.dirname(dir);
193+
if (parent === dir) {
194+
return false;
195+
}
196+
dir = parent;
197+
}
149198
}
150199
}
151200

0 commit comments

Comments
 (0)