diff --git a/packages/compiler-cli/ngcc/main-ngcc.ts b/packages/compiler-cli/ngcc/main-ngcc.ts index 41a10d0112..394d9cb52d 100644 --- a/packages/compiler-cli/ngcc/main-ngcc.ts +++ b/packages/compiler-cli/ngcc/main-ngcc.ts @@ -39,7 +39,8 @@ if (require.main === module) { .option('t', { alias: 'target', describe: - 'A relative path (from the `source` path) to a single entry-point to process (plus its dependencies).', + 'A relative path (from the `source` path) to a single entry-point to process (plus its dependencies).\n' + + 'If this property is provided then `error-on-failed-entry-point` is forced to true', }) .option('first-only', { describe: @@ -83,6 +84,14 @@ if (require.main === module) { type: 'boolean', default: false, }) + .option('error-on-failed-entry-point', { + describe: + 'Set this option in order to terminate immediately with an error code if an entry-point fails to be processed.\n' + + 'If `-t`/`--target` is provided then this property is always true and cannot be changed. Otherwise the default is false.\n' + + 'When set to false, ngcc will continue to process entry-points after a failure. In which case it will log an error and resume processing other entry-points.', + type: 'boolean', + default: false, + }) .strict() .help() .parse(args); @@ -103,6 +112,7 @@ if (require.main === module) { const logLevel = options['l'] as keyof typeof LogLevel | undefined; const enableI18nLegacyMessageIdFormat = options['legacy-message-ids']; const invalidateEntryPointManifest = options['invalidate-entry-point-manifest']; + const errorOnFailedEntryPoint = options['error-on-failed-entry-point']; (async() => { try { @@ -116,7 +126,7 @@ if (require.main === module) { createNewEntryPointFormats, logger, enableI18nLegacyMessageIdFormat, - async: options['async'], invalidateEntryPointManifest, + async: options['async'], invalidateEntryPointManifest, errorOnFailedEntryPoint, }); if (logger) { diff --git a/packages/compiler-cli/ngcc/src/execution/tasks/api.ts b/packages/compiler-cli/ngcc/src/execution/tasks/api.ts index 60ec3719db..ed2fd2165b 100644 --- a/packages/compiler-cli/ngcc/src/execution/tasks/api.ts +++ b/packages/compiler-cli/ngcc/src/execution/tasks/api.ts @@ -110,6 +110,13 @@ export interface TaskQueue { */ markTaskCompleted(task: Task): void; + /** + * Mark a task as failed. + * + * Do not process the tasks that depend upon the given task. + */ + markAsFailed(task: Task): void; + /** * Return a string representation of the task queue (for debugging purposes). * diff --git a/packages/compiler-cli/ngcc/src/execution/tasks/completion.ts b/packages/compiler-cli/ngcc/src/execution/tasks/completion.ts index 4f6a060a3d..5a8111fa9a 100644 --- a/packages/compiler-cli/ngcc/src/execution/tasks/completion.ts +++ b/packages/compiler-cli/ngcc/src/execution/tasks/completion.ts @@ -6,10 +6,11 @@ * found in the LICENSE file at https://angular.io/license */ import {FileSystem, resolve} from '../../../../src/ngtsc/file_system'; +import {Logger} from '../../logging/logger'; import {markAsProcessed} from '../../packages/build_marker'; import {PackageJsonFormatProperties, getEntryPointFormat} from '../../packages/entry_point'; import {PackageJsonUpdater} from '../../writing/package_json_updater'; -import {Task, TaskCompletedCallback, TaskProcessingOutcome} from './api'; +import {Task, TaskCompletedCallback, TaskProcessingOutcome, TaskQueue} from './api'; /** * A function that can handle a specific outcome of a task completion. @@ -70,3 +71,17 @@ export function createThrowErrorHandler(fs: FileSystem): TaskCompletedHandler { (message !== null ? ` due to ${message}` : '')); }; } + +/** + * Create a handler that logs an error and marks the task as failed. + */ +export function createLogErrorHandler( + logger: Logger, fs: FileSystem, taskQueue: TaskQueue): TaskCompletedHandler { + return (task: Task, message: string | null): void => { + taskQueue.markAsFailed(task); + const format = getEntryPointFormat(fs, task.entryPoint, task.formatProperty); + logger.error( + `Failed to compile entry-point ${task.entryPoint.name} (${task.formatProperty} as ${format})` + + (message !== null ? ` due to ${message}` : '')); + }; +} diff --git a/packages/compiler-cli/ngcc/src/execution/tasks/queues/base_task_queue.ts b/packages/compiler-cli/ngcc/src/execution/tasks/queues/base_task_queue.ts index 034fa70dbe..de1497fb66 100644 --- a/packages/compiler-cli/ngcc/src/execution/tasks/queues/base_task_queue.ts +++ b/packages/compiler-cli/ngcc/src/execution/tasks/queues/base_task_queue.ts @@ -5,7 +5,7 @@ * 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 {Logger} from '../../../logging/logger'; import {PartiallyOrderedTasks, Task, TaskDependencies, TaskQueue} from '../api'; import {stringifyTask} from '../utils'; @@ -19,9 +19,40 @@ export abstract class BaseTaskQueue implements TaskQueue { } protected inProgressTasks = new Set(); - constructor(protected tasks: PartiallyOrderedTasks, protected dependencies: TaskDependencies) {} + /** + * A map of tasks that should be skipped, mapped to the task that caused them to be skipped. + */ + private tasksToSkip = new Map(); - abstract getNextTask(): Task|null; + constructor( + protected logger: Logger, protected tasks: PartiallyOrderedTasks, + protected dependencies: TaskDependencies) {} + + protected abstract computeNextTask(): Task|null; + + getNextTask(): Task|null { + let nextTask = this.computeNextTask(); + while (nextTask !== null) { + if (!this.tasksToSkip.has(nextTask)) { + break; + } + // We are skipping this task so mark it as complete + this.markTaskCompleted(nextTask); + const failedTask = this.tasksToSkip.get(nextTask) !; + this.logger.warn( + `Skipping processing of ${nextTask.entryPoint.name} because its dependency ${failedTask.entryPoint.name} failed to compile.`); + nextTask = this.computeNextTask(); + } + return nextTask; + } + + markAsFailed(task: Task) { + if (this.dependencies.has(task)) { + for (const dependentTask of this.dependencies.get(task) !) { + this.skipDependentTasks(dependentTask, task); + } + } + } markTaskCompleted(task: Task): void { if (!this.inProgressTasks.has(task)) { @@ -41,6 +72,21 @@ export abstract class BaseTaskQueue implements TaskQueue { ` In-progress tasks (${inProgTasks.length}): ${this.stringifyTasks(inProgTasks, ' ')}`; } + /** + * Mark the given `task` as to be skipped, then recursive skip all its dependents. + * + * @param task The task to skip + * @param failedTask The task that failed, causing this task to be skipped + */ + protected skipDependentTasks(task: Task, failedTask: Task) { + this.tasksToSkip.set(task, failedTask); + if (this.dependencies.has(task)) { + for (const dependentTask of this.dependencies.get(task) !) { + this.skipDependentTasks(dependentTask, failedTask); + } + } + } + protected stringifyTasks(tasks: Task[], indentation: string): string { return tasks.map(task => `\n${indentation}- ${stringifyTask(task)}`).join(''); } diff --git a/packages/compiler-cli/ngcc/src/execution/tasks/queues/parallel_task_queue.ts b/packages/compiler-cli/ngcc/src/execution/tasks/queues/parallel_task_queue.ts index 5c6cb4d6e8..b7f7ef9fc2 100644 --- a/packages/compiler-cli/ngcc/src/execution/tasks/queues/parallel_task_queue.ts +++ b/packages/compiler-cli/ngcc/src/execution/tasks/queues/parallel_task_queue.ts @@ -5,6 +5,7 @@ * 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 {Logger} from '../../../logging/logger'; import {PartiallyOrderedTasks, Task, TaskDependencies} from '../api'; import {getBlockedTasks, sortTasksByPriority, stringifyTask} from '../utils'; import {BaseTaskQueue} from './base_task_queue'; @@ -21,12 +22,12 @@ export class ParallelTaskQueue extends BaseTaskQueue { */ private blockedTasks: Map>; - constructor(tasks: PartiallyOrderedTasks, dependents: TaskDependencies) { - super(sortTasksByPriority(tasks, dependents), dependents); - this.blockedTasks = getBlockedTasks(dependents); + constructor(logger: Logger, tasks: PartiallyOrderedTasks, dependencies: TaskDependencies) { + super(logger, sortTasksByPriority(tasks, dependencies), dependencies); + this.blockedTasks = getBlockedTasks(dependencies); } - getNextTask(): Task|null { + computeNextTask(): Task|null { // Look for the first available (i.e. not blocked) task. // (NOTE: Since tasks are sorted by priority, the first available one is the best choice.) const nextTaskIdx = this.tasks.findIndex(task => !this.blockedTasks.has(task)); diff --git a/packages/compiler-cli/ngcc/src/execution/tasks/queues/serial_task_queue.ts b/packages/compiler-cli/ngcc/src/execution/tasks/queues/serial_task_queue.ts index 2ffd6984a6..869f0aeb52 100644 --- a/packages/compiler-cli/ngcc/src/execution/tasks/queues/serial_task_queue.ts +++ b/packages/compiler-cli/ngcc/src/execution/tasks/queues/serial_task_queue.ts @@ -17,7 +17,7 @@ import {BaseTaskQueue} from './base_task_queue'; * before requesting the next one. */ export class SerialTaskQueue extends BaseTaskQueue { - getNextTask(): Task|null { + computeNextTask(): Task|null { const nextTask = this.tasks.shift() || null; if (nextTask) { diff --git a/packages/compiler-cli/ngcc/src/main.ts b/packages/compiler-cli/ngcc/src/main.ts index 01ff4baa8d..d332866d54 100644 --- a/packages/compiler-cli/ngcc/src/main.ts +++ b/packages/compiler-cli/ngcc/src/main.ts @@ -29,7 +29,7 @@ import {ClusterExecutor} from './execution/cluster/executor'; 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'; -import {composeTaskCompletedCallbacks, createMarkAsProcessedHandler, createThrowErrorHandler} from './execution/tasks/completion'; +import {composeTaskCompletedCallbacks, createLogErrorHandler, createMarkAsProcessedHandler, createThrowErrorHandler} from './execution/tasks/completion'; import {ParallelTaskQueue} from './execution/tasks/queues/parallel_task_queue'; import {SerialTaskQueue} from './execution/tasks/queues/serial_task_queue'; import {computeTaskDependencies} from './execution/tasks/utils'; @@ -63,6 +63,8 @@ export interface SyncNgccOptions { * `basePath`. * * All its dependencies will need to be processed too. + * + * If this property is provided then `errorOnFailedEntryPoint` is forced to true. */ targetEntryPointPath?: string; @@ -108,6 +110,18 @@ export interface SyncNgccOptions { */ async?: false; + /** + * Set to true in order to terminate immediately with an error code if an entry-point fails to be + * processed. + * + * If `targetEntryPointPath` is provided then this property is always true and cannot be + * changed. Otherwise the default is false. + * + * When set to false, ngcc will continue to process entry-points after a failure. In which case it + * will log an error and resume processing other entry-points. + */ + errorOnFailedEntryPoint?: boolean; + /** * Render `$localize` messages with legacy format ids. * @@ -155,8 +169,13 @@ export function mainNgcc({basePath, targetEntryPointPath, propertiesToConsider = SUPPORTED_FORMAT_PROPERTIES, compileAllFormats = true, createNewEntryPointFormats = false, logger = new ConsoleLogger(LogLevel.info), pathMappings, async = false, - enableI18nLegacyMessageIdFormat = true, + errorOnFailedEntryPoint = false, enableI18nLegacyMessageIdFormat = true, invalidateEntryPointManifest = false}: NgccOptions): void|Promise { + if (!!targetEntryPointPath) { + // targetEntryPointPath forces us to error if an entry-point fails. + errorOnFailedEntryPoint = true; + } + // Execute in parallel, if async execution is acceptable and there are more than 1 CPU cores. const inParallel = async && (os.cpus().length > 1); @@ -253,7 +272,7 @@ export function mainNgcc({basePath, targetEntryPointPath, `Analyzed ${entryPoints.length} entry-points in ${duration}s. ` + `(Total tasks: ${tasks.length})`); - return getTaskQueue(inParallel, tasks, graph); + return getTaskQueue(logger, inParallel, tasks, graph); }; // The function for creating the `compile()` function. @@ -294,20 +313,21 @@ export function mainNgcc({basePath, targetEntryPointPath, ts.formatDiagnosticsWithColorAndContext(result.diagnostics, bundle.src.host))); } fileWriter.writeBundle(bundle, result.transformedFiles, formatPropertiesToMarkAsProcessed); + + logger.debug(` Successfully compiled ${entryPoint.name} : ${formatProperty}`); + + onTaskCompleted(task, TaskProcessingOutcome.Processed, null); } else { const errors = replaceTsWithNgInErrors( ts.formatDiagnosticsWithColorAndContext(result.diagnostics, bundle.src.host)); onTaskCompleted(task, TaskProcessingOutcome.Failed, `compilation errors:\n${errors}`); } - - logger.debug(` Successfully compiled ${entryPoint.name} : ${formatProperty}`); - - onTaskCompleted(task, TaskProcessingOutcome.Processed, null); }; }; // The executor for actually planning and getting the work done. - const createTaskCompletedCallback = getCreateTaskCompletedCallback(pkgJsonUpdater, fileSystem); + const createTaskCompletedCallback = + getCreateTaskCompletedCallback(pkgJsonUpdater, errorOnFailedEntryPoint, logger, fileSystem); const executor = getExecutor( async, inParallel, logger, pkgJsonUpdater, fileSystem, createTaskCompletedCallback); @@ -349,17 +369,21 @@ function getFileWriter( } function getTaskQueue( - inParallel: boolean, tasks: PartiallyOrderedTasks, graph: DepGraph): TaskQueue { + logger: Logger, inParallel: boolean, tasks: PartiallyOrderedTasks, + graph: DepGraph): TaskQueue { const dependencies = computeTaskDependencies(tasks, graph); - return inParallel ? new ParallelTaskQueue(tasks, dependencies) : - new SerialTaskQueue(tasks, dependencies); + return inParallel ? new ParallelTaskQueue(logger, tasks, dependencies) : + new SerialTaskQueue(logger, tasks, dependencies); } function getCreateTaskCompletedCallback( - pkgJsonUpdater: PackageJsonUpdater, fileSystem: FileSystem): CreateTaskCompletedCallback { - return _taskQueue => composeTaskCompletedCallbacks({ + pkgJsonUpdater: PackageJsonUpdater, errorOnFailedEntryPoint: boolean, logger: Logger, + fileSystem: FileSystem): CreateTaskCompletedCallback { + return taskQueue => composeTaskCompletedCallbacks({ [TaskProcessingOutcome.Processed]: createMarkAsProcessedHandler(pkgJsonUpdater), - [TaskProcessingOutcome.Failed]: createThrowErrorHandler(fileSystem), + [TaskProcessingOutcome.Failed]: + errorOnFailedEntryPoint ? createThrowErrorHandler(fileSystem) : + createLogErrorHandler(logger, fileSystem, taskQueue), }); } diff --git a/packages/compiler-cli/ngcc/test/execution/tasks/queues/parallel_task_queue_spec.ts b/packages/compiler-cli/ngcc/test/execution/tasks/queues/parallel_task_queue_spec.ts index 7a33b10e24..03d196fe37 100644 --- a/packages/compiler-cli/ngcc/test/execution/tasks/queues/parallel_task_queue_spec.ts +++ b/packages/compiler-cli/ngcc/test/execution/tasks/queues/parallel_task_queue_spec.ts @@ -9,6 +9,7 @@ import {PartiallyOrderedTasks, TaskQueue} from '../../../../src/execution/tasks/api'; import {ParallelTaskQueue} from '../../../../src/execution/tasks/queues/parallel_task_queue'; import {computeTaskDependencies} from '../../../../src/execution/tasks/utils'; +import {MockLogger} from '../../../helpers/mock_logger'; import {createTasksAndGraph} from '../../helpers'; describe('ParallelTaskQueue', () => { @@ -36,7 +37,7 @@ describe('ParallelTaskQueue', () => { const {tasks, graph} = createTasksAndGraph(entryPointCount, tasksPerEntryPointCount, entryPointDeps); const dependencies = computeTaskDependencies(tasks, graph); - return {tasks, queue: new ParallelTaskQueue(tasks.slice(), dependencies)}; + return {tasks, queue: new ParallelTaskQueue(new MockLogger(), tasks.slice(), dependencies)}; } /** diff --git a/packages/compiler-cli/ngcc/test/execution/tasks/queues/serial_task_queue_spec.ts b/packages/compiler-cli/ngcc/test/execution/tasks/queues/serial_task_queue_spec.ts index 02fd212438..ea53af115e 100644 --- a/packages/compiler-cli/ngcc/test/execution/tasks/queues/serial_task_queue_spec.ts +++ b/packages/compiler-cli/ngcc/test/execution/tasks/queues/serial_task_queue_spec.ts @@ -11,6 +11,7 @@ import {PartiallyOrderedTasks, Task, TaskQueue} from '../../../../src/execution/ import {SerialTaskQueue} from '../../../../src/execution/tasks/queues/serial_task_queue'; import {computeTaskDependencies} from '../../../../src/execution/tasks/utils'; import {EntryPoint} from '../../../../src/packages/entry_point'; +import {MockLogger} from '../../../helpers/mock_logger'; describe('SerialTaskQueue', () => { @@ -38,7 +39,7 @@ describe('SerialTaskQueue', () => { graph.addNode(entryPoint.path); } const dependencies = computeTaskDependencies(tasks, graph); - return {tasks, queue: new SerialTaskQueue(tasks.slice(), dependencies)}; + return {tasks, queue: new SerialTaskQueue(new MockLogger(), tasks.slice(), dependencies)}; }; /** diff --git a/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts b/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts index fe44a49b46..888c854415 100644 --- a/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts +++ b/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts @@ -1085,45 +1085,130 @@ runInEachFileSystem(() => { }); describe('diagnostics', () => { - it('should fail with formatted diagnostics when an error diagnostic is produced', () => { - loadTestFiles([ - { - name: _('/node_modules/fatal-error/package.json'), - contents: '{"name": "fatal-error", "es2015": "./index.js", "typings": "./index.d.ts"}', - }, - {name: _('/node_modules/fatal-error/index.metadata.json'), contents: 'DUMMY DATA'}, - { - name: _('/node_modules/fatal-error/index.js'), - contents: ` + it('should fail with formatted diagnostics when an error diagnostic is produced, if targetEntryPointPath is provided', + () => { + loadTestFiles([ + { + name: _('/node_modules/fatal-error/package.json'), + contents: + '{"name": "fatal-error", "es2015": "./index.js", "typings": "./index.d.ts"}', + }, + {name: _('/node_modules/fatal-error/index.metadata.json'), contents: 'DUMMY DATA'}, + { + name: _('/node_modules/fatal-error/index.js'), + contents: ` import {Component} from '@angular/core'; export class FatalError {} FatalError.decorators = [ {type: Component, args: [{selector: 'fatal-error'}]} ]; `, - }, - { - name: _('/node_modules/fatal-error/index.d.ts'), - contents: ` + }, + { + name: _('/node_modules/fatal-error/index.d.ts'), + contents: ` export declare class FatalError {} `, - }, - ]); + }, + ]); - try { - mainNgcc({ - basePath: '/node_modules', - targetEntryPointPath: 'fatal-error', - propertiesToConsider: ['es2015'] - }); - fail('should have thrown'); - } catch (e) { - expect(e.message).toContain( - 'Failed to compile entry-point fatal-error (es2015 as esm2015) due to compilation errors:'); - expect(e.message).toContain('NG2001'); - expect(e.message).toContain('component is missing a template'); - } - }); + try { + mainNgcc({ + basePath: '/node_modules', + targetEntryPointPath: 'fatal-error', + propertiesToConsider: ['es2015'] + }); + fail('should have thrown'); + } catch (e) { + expect(e.message).toContain( + 'Failed to compile entry-point fatal-error (es2015 as esm2015) due to compilation errors:'); + expect(e.message).toContain('NG2001'); + expect(e.message).toContain('component is missing a template'); + } + }); + + it('should not fail but log an error with formatted diagnostics when an error diagnostic is produced, if targetEntryPoint is not provided and errorOnFailedEntryPoint is false', + () => { + loadTestFiles([ + { + name: _('/node_modules/fatal-error/package.json'), + contents: + '{"name": "fatal-error", "es2015": "./index.js", "typings": "./index.d.ts"}', + }, + {name: _('/node_modules/fatal-error/index.metadata.json'), contents: 'DUMMY DATA'}, + { + name: _('/node_modules/fatal-error/index.js'), + contents: ` + import {Component} from '@angular/core'; + export class FatalError {} + FatalError.decorators = [ + {type: Component, args: [{selector: 'fatal-error'}]} + ];`, + }, + { + name: _('/node_modules/fatal-error/index.d.ts'), + contents: `export declare class FatalError {}`, + }, + { + name: _('/node_modules/dependent/package.json'), + contents: '{"name": "dependent", "es2015": "./index.js", "typings": "./index.d.ts"}', + }, + {name: _('/node_modules/dependent/index.metadata.json'), contents: 'DUMMY DATA'}, + { + name: _('/node_modules/dependent/index.js'), + contents: ` + import {Component} from '@angular/core'; + import {FatalError} from 'fatal-error'; + export class Dependent {} + Dependent.decorators = [ + {type: Component, args: [{selector: 'dependent', template: ''}]} + ];`, + }, + { + name: _('/node_modules/dependent/index.d.ts'), + contents: `export declare class Dependent {}`, + }, + { + name: _('/node_modules/independent/package.json'), + contents: + '{"name": "independent", "es2015": "./index.js", "typings": "./index.d.ts"}', + }, + {name: _('/node_modules/independent/index.metadata.json'), contents: 'DUMMY DATA'}, + { + name: _('/node_modules/independent/index.js'), + contents: ` + import {Component} from '@angular/core'; + export class Independent {} + Independent.decorators = [ + {type: Component, args: [{selector: 'independent', template: ''}]} + ];`, + }, + { + name: _('/node_modules/independent/index.d.ts'), + contents: `export declare class Independent {}`, + }, + ]); + + const logger = new MockLogger(); + mainNgcc({ + basePath: '/node_modules', + propertiesToConsider: ['es2015'], + errorOnFailedEntryPoint: false, logger, + }); + expect(logger.logs.error.length).toEqual(1); + const message = logger.logs.error[0][0]; + expect(message).toContain( + 'Failed to compile entry-point fatal-error (es2015 as esm2015) due to compilation errors:'); + expect(message).toContain('NG2001'); + expect(message).toContain('component is missing a template'); + + expect(hasBeenProcessed(loadPackage('fatal-error', _('/node_modules')), 'es2015')) + .toBe(false); + expect(hasBeenProcessed(loadPackage('dependent', _('/node_modules')), 'es2015')) + .toBe(false); + expect(hasBeenProcessed(loadPackage('independent', _('/node_modules')), 'es2015')) + .toBe(true); + }); }); describe('logger', () => {