From 97ef37136359588d8dcb3f3dc05d2c271c90089a Mon Sep 17 00:00:00 2001 From: Andrei Alecu Date: Tue, 18 Aug 2020 13:12:34 +0300 Subject: [PATCH] fix(cli): "fancy" progress reporting not disabled on all CI systems (#9516) Fixes: #8696 #8893 Detects whether the `CI` environment variable is set and reverts output to `standard` behavior. The fancy output was especially broken on CircleCI. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- packages/aws-cdk/bin/cdk.ts | 3 ++- .../aws-cdk/lib/api/cloudformation-deployments.ts | 8 ++++++++ packages/aws-cdk/lib/api/deploy-stack.ts | 7 +++++++ .../util/cloudformation/stack-activity-monitor.ts | 14 +++++++++++++- packages/aws-cdk/lib/cdk-toolkit.ts | 8 ++++++++ 5 files changed, 38 insertions(+), 2 deletions(-) diff --git a/packages/aws-cdk/bin/cdk.ts b/packages/aws-cdk/bin/cdk.ts index 0b615b379d4f1..016a0855494be 100644 --- a/packages/aws-cdk/bin/cdk.ts +++ b/packages/aws-cdk/bin/cdk.ts @@ -82,7 +82,7 @@ async function parseCommandLineArguments() { .option('build-exclude', { type: 'array', alias: 'E', nargs: 1, desc: 'Do not rebuild asset with the given ID. Can be specified multiple times.', default: [] }) .option('exclusively', { type: 'boolean', alias: 'e', desc: 'Only deploy requested stacks, don\'t include dependencies' }) .option('require-approval', { type: 'string', choices: [RequireApproval.Never, RequireApproval.AnyChange, RequireApproval.Broadening], desc: 'What security-sensitive changes need manual approval' }) - .option('ci', { type: 'boolean', desc: 'Force CI detection (deprecated)', default: process.env.CI !== undefined }) + .option('ci', { type: 'boolean', desc: 'Force CI detection', default: process.env.CI !== undefined }) .option('notification-arns', { type: 'array', desc: 'ARNs of SNS topics that CloudFormation will notify with stack related events', nargs: 1, requiresArg: true }) .option('tags', { type: 'array', alias: 't', desc: 'Tags to add to the stack (KEY=VALUE)', nargs: 1, requiresArg: true }) .option('execute', { type: 'boolean', desc: 'Whether to execute ChangeSet (--no-execute will NOT execute the ChangeSet)', default: true }) @@ -279,6 +279,7 @@ async function initCommandLine() { parameters: parameterMap, usePreviousParameters: args['previous-parameters'], outputsFile: args.outputsFile, + ci: args.ci, }); case 'destroy': diff --git a/packages/aws-cdk/lib/api/cloudformation-deployments.ts b/packages/aws-cdk/lib/api/cloudformation-deployments.ts index 0ee09812629c9..dac91e4b9e5e6 100644 --- a/packages/aws-cdk/lib/api/cloudformation-deployments.ts +++ b/packages/aws-cdk/lib/api/cloudformation-deployments.ts @@ -88,6 +88,13 @@ export interface DeployStackOptions { * @default true */ usePreviousParameters?: boolean; + + /** + * Whether we are on a CI system + * + * @default false + */ + readonly ci?: boolean; } export interface DestroyStackOptions { @@ -156,6 +163,7 @@ export class CloudFormationDeployments { force: options.force, parameters: options.parameters, usePreviousParameters: options.usePreviousParameters, + ci: options.ci, }); } diff --git a/packages/aws-cdk/lib/api/deploy-stack.ts b/packages/aws-cdk/lib/api/deploy-stack.ts index f2f2c267fc586..454ff8452b566 100644 --- a/packages/aws-cdk/lib/api/deploy-stack.ts +++ b/packages/aws-cdk/lib/api/deploy-stack.ts @@ -158,6 +158,13 @@ export interface DeployStackOptions { * @default false */ force?: boolean; + + /** + * Whether we are on a CI system + * + * @default false + */ + readonly ci?: boolean; } const LARGE_TEMPLATE_SIZE_KB = 50; diff --git a/packages/aws-cdk/lib/api/util/cloudformation/stack-activity-monitor.ts b/packages/aws-cdk/lib/api/util/cloudformation/stack-activity-monitor.ts index af192ec162e49..9f4c1415a4989 100644 --- a/packages/aws-cdk/lib/api/util/cloudformation/stack-activity-monitor.ts +++ b/packages/aws-cdk/lib/api/util/cloudformation/stack-activity-monitor.ts @@ -35,6 +35,15 @@ export interface StackActivityMonitorProps { * @default - Use value from logging.logLevel */ readonly logLevel?: LogLevel; + + /** + * Whether we are on a CI system + * + * If so, disable the "optimized" stack monitor. + * + * @default false + */ + readonly ci?: boolean; } export class StackActivityMonitor { @@ -79,7 +88,10 @@ export class StackActivityMonitor { const isWindows = process.platform === 'win32'; const verbose = options.logLevel ?? logLevel; - const fancyOutputAvailable = !isWindows && stream.isTTY; + // On some CI systems (such as CircleCI) output still reports as a TTY so we also + // need an individual check for whether we're running on CI. + // see: https://discuss.circleci.com/t/circleci-terminal-is-a-tty-but-term-is-not-set/9965 + const fancyOutputAvailable = !isWindows && stream.isTTY && !options.ci; this.printer = fancyOutputAvailable && !verbose ? new CurrentActivityPrinter(props) diff --git a/packages/aws-cdk/lib/cdk-toolkit.ts b/packages/aws-cdk/lib/cdk-toolkit.ts index e9da7e14eea15..3aec3c4eeef18 100644 --- a/packages/aws-cdk/lib/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cdk-toolkit.ts @@ -190,6 +190,7 @@ export class CdkToolkit { force: options.force, parameters: Object.assign({}, parameterMap['*'], parameterMap[stack.stackName]), usePreviousParameters: options.usePreviousParameters, + ci: options.ci, }); const message = result.noOp @@ -583,6 +584,13 @@ export interface DeployOptions { * @default - Outputs are not written to any file */ outputsFile?: string; + + /** + * Whether we are on a CI system + * + * @default false + */ + readonly ci?: boolean; } export interface DestroyOptions {