fix(cli): large context causes E2BIG error during synthesis on Linux #21230

otaviomacedo · 2022-07-19T12:59:20Z

Instead of passing the context in an environment variable, the CLI now writes the context to a temporary file and sets an environment variable only with the location. The app then uses that location to read from the file.

Also tested manually on a Linux machine.

Fixes #19261.

All Submissions:

Have you followed the guidelines in our Contributing guide?

Adding new Unconventional Dependencies:

This PR adds new unconventional dependencies following the process described here

New Features

Have you added the new feature to an integration test?
- Did you use yarn integ to deploy the infrastructure and generate the snapshot (i.e. yarn integ without --dry-run)?

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

…text from the CLI to the app

gitpod-io · 2022-07-19T12:59:28Z

rix0rrr

Provisional approval

rix0rrr · 2022-07-22T15:50:01Z

packages/@aws-cdk/core/lib/app.ts

+
+  private readContextFromTempFile() {
+    const location = process.env[cxapi.CONTEXT_LOCATION_ENV];
+    return location != null ? fs.readJSONSync(location) : null;


We generally avoid using null, preferring undefined instead.

rix0rrr · 2022-07-22T15:50:07Z

packages/@aws-cdk/core/lib/app.ts

+  // for backward compatibility with old versions of the CLI
+  private readContextFromEnvironment() {
+    const contextJson = process.env[cxapi.CONTEXT_ENV];
+    return contextJson ? JSON.parse(contextJson) : null;


rix0rrr · 2022-07-22T15:50:32Z

packages/aws-cdk/lib/api/cxapp/exec.ts

@@ -44,7 +45,11 @@ export async function execProgram(aws: SdkProvider, config: Configuration): Prom
  context[cxapi.BUNDLING_STACKS] = bundlingStacks;

  debug('context:', context);
-  env[cxapi.CONTEXT_ENV] = JSON.stringify(context);
+
+  const contextDir = await fs.mkdtemp(path.join(os.tmpdir(), 'cdk-context'));


Do we clean this directory up anywhere?

mergify · 2022-07-27T10:23:13Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

aws-cdk-automation · 2022-07-27T11:00:31Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: c10bd0a
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2022-07-27T11:00:48Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

…n Linux (#21230)" This reverts commit 6df82b7, as it appears to cause regression test failures.

RomainMuller · 2022-07-27T13:23:25Z

This PR was reverted due to emergent regression test failures apparently related to it. Discussed with @otaviomacedo & provided support to re-create.

Instead of passing the context in an environment variable, the CLI now writes the context to a temporary file and sets an environment variable only with the location. The app then uses that location to read from the file. Also tested manually on a Linux machine. Re-roll #21230 Fixes #19261

…ws#21230) Instead of passing the context in an environment variable, the CLI now writes the context to a temporary file and sets an environment variable only with the location. The app then uses that location to read from the file. Also tested manually on a Linux machine. Fixes aws#19261. ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

…n Linux (aws#21230)" This reverts commit 6df82b7, as it appears to cause regression test failures.

…ws#21230) Instead of passing the context in an environment variable, the CLI now writes the context to a temporary file and sets an environment variable only with the location. The app then uses that location to read from the file. Also tested manually on a Linux machine. Fixes aws#19261. ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

…n Linux (aws#21230)" This reverts commit 6df82b7, as it appears to cause regression test failures.

…ws#21230) Instead of passing the context in an environment variable, the CLI now writes the context to a temporary file and sets an environment variable only with the location. The app then uses that location to read from the file. Also tested manually on a Linux machine. Fixes aws#19261. ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

…n Linux (aws#21230)" This reverts commit 6df82b7, as it appears to cause regression test failures.

…ws#21230) Instead of passing the context in an environment variable, the CLI now writes the context to a temporary file and sets an environment variable only with the location. The app then uses that location to read from the file. Also tested manually on a Linux machine. Fixes aws#19261. ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

…n Linux (aws#21230)" This reverts commit 6df82b7, as it appears to cause regression test failures.

…21373) Linux systems don't support environment variables larger than 128KiB. This change splits the context into two if it's larger than that and stores the overflow into a temporary file. The application then re-constructs the original context from these two sources. A special case is when this new version of the CLI is used to synthesize an application that depends on an old version of the framework. The application will still consume part of the context, but the CLI warns the user that some of it has been lost. Since the tree manipulation logic is basically the same as the one used for displaying notices, it was extracted into its own file. Re-roll #21230 Fixes #19261 ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* Instead of passing the context in an environment variable, the CLI now writes the context to a temporary file and sets an environment variable only with the location. The app then uses that location to read from the file. Also tested manually on a Linux machine. Re-roll #21230 Fixes #19261

…ws#21373) Linux systems don't support environment variables larger than 128KiB. This change splits the context into two if it's larger than that and stores the overflow into a temporary file. The application then re-constructs the original context from these two sources. A special case is when this new version of the CLI is used to synthesize an application that depends on an old version of the framework. The application will still consume part of the context, but the CLI warns the user that some of it has been lost. Since the tree manipulation logic is basically the same as the one used for displaying notices, it was extracted into its own file. Re-roll aws#21230 Fixes aws#19261 ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* Instead of passing the context in an environment variable, the CLI now writes the context to a temporary file and sets an environment variable only with the location. The app then uses that location to read from the file. Also tested manually on a Linux machine. Re-roll aws#21230 Fixes aws#19261

otaviomacedo added 3 commits July 19, 2022 09:50

Using a temp file instead of the environment variable to send the con…

142455b

…text from the CLI to the app

Not setting cxapi.CONTEXT_ENV anymore

db47bcf

Reading the context properly

4eba273

aws-cdk-automation requested a review from a team July 19, 2022 12:59

github-actions bot added bug This issue is a bug. effort/medium Medium work item – several days of effort p1 labels Jul 19, 2022

mergify bot added the contribution/core This is a PR that came from AWS. label Jul 19, 2022

otaviomacedo added 9 commits July 20, 2022 12:12

Updated integration tests

d7dd451

Merge branch 'main' into otaviom/context-temp-file

2a6df0f

Updated integ tests

04c0554

Update snapshot

67682d2

Updated athena snapshot

69f8b89

Updated servicecatalogappregistry snapshots

0cbc423

Reverted templates

bd10620

Falling back to the env variable when the location is undefined

5ad9f53

More tests

56d02d3

rix0rrr added the pr/do-not-merge This PR should not be merged at this time. label Jul 22, 2022

rix0rrr approved these changes Jul 22, 2022

View reviewed changes

otaviomacedo added 2 commits July 26, 2022 19:05

Cleaning up temp directory

d463647

dirname

7c73e7a

otaviomacedo removed the pr/do-not-merge This PR should not be merged at this time. label Jul 27, 2022

Merge branch 'main' into otaviom/context-temp-file

c10bd0a

mergify bot merged commit 6df82b7 into main Jul 27, 2022

mergify bot deleted the otaviom/context-temp-file branch July 27, 2022 11:00

RomainMuller added a commit that referenced this pull request Jul 27, 2022

Revert "fix(cli): large context causes E2BIG error during synthesis o…

d7aa193

…n Linux (#21230)" This reverts commit 6df82b7, as it appears to cause regression test failures.

RomainMuller restored the otaviom/context-temp-file branch July 27, 2022 13:20

mrgrain pushed a commit to mrgrain/aws-cdk that referenced this pull request Jul 27, 2022

Revert "fix(cli): large context causes E2BIG error during synthesis o…

df313e3

…n Linux (aws#21230)" This reverts commit 6df82b7, as it appears to cause regression test failures.

mrgrain pushed a commit to mrgrain/aws-cdk that referenced this pull request Jul 28, 2022

Revert "fix(cli): large context causes E2BIG error during synthesis o…

0ddd5f4

…n Linux (aws#21230)" This reverts commit 6df82b7, as it appears to cause regression test failures.

mrgrain pushed a commit to mrgrain/aws-cdk that referenced this pull request Jul 28, 2022

Revert "fix(cli): large context causes E2BIG error during synthesis o…

daa66c7

…n Linux (aws#21230)" This reverts commit 6df82b7, as it appears to cause regression test failures.

otaviomacedo mentioned this pull request Jul 29, 2022

fix(cli): large context causes E2BIG error during synthesis on Linux #21373

Merged

4 tasks

josephedward pushed a commit to josephedward/aws-cdk that referenced this pull request Aug 30, 2022

Revert "fix(cli): large context causes E2BIG error during synthesis o…

af39635

…n Linux (aws#21230)" This reverts commit 6df82b7, as it appears to cause regression test failures.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(cli): large context causes E2BIG error during synthesis on Linux #21230

fix(cli): large context causes E2BIG error during synthesis on Linux #21230

otaviomacedo commented Jul 19, 2022 •

edited

Loading

gitpod-io bot commented Jul 19, 2022

rix0rrr left a comment

rix0rrr Jul 22, 2022

rix0rrr Jul 22, 2022

rix0rrr Jul 22, 2022

mergify bot commented Jul 27, 2022

aws-cdk-automation commented Jul 27, 2022

mergify bot commented Jul 27, 2022

RomainMuller commented Jul 27, 2022

fix(cli): large context causes E2BIG error during synthesis on Linux #21230

fix(cli): large context causes E2BIG error during synthesis on Linux #21230

Conversation

otaviomacedo commented Jul 19, 2022 • edited Loading

All Submissions:

Adding new Unconventional Dependencies:

New Features

gitpod-io bot commented Jul 19, 2022

rix0rrr left a comment

Choose a reason for hiding this comment

rix0rrr Jul 22, 2022

Choose a reason for hiding this comment

rix0rrr Jul 22, 2022

Choose a reason for hiding this comment

rix0rrr Jul 22, 2022

Choose a reason for hiding this comment

mergify bot commented Jul 27, 2022

aws-cdk-automation commented Jul 27, 2022

AWS CodeBuild CI Report

mergify bot commented Jul 27, 2022

RomainMuller commented Jul 27, 2022

otaviomacedo commented Jul 19, 2022 •

edited

Loading