From 2ed714639384848dc398c01eb29a39ac218c959d Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Thu, 16 Apr 2020 09:39:28 +0100 Subject: [PATCH] Revert "fix(ngcc): do not spawn unlocker processes on cluster workers (#36569)" (#36637) This reverts commit 66effde9f3f9796f9507cd80a85692f0e04e96f5. PR Close #36637 --- .../cluster/lock_file_with_child_process.ts | 42 --------- .../lock_file_with_child_process/index.ts | 2 +- packages/compiler-cli/ngcc/src/main.ts | 4 +- .../lock_file_with_child_process_spec.ts | 87 ------------------- 4 files changed, 2 insertions(+), 133 deletions(-) delete mode 100644 packages/compiler-cli/ngcc/src/execution/cluster/lock_file_with_child_process.ts delete mode 100644 packages/compiler-cli/ngcc/test/execution/cluster/lock_file_with_child_process_spec.ts diff --git a/packages/compiler-cli/ngcc/src/execution/cluster/lock_file_with_child_process.ts b/packages/compiler-cli/ngcc/src/execution/cluster/lock_file_with_child_process.ts deleted file mode 100644 index b6354976a6..0000000000 --- a/packages/compiler-cli/ngcc/src/execution/cluster/lock_file_with_child_process.ts +++ /dev/null @@ -1,42 +0,0 @@ -/** - * @license - * Copyright Google Inc. All Rights Reserved. - * - * Use of this source code is governed by an MIT-style license that can be - * found in the LICENSE file at https://angular.io/license - */ - -/// - -import {ChildProcess} from 'child_process'; -import * as cluster from 'cluster'; - -import {AbsoluteFsPath} from '../../../../src/ngtsc/file_system'; -import {LockFileWithChildProcess} from '../../locking/lock_file_with_child_process'; - - -/** - * A `LockFileWithChildProcess` that is `cluster`-aware and does not spawn unlocker processes from - * worker processes (only from the master process, which does the locking). - */ -export class ClusterLockFileWithChildProcess extends LockFileWithChildProcess { - write(): void { - if (!cluster.isMaster) { - // This is a worker process: - // This method should only be on the master process. - throw new Error('Tried to create a lock-file from a worker process.'); - } - - return super.write(); - } - - protected createUnlocker(path: AbsoluteFsPath): ChildProcess|null { - if (cluster.isMaster) { - // This is the master process: - // Create the unlocker. - return super.createUnlocker(path); - } - - return null; - } -} diff --git a/packages/compiler-cli/ngcc/src/locking/lock_file_with_child_process/index.ts b/packages/compiler-cli/ngcc/src/locking/lock_file_with_child_process/index.ts index a544addf0d..9dcb848829 100644 --- a/packages/compiler-cli/ngcc/src/locking/lock_file_with_child_process/index.ts +++ b/packages/compiler-cli/ngcc/src/locking/lock_file_with_child_process/index.ts @@ -78,7 +78,7 @@ export class LockFileWithChildProcess implements LockFile { } } - protected createUnlocker(path: AbsoluteFsPath): ChildProcess|null { + protected createUnlocker(path: AbsoluteFsPath): ChildProcess { this.logger.debug('Forking unlocker child-process'); const logLevel = this.logger.level !== undefined ? this.logger.level.toString() : LogLevel.info.toString(); diff --git a/packages/compiler-cli/ngcc/src/main.ts b/packages/compiler-cli/ngcc/src/main.ts index 0d072866a1..1f63a029bb 100644 --- a/packages/compiler-cli/ngcc/src/main.ts +++ b/packages/compiler-cli/ngcc/src/main.ts @@ -27,7 +27,6 @@ import {EntryPointFinder} from './entry_point_finder/interface'; import {TargetedEntryPointFinder} from './entry_point_finder/targeted_entry_point_finder'; import {AnalyzeEntryPointsFn, CreateCompileFn, Executor} from './execution/api'; import {ClusterExecutor} from './execution/cluster/executor'; -import {ClusterLockFileWithChildProcess} from './execution/cluster/lock_file_with_child_process'; import {ClusterPackageJsonUpdater} from './execution/cluster/package_json_updater'; import {SingleProcessExecutorAsync, SingleProcessExecutorSync} from './execution/single_process_executor'; import {CreateTaskCompletedCallback, PartiallyOrderedTasks, Task, TaskProcessingOutcome, TaskQueue} from './execution/tasks/api'; @@ -429,8 +428,7 @@ function getCreateTaskCompletedCallback( function getExecutor( async: boolean, inParallel: boolean, logger: Logger, pkgJsonUpdater: PackageJsonUpdater, fileSystem: FileSystem, createTaskCompletedCallback: CreateTaskCompletedCallback): Executor { - const lockFile = inParallel ? new ClusterLockFileWithChildProcess(fileSystem, logger) : - new LockFileWithChildProcess(fileSystem, logger); + const lockFile = new LockFileWithChildProcess(fileSystem, logger); if (async) { // Execute asynchronously (either serially or in parallel) const locker = new AsyncLocker(lockFile, logger, 500, 50); diff --git a/packages/compiler-cli/ngcc/test/execution/cluster/lock_file_with_child_process_spec.ts b/packages/compiler-cli/ngcc/test/execution/cluster/lock_file_with_child_process_spec.ts deleted file mode 100644 index 17501508d5..0000000000 --- a/packages/compiler-cli/ngcc/test/execution/cluster/lock_file_with_child_process_spec.ts +++ /dev/null @@ -1,87 +0,0 @@ -/** - * @license - * Copyright Google Inc. All Rights Reserved. - * - * Use of this source code is governed by an MIT-style license that can be - * found in the LICENSE file at https://angular.io/license - */ - -/// - -import {ChildProcess} from 'child_process'; -import * as cluster from 'cluster'; - -import {getFileSystem} from '../../../../src/ngtsc/file_system'; -import {runInEachFileSystem} from '../../../../src/ngtsc/file_system/testing'; -import {ClusterLockFileWithChildProcess} from '../../../src/execution/cluster/lock_file_with_child_process'; -import {LockFileWithChildProcess} from '../../../src/locking/lock_file_with_child_process'; -import {MockLogger} from '../../helpers/mock_logger'; -import {mockProperty} from '../../helpers/spy_utils'; - - -runInEachFileSystem(() => { - describe('ClusterLockFileWithChildProcess', () => { - const runAsClusterMaster = mockProperty(cluster, 'isMaster'); - const mockUnlockerProcess = {} as ChildProcess; - let lockFileWithChildProcessSpies: - Record<'createUnlocker'|'read'|'remove'|'write', jasmine.Spy>; - - beforeEach(() => { - lockFileWithChildProcessSpies = { - createUnlocker: spyOn(LockFileWithChildProcess.prototype as any, 'createUnlocker') - .and.returnValue(mockUnlockerProcess), - read: spyOn(LockFileWithChildProcess.prototype, 'read').and.returnValue('{unknown}'), - remove: spyOn(LockFileWithChildProcess.prototype, 'remove'), - write: spyOn(LockFileWithChildProcess.prototype, 'write'), - }; - }); - - it('should be an instance of `LockFileWithChildProcess`', () => { - const lockFile = new ClusterLockFileWithChildProcess(getFileSystem(), new MockLogger()); - - expect(lockFile).toEqual(jasmine.any(ClusterLockFileWithChildProcess)); - expect(lockFile).toEqual(jasmine.any(LockFileWithChildProcess)); - }); - - describe('write()', () => { - it('should create the lock-file when called on the cluster master', () => { - runAsClusterMaster(true); - const lockFile = new ClusterLockFileWithChildProcess(getFileSystem(), new MockLogger()); - - expect(lockFileWithChildProcessSpies.write).not.toHaveBeenCalled(); - - lockFile.write(); - expect(lockFileWithChildProcessSpies.write).toHaveBeenCalledWith(); - }); - - it('should throw an error when called on a cluster worker', () => { - runAsClusterMaster(false); - const lockFile = new ClusterLockFileWithChildProcess(getFileSystem(), new MockLogger()); - - expect(() => lockFile.write()) - .toThrowError('Tried to create a lock-file from a worker process.'); - expect(lockFileWithChildProcessSpies.write).not.toHaveBeenCalled(); - }); - }); - - describe('createUnlocker()', () => { - it('should create the unlocker when called on the cluster master', () => { - runAsClusterMaster(true); - const lockFile = new ClusterLockFileWithChildProcess(getFileSystem(), new MockLogger()); - - lockFileWithChildProcessSpies.createUnlocker.calls.reset(); - - expect((lockFile as any).createUnlocker(lockFile.path)).toBe(mockUnlockerProcess); - expect(lockFileWithChildProcessSpies.createUnlocker).toHaveBeenCalledWith(lockFile.path); - }); - - it('should not create the unlocker when called on a cluster worker', () => { - runAsClusterMaster(false); - const lockFile = new ClusterLockFileWithChildProcess(getFileSystem(), new MockLogger()); - - expect((lockFile as any).createUnlocker(lockFile.path)).toBeNull(); - expect(lockFileWithChildProcessSpies.createUnlocker).not.toHaveBeenCalled(); - }); - }); - }); -});