Skip to content

Commit 7952e57

Browse files
alan-agius4clydin
alan-agius4
authored and
clydin
committed
fix(@angular/cli): avoid creating unnecessary global configuration
Previously, when a global configuration was not found on disk we forcefully created it even when it was not needed. This causes issues in CI when multiple `ng` commands would run in parallel as it caused a read/write race conditions. See: https://angular-team.slack.com/archives/C46U16D4Z/p1654089933910829 (cherry picked from commit 08c87c8)
1 parent e4fb966 commit 7952e57

File tree

6 files changed

+52
-33
lines changed

6 files changed

+52
-33
lines changed

‎packages/angular/cli/src/command-builder/command-module.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ export interface CommandContext {
4040
currentDirectory: string;
4141
root: string;
4242
workspace?: AngularWorkspace;
43-
globalConfiguration?: AngularWorkspace;
43+
globalConfiguration: AngularWorkspace;
4444
logger: logging.Logger;
4545
packageManager: PackageManagerUtils;
4646
/** Arguments parsed in free-from without parser configuration. */

‎packages/angular/cli/src/command-builder/command-runner.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ export async function runCommand(args: string[], logger: logging.Logger): Promis
7777
const positional = getYargsCompletions ? _.slice(1) : _;
7878

7979
let workspace: AngularWorkspace | undefined;
80-
let globalConfiguration: AngularWorkspace | undefined;
80+
let globalConfiguration: AngularWorkspace;
8181
try {
8282
[workspace, globalConfiguration] = await Promise.all([
8383
getWorkspace('local'),

‎packages/angular/cli/src/command-builder/schematics-command-module.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,7 @@ export abstract class SchematicsCommandModule
304304

305305
const value =
306306
getSchematicCollections(workspace?.getCli()) ??
307-
getSchematicCollections(globalConfiguration?.getCli());
307+
getSchematicCollections(globalConfiguration.getCli());
308308
if (value) {
309309
return value;
310310
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ export class ConfigCommandModule
5757

5858
async run(options: Options<ConfigCommandArgs>): Promise<number | void> {
5959
const level = options.global ? 'global' : 'local';
60-
const [config] = getWorkspaceRaw(level);
60+
const [config] = await getWorkspaceRaw(level);
6161

6262
if (options.value == undefined) {
6363
if (!config) {
@@ -118,7 +118,7 @@ export class ConfigCommandModule
118118
throw new CommandModuleError('Invalid Path.');
119119
}
120120

121-
const [config, configPath] = getWorkspaceRaw(options.global ? 'global' : 'local');
121+
const [config, configPath] = await getWorkspaceRaw(options.global ? 'global' : 'local');
122122
const { logger } = this.context;
123123

124124
if (!config || !configPath) {

‎packages/angular/cli/src/utilities/config.ts‎

Lines changed: 45 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*/
88

99
import { json, workspaces } from '@angular-devkit/core';
10-
import { existsSync, promises as fs, writeFileSync } from 'fs';
10+
import { existsSync, promises as fs } from 'fs';
1111
import * as os from 'os';
1212
import * as path from 'path';
1313
import { PackageManager } from '../../lib/config/workspace-schema';
@@ -51,6 +51,7 @@ export const workspaceSchemaPath = path.join(__dirname, '../../lib/config/schema
5151

5252
const configNames = ['angular.json', '.angular.json'];
5353
const globalFileName = '.angular-config.json';
54+
const defaultGlobalFilePath = path.join(os.homedir(), globalFileName);
5455

5556
function xdgConfigHome(home: string, configFile?: string): string {
5657
// https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html
@@ -106,9 +107,8 @@ function globalFilePath(): string | null {
106107
return xdgConfigOld;
107108
}
108109

109-
const p = path.join(home, globalFileName);
110-
if (existsSync(p)) {
111-
return p;
110+
if (existsSync(defaultGlobalFilePath)) {
111+
return defaultGlobalFilePath;
112112
}
113113

114114
return null;
@@ -147,7 +147,12 @@ export class AngularWorkspace {
147147
}
148148

149149
save(): Promise<void> {
150-
return workspaces.writeWorkspace(this.workspace, createWorkspaceHost(), this.filePath);
150+
return workspaces.writeWorkspace(
151+
this.workspace,
152+
createWorkspaceHost(),
153+
this.filePath,
154+
workspaces.WorkspaceFormat.JSON,
155+
);
151156
}
152157

153158
static async load(workspaceFilePath: string): Promise<AngularWorkspace> {
@@ -162,22 +167,38 @@ export class AngularWorkspace {
162167
}
163168

164169
const cachedWorkspaces = new Map<string, AngularWorkspace | undefined>();
170+
171+
export async function getWorkspace(level: 'global'): Promise<AngularWorkspace>;
172+
export async function getWorkspace(level: 'local'): Promise<AngularWorkspace | undefined>;
165173
export async function getWorkspace(
166-
level: 'local' | 'global' = 'local',
174+
level: 'local' | 'global',
175+
): Promise<AngularWorkspace | undefined>;
176+
177+
export async function getWorkspace(
178+
level: 'local' | 'global',
167179
): Promise<AngularWorkspace | undefined> {
168180
if (cachedWorkspaces.has(level)) {
169181
return cachedWorkspaces.get(level);
170182
}
171183

172-
let configPath = level === 'local' ? projectFilePath() : globalFilePath();
184+
const configPath = level === 'local' ? projectFilePath() : globalFilePath();
173185
if (!configPath) {
174-
if (level === 'local') {
175-
cachedWorkspaces.set(level, undefined);
186+
if (level === 'global') {
187+
// Unlike a local config, a global config is not mandatory.
188+
// So we create an empty one in memory and keep it as such until it has been modified and saved.
189+
const globalWorkspace = new AngularWorkspace(
190+
{ extensions: {}, projects: new workspaces.ProjectDefinitionCollection() },
191+
defaultGlobalFilePath,
192+
);
193+
194+
cachedWorkspaces.set(level, globalWorkspace);
176195

177-
return undefined;
196+
return globalWorkspace;
178197
}
179198

180-
configPath = createGlobalSettings();
199+
cachedWorkspaces.set(level, undefined);
200+
201+
return undefined;
181202
}
182203

183204
try {
@@ -193,26 +214,24 @@ export async function getWorkspace(
193214
}
194215
}
195216

196-
export function createGlobalSettings(): string {
197-
const home = os.homedir();
198-
if (!home) {
199-
throw new Error('No home directory found.');
200-
}
201-
202-
const globalPath = path.join(home, globalFileName);
203-
writeFileSync(globalPath, JSON.stringify({ version: 1 }));
204-
205-
return globalPath;
206-
}
207-
208-
export function getWorkspaceRaw(
217+
/**
218+
* This method will load the workspace configuration in raw JSON format.
219+
* When `level` is `global` and file doesn't exists, it will be created.
220+
*
221+
* NB: This method is intended to be used only for `ng config`.
222+
*/
223+
export async function getWorkspaceRaw(
209224
level: 'local' | 'global' = 'local',
210-
): [JSONFile | null, string | null] {
225+
): Promise<[JSONFile | null, string | null]> {
211226
let configPath = level === 'local' ? projectFilePath() : globalFilePath();
212227

213228
if (!configPath) {
214229
if (level === 'global') {
215-
configPath = createGlobalSettings();
230+
configPath = defaultGlobalFilePath;
231+
// Config doesn't exist, force create it.
232+
233+
const globalWorkspace = await getWorkspace('global');
234+
await globalWorkspace.save();
216235
} else {
217236
return [null, null];
218237
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ interface PackageManagerOptions {
2626
}
2727

2828
export interface PackageManagerUtilsContext {
29-
globalConfiguration?: AngularWorkspace;
29+
globalConfiguration: AngularWorkspace;
3030
workspace?: AngularWorkspace;
3131
root: string;
3232
}
@@ -326,7 +326,7 @@ export class PackageManagerUtils {
326326
}
327327

328328
if (!result) {
329-
result = getPackageManager(globalWorkspace?.extensions['cli']);
329+
result = getPackageManager(globalWorkspace.extensions['cli']);
330330
}
331331

332332
return result;

0 commit comments

Comments
 (0)