cli-module-build: pack packages in batches inside createDistWorkspace
When passing --always-yarn-pack, we previously packed all packages in parallel. Since package.json files are rewritten during packing, this could cause intermittent "No local workspace found for this range" failures. To fix this, we now pack packages in batches, starting with the ones that have no workspace dependencies and expanding out to include packages whose dependencies have already been packed. Signed-off-by: MT Lewis <mtlewis@users.noreply.github.com>
This commit is contained in:
@@ -0,0 +1,139 @@
|
||||
/*
|
||||
* Copyright 2026 The Backstage Authors
|
||||
*
|
||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||
* you may not use this file except in compliance with the License.
|
||||
* You may obtain a copy of the License at
|
||||
*
|
||||
* http://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing, software
|
||||
* distributed under the License is distributed on an "AS IS" BASIS,
|
||||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
* See the License for the specific language governing permissions and
|
||||
* limitations under the License.
|
||||
*/
|
||||
|
||||
import { PackageGraphNode } from '@backstage/cli-node';
|
||||
import { computeTopologicalLayers } from './computeTopologicalLayers';
|
||||
|
||||
function makeNode(
|
||||
name: string,
|
||||
deps: PackageGraphNode[] = [],
|
||||
): PackageGraphNode {
|
||||
return {
|
||||
name,
|
||||
dir: `/packages/${name}`,
|
||||
packageJson: { name, version: '1.0.0' },
|
||||
allLocalDependencies: new Map(),
|
||||
publishedLocalDependencies: new Map(deps.map(d => [d.name, d])),
|
||||
localDependencies: new Map(),
|
||||
localDevDependencies: new Map(),
|
||||
localOptionalDependencies: new Map(),
|
||||
allLocalDependents: new Map(),
|
||||
publishedLocalDependents: new Map(),
|
||||
localDependents: new Map(),
|
||||
localDevDependents: new Map(),
|
||||
localOptionalDependents: new Map(),
|
||||
} as PackageGraphNode;
|
||||
}
|
||||
|
||||
describe('computeTopologicalLayers', () => {
|
||||
it('returns an empty array for no packages', () => {
|
||||
expect(computeTopologicalLayers([])).toEqual([]);
|
||||
});
|
||||
|
||||
it('returns a single layer when packages have no dependencies', () => {
|
||||
const a = makeNode('a');
|
||||
const b = makeNode('b');
|
||||
const c = makeNode('c');
|
||||
|
||||
const layers = computeTopologicalLayers([a, b, c]);
|
||||
expect(layers).toHaveLength(1);
|
||||
expect(layers[0]).toEqual(expect.arrayContaining([a, b, c]));
|
||||
});
|
||||
|
||||
it('separates packages into layers based on dependency order', () => {
|
||||
const a = makeNode('a');
|
||||
const b = makeNode('b', [a]);
|
||||
const c = makeNode('c', [b]);
|
||||
|
||||
const layers = computeTopologicalLayers([a, b, c]);
|
||||
expect(layers).toHaveLength(3);
|
||||
expect(layers[0]).toEqual([a]);
|
||||
expect(layers[1]).toEqual([b]);
|
||||
expect(layers[2]).toEqual([c]);
|
||||
});
|
||||
|
||||
it('groups independent packages into the same layer', () => {
|
||||
// a
|
||||
// / \
|
||||
// b c
|
||||
// \ /
|
||||
// d
|
||||
const a = makeNode('a');
|
||||
const b = makeNode('b', [a]);
|
||||
const c = makeNode('c', [a]);
|
||||
const d = makeNode('d', [b, c]);
|
||||
|
||||
const layers = computeTopologicalLayers([a, b, c, d]);
|
||||
expect(layers).toHaveLength(3);
|
||||
expect(layers[0]).toEqual([a]);
|
||||
expect(layers[1]).toEqual(expect.arrayContaining([b, c]));
|
||||
expect(layers[1]).toHaveLength(2);
|
||||
expect(layers[2]).toEqual([d]);
|
||||
});
|
||||
|
||||
it('falls back to a single layer on circular dependencies', () => {
|
||||
const a = makeNode('a');
|
||||
const b = makeNode('b');
|
||||
// Create a cycle: a -> b -> a
|
||||
a.publishedLocalDependencies.set('b', b);
|
||||
b.publishedLocalDependencies.set('a', a);
|
||||
|
||||
const layers = computeTopologicalLayers([a, b]);
|
||||
// Should still return all packages rather than hanging or throwing
|
||||
expect(layers).toHaveLength(1);
|
||||
expect(layers[0]).toEqual(expect.arrayContaining([a, b]));
|
||||
});
|
||||
|
||||
it('handles a partial cycle with non-cyclic packages separated into earlier layers', () => {
|
||||
const a = makeNode('a');
|
||||
const b = makeNode('b', [a]);
|
||||
const c = makeNode('c');
|
||||
// Create cycle: b -> c -> b (but a is not in the cycle)
|
||||
b.publishedLocalDependencies.set('c', c);
|
||||
c.publishedLocalDependencies.set('b', b);
|
||||
|
||||
const layers = computeTopologicalLayers([a, b, c]);
|
||||
// a has no deps, so it goes in layer 0
|
||||
expect(layers[0]).toEqual([a]);
|
||||
// b and c form a cycle, so they are dumped together
|
||||
expect(layers[1]).toEqual(expect.arrayContaining([b, c]));
|
||||
expect(layers).toHaveLength(2);
|
||||
});
|
||||
|
||||
it('produces correct layers regardless of input order', () => {
|
||||
const a = makeNode('a');
|
||||
const b = makeNode('b', [a]);
|
||||
const c = makeNode('c', [a]);
|
||||
|
||||
// Reverse input order
|
||||
const layers = computeTopologicalLayers([c, b, a]);
|
||||
expect(layers).toHaveLength(2);
|
||||
expect(layers[0]).toEqual([a]);
|
||||
expect(layers[1]).toEqual(expect.arrayContaining([b, c]));
|
||||
});
|
||||
|
||||
it('only considers publishedLocalDependencies, not other dependency types', () => {
|
||||
const a = makeNode('a');
|
||||
const b = makeNode('b');
|
||||
// b has a as a dev dependency but not a published dependency
|
||||
b.localDevDependencies.set('a', a);
|
||||
|
||||
const layers = computeTopologicalLayers([a, b]);
|
||||
// Both should be in the same layer since published deps are empty
|
||||
expect(layers).toHaveLength(1);
|
||||
expect(layers[0]).toEqual(expect.arrayContaining([a, b]));
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,63 @@
|
||||
/*
|
||||
* Copyright 2026 The Backstage Authors
|
||||
*
|
||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||
* you may not use this file except in compliance with the License.
|
||||
* You may obtain a copy of the License at
|
||||
*
|
||||
* http://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing, software
|
||||
* distributed under the License is distributed on an "AS IS" BASIS,
|
||||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
* See the License for the specific language governing permissions and
|
||||
* limitations under the License.
|
||||
*/
|
||||
|
||||
import { PackageGraphNode } from '@backstage/cli-node';
|
||||
|
||||
/**
|
||||
* Groups packages into topological layers based on their published local
|
||||
* dependencies. Packages in the same layer have no inter-dependencies and
|
||||
* can be safely packed in parallel. Each successive layer depends only on
|
||||
* packages in earlier (already-packed) layers.
|
||||
*
|
||||
* If a dependency cycle is detected the remaining packages are returned as
|
||||
* a single final layer — this matches the previous behaviour of packing
|
||||
* everything in parallel and avoids blocking the build entirely.
|
||||
*/
|
||||
export function computeTopologicalLayers(
|
||||
packages: PackageGraphNode[],
|
||||
): PackageGraphNode[][] {
|
||||
const remaining = new Map(packages.map(p => [p.name, p]));
|
||||
const layers: PackageGraphNode[][] = [];
|
||||
|
||||
while (remaining.size > 0) {
|
||||
const layer: PackageGraphNode[] = [];
|
||||
|
||||
for (const [, pkg] of remaining) {
|
||||
// A package is ready when none of its published local deps are still
|
||||
// waiting to be packed.
|
||||
const blocked = Array.from(pkg.publishedLocalDependencies.keys()).some(
|
||||
dep => remaining.has(dep),
|
||||
);
|
||||
if (!blocked) {
|
||||
layer.push(pkg);
|
||||
}
|
||||
}
|
||||
|
||||
if (layer.length === 0) {
|
||||
// Circular dependency — fall back to packing everything remaining
|
||||
// together, accepting the (pre-existing) race risk for this cycle.
|
||||
layers.push(Array.from(remaining.values()));
|
||||
break;
|
||||
}
|
||||
|
||||
for (const pkg of layer) {
|
||||
remaining.delete(pkg.name);
|
||||
}
|
||||
layers.push(layer);
|
||||
}
|
||||
|
||||
return layers;
|
||||
}
|
||||
@@ -26,6 +26,7 @@ import * as tar from 'tar';
|
||||
import partition from 'lodash/partition';
|
||||
|
||||
import { run, targetPaths } from '@backstage/cli-common';
|
||||
import { computeTopologicalLayers } from './computeTopologicalLayers';
|
||||
import {
|
||||
dependencies as cliDependencies,
|
||||
devDependencies as cliDevDependencies,
|
||||
@@ -347,8 +348,14 @@ async function moveToDistWorkspace(
|
||||
|
||||
// Old flow is below, which calls `yarn pack` and extracts the tarball
|
||||
|
||||
async function pack(target: PackageGraphNode, archive: string) {
|
||||
let archiveIndex = 0;
|
||||
|
||||
async function pack(
|
||||
target: PackageGraphNode,
|
||||
archive: string = `temp-package-${archiveIndex++}.tgz`,
|
||||
) {
|
||||
logger.log(`Repacking ${target.name} into dist workspace`);
|
||||
|
||||
const absoluteOutputPath = resolvePath(
|
||||
workspaceDir,
|
||||
relativePath(targetPaths.rootDir, target.dir),
|
||||
@@ -392,13 +399,18 @@ async function moveToDistWorkspace(
|
||||
await pack(target, `temp-package.tgz`);
|
||||
}
|
||||
|
||||
// Repacking in parallel is much faster and safe for all packages outside of the Backstage repo
|
||||
await runConcurrentTasks({
|
||||
items: safePackages.map((target, index) => ({ target, index })),
|
||||
worker: async ({ target, index }) => {
|
||||
await pack(target, `temp-package-${index}.tgz`);
|
||||
},
|
||||
});
|
||||
// Pack safe packages in topological layers so that a package is never
|
||||
// packed concurrently with its own dependencies. `yarn pack` temporarily
|
||||
// rewrites each package's package.json to resolve workspace:^ references;
|
||||
// packing a package while a dependency's manifest is mid-rewrite causes
|
||||
// intermittent "No local workspace found for this range" failures.
|
||||
const layers = computeTopologicalLayers(safePackages);
|
||||
for (const layer of layers) {
|
||||
await runConcurrentTasks({
|
||||
items: layer,
|
||||
worker: pack,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user