From fbdb48941fe0e83130b13cb2b73ac28c6e0362f8 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 21 Dec 2022 16:41:35 +0100 Subject: [PATCH] cli: use package graph to look up local lockfile versions Signed-off-by: Patrik Oldsberg --- .changeset/bright-pants-cry.md | 2 +- .../cli/src/commands/start/startFrontend.ts | 4 + packages/cli/src/commands/versions/bump.ts | 5 ++ packages/cli/src/commands/versions/lint.ts | 4 + .../cli/src/lib/versioning/Lockfile.test.ts | 79 ++++++++++++++++++- packages/cli/src/lib/versioning/Lockfile.ts | 38 ++++++--- 6 files changed, 118 insertions(+), 14 deletions(-) diff --git a/.changeset/bright-pants-cry.md b/.changeset/bright-pants-cry.md index 53ba0c9521..88d7356640 100644 --- a/.changeset/bright-pants-cry.md +++ b/.changeset/bright-pants-cry.md @@ -2,4 +2,4 @@ '@backstage/cli': patch --- -Fixing the error `Error: No existing version was accepted for range ^0.12.0, searching through 0.11.2,0.0.0-use.local, for package @backstage/core-components`, which can happen when there are multiple matching versions for a package, and one of them uses `workspace:^` as its range. +Fixed an issue where the CLI would fail to function when there was a mix of workspace and non-workspace versions of the same package in `yarn.lock` when using Yarn 3. diff --git a/packages/cli/src/commands/start/startFrontend.ts b/packages/cli/src/commands/start/startFrontend.ts index 689fb28466..b7b81e6bff 100644 --- a/packages/cli/src/commands/start/startFrontend.ts +++ b/packages/cli/src/commands/start/startFrontend.ts @@ -22,6 +22,7 @@ import { loadCliConfig } from '../../lib/config'; import { paths } from '../../lib/paths'; import { Lockfile } from '../../lib/versioning'; import { forbiddenDuplicatesFilter, includedFilter } from '../versions/lint'; +import { PackageGraph } from '../../lib/monorepo'; interface StartAppOptions { verifyVersions?: boolean; @@ -36,6 +37,9 @@ export async function startFrontend(options: StartAppOptions) { const lockfile = await Lockfile.load(paths.resolveTargetRoot('yarn.lock')); const result = lockfile.analyze({ filter: includedFilter, + localPackages: PackageGraph.fromPackages( + await PackageGraph.listTargetPackages(), + ), }); const problemPackages = [...result.newVersions, ...result.newRanges] .map(({ name }) => name) diff --git a/packages/cli/src/commands/versions/bump.ts b/packages/cli/src/commands/versions/bump.ts index 1905aded20..37eb858dcf 100644 --- a/packages/cli/src/commands/versions/bump.ts +++ b/packages/cli/src/commands/versions/bump.ts @@ -39,6 +39,7 @@ import { ReleaseManifest, } from '@backstage/release-manifests'; import 'global-agent/bootstrap'; +import { PackageGraph } from '../../lib/monorepo'; const DEP_TYPES = [ 'dependencies', @@ -305,8 +306,12 @@ export default async (opts: OptionValues) => { // Finally we make sure the new lockfile doesn't have any duplicates const dedupLockfile = await Lockfile.load(lockfilePath); + const result = dedupLockfile.analyze({ filter, + localPackages: PackageGraph.fromPackages( + await PackageGraph.listTargetPackages(), + ), }); if (result.newVersions.length > 0) { diff --git a/packages/cli/src/commands/versions/lint.ts b/packages/cli/src/commands/versions/lint.ts index 9939b2ba51..e6bc119b71 100644 --- a/packages/cli/src/commands/versions/lint.ts +++ b/packages/cli/src/commands/versions/lint.ts @@ -18,6 +18,7 @@ import { OptionValues } from 'commander'; import { Lockfile } from '../../lib/versioning'; import { paths } from '../../lib/paths'; import partition from 'lodash/partition'; +import { PackageGraph } from '../../lib/monorepo'; // Packages that we try to avoid duplicates for const INCLUDED = [/^@backstage\//]; @@ -55,6 +56,9 @@ export default async (cmd: OptionValues) => { const lockfile = await Lockfile.load(paths.resolveTargetRoot('yarn.lock')); const result = lockfile.analyze({ filter: includedFilter, + localPackages: PackageGraph.fromPackages( + await PackageGraph.listTargetPackages(), + ), }); logArray( diff --git a/packages/cli/src/lib/versioning/Lockfile.test.ts b/packages/cli/src/lib/versioning/Lockfile.test.ts index 4621b43372..e663ec4a93 100644 --- a/packages/cli/src/lib/versioning/Lockfile.test.ts +++ b/packages/cli/src/lib/versioning/Lockfile.test.ts @@ -16,6 +16,7 @@ import fs from 'fs-extra'; import mockFs from 'mock-fs'; +import { ExtendedPackage } from '../monorepo'; import { Lockfile } from './Lockfile'; const HEADER = `# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY. @@ -91,7 +92,7 @@ describe('Lockfile', () => { }); const lockfile = await Lockfile.load('/yarn.lock'); - const result = lockfile.analyze(); + const result = lockfile.analyze({ localPackages: new Map() }); expect(result).toEqual({ invalidRanges: [], newRanges: [], @@ -120,7 +121,7 @@ describe('Lockfile', () => { }); const lockfile = await Lockfile.load('/yarn.lock'); - const result = lockfile.analyze(); + const result = lockfile.analyze({ localPackages: new Map() }); expect(result).toEqual({ invalidRanges: [], newRanges: [ @@ -192,6 +193,36 @@ b@^2: version: 2.0.1 `; +const mockANewLocal = `${newHeader} +a@^1: + version: 1.0.1 + dependencies: + b: ^2 + integrity: sha512-xyz + resolved: "https://my-registry/a-1.0.01.tgz#abc123" + +"b@2.0.x, b@^2.0.1": + version: 0.0.0-use.local + +b@^2: + version: 2.0.0 +`; + +const mockANewLocalDedup = `${newHeader} +a@^1: + version: 1.0.1 + dependencies: + b: ^2 + integrity: sha512-xyz + resolved: "https://my-registry/a-1.0.01.tgz#abc123" + +"b@2.0.x, b@^2.0.1": + version: 0.0.0-use.local + +b@^2: + version: 0.0.0-use.local +`; + describe('New Lockfile', () => { afterEach(() => { mockFs.restore(); @@ -218,7 +249,7 @@ describe('New Lockfile', () => { }); const lockfile = await Lockfile.load('/yarn.lock'); - const result = lockfile.analyze(); + const result = lockfile.analyze({ localPackages: new Map() }); expect(result).toEqual({ invalidRanges: [], newRanges: [], @@ -242,4 +273,46 @@ describe('New Lockfile', () => { mockANewDedup, ); }); + + it('should deduplicate and save mockANewLocal', async () => { + mockFs({ + '/yarn.lock': mockANewLocal, + }); + + const lockfile = await Lockfile.load('/yarn.lock'); + const result = lockfile.analyze({ + localPackages: new Map([ + [ + 'b', + { + packageJson: { version: '2.0.1' }, + } as ExtendedPackage, + ], + ]), + }); + expect(result).toEqual({ + invalidRanges: [], + newRanges: [], + newVersions: [ + { + name: 'b', + range: '^2', + oldVersion: '2.0.0', + newVersion: '0.0.0-use.local', + }, + ], + }); + + expect(lockfile.toString()).toBe(mockANewLocal); + lockfile.replaceVersions(result.newVersions); + expect(lockfile.toString()).toBe(mockANewLocalDedup); + + await expect(fs.readFile('/yarn.lock', 'utf8')).resolves.toBe( + mockANewLocal, + ); + await expect(lockfile.save()).resolves.toBeUndefined(); + await expect(fs.readFile('/yarn.lock', 'utf8')).resolves.toBe( + mockANewLocalDedup, + ); + }); }); diff --git a/packages/cli/src/lib/versioning/Lockfile.ts b/packages/cli/src/lib/versioning/Lockfile.ts index ac2206cd83..0559ca86a1 100644 --- a/packages/cli/src/lib/versioning/Lockfile.ts +++ b/packages/cli/src/lib/versioning/Lockfile.ts @@ -18,6 +18,7 @@ import fs from 'fs-extra'; import semver from 'semver'; import { parseSyml, stringifySyml } from '@yarnpkg/parsers'; import { stringify as legacyStringifyLockfile } from '@yarnpkg/lockfile'; +import { ExtendedPackage } from '../monorepo'; const ENTRY_PATTERN = /^((?:@[^/]+\/)?[^@/]+)@(.+)$/; @@ -164,8 +165,11 @@ export class Lockfile { } /** Analyzes the lockfile to identify possible actions and warnings for the entries */ - analyze(options?: { filter?: (name: string) => boolean }): AnalyzeResult { - const { filter } = options ?? {}; + analyze(options: { + filter?: (name: string) => boolean; + localPackages: Map; + }): AnalyzeResult { + const { filter, localPackages } = options; const result: AnalyzeResult = { invalidRanges: [], newVersions: [], @@ -195,38 +199,52 @@ export class Lockfile { const versions = Array.from(new Set(entries.map(e => e.version))) .map(v => { // Translate workspace:^ references to the actual version - return v === '0.0.0-use.local' - ? require(`${name}/package.json`).version - : v; + if (v === '0.0.0-use.local') { + const local = localPackages.get(name); + if (!local) { + throw new Error(`No local package found for ${name}`); + } + if (!local.packageJson.version) { + throw new Error(`No version found for local package ${name}`); + } + return { + entryVersion: v, + actualVersion: local.packageJson.version, + }; + } + return { entryVersion: v, actualVersion: v }; }) - .sort((v1, v2) => semver.rcompare(v1, v2)); + .sort((v1, v2) => semver.rcompare(v1.actualVersion, v2.actualVersion)); // If we're not using at least 2 different versions we're done if (versions.length < 2) { continue; } + // TODO(Rugvip): Support bumping into workspace ranges too const acceptedVersions = new Set(); for (const { version, range } of entries) { // Finds the highest matching version from the the known versions // TODO(Rugvip): We may want to select the version that satisfies the most ranges rather than the highest one - const acceptedVersion = versions.find(v => semver.satisfies(v, range)); + const acceptedVersion = versions.find(v => + semver.satisfies(v.actualVersion, range), + ); if (!acceptedVersion) { throw new Error( `No existing version was accepted for range ${range}, searching through ${versions}, for package ${name}`, ); } - if (acceptedVersion !== version) { + if (acceptedVersion.entryVersion !== version) { result.newVersions.push({ name, range, - newVersion: acceptedVersion, + newVersion: acceptedVersion.entryVersion, oldVersion: version, }); } - acceptedVersions.add(acceptedVersion); + acceptedVersions.add(acceptedVersion.actualVersion); } // If all ranges were able to accept the same version, we're done