Skip to content

Commit 5adc742

Browse files
alan-agius4alan-agius4
committed
fix(@angular/cli): use dedicated cache directory for temporary package installs
Relocates temporary package installations to a dedicated cache directory. This ensures that the project's `.npmrc` is correctly respected, as the installer does relies on the current working directory (CWD) rather than flags like `--prefix`.
1 parent 1944008 commit 5adc742

File tree

6 files changed

+117
-92
lines changed

6 files changed

+117
-92
lines changed

‎packages/angular/cli/src/commands/add/cli.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import {
3333
import { assertIsError } from '../../utilities/error';
3434
import { isTTY } from '../../utilities/tty';
3535
import { VERSION } from '../../utilities/version';
36+
import { getCacheConfig } from '../cache/utilities';
3637

3738
class CommandError extends Error {}
3839

@@ -299,7 +300,8 @@ export default class AddCommandModule
299300
task: AddCommandTaskWrapper,
300301
): Promise<void> {
301302
context.packageManager = await createPackageManager({
302-
cwd: this.context.root,
303+
cacheDirectory: getCacheConfig(this.context.workspace).path,
304+
root: this.context.root,
303305
logger: this.context.logger,
304306
dryRun: context.dryRun,
305307
});

‎packages/angular/cli/src/commands/update/cli.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import {
3030
getProjectDependencies,
3131
readPackageJson,
3232
} from '../../utilities/package-tree';
33+
import { getCacheConfig } from '../cache/utilities';
3334
import {
3435
checkCLIVersion,
3536
coerceVersionNumber,
@@ -172,7 +173,8 @@ export default class UpdateCommandModule extends CommandModule<UpdateCommandArgs
172173
const { logger } = this.context;
173174
// Instantiate the package manager
174175
const packageManager = await createPackageManager({
175-
cwd: this.context.root,
176+
root: this.context.root,
177+
cacheDirectory: getCacheConfig(this.context.workspace).path,
176178
logger,
177179
configuredPackageManager: this.context.packageManager.name,
178180
});

‎packages/angular/cli/src/package-managers/factory.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
import { major } from 'semver';
1010
import { discover } from './discovery';
11-
import { Host, NodeJS_HOST } from './host';
11+
import { Host, createNodeJsHost } from './host';
1212
import { Logger } from './logger';
1313
import { PackageManager } from './package-manager';
1414
import { PackageManagerName, SUPPORTED_PACKAGE_MANAGERS } from './package-manager-descriptor';
@@ -106,13 +106,14 @@ async function determinePackageManager(
106106
* @returns A promise that resolves to a new `PackageManager` instance.
107107
*/
108108
export async function createPackageManager(options: {
109-
cwd: string;
109+
root: string;
110+
cacheDirectory: string;
110111
configuredPackageManager?: PackageManagerName;
111112
logger?: Logger;
112113
dryRun?: boolean;
113114
}): Promise<PackageManager> {
114-
const { cwd, configuredPackageManager, logger, dryRun } = options;
115-
const host = NodeJS_HOST;
115+
const { root: cwd, cacheDirectory, configuredPackageManager, logger, dryRun } = options;
116+
const host = createNodeJsHost(cwd, cacheDirectory);
116117

117118
const { name, source } = await determinePackageManager(
118119
host,

‎packages/angular/cli/src/package-managers/host.ts‎

Lines changed: 91 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,9 @@
1414
*/
1515

1616
import { type SpawnOptions, spawn } from 'node:child_process';
17-
import { Stats } from 'node:fs';
18-
import { mkdtemp, readFile, readdir, rm, stat, writeFile } from 'node:fs/promises';
19-
import { platform, tmpdir } from 'node:os';
17+
import { Stats, existsSync } from 'node:fs';
18+
import { copyFile, mkdir, mkdtemp, readFile, readdir, rm, stat, writeFile } from 'node:fs/promises';
19+
import { platform } from 'node:os';
2020
import { join } from 'node:path';
2121
import { PackageManagerError } from './error';
2222

@@ -87,68 +87,95 @@ export interface Host {
8787
}
8888

8989
/**
90-
* A concrete implementation of the `Host` interface that uses the Node.js APIs.
90+
* The package manager configuration files that are copied to the temp directory.
9191
*/
92-
export const NodeJS_HOST: Host = {
93-
stat,
94-
readdir,
95-
readFile: (path: string) => readFile(path, { encoding: 'utf8' }),
96-
writeFile,
97-
createTempDirectory: () => mkdtemp(join(tmpdir(), 'angular-cli-')),
98-
deleteDirectory: (path: string) => rm(path, { recursive: true, force: true }),
99-
runCommand: async (
100-
command: string,
101-
args: readonly string[],
102-
options: {
103-
timeout?: number;
104-
stdio?: 'pipe' | 'ignore';
105-
cwd?: string;
106-
env?: Record<string, string>;
107-
} = {},
108-
): Promise<{ stdout: string; stderr: string }> => {
109-
const signal = options.timeout ? AbortSignal.timeout(options.timeout) : undefined;
110-
const isWin32 = platform() === 'win32';
111-
112-
return new Promise((resolve, reject) => {
113-
const spawnOptions = {
114-
shell: isWin32,
115-
stdio: options.stdio ?? 'pipe',
116-
signal,
117-
cwd: options.cwd,
118-
env: {
119-
...process.env,
120-
...options.env,
121-
},
122-
} satisfies SpawnOptions;
123-
const childProcess = isWin32
124-
? spawn(`${command} ${args.join(' ')}`, spawnOptions)
125-
: spawn(command, args, spawnOptions);
126-
127-
let stdout = '';
128-
childProcess.stdout?.on('data', (data) => (stdout += data.toString()));
129-
130-
let stderr = '';
131-
childProcess.stderr?.on('data', (data) => (stderr += data.toString()));
132-
133-
childProcess.on('close', (code) => {
134-
if (code === 0) {
135-
resolve({ stdout, stderr });
136-
} else {
137-
const message = `Process exited with code ${code}.`;
138-
reject(new PackageManagerError(message, stdout, stderr, code));
139-
}
140-
});
92+
const PACKAGE_MANAGER_CONFIG_FILES = ['.npmrc', 'bunfig.toml'];
14193

142-
childProcess.on('error', (err) => {
143-
if (err.name === 'AbortError') {
144-
const message = `Process timed out.`;
94+
/**
95+
* A concrete implementation of the `Host` interface that uses the Node.js APIs.
96+
* @param root The root directory of the project.
97+
* @param cacheDirectory The directory to use for caching.
98+
* @returns A host that uses the Node.js APIs.
99+
*/
100+
export function createNodeJsHost(root: string, cacheDirectory: string): Host {
101+
return {
102+
stat,
103+
readdir,
104+
readFile: (path: string) => readFile(path, { encoding: 'utf8' }),
105+
writeFile,
106+
createTempDirectory: async () => {
107+
await mkdir(cacheDirectory, { recursive: true });
108+
const tmpDir = await mkdtemp(join(cacheDirectory, 'package-manager-tmp-'));
109+
110+
// Copy the configs as bun does not read files up the tree.
111+
await Promise.all(
112+
PACKAGE_MANAGER_CONFIG_FILES.map((configFile) => {
113+
const sourcePath = join(root, configFile);
114+
const destinationPath = join(tmpDir, configFile);
115+
116+
if (existsSync(sourcePath)) {
117+
return copyFile(sourcePath, destinationPath);
118+
}
119+
}),
120+
);
121+
122+
return tmpDir;
123+
},
124+
deleteDirectory: (path: string) => rm(path, { recursive: true, force: true }),
125+
runCommand: async (
126+
command: string,
127+
args: readonly string[],
128+
options: {
129+
timeout?: number;
130+
stdio?: 'pipe' | 'ignore';
131+
cwd?: string;
132+
env?: Record<string, string>;
133+
} = {},
134+
): Promise<{ stdout: string; stderr: string }> => {
135+
const signal = options.timeout ? AbortSignal.timeout(options.timeout) : undefined;
136+
const isWin32 = platform() === 'win32';
137+
138+
return new Promise((resolve, reject) => {
139+
const spawnOptions = {
140+
shell: isWin32,
141+
stdio: options.stdio ?? 'pipe',
142+
signal,
143+
cwd: options.cwd,
144+
env: {
145+
...process.env,
146+
...options.env,
147+
},
148+
} satisfies SpawnOptions;
149+
const childProcess = isWin32
150+
? spawn(`${command} ${args.join(' ')}`, spawnOptions)
151+
: spawn(command, args, spawnOptions);
152+
153+
let stdout = '';
154+
childProcess.stdout?.on('data', (data) => (stdout += data.toString()));
155+
156+
let stderr = '';
157+
childProcess.stderr?.on('data', (data) => (stderr += data.toString()));
158+
159+
childProcess.on('close', (code) => {
160+
if (code === 0) {
161+
resolve({ stdout, stderr });
162+
} else {
163+
const message = `Process exited with code ${code}.`;
164+
reject(new PackageManagerError(message, stdout, stderr, code));
165+
}
166+
});
167+
168+
childProcess.on('error', (err) => {
169+
if (err.name === 'AbortError') {
170+
const message = `Process timed out.`;
171+
reject(new PackageManagerError(message, stdout, stderr, null));
172+
173+
return;
174+
}
175+
const message = `Process failed with error: ${err.message}`;
145176
reject(new PackageManagerError(message, stdout, stderr, null));
146-
147-
return;
148-
}
149-
const message = `Process failed with error: ${err.message}`;
150-
reject(new PackageManagerError(message, stdout, stderr, null));
177+
});
151178
});
152-
});
153-
},
154-
};
179+
},
180+
};
181+
}

‎packages/angular/cli/src/package-managers/package-manager.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -546,9 +546,11 @@ export class PackageManager {
546546
// Writing an empty package.json file beforehand prevents this.
547547
await this.host.writeFile(join(workingDirectory, 'package.json'), '{}');
548548

549-
const flags = [options.ignoreScripts ? this.descriptor.ignoreScriptsFlag : ''].filter(
550-
(flag) => flag,
551-
);
549+
const flags = [];
550+
if (options.ignoreScripts) {
551+
flags.push(this.descriptor.ignoreScriptsFlag);
552+
}
553+
552554
const args: readonly string[] = [this.descriptor.addCommand, specifier, ...flags];
553555

554556
try {

‎tests/e2e/utils/registry.ts‎

Lines changed: 10 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -70,30 +70,21 @@ export async function createNpmConfigForAuthentication(
7070
const token = invalidToken ? `invalid=` : VALID_TOKEN;
7171
const registry = (getGlobalVariable('package-secure-registry') as string).replace(/^\w+:/, '');
7272

73+
// `always-auth is required for yarn classic.
74+
// See: https://www.verdaccio.org/docs/setup-yarn#yarn-classic-1x
7375
await writeFile(
7476
'.npmrc',
7577
scopedAuthentication
7678
? `
77-
${registry}/:_auth="${token}"
78-
registry=http:${registry}
79-
`
79+
${registry}/:_auth="${token}"
80+
registry=http:${registry}
81+
always-auth = true
82+
`
8083
: `
81-
_auth="${token}"
82-
registry=http:${registry}
83-
`,
84-
);
85-
86-
await writeFile(
87-
'.yarnrc',
88-
scopedAuthentication
89-
? `
90-
${registry}/:_auth "${token}"
91-
registry http:${registry}
92-
`
93-
: `
94-
_auth "${token}"
95-
registry http:${registry}
96-
`,
84+
_auth="${token}"
85+
registry=http:${registry}
86+
always-auth = true
87+
`,
9788
);
9889
}
9990

0 commit comments

Comments
 (0)