From 6321746e20695095706625443bcf78c3a7f0e0f6 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 26 Nov 2020 13:33:06 +0100 Subject: [PATCH] cli: make versions:bump properly bump the versions of all @backstage packages --- .changeset/long-birds-rush.md | 2 +- .../cli/src/commands/versions/bump.test.ts | 15 ++- packages/cli/src/commands/versions/bump.ts | 96 ++++++++++++------- packages/cli/src/lib/versioning/Lockfile.ts | 6 ++ 4 files changed, 78 insertions(+), 41 deletions(-) diff --git a/.changeset/long-birds-rush.md b/.changeset/long-birds-rush.md index db4da34789..b1ce6aa125 100644 --- a/.changeset/long-birds-rush.md +++ b/.changeset/long-birds-rush.md @@ -2,4 +2,4 @@ '@backstage/cli': patch --- -Make versions:bump install new versions of dependencies that were within the specified range +Make versions:bump install new versions of dependencies that were within the specified range as well as install new versions of transitive @backstage dependencies. diff --git a/packages/cli/src/commands/versions/bump.test.ts b/packages/cli/src/commands/versions/bump.test.ts index f4d69c52be..1325f0e2fd 100644 --- a/packages/cli/src/commands/versions/bump.test.ts +++ b/packages/cli/src/commands/versions/bump.test.ts @@ -25,6 +25,7 @@ import { withLogCollector } from '@backstage/test-utils'; const REGISTRY_VERSIONS: { [name: string]: string } = { '@backstage/core': '1.0.6', + '@backstage/core-api': '1.0.7', '@backstage/theme': '2.0.0', }; @@ -54,11 +55,8 @@ const lockfileMock = `${HEADER} version "1.0.3" `; -// This resulting lockfile isn't a real world example, since it doesn't include the package bumps +// This is the lockfile that we produce to unlock versions before we run yarn install const lockfileMockResult = `${HEADER} -"@backstage/core-api@^1.0.3", "@backstage/core-api@^1.0.6": - version "1.0.6" - "@backstage/core@^1.0.5": version "1.0.6" dependencies: @@ -121,15 +119,16 @@ describe('bump', () => { expect(logs.filter(Boolean)).toEqual([ 'Checking for updates of @backstage/theme', 'Checking for updates of @backstage/core', + 'Checking for updates of @backstage/core-api', 'Some packages are outdated, updating', 'Removing lockfile entry for @backstage/core@^1.0.3 to bump to 1.0.6', + 'Removing lockfile entry for @backstage/core-api@^1.0.6 to bump to 1.0.7', + 'Removing lockfile entry for @backstage/core-api@^1.0.3 to bump to 1.0.7', 'Bumping @backstage/theme in b to ^2.0.0', "Running 'yarn install' to install new versions", - 'Removing duplicate dependencies from yarn.lock', - "Running 'yarn install' to remove duplicates from node_modules", ]); - expect(runObj.runPlain).toHaveBeenCalledTimes(2); + expect(runObj.runPlain).toHaveBeenCalledTimes(3); expect(runObj.runPlain).toHaveBeenCalledWith( 'yarn', 'info', @@ -143,7 +142,7 @@ describe('bump', () => { '@backstage/theme', ); - expect(runObj.run).toHaveBeenCalledTimes(2); + expect(runObj.run).toHaveBeenCalledTimes(1); expect(runObj.run).toHaveBeenCalledWith('yarn', ['install']); const lockfileContents = await fs.readFile('/yarn.lock', 'utf8'); diff --git a/packages/cli/src/commands/versions/bump.ts b/packages/cli/src/commands/versions/bump.ts index 1883d87bac..441f65f353 100644 --- a/packages/cli/src/commands/versions/bump.ts +++ b/packages/cli/src/commands/versions/bump.ts @@ -41,6 +41,9 @@ type PkgVersionInfo = { export default async () => { const lockfilePath = paths.resolveTargetRoot('yarn.lock'); + const lockfile = await Lockfile.load(lockfilePath); + + const findTargetVersion = createVersionFinder(); // First we discover all Backstage dependencies within our own repo const dependencyMap = await mapDependencies(paths.targetDir); @@ -48,20 +51,16 @@ export default async () => { // Next check with the package registry to see which dependency ranges we need to bump const versionBumps = new Map(); // Track package versions that we want to remove from yarn.lock in order to trigger a bump - const unlocked = Array<{ name: string; range: string; latest: string }>(); + const unlocked = Array<{ name: string; range: string; target: string }>(); await workerThreads(16, dependencyMap.entries(), async ([name, pkgs]) => { - console.log(`Checking for updates of ${name}`); - const info = await fetchPackageInfo(name); - const latest = info['dist-tags'].latest; - if (!latest) { - throw new Error(`No latest version found for ${name}`); - } + const target = await findTargetVersion(name); for (const pkg of pkgs) { - if (semver.satisfies(latest, pkg.range)) { - if (semver.minVersion(pkg.range)?.version !== latest) { - unlocked.push({ name, range: pkg.range, latest }); + if (semver.satisfies(target, pkg.range)) { + if (semver.minVersion(pkg.range)?.version !== target) { + unlocked.push({ name, range: pkg.range, target }); } + continue; } versionBumps.set( @@ -69,12 +68,32 @@ export default async () => { (versionBumps.get(pkg.name) ?? []).concat({ name, location: pkg.location, - range: `^${latest}`, // TODO(Rugvip): Option to use something else than ^? + range: `^${target}`, // TODO(Rugvip): Option to use something else than ^? }), ); } }); + // Check for updates of transitive backstage dependencies + await workerThreads(16, lockfile.keys(), async name => { + // Only check @backstage packages and friends, we don't want this to do a full update of all deps + if (!includedFilter(name)) { + return; + } + + const target = await findTargetVersion(name); + + for (const entry of lockfile.get(name) ?? []) { + // Ignore lockfile entries that don't satisfy the version range, since + // these can't cause the package to be locked to an older version + if (!semver.satisfies(target, entry.range)) { + continue; + } + // Unlock all entries that are within range but on the old version + unlocked.push({ name, range: entry.range, target }); + } + }); + console.log(); // Write all discovered version bumps to package.json in this repo @@ -85,17 +104,21 @@ export default async () => { console.log(); if (unlocked.length > 0) { - const lockfile = await Lockfile.load(lockfilePath); - for (const { name, range, latest } of unlocked) { + const removed = new Set(); + for (const { name, range, target } of unlocked) { // Don't bother removing lockfile entries if they're already on the correct version const existingEntry = lockfile.get(name)?.find(e => e.range === range); - if (existingEntry?.version === latest) { + if (existingEntry?.version === target) { continue; } - console.log( - `Removing lockfile entry for ${name}@${range} to bump to ${latest}`, - ); - lockfile.remove(name, range); + const key = JSON.stringify({ name, range }); + if (!removed.has(key)) { + removed.add(key); + console.log( + `Removing lockfile entry for ${name}@${range} to bump to ${target}`, + ); + lockfile.remove(name, range); + } } await lockfile.save(); } @@ -126,24 +149,13 @@ export default async () => { console.log(); // Finally we make sure the new lockfile doesn't have any duplicates - const lockfile = await Lockfile.load(lockfilePath); - const result = lockfile.analyze({ + const dedupLockfile = await Lockfile.load(lockfilePath); + const result = dedupLockfile.analyze({ filter: includedFilter, }); if (result.newVersions.length > 0) { - console.log(); - console.log('Removing duplicate dependencies from yarn.lock'); - lockfile.replaceVersions(result.newVersions); - await lockfile.save(); - - console.log( - "Running 'yarn install' to remove duplicates from node_modules", - ); - console.log(); - await run('yarn', ['install']); - - console.log(); + throw new Error('Duplicate versions present after package bump'); } const forbiddenNewRanges = result.newRanges.filter(({ name }) => @@ -158,6 +170,26 @@ export default async () => { } }; +function createVersionFinder() { + const found = new Map(); + + return async function findTargetVersion(name: string) { + const existing = found.get(name); + if (existing) { + return existing; + } + + console.log(`Checking for updates of ${name}`); + const info = await fetchPackageInfo(name); + const latest = info['dist-tags'].latest; + if (!latest) { + throw new Error(`No latest version found for ${name}`); + } + found.set(name, latest); + return latest; + }; +} + async function workerThreads( count: number, items: IterableIterator, diff --git a/packages/cli/src/lib/versioning/Lockfile.ts b/packages/cli/src/lib/versioning/Lockfile.ts index f9bc789d95..fd7c189b40 100644 --- a/packages/cli/src/lib/versioning/Lockfile.ts +++ b/packages/cli/src/lib/versioning/Lockfile.ts @@ -100,10 +100,16 @@ export class Lockfile { private readonly data: LockfileData, ) {} + /** Get the entries for a single package in the lockfile */ get(name: string): LockfileQueryEntry[] | undefined { return this.packages.get(name); } + /** Returns the name of all packages available in the lockfile */ + keys(): IterableIterator { + return this.packages.keys(); + } + /** Analyzes the lockfile to identify possible actions and warnings for the entries */ analyze(options?: { filter?: (name: string) => boolean }): AnalyzeResult { const { filter } = options ?? {};