fix(@angular/cli): avoid creating unnecessary global configuration

alan-agius4 · clydin · commit 08c87c8d473e · 2022-06-03T14:52:52.000-04:00
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
diff --git 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
@@ -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
@@ -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
@@ -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) {
@@ -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
@@ -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<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> {
@@ -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 {
@@ -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
@@ -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;

Original file line number	Diff line number	Diff line change
`@@ -304,7 +304,7 @@ export abstract class SchematicsCommandModule`
`304`	`304`
`305`	`305`	`const value =`
`306`	`306`	`getSchematicCollections(workspace?.getCli()) ??`
`307`		`- getSchematicCollections(globalConfiguration?.getCli());`
	`307`	`+ getSchematicCollections(globalConfiguration.getCli());`
`308`	`308`	`if (value) {`
`309`	`309`	`return value;`
`310`	`310`	`}`
Original file line number	Diff line number	Diff line change
`@@ -26,7 +26,7 @@ interface PackageManagerOptions {`
`26`	`26`	`}`
`27`	`27`
`28`	`28`	`export interface PackageManagerUtilsContext {`
`29`		`- globalConfiguration?: AngularWorkspace;`
	`29`	`+ globalConfiguration: AngularWorkspace;`
`30`	`30`	`workspace?: AngularWorkspace;`
`31`	`31`	`root: string;`
`32`	`32`	`}`
`@@ -326,7 +326,7 @@ export class PackageManagerUtils {`
`326`	`326`	`}`
`327`	`327`
`328`	`328`	`if (!result) {`
`329`		`- result = getPackageManager(globalWorkspace?.extensions['cli']);`
	`329`	`+ result = getPackageManager(globalWorkspace.extensions['cli']);`
`330`	`330`	`}`
`331`	`331`
`332`	`332`	`return result;`