[WIP] chore: clarify build dependencies for future hermetic builds #10039

rix0rrr · 2020-08-28T11:56:59Z

This PR is in preparation of future changes. It cleans up various parts of our build process that made it impossible to build packages individually, including:

Dependencies on specific source tree layout (relative directories between packages)
node_modules dependencies that were used but not declared, and happened to work because they were hoisted by yarn (not the runtime ones, because we have an eslint rule for that, but mostly around command-line tools and typings).

This is all in preparation for a potential future new build tool that can build in isolation and can aggressively cache artifacts (locally and in S3). You can try it today:

$ yarn install --frozen-lockfile
$ npx nozem from-lerna
$ npx nozem build

New concepts introduced as part of this change: I've added some annotations into the package.jsons that are needed by nozem to do a proper hermetic build. Notably: tell it what type of OS tools are necessary in order to execute the build (sed/grep/awk/docker/...).

The build tool is not production-quality yet and this is not a bid to switch over to it. However, it's starting to be onerous to maintain these changes in a separate branch, so I'd like to start merging them in.

Not everything builds yet but we have to start somewhere.

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

Remove `eslintrc` references to outside directories, consolidate `.gitignores`. Annotate package.jsons with a new field "ostools" which the new build tool needs to build a properly hermetic build environment. Replace `require('../.....')` in integ-helpers. Add undeclared `@types` dependencies missing in various `package.json`s. Turn straggling eslint.json into an eslintrc.js. Stop relying on ambient import of `@types/aws-lambda`, it doesn't play well with downstream packages. Make it an explicit import. Rewrite cloudformation-include/decdk/monocdk codegen scripts to be out-of-source-build aware.

jogold · 2020-08-28T12:32:24Z

packages/awslint/.eslintrc.js

-const baseConfig = require('cdk-build-tools/config/eslintrc');
-baseConfig.parserOptions.project = __dirname + '/tsconfig.json';
-module.exports = baseConfig;
+// This cannot reference the build rules from cdk-build-tools as this


The content here is not equivalent to the imported eslintrc.js anymore (the style part has evolved). I understand the circular dependency issue here, how about moving the /config part of cdk-build-tools to a separate package cdk-build-tools-config?

jogold · 2020-08-28T12:36:51Z

can aggressively cache artifacts (locally and in S3)

@rix0rrr does this mean that I will be able to "sync/download" those artifacts to my local dev env from a public S3 (updated by the CI)?

rix0rrr · 2020-08-28T13:41:46Z

@rix0rrr does this mean that I will be able to "sync/download" those artifacts to my local dev env from a public S3 (updated by the CI)?

That's the idea, yes.

eladb · 2020-08-28T15:01:08Z

packages/@aws-cdk/aws-lambda-nodejs/test/function.test.ts

@@ -7,6 +7,10 @@ import { Stack } from '@aws-cdk/core';
 import { NodejsFunction } from '../lib';
 import { Bundling } from '../lib/bundling';

+const PROJECT_ROOT = process.env.NZM_SOURCE_PACKAGE


Is there a way to avoid taking this dependency on the nozem
Environment?

I don't think so. I'll be honest with you, the aws-lambda-nodejs package is the bane of my existence here.

In this case, this exists to tell it where the .git directory of the source repository is. I don't know why it needs to know that, but right now I'm just telling it in an attempt to get the build to pass.

We lookup the project root because this is what we mount as /asset-input to be sure to have access to all node modules and dependencies when bundling in the container. Would a fallback on looking for a lockfile (yarn.lock, package-lock.json) improve things here?

I feel auto discovery of the project root is a source of confusion and odd issues for many users. Maybe we should just make that explicitly required?

I think it's nice to have it auto discovered.

What about this?

// Find project root const projectRoot = options.projectRoot ?? findUp(`.git${path.sep}`) ?? findUp('yarn.lock') ?? findUp('package-lock.json') ?? findUp('package.json');

here

aws-cdk/packages/@aws-cdk/aws-lambda-nodejs/lib/bundling.ts

Lines 117 to 121 in 9ffb268

// Find project root

const projectRoot = options.projectRoot ?? findUp(`.git${path.sep}`);

if (!projectRoot) {

throw new Error('Cannot find project root. Please specify it with `projectRoot`.');

}

Using a lockfile in this case also doesn't help me. I'm trying to do an isolated build of the package. There's a yarn.lock somewhere but you shouldn't care about that.

My build script has set up the build environment exactly as Node-based tools should expect it (node_modules in the right place and everything). There's no yarn.lock or package-lock.json in sight.

But there's a package.json right? so it should work.

eladb · 2020-08-28T15:01:57Z

packages/@aws-cdk/aws-lambda-nodejs/test/integ.function.ts

@@ -7,14 +7,23 @@ class TestStack extends Stack {
  constructor(scope: Construct, id: string, props?: StackProps) {
    super(scope, id, props);

+    const projectRoot = process.env.NZM_PACKAGE_SOURCE


Feels odd that we will need to do that everywhere

That's the least odd part about it. If we need to do it once we need to do it everywhere :)

rix0rrr · 2020-08-28T15:44:06Z

@jogold is the regular aws-lambda-nodejs build supposed to use a Docker build or a Local build?

Right now I'm seeing this and I have no idea what I'm supposed to expect:

Incorrect parcel version detected: 3.5.1 <> 2.0.0-beta.1. Switching to Docker bundling.
...
Bundling asset cdk-integ-lambda-nodejs-dependencies/external/Code/Stage...
[Error: EACCES: permission denied, mkdir '/.parcel-cache'] {
  errno: -13,
  code: 'EACCES',
  syscall: 'mkdir',
  path: '/.parcel-cache'
}
/Users/huijbers/.nozem-build/cache/@aws-cdk-core/291b3a8cd1e341503e8980cda41d45d407b70912/out/lib/asset-staging.js:184
            throw new Error(`Failed to bundle asset ${this.node.path}, bundle output is located at ${bundleErrorDir}: ${err}`);
            ^

Error: Failed to bundle asset cdk-integ-lambda-nodejs-dependencies/external/Code/Stage, bundle output is located at /Users/huijbers/.nozem-build/build/@aws-cdk-aws-lambda-nodejs-test/src/test/cdk-integ.out/bundling-temp-RIj5yi-error: Error: docker exited with status 1
    at AssetStaging.bundle (/Users/huijbers/.nozem-build/cache/@aws-cdk-core/291b3a8cd1e341503e8980cda41d45d407b70912/out/lib/asset-staging.js:184:19)
    at new AssetStaging (/Users/huijbers/.nozem-build/cache/@aws-cdk-core/291b3a8cd1e341503e8980cda41d45d407b70912/out/lib/asset-staging.js:50:35)
    at new Asset (/Users/huijbers/.nozem-build/cache/@aws-cdk-aws-s3-assets/3a6aa69da06de975a7fbb195f59c78f6d2688446/out/lib/asset.js:21:25)
    at AssetCode.bind (/Users/huijbers/.nozem-build/cache/@aws-cdk-aws-lambda/83030f006ab86091081a646930e3cb29f394f3ec/out/lib/code.js:137:26)
    at new Function (/Users/huijbers/.nozem-build/cache/@aws-cdk-aws-lambda/83030f006ab86091081a646930e3cb29f394f3ec/out/lib/function.js:83:33)
    at new NodejsFunction (/Users/huijbers/.nozem-build/build/@aws-cdk-aws-lambda-nodejs-test/src/lib/function.js:30:13)
    at new TestStack (/Users/huijbers/.nozem-build/build/@aws-cdk-aws-lambda-nodejs-test/src/test/integ.dependencies.js:14:9)
    at Object.<anonymous> (/Users/huijbers/.nozem-build/build/@aws-cdk-aws-lambda-nodejs-test/src/test/integ.dependencies.js:28:1)
    at Module._compile (internal/modules/cjs/loader.js:956:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:973:10)

jogold · 2020-08-28T16:03:37Z

Does it work when not forcing Docker for integ.dependencies.js?

aws-cdk/packages/@aws-cdk/aws-lambda-nodejs/test/integ.dependencies.ts

Line 19 in 9ffb268

forceDockerBundling: true,

rix0rrr · 2020-08-28T16:16:04Z

@rix0rrr what is the value of process.env.NZM_SOURCE_PACKAGE and what type of folder is this?

It's /Users/huijbers/...ooohhsecret..../aws-cdk which is my repository root (where .git lives). Notably, this is different from where I'm currently running the build, which is /Users/huijbers/.nozem-build/build/@aws-cdk-aws-lambda-nodejs-test/src)

rix0rrr · 2020-08-28T16:20:27Z

A local bundling fails with

Error: Cannot find module '@babel/core' from '/Users/huijbers/.nozem-build/build/@aws-cdk-aws-lambda-nodejs-test/src/test/integ-handlers'

Not quite sure why parcel needs us to depend on @babel/core...

jogold · 2020-08-28T16:25:40Z

A local bundling fails with
Error: Cannot find module '@babel/core' from '/Users/huijbers/.nozem-build/build/@aws-cdk-aws-lambda-nodejs-test/src/test/integ-handlers'
Not quite sure why parcel needs us to depend on @babel/core...

parcel is a dev dependency of aws-lambda-nodejs not a dependency (and parcel needs @babel/core), are dev dependencies and their dependencies available in your isolated build?

It's a dev dependency to allow one of the integ test to run locally...

rix0rrr · 2020-08-28T16:31:59Z

parcel itself is available, and @babel/core is available as a dependency directly to parcel in the directory where it's located.

But that's apparently not what it's doing--it's searching for @babel/core from the directory of files that it's processing, not its own source directory.

rix0rrr · 2020-08-28T16:36:56Z

I guess I'll just add the dependency on @babel/core...

eladb · 2020-08-28T16:41:16Z

@jogold see https://github.com/eladb/projen/issues/90

jogold · 2020-08-28T17:30:22Z

I think that there is an issue when entry is not located in the auto discovered projectRoot which seems to be the case for you @rix0rrr, might come from this

aws-cdk/packages/@aws-cdk/aws-lambda-nodejs/lib/bundling.ts

Line 122 in af41534

    
           const relativeEntryPath = path.relative(projectRoot, path.resolve(options.entry));

then somehow / is mounted and then you have errors because you cannot create .parcel-cache in there + other problems when building locally.

Can you give me the tools to reproduce (nozem setup etc.) so I can try to fix this?

lambda

jogold · 2020-09-04T12:51:34Z

For this error (when Docker ran):

EACCES: permission denied, mkdir '/.parcel-cache'

The fix is here #10181 (would allow to revert the change on forceDockerBundling: true).

aws-cdk-automation · 2020-09-30T18:17:41Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: a2da71d
Result: FAILED
Build Logs (available for 30 days)

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

Enhance our `package.json`s with enough information to make hermetic, cacheable builds possible. Mostly consists of: * Adding missing `@types/*` packages to `devDepencendies` everywhere. Those dependencies were always required but undeclared. * Add `nozem.ostools` to various packages to indicate the non-hermetic binaries these scripts depend on. * Add `/// !cdk-integ pragma:ignore-assets` to all integ tests that use assets. * Fix some `.gitignores` that were ignoring files that were already committed inside git. * Update `pkglint` to not be reliant on its *own* location inside the monorepo, but instead search upwards for the monorepo root from the package it's checking. To give this a shot yourself, run: ``` $ npx nozem ``` This is a revival of #10039, which is now obsolete. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

…4341) Enhance our `package.json`s with enough information to make hermetic, cacheable builds possible. Mostly consists of: * Adding missing `@types/*` packages to `devDepencendies` everywhere. Those dependencies were always required but undeclared. * Add `nozem.ostools` to various packages to indicate the non-hermetic binaries these scripts depend on. * Add `/// !cdk-integ pragma:ignore-assets` to all integ tests that use assets. * Fix some `.gitignores` that were ignoring files that were already committed inside git. * Update `pkglint` to not be reliant on its *own* location inside the monorepo, but instead search upwards for the monorepo root from the package it's checking. To give this a shot yourself, run: ``` $ npx nozem ``` This is a revival of aws#10039, which is now obsolete. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

rix0rrr added 14 commits July 9, 2020 13:30

Merge remote-tracking branch 'origin/master' into huijbers/cleanup-build

f526503

Fix build errors

2bf1c29

Ignore nazel.json

e1f076d

Need to move junit-dep to top to make it correct

d0a4339

Fixing assumptions on source tree layout

1eaf9b2

Always chasing after lambda-nodejs

99c1b5c

Rewrite package.jsons to match rename nazel => nozem

1af9d2a

Rename NZL_PACKAGE_SOURCE => NZM_PACKAGE_SOURCE

ed96171

Merge remote-tracking branch 'origin/master' into huijbers/cleanup-build

1d4f5bc

Fixing eslint references

cc69d96

More renames to NZM

d770189

Add dependency

496661a

Merge remote-tracking branch 'origin/master' into huijbers/cleanup-build

34b56a4

rix0rrr requested a review from a team August 28, 2020 11:56

rix0rrr self-assigned this Aug 28, 2020

mergify bot added the contribution/core This is a PR that came from AWS. label Aug 28, 2020

rix0rrr changed the title ~~chore: cleanup build promiscuity~~ chore: clarify build dependencies for future hermetic builds Aug 28, 2020

Commit missing change

e4576b8

jogold reviewed Aug 28, 2020

View reviewed changes

rix0rrr added 2 commits August 28, 2020 15:27

Fix incorrect constructs version

05067a3

Some more broken references

3fbe588

rix0rrr added 2 commits August 28, 2020 16:03

Fix engine spec

7ee5c4e

Except canary source file from gitignore

c9f896e

eladb suggested changes Aug 28, 2020

View reviewed changes

rix0rrr added 2 commits August 28, 2020 17:03

make aws-lambda-python build

e8a75fc

Update aws-lambda expectations

8fe0f06

Make aws-lambda-nodejs build

cd05018

rix0rrr added 4 commits August 28, 2020 20:12

Fix jest.config.js reference

989da8e

Make lambda-python less stringent about its testing requirements

9f0d6c3

Make aws-certificatemanager not accidentally run the tests from the

98446f8

lambda

Update version

52de92d

rix0rrr added the pr/do-not-merge This PR should not be merged at this time. label Aug 31, 2020

Fix decdk build

69aed52

rix0rrr changed the title ~~chore: clarify build dependencies for future hermetic builds~~ [WIP] chore: clarify build dependencies for future hermetic builds Sep 8, 2020

rix0rrr added 6 commits September 30, 2020 17:19

Merge remote-tracking branch 'origin/master' into huijbers/cleanup-build

781b5f4

Don't disable pkglint

5cb1200

Build rule tweaks

704e65e

Watch using cdk-watch

6341603

NO longer need yaml

0dae350

Commmaaaaaa

a2da71d

rix0rrr mentioned this pull request Apr 23, 2021

chore: add enough information to make package builds cacheable #14341

Merged

rix0rrr closed this Apr 23, 2021

njlynch deleted the huijbers/cleanup-build branch October 19, 2021 14:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] chore: clarify build dependencies for future hermetic builds #10039

[WIP] chore: clarify build dependencies for future hermetic builds #10039

rix0rrr commented Aug 28, 2020

jogold Aug 28, 2020

jogold commented Aug 28, 2020

rix0rrr commented Aug 28, 2020

eladb Aug 28, 2020

rix0rrr Aug 28, 2020

jogold Aug 28, 2020 •

edited

Loading

eladb Aug 28, 2020

jogold Aug 28, 2020

rix0rrr Aug 28, 2020

jogold Aug 28, 2020

eladb Aug 28, 2020

rix0rrr Aug 28, 2020

rix0rrr commented Aug 28, 2020

jogold commented Aug 28, 2020

rix0rrr commented Aug 28, 2020

rix0rrr commented Aug 28, 2020

jogold commented Aug 28, 2020 •

edited

Loading

rix0rrr commented Aug 28, 2020 •

edited

Loading

rix0rrr commented Aug 28, 2020

eladb commented Aug 28, 2020

jogold commented Aug 28, 2020 •

edited

Loading

jogold commented Sep 4, 2020 •

edited

Loading

aws-cdk-automation commented Sep 30, 2020

	// Find project root
	const projectRoot = options.projectRoot ?? findUp(`.git${path.sep}`);
	if (!projectRoot) {
	throw new Error('Cannot find project root. Please specify it with `projectRoot`.');
	}

[WIP] chore: clarify build dependencies for future hermetic builds #10039

[WIP] chore: clarify build dependencies for future hermetic builds #10039

Conversation

rix0rrr commented Aug 28, 2020

Choose a reason for hiding this comment

jogold commented Aug 28, 2020

rix0rrr commented Aug 28, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jogold Aug 28, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rix0rrr commented Aug 28, 2020

jogold commented Aug 28, 2020

rix0rrr commented Aug 28, 2020

rix0rrr commented Aug 28, 2020

jogold commented Aug 28, 2020 • edited Loading

rix0rrr commented Aug 28, 2020 • edited Loading

rix0rrr commented Aug 28, 2020

eladb commented Aug 28, 2020

jogold commented Aug 28, 2020 • edited Loading

jogold commented Sep 4, 2020 • edited Loading

aws-cdk-automation commented Sep 30, 2020

AWS CodeBuild CI Report

jogold Aug 28, 2020 •

edited

Loading

jogold commented Aug 28, 2020 •

edited

Loading

rix0rrr commented Aug 28, 2020 •

edited

Loading

jogold commented Aug 28, 2020 •

edited

Loading

jogold commented Sep 4, 2020 •

edited

Loading