Skip to content

Commit

Permalink
chore(core): allow the bundler to re-use pre-existing bundler output (a…
Browse files Browse the repository at this point in the history
…ws#8916)

This PR changes `AssetStaging` so that the bundler will re-use pre-existing output. Before, the bundler would re-run Docker without considering pre-existing assets, which was slow. Now, when handling a `SOURCE` hash type, the bundler detects and returns pre-existing asset output without re-running Docker. For all other hash types, the bundler outputs to a temp directory before calculating asset hashes.

Closes aws#8882 

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
misterjoshua authored Jul 13, 2020
1 parent 28949bd commit 31d6e65
Show file tree
Hide file tree
Showing 5 changed files with 251 additions and 37 deletions.
1 change: 1 addition & 0 deletions packages/@aws-cdk/aws-lambda/test/integ.bundling.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/// !cdk-integ pragma:ignore-assets
import * as path from 'path';
import { App, CfnOutput, Construct, Stack, StackProps } from '@aws-cdk/core';
import * as lambda from '../lib';
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/// !cdk-integ pragma:ignore-assets
import * as path from 'path';
import * as iam from '@aws-cdk/aws-iam';
import { App, BundlingDockerImage, Construct, Stack, StackProps } from '@aws-cdk/core';
Expand Down
102 changes: 80 additions & 22 deletions packages/@aws-cdk/core/lib/asset-staging.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,23 @@ export class AssetStaging extends Construct {
this.sourcePath = props.sourcePath;
this.fingerprintOptions = props;

// Determine the hash type based on the props as props.assetHashType is
// optional from a caller perspective.
const hashType = this.determineHashType(props.assetHashType, props.assetHash);

if (props.bundling) {
this.bundleDir = this.bundle(props.bundling);
// Determine the source hash in advance of bundling if the asset hash type
// is source so that the bundler can opt to re-use its bundle dir.
const sourceHash = hashType === AssetHashType.SOURCE
? this.calculateHash(hashType, props.assetHash, props.bundling)
: undefined;

this.bundleDir = this.bundle(props.bundling, sourceHash);
this.assetHash = sourceHash ?? this.calculateHash(hashType, props.assetHash, props.bundling);
} else {
this.assetHash = this.calculateHash(hashType, props.assetHash);
}

this.assetHash = this.calculateHash(props);

const stagingDisabled = this.node.tryGetContext(cxapi.DISABLE_ASSET_STAGING_CONTEXT);
if (stagingDisabled) {
this.stagedPath = this.bundleDir ?? this.sourcePath;
Expand Down Expand Up @@ -137,15 +148,37 @@ export class AssetStaging extends Construct {
}
}

private bundle(options: BundlingOptions): string {
/**
* @Property sourceHash The source hash of the asset. If specified, the bundler
* will attempt to re-use the bundling directory, which is based on a hash of
* both the sourceHash and options. If bundling finds a pre-existing directory,
* the bundler will return it as-is and won't regenerate the bundle.
*/
private bundle(options: BundlingOptions, sourceHash?: string): string {
// Temp staging directory in the working directory
const stagingTmp = path.join('.', STAGING_TMP);
fs.ensureDirSync(stagingTmp);

// Create temp directory for bundling inside the temp staging directory
const bundleDir = path.resolve(fs.mkdtempSync(path.join(stagingTmp, 'asset-bundle-')));
// Chmod the bundleDir to full access.
fs.chmodSync(bundleDir, 0o777);
let bundleDir: string;
if (sourceHash) {
// When an asset hash is known in advance of bundling, bundling is done into a dedicated staging directory.
bundleDir = path.resolve(path.join(stagingTmp, 'asset-bundle-hash-' + sourceHash));

if (fs.existsSync(bundleDir)) {
// Pre-existing bundle directory. The bundle has already been generated once before, so lets provide it
// as-is to the caller.
return bundleDir;
}

fs.ensureDirSync(bundleDir);
} else {
// When the asset hash isn't known in advance, bundling is done into a temporary staging directory.

// Create temp directory for bundling inside the temp staging directory
bundleDir = path.resolve(fs.mkdtempSync(path.join(stagingTmp, 'asset-bundle-temp-')));
// Chmod the bundleDir to full access.
fs.chmodSync(bundleDir, 0o777);
}

let user: string;
if (options.user) {
Expand Down Expand Up @@ -181,7 +214,18 @@ export class AssetStaging extends Construct {
workingDirectory: options.workingDirectory ?? AssetStaging.BUNDLING_INPUT_DIR,
});
} catch (err) {
throw new Error(`Failed to run bundling Docker image for asset ${this.node.path}: ${err}`);
// When bundling fails, keep the bundle output for diagnosability, but
// rename it out of the way so that the next run doesn't assume it has a
// valid bundleDir.

const bundleErrorDir = bundleDir + '-error';
if (fs.existsSync(bundleErrorDir)) {
// Remove the last bundleErrorDir.
fs.removeSync(bundleErrorDir);
}

fs.renameSync(bundleDir, bundleErrorDir);
throw new Error(`Failed to run bundling Docker image for asset ${this.node.path}, bundle output is located at ${bundleErrorDir}: ${err}`);
}

if (FileSystem.isEmpty(bundleDir)) {
Expand All @@ -191,35 +235,49 @@ export class AssetStaging extends Construct {
return bundleDir;
}

private calculateHash(props: AssetStagingProps): string {
let hashType: AssetHashType;

if (props.assetHash) {
if (props.assetHashType && props.assetHashType !== AssetHashType.CUSTOM) {
throw new Error(`Cannot specify \`${props.assetHashType}\` for \`assetHashType\` when \`assetHash\` is specified. Use \`CUSTOM\` or leave \`undefined\`.`);
/**
* Determines the hash type from user-given prop values.
*
* @param assetHashType Asset hash type construct prop
* @param assetHash Asset hash given in the construct props
*/
private determineHashType(assetHashType?: AssetHashType, assetHash?: string) {
if (assetHash) {
if (assetHashType && assetHashType !== AssetHashType.CUSTOM) {
throw new Error(`Cannot specify \`${assetHashType}\` for \`assetHashType\` when \`assetHash\` is specified. Use \`CUSTOM\` or leave \`undefined\`.`);
}
hashType = AssetHashType.CUSTOM;
} else if (props.assetHashType) {
hashType = props.assetHashType;
return AssetHashType.CUSTOM;
} else if (assetHashType) {
return assetHashType;
} else {
hashType = AssetHashType.SOURCE;
return AssetHashType.SOURCE;
}
}

private calculateHash(hashType: AssetHashType, assetHash?: string, bundling?: BundlingOptions): string {
switch (hashType) {
case AssetHashType.SOURCE:
return FileSystem.fingerprint(this.sourcePath, this.fingerprintOptions);
const sourceHash = FileSystem.fingerprint(this.sourcePath, this.fingerprintOptions);
if (bundling) {
return crypto.createHash('sha256')
.update(JSON.stringify(bundling))
.update(sourceHash)
.digest('hex');
} else {
return sourceHash;
}
case AssetHashType.BUNDLE:
if (!this.bundleDir) {
throw new Error('Cannot use `AssetHashType.BUNDLE` when `bundling` is not specified.');
}
return FileSystem.fingerprint(this.bundleDir, this.fingerprintOptions);
case AssetHashType.CUSTOM:
if (!props.assetHash) {
if (!assetHash) {
throw new Error('`assetHash` must be specified when `assetHashType` is set to `AssetHashType.CUSTOM`.');
}
// Hash the hash to make sure we can use it in a file/directory name.
// The resulting hash will also have the same length as for the other hash types.
return crypto.createHash('sha256').update(props.assetHash).digest('hex');
return crypto.createHash('sha256').update(assetHash).digest('hex');
default:
throw new Error('Unknown asset hash type.');
}
Expand Down
1 change: 1 addition & 0 deletions packages/@aws-cdk/core/test/docker-stub.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ set -euo pipefail
# `/tmp/docker-stub.input` and accepts one of 3 commands that impact it's
# behavior.

echo "$@" >> /tmp/docker-stub.input.concat
echo "$@" > /tmp/docker-stub.input

if echo "$@" | grep "DOCKER_STUB_SUCCESS_NO_OUTPUT"; then
Expand Down
Loading

0 comments on commit 31d6e65

Please sign in to comment.