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.

Sign up for GitHub

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

Jump to bottom

fix(cli): assets shared between stages lead to an error #25907

Merged
merged 5 commits into from
Jun 8, 2023
Merged

fix(cli): assets shared between stages lead to an error #25907

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c0e7743
fix(cli): assets shared between stages lead to an error
rix0rrr Jun 8, 2023
895e638
Update tests
rix0rrr Jun 8, 2023
63a3334
Clarify with some comments
rix0rrr Jun 8, 2023
0e7d71c
Test
rix0rrr Jun 8, 2023
b68e8f1
Merge branch 'main' into huijbers/fix-asset-dupe
mergify[bot] Jun 8, 2023
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
19 changes: 9 additions & 10 deletions packages/aws-cdk/lib/util/work-graph-builder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ export class WorkGraphBuilder {
'stack': 5,
};
private readonly graph = new WorkGraph();
private readonly assetBuildNodes = new Map<string, AssetBuildNode>;

constructor(private readonly prebuildAssets: boolean, private readonly idPrefix = '') { }

Expand All @@ -39,12 +38,16 @@ export class WorkGraphBuilder {
*/
// eslint-disable-next-line max-len
private addAsset(parentStack: cxapi.CloudFormationStackArtifact, assetArtifact: cxapi.AssetManifestArtifact, assetManifest: AssetManifest, asset: IManifestEntry) {
const buildId = `${this.idPrefix}${asset.id}-build`;
// Just the artifact identifier
const assetId = asset.id.assetId;
// Unique per destination where the artifact needs to go
const assetDestinationId = `${asset.id}`;

// Add the build node, but only one per "source"
// The genericSource includes a relative path we could make absolute to do more effective deduplication of build steps. Not doing that right now.
const assetBuildNodeKey = JSON.stringify(asset.genericSource);
if (!this.assetBuildNodes.has(assetBuildNodeKey)) {
const buildId = `${this.idPrefix}${assetId}-build`;
const publishNodeId = `${this.idPrefix}${assetDestinationId}-publish`;

// Build node only gets added once because they are all the same
if (!this.graph.tryGetNode(buildId)) {
const node: AssetBuildNode = {
type: 'asset-build',
id: buildId,
Expand All @@ -60,13 +63,9 @@ export class WorkGraphBuilder {
deploymentState: DeploymentState.PENDING,
priority: WorkGraphBuilder.PRIORITIES['asset-build'],
};
this.assetBuildNodes.set(assetBuildNodeKey, node);
this.graph.addNodes(node);
}

// Always add the publish
const publishNodeId = `${this.idPrefix}${asset.id}-publish`;

const publishNode = this.graph.tryGetNode(publishNodeId);
if (!publishNode) {
this.graph.addNodes({
Expand Down