Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "chore(core): allow the bundler to re-use pre-existing bundler output (#8916)" #9037

Merged
merged 1 commit into from
Jul 13, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion packages/@aws-cdk/aws-lambda/test/integ.bundling.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
/// !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,4 +1,3 @@
/// !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: 22 additions & 80 deletions packages/@aws-cdk/core/lib/asset-staging.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,23 +88,12 @@ 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) {
// 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.bundleDir = this.bundle(props.bundling);
}

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 @@ -148,37 +137,15 @@ export class AssetStaging extends Construct {
}
}

/**
* @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 {
private bundle(options: BundlingOptions): string {
// Temp staging directory in the working directory
const stagingTmp = path.join('.', STAGING_TMP);
fs.ensureDirSync(stagingTmp);

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);
}
// 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 user: string;
if (options.user) {
Expand Down Expand Up @@ -214,18 +181,7 @@ export class AssetStaging extends Construct {
workingDirectory: options.workingDirectory ?? AssetStaging.BUNDLING_INPUT_DIR,
});
} catch (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}`);
throw new Error(`Failed to run bundling Docker image for asset ${this.node.path}: ${err}`);
}

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

/**
* 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\`.`);
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\`.`);
}
return AssetHashType.CUSTOM;
} else if (assetHashType) {
return assetHashType;
hashType = AssetHashType.CUSTOM;
} else if (props.assetHashType) {
hashType = props.assetHashType;
} else {
return AssetHashType.SOURCE;
hashType = AssetHashType.SOURCE;
}
}

private calculateHash(hashType: AssetHashType, assetHash?: string, bundling?: BundlingOptions): string {
switch (hashType) {
case AssetHashType.SOURCE:
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;
}
return FileSystem.fingerprint(this.sourcePath, this.fingerprintOptions);
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 (!assetHash) {
if (!props.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(assetHash).digest('hex');
return crypto.createHash('sha256').update(props.assetHash).digest('hex');
default:
throw new Error('Unknown asset hash type.');
}
Expand Down
1 change: 0 additions & 1 deletion packages/@aws-cdk/core/test/docker-stub.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ 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