Skip to content

Commit

Permalink
fix(@angular/cli): avoid creating unnecessary global configuration
Browse files Browse the repository at this point in the history
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)
  • Loading branch information
alan-agius4 authored and clydin committed Jun 3, 2022
1 parent e4fb966 commit 7952e57
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 33 deletions.
2 changes: 1 addition & 1 deletion packages/angular/cli/src/command-builder/command-module.ts
Expand Up @@ -40,7 +40,7 @@ export interface CommandContext {
currentDirectory: string;
root: string;
workspace?: AngularWorkspace;
globalConfiguration?: AngularWorkspace;
globalConfiguration: AngularWorkspace;
logger: logging.Logger;
packageManager: PackageManagerUtils;
/** Arguments parsed in free-from without parser configuration. */
Expand Down
2 changes: 1 addition & 1 deletion packages/angular/cli/src/command-builder/command-runner.ts
Expand Up @@ -77,7 +77,7 @@ export async function runCommand(args: string[], logger: logging.Logger): Promis
const positional = getYargsCompletions ? _.slice(1) : _;

let workspace: AngularWorkspace | undefined;
let globalConfiguration: AngularWorkspace | undefined;
let globalConfiguration: AngularWorkspace;
try {
[workspace, globalConfiguration] = await Promise.all([
getWorkspace('local'),
Expand Down
Expand Up @@ -304,7 +304,7 @@ export abstract class SchematicsCommandModule

const value =
getSchematicCollections(workspace?.getCli()) ??
getSchematicCollections(globalConfiguration?.getCli());
getSchematicCollections(globalConfiguration.getCli());
if (value) {
return value;
}
Expand Down
4 changes: 2 additions & 2 deletions packages/angular/cli/src/commands/config/cli.ts
Expand Up @@ -57,7 +57,7 @@ export class ConfigCommandModule

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

if (options.value == undefined) {
if (!config) {
Expand Down Expand Up @@ -118,7 +118,7 @@ export class ConfigCommandModule
throw new CommandModuleError('Invalid Path.');
}

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

if (!config || !configPath) {
Expand Down
71 changes: 45 additions & 26 deletions packages/angular/cli/src/utilities/config.ts
Expand Up @@ -7,7 +7,7 @@
*/

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

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

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

const p = path.join(home, globalFileName);
if (existsSync(p)) {
return p;
if (existsSync(defaultGlobalFilePath)) {
return defaultGlobalFilePath;
}

return null;
Expand Down Expand Up @@ -147,7 +147,12 @@ export class AngularWorkspace {
}

save(): Promise<void> {
return workspaces.writeWorkspace(this.workspace, createWorkspaceHost(), this.filePath);
return workspaces.writeWorkspace(
this.workspace,
createWorkspaceHost(),
this.filePath,
workspaces.WorkspaceFormat.JSON,
);
}

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

const cachedWorkspaces = new Map<string, AngularWorkspace | undefined>();

export async function getWorkspace(level: 'global'): Promise<AngularWorkspace>;
export async function getWorkspace(level: 'local'): Promise<AngularWorkspace | undefined>;
export async function getWorkspace(
level: 'local' | 'global' = 'local',
level: 'local' | 'global',
): Promise<AngularWorkspace | undefined>;

export async function getWorkspace(
level: 'local' | 'global',
): Promise<AngularWorkspace | undefined> {
if (cachedWorkspaces.has(level)) {
return cachedWorkspaces.get(level);
}

let configPath = level === 'local' ? projectFilePath() : globalFilePath();
const configPath = level === 'local' ? projectFilePath() : globalFilePath();
if (!configPath) {
if (level === 'local') {
cachedWorkspaces.set(level, undefined);
if (level === 'global') {
// Unlike a local config, a global config is not mandatory.
// So we create an empty one in memory and keep it as such until it has been modified and saved.
const globalWorkspace = new AngularWorkspace(
{ extensions: {}, projects: new workspaces.ProjectDefinitionCollection() },
defaultGlobalFilePath,
);

cachedWorkspaces.set(level, globalWorkspace);

return undefined;
return globalWorkspace;
}

configPath = createGlobalSettings();
cachedWorkspaces.set(level, undefined);

return undefined;
}

try {
Expand All @@ -193,26 +214,24 @@ export async function getWorkspace(
}
}

export function createGlobalSettings(): string {
const home = os.homedir();
if (!home) {
throw new Error('No home directory found.');
}

const globalPath = path.join(home, globalFileName);
writeFileSync(globalPath, JSON.stringify({ version: 1 }));

return globalPath;
}

export function getWorkspaceRaw(
/**
* This method will load the workspace configuration in raw JSON format.
* When `level` is `global` and file doesn't exists, it will be created.
*
* NB: This method is intended to be used only for `ng config`.
*/
export async function getWorkspaceRaw(
level: 'local' | 'global' = 'local',
): [JSONFile | null, string | null] {
): Promise<[JSONFile | null, string | null]> {
let configPath = level === 'local' ? projectFilePath() : globalFilePath();

if (!configPath) {
if (level === 'global') {
configPath = createGlobalSettings();
configPath = defaultGlobalFilePath;
// Config doesn't exist, force create it.

const globalWorkspace = await getWorkspace('global');
await globalWorkspace.save();
} else {
return [null, null];
}
Expand Down
4 changes: 2 additions & 2 deletions packages/angular/cli/src/utilities/package-manager.ts
Expand Up @@ -26,7 +26,7 @@ interface PackageManagerOptions {
}

export interface PackageManagerUtilsContext {
globalConfiguration?: AngularWorkspace;
globalConfiguration: AngularWorkspace;
workspace?: AngularWorkspace;
root: string;
}
Expand Down Expand Up @@ -326,7 +326,7 @@ export class PackageManagerUtils {
}

if (!result) {
result = getPackageManager(globalWorkspace?.extensions['cli']);
result = getPackageManager(globalWorkspace.extensions['cli']);
}

return result;
Expand Down

0 comments on commit 7952e57

Please sign in to comment.