From 7952e579066f7191f4b82a10816c6a41a4ea5644 Mon Sep 17 00:00:00 2001 From: Alan Agius Date: Thu, 2 Jun 2022 09:37:38 +0000 Subject: [PATCH] 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 08c87c8d473e4cc56830ea40d562fd9a1eb51e4e) --- .../cli/src/command-builder/command-module.ts | 2 +- .../cli/src/command-builder/command-runner.ts | 2 +- .../schematics-command-module.ts | 2 +- .../angular/cli/src/commands/config/cli.ts | 4 +- packages/angular/cli/src/utilities/config.ts | 71 ++++++++++++------- .../cli/src/utilities/package-manager.ts | 4 +- 6 files changed, 52 insertions(+), 33 deletions(-) diff --git a/packages/angular/cli/src/command-builder/command-module.ts b/packages/angular/cli/src/command-builder/command-module.ts index 0ca5b12a742b..b476c392dd5b 100644 --- a/packages/angular/cli/src/command-builder/command-module.ts +++ b/packages/angular/cli/src/command-builder/command-module.ts @@ -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. */ diff --git a/packages/angular/cli/src/command-builder/command-runner.ts b/packages/angular/cli/src/command-builder/command-runner.ts index a1683d3a124b..a0b6d06f8806 100644 --- a/packages/angular/cli/src/command-builder/command-runner.ts +++ b/packages/angular/cli/src/command-builder/command-runner.ts @@ -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'), diff --git a/packages/angular/cli/src/command-builder/schematics-command-module.ts b/packages/angular/cli/src/command-builder/schematics-command-module.ts index 13102644e820..830ccd3077a2 100644 --- a/packages/angular/cli/src/command-builder/schematics-command-module.ts +++ b/packages/angular/cli/src/command-builder/schematics-command-module.ts @@ -304,7 +304,7 @@ export abstract class SchematicsCommandModule const value = getSchematicCollections(workspace?.getCli()) ?? - getSchematicCollections(globalConfiguration?.getCli()); + getSchematicCollections(globalConfiguration.getCli()); if (value) { return value; } diff --git a/packages/angular/cli/src/commands/config/cli.ts b/packages/angular/cli/src/commands/config/cli.ts index c0e09de137e0..4fe22df391a9 100644 --- a/packages/angular/cli/src/commands/config/cli.ts +++ b/packages/angular/cli/src/commands/config/cli.ts @@ -57,7 +57,7 @@ export class ConfigCommandModule async run(options: Options): Promise { const level = options.global ? 'global' : 'local'; - const [config] = getWorkspaceRaw(level); + const [config] = await getWorkspaceRaw(level); if (options.value == undefined) { if (!config) { @@ -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) { diff --git a/packages/angular/cli/src/utilities/config.ts b/packages/angular/cli/src/utilities/config.ts index 8a9263d518cc..601d609c2e10 100644 --- a/packages/angular/cli/src/utilities/config.ts +++ b/packages/angular/cli/src/utilities/config.ts @@ -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'; @@ -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 @@ -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; @@ -147,7 +147,12 @@ export class AngularWorkspace { } save(): Promise { - 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 { @@ -162,22 +167,38 @@ export class AngularWorkspace { } const cachedWorkspaces = new Map(); + +export async function getWorkspace(level: 'global'): Promise; +export async function getWorkspace(level: 'local'): Promise; export async function getWorkspace( - level: 'local' | 'global' = 'local', + level: 'local' | 'global', +): Promise; + +export async function getWorkspace( + level: 'local' | 'global', ): Promise { 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 { @@ -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]; } diff --git a/packages/angular/cli/src/utilities/package-manager.ts b/packages/angular/cli/src/utilities/package-manager.ts index d05219b1dec9..95799efd8747 100644 --- a/packages/angular/cli/src/utilities/package-manager.ts +++ b/packages/angular/cli/src/utilities/package-manager.ts @@ -26,7 +26,7 @@ interface PackageManagerOptions { } export interface PackageManagerUtilsContext { - globalConfiguration?: AngularWorkspace; + globalConfiguration: AngularWorkspace; workspace?: AngularWorkspace; root: string; } @@ -326,7 +326,7 @@ export class PackageManagerUtils { } if (!result) { - result = getPackageManager(globalWorkspace?.extensions['cli']); + result = getPackageManager(globalWorkspace.extensions['cli']); } return result;