Merge pull request #15364 from backstage/freben/fix-referenced-locals

attempt to unbreak the duplicated versions problem in master
This commit is contained in:
Patrik Oldsberg
2022-12-22 09:34:25 +01:00
committed by GitHub
8 changed files with 143 additions and 17 deletions
+5
View File
@@ -0,0 +1,5 @@
---
'@backstage/cli': patch
---
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.
+5
View File
@@ -0,0 +1,5 @@
---
'@backstage/cli': patch
---
The frontend serve task now filters out allowed package duplicates during its package check, just like `versions:bump` and `versions:check`.
+5
View File
@@ -0,0 +1,5 @@
---
'@backstage/cli': patch
---
Workspace ranges are no longer considered invalid by version commands.
@@ -21,7 +21,8 @@ import { serveBundle } from '../../lib/bundler';
import { loadCliConfig } from '../../lib/config';
import { paths } from '../../lib/paths';
import { Lockfile } from '../../lib/versioning';
import { includedFilter } from '../versions/lint';
import { forbiddenDuplicatesFilter, includedFilter } from '../versions/lint';
import { PackageGraph } from '../../lib/monorepo';
interface StartAppOptions {
verifyVersions?: boolean;
@@ -36,10 +37,13 @@ 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,
);
const problemPackages = [...result.newVersions, ...result.newRanges]
.map(({ name }) => name)
.filter(forbiddenDuplicatesFilter);
if (problemPackages.length > 1) {
console.log(
@@ -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) {
@@ -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(
@@ -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,
);
});
});
+35 -10
View File
@@ -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<string, ExtendedPackage>;
}): AnalyzeResult {
const { filter, localPackages } = options;
const result: AnalyzeResult = {
invalidRanges: [],
newVersions: [],
@@ -178,7 +182,9 @@ export class Lockfile {
}
// Get rid of and signal any invalid ranges upfront
const invalid = allEntries.filter(e => !semver.validRange(e.range));
const invalid = allEntries.filter(
e => !semver.validRange(e.range) && !e.range.startsWith('workspace:'),
);
result.invalidRanges.push(
...invalid.map(({ range }) => ({ name, range })),
);
@@ -190,36 +196,55 @@ export class Lockfile {
}
// Find all versions currently in use
const versions = Array.from(new Set(entries.map(e => e.version))).sort(
(v1, v2) => semver.rcompare(v1, v2),
);
const versions = Array.from(new Set(entries.map(e => e.version)))
.map(v => {
// Translate workspace:^ references to the actual version
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.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<string>();
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