From baf8c0d39e78c61f7aedb81ed1daaf03cfea76a9 Mon Sep 17 00:00:00 2001 From: Brandon Cheng Date: Mon, 30 May 2022 13:27:06 -0400 Subject: [PATCH] feat: add experimental use-inline-specifiers-lockfile-format --- .changeset/fifty-rats-jog.md | 8 ++ packages/config/src/Config.ts | 3 + packages/config/src/index.ts | 2 + .../core/src/install/extendInstallOptions.ts | 2 + packages/core/src/install/index.ts | 7 +- .../experiments/InlineSpecifiersLockfile.ts | 38 +++++++ .../inlineSpecifiersLockfileConverters.ts | 105 ++++++++++++++++++ packages/lockfile-file/src/read.ts | 3 +- .../lockfile-file/src/sortLockfileKeys.ts | 2 + packages/lockfile-file/src/write.ts | 23 +++- 10 files changed, 186 insertions(+), 7 deletions(-) create mode 100644 .changeset/fifty-rats-jog.md create mode 100644 packages/lockfile-file/src/experiments/InlineSpecifiersLockfile.ts create mode 100644 packages/lockfile-file/src/experiments/inlineSpecifiersLockfileConverters.ts diff --git a/.changeset/fifty-rats-jog.md b/.changeset/fifty-rats-jog.md new file mode 100644 index 00000000000..0ca4205bdfb --- /dev/null +++ b/.changeset/fifty-rats-jog.md @@ -0,0 +1,8 @@ +--- +"@pnpm/config": minor +"@pnpm/core": minor +"@pnpm/lockfile-file": minor +"pnpm": minor +--- + +Add experimental lockfile format that should merge conflict less in the `importers` section. Enabled by setting the `use-inline-specifiers-lockfile-format = true` feature flag in `.npmrc`. diff --git a/packages/config/src/Config.ts b/packages/config/src/Config.ts index e155d1c12b0..654fa9f1549 100644 --- a/packages/config/src/Config.ts +++ b/packages/config/src/Config.ts @@ -157,6 +157,9 @@ export interface Config { changedFilesIgnorePattern?: string[] rootProjectManifest?: ProjectManifest userConfig: Record + + // feature flags for experimental testing + useInlineSpecifiersLockfileFormat?: boolean // For https://github.com/pnpm/pnpm/issues/4725 } export interface ConfigWithDeprecatedSettings extends Config { diff --git a/packages/config/src/index.ts b/packages/config/src/index.ts index 2bd1f1a2926..0bbf968b08a 100644 --- a/packages/config/src/index.ts +++ b/packages/config/src/index.ts @@ -108,6 +108,7 @@ export const types = Object.assign({ stream: Boolean, 'strict-peer-dependencies': Boolean, 'use-beta-cli': Boolean, + 'use-inline-specifiers-lockfile-format': Boolean, 'use-node-version': String, 'use-running-store-server': Boolean, 'use-store-server': Boolean, @@ -222,6 +223,7 @@ export default async ( 'strict-peer-dependencies': true, 'unsafe-perm': npmDefaults['unsafe-perm'], 'use-beta-cli': false, + 'use-inline-specifiers-lockfile-format': false, userconfig: npmDefaults.userconfig, 'virtual-store-dir': 'node_modules/.pnpm', 'workspace-concurrency': 4, diff --git a/packages/core/src/install/extendInstallOptions.ts b/packages/core/src/install/extendInstallOptions.ts index 8d976fadec5..2d4e99fb994 100644 --- a/packages/core/src/install/extendInstallOptions.ts +++ b/packages/core/src/install/extendInstallOptions.ts @@ -28,6 +28,7 @@ export interface StrictInstallOptions { saveLockfile: boolean useGitBranchLockfile: boolean mergeGitBranchLockfiles: boolean + useInlineSpecifiersLockfileFormat: boolean linkWorkspacePackagesDepth: number lockfileOnly: boolean fixLockfile: boolean @@ -176,6 +177,7 @@ const defaults = async (opts: InstallOptions) => { useLockfile: true, saveLockfile: true, useGitBranchLockfile: false, + useInlineSpecifiersLockfileFormat: false, mergeGitBranchLockfiles: false, userAgent: `${packageManager.name}/${packageManager.version} npm/? node/${process.version} ${process.platform} ${process.arch}`, verifyStoreIntegrity: true, diff --git a/packages/core/src/install/index.ts b/packages/core/src/install/index.ts index 3af809f07e1..461e13ceea3 100644 --- a/packages/core/src/install/index.ts +++ b/packages/core/src/install/index.ts @@ -857,7 +857,12 @@ const _installInContext: InstallFunction = async (projects, ctx, opts) => { } const depsStateCache: DepsStateCache = {} - const lockfileOpts = { forceSharedFormat: opts.forceSharedLockfile, useGitBranchLockfile: opts.useGitBranchLockfile, mergeGitBranchLockfiles: opts.mergeGitBranchLockfiles } + const lockfileOpts = { + forceSharedFormat: opts.forceSharedLockfile, + useInlineSpecifiersFormat: opts.useInlineSpecifiersLockfileFormat, + useGitBranchLockfile: opts.useGitBranchLockfile, + mergeGitBranchLockfiles: opts.mergeGitBranchLockfiles, + } if (!opts.lockfileOnly && opts.enableModulesDir) { const result = await linkPackages( projects, diff --git a/packages/lockfile-file/src/experiments/InlineSpecifiersLockfile.ts b/packages/lockfile-file/src/experiments/InlineSpecifiersLockfile.ts new file mode 100644 index 00000000000..062b1835d9c --- /dev/null +++ b/packages/lockfile-file/src/experiments/InlineSpecifiersLockfile.ts @@ -0,0 +1,38 @@ +import type { Lockfile } from '@pnpm/lockfile-types' +import type { DependenciesMeta } from '@pnpm/types' + +export const InlineSpecifiersFormatLockfileKey = 'inlineSpecifiersFormat' + +/** + * Similar to the current Lockfile importers format (lockfile version 5.4 at + * time of writing), but specifiers are moved to each ResolvedDependencies block + * instead of being declared on its own dictionary. + * + * This is an experiment to reduce one flavor of merge conflicts in lockfiles. + * For more info: https://github.com/pnpm/pnpm/issues/4725. + */ +export interface InlineSpecifiersLockfile extends Omit { + [InlineSpecifiersFormatLockfileKey]: true + importers: Record +} + +/** + * Similar to the current ProjectSnapshot interface, but omits the "specifiers" + * field in favor of inlining each specifier next to its version resolution in + * dependency blocks. + */ +export interface InlineSpecifiersProjectSnapshot { + dependencies?: InlineSpecifiersResolvedDependencies + devDependencies?: InlineSpecifiersResolvedDependencies + optionalDependencies?: InlineSpecifiersResolvedDependencies + dependenciesMeta?: DependenciesMeta +} + +export interface InlineSpecifiersResolvedDependencies { + [depName: string]: SpecifierAndResolution +} + +export interface SpecifierAndResolution { + specifier: string + version: string +} diff --git a/packages/lockfile-file/src/experiments/inlineSpecifiersLockfileConverters.ts b/packages/lockfile-file/src/experiments/inlineSpecifiersLockfileConverters.ts new file mode 100644 index 00000000000..34003a0e973 --- /dev/null +++ b/packages/lockfile-file/src/experiments/inlineSpecifiersLockfileConverters.ts @@ -0,0 +1,105 @@ +import type { Lockfile, ProjectSnapshot, ResolvedDependencies } from '@pnpm/lockfile-types' +import type { InlineSpecifiersLockfile, InlineSpecifiersProjectSnapshot, InlineSpecifiersResolvedDependencies } from './InlineSpecifiersLockfile' + +export function isExperimentalInlineSpecifiersFormat ( + lockfile: InlineSpecifiersLockfile | Lockfile +): lockfile is InlineSpecifiersLockfile { + return (lockfile as InlineSpecifiersLockfile).inlineSpecifiersFormat ?? false +} + +export function convertToInlineSpecifiersFormat (lockfile: Lockfile): InlineSpecifiersLockfile { + return { + inlineSpecifiersFormat: true, + ...lockfile, + importers: mapValues(lockfile.importers, convertProjectSnapshotToInlineSpecifiersFormat), + } +} + +export function revertFromInlineSpecifiersFormatIfNecessary (lockfile: Lockfile | InlineSpecifiersLockfile): Lockfile { + return isExperimentalInlineSpecifiersFormat(lockfile) + ? revertFromInlineSpecifiersFormat(lockfile) + : lockfile +} + +export function revertFromInlineSpecifiersFormat (lockfile: InlineSpecifiersLockfile): Lockfile { + const { importers, inlineSpecifiersFormat, ...rest } = lockfile + + return { + ...rest, + importers: mapValues(importers, revertProjectSnapshot), + } +} + +function convertProjectSnapshotToInlineSpecifiersFormat ( + projectSnapshot: ProjectSnapshot +): InlineSpecifiersProjectSnapshot { + const { specifiers } = projectSnapshot + return { + dependencies: projectSnapshot.dependencies != null + ? convertResolvedDependenciesToInlineSpecifiersFormat(projectSnapshot.dependencies, { specifiers }) + : projectSnapshot.dependencies, + optionalDependencies: projectSnapshot.optionalDependencies != null + ? convertResolvedDependenciesToInlineSpecifiersFormat(projectSnapshot.optionalDependencies, { specifiers }) + : projectSnapshot.optionalDependencies, + devDependencies: projectSnapshot.devDependencies != null + ? convertResolvedDependenciesToInlineSpecifiersFormat(projectSnapshot.devDependencies, { specifiers }) + : projectSnapshot.devDependencies, + dependenciesMeta: projectSnapshot.dependenciesMeta, + } +} + +function convertResolvedDependenciesToInlineSpecifiersFormat ( + resolvedDependencies: ResolvedDependencies, + { specifiers }: { specifiers: ResolvedDependencies} +): InlineSpecifiersResolvedDependencies { + const result: InlineSpecifiersResolvedDependencies = {} + for (const [depName, version] of Object.entries(resolvedDependencies)) { + const specifier = specifiers[depName] + result[depName] = { specifier, version } + } + return result +} + +function revertProjectSnapshot (from: InlineSpecifiersProjectSnapshot): ProjectSnapshot { + const specifiers: ResolvedDependencies = {} + + function moveSpecifiers (from: InlineSpecifiersResolvedDependencies): ResolvedDependencies { + const resolvedDependencies: ResolvedDependencies = {} + for (const [depName, { specifier, version }] of Object.entries(from)) { + const existingValue = specifiers[depName] + if (existingValue != null && existingValue !== specifier) { + throw new Error(`Project snapshot lists the same dependency more than once with conflicting versions: ${depName}`) + } + + specifiers[depName] = specifier + resolvedDependencies[depName] = version + } + return resolvedDependencies + } + + const dependencies = from.dependencies == null + ? from.dependencies + : moveSpecifiers(from.dependencies) + const devDependencies = from.devDependencies == null + ? from.devDependencies + : moveSpecifiers(from.devDependencies) + const optionalDependencies = from.optionalDependencies == null + ? from.optionalDependencies + : moveSpecifiers(from.optionalDependencies) + + return { + ...from, + specifiers, + dependencies, + devDependencies, + optionalDependencies, + } +} + +function mapValues (obj: Record, mapper: (el: T) => U): Record { + const result = {} + for (const [key, value] of Object.entries(obj)) { + result[key] = mapper(value) + } + return result +} diff --git a/packages/lockfile-file/src/read.ts b/packages/lockfile-file/src/read.ts index fca6a0f479b..7cbfc7ce13d 100644 --- a/packages/lockfile-file/src/read.ts +++ b/packages/lockfile-file/src/read.ts @@ -18,6 +18,7 @@ import logger from './logger' import { LockfileFile } from './write' import { getWantedLockfileName } from './lockfileName' import { getGitBranchLockfileNames } from './gitBranchLockfile' +import { revertFromInlineSpecifiersFormatIfNecessary } from './experiments/inlineSpecifiersLockfileConverters' export async function readCurrentLockfile ( virtualStoreDir: string, @@ -101,7 +102,7 @@ async function _read ( }) } if (lockfileFile) { - const lockfile = convertFromLockfileFileMutable(lockfileFile) + const lockfile = revertFromInlineSpecifiersFormatIfNecessary(convertFromLockfileFileMutable(lockfileFile)) const lockfileSemver = comverToSemver((lockfile.lockfileVersion ?? 0).toString()) /* eslint-enable @typescript-eslint/dot-notation */ if (typeof opts.wantedVersion !== 'number' || semver.major(lockfileSemver) === semver.major(comverToSemver(opts.wantedVersion.toString()))) { diff --git a/packages/lockfile-file/src/sortLockfileKeys.ts b/packages/lockfile-file/src/sortLockfileKeys.ts index 8c1e11b4043..0e644d6c679 100644 --- a/packages/lockfile-file/src/sortLockfileKeys.ts +++ b/packages/lockfile-file/src/sortLockfileKeys.ts @@ -1,4 +1,5 @@ import sortKeys from 'sort-keys' +import { InlineSpecifiersFormatLockfileKey } from './experiments/InlineSpecifiersLockfile' import { LockfileFile } from './write' const ORDERED_KEYS = { @@ -38,6 +39,7 @@ const ROOT_KEYS_ORDER = { overrides: 3, packageExtensionsChecksum: 4, patchedDependencies: 5, + [InlineSpecifiersFormatLockfileKey]: 6, specifiers: 10, dependencies: 11, optionalDependencies: 12, diff --git a/packages/lockfile-file/src/write.ts b/packages/lockfile-file/src/write.ts index bbca62cb19e..e82649bce08 100644 --- a/packages/lockfile-file/src/write.ts +++ b/packages/lockfile-file/src/write.ts @@ -11,6 +11,7 @@ import writeFileAtomicCB from 'write-file-atomic' import logger from './logger' import { sortLockfileKeys } from './sortLockfileKeys' import { getWantedLockfileName } from './lockfileName' +import { convertToInlineSpecifiersFormat } from './experiments/inlineSpecifiersLockfileConverters' async function writeFileAtomic (filename: string, data: string) { return new Promise((resolve, reject) => writeFileAtomicCB(filename, data, {}, (err?: Error) => (err != null) ? reject(err) : resolve())) @@ -29,6 +30,7 @@ export async function writeWantedLockfile ( wantedLockfile: Lockfile, opts?: { forceSharedFormat?: boolean + useInlineSpecifiersFormat?: boolean useGitBranchLockfile?: boolean mergeGitBranchLockfiles?: boolean } @@ -48,13 +50,16 @@ export async function writeCurrentLockfile ( return writeLockfile('lock.yaml', virtualStoreDir, currentLockfile, opts) } +interface LockfileFormatOptions { + forceSharedFormat?: boolean + useInlineSpecifiersFormat?: boolean +} + async function writeLockfile ( lockfileFilename: string, pkgPath: string, wantedLockfile: Lockfile, - opts?: { - forceSharedFormat?: boolean - } + opts?: LockfileFormatOptions ) { const lockfilePath = path.join(pkgPath, lockfileFilename) @@ -63,7 +68,11 @@ async function writeLockfile ( return rimraf(lockfilePath) } - const yamlDoc = yamlStringify(wantedLockfile, opts?.forceSharedFormat === true) + const lockfileToStringify = (opts?.useInlineSpecifiersFormat ?? false) + ? convertToInlineSpecifiersFormat(wantedLockfile) as unknown as Lockfile + : wantedLockfile + + const yamlDoc = yamlStringify(lockfileToStringify, opts?.forceSharedFormat === true) return writeFileAtomic(lockfilePath, yamlDoc) } @@ -145,6 +154,7 @@ export function normalizeLockfile (lockfile: Lockfile, forceSharedFormat: boolea export default async function writeLockfiles ( opts: { forceSharedFormat?: boolean + useInlineSpecifiersFormat?: boolean wantedLockfile: Lockfile wantedLockfileDir: string currentLockfile: Lockfile @@ -167,7 +177,10 @@ export default async function writeLockfiles ( } const forceSharedFormat = opts?.forceSharedFormat === true - const yamlDoc = yamlStringify(opts.wantedLockfile, forceSharedFormat) + const wantedLockfileToStringify = (opts.useInlineSpecifiersFormat ?? false) + ? convertToInlineSpecifiersFormat(opts.wantedLockfile) as unknown as Lockfile + : opts.wantedLockfile + const yamlDoc = yamlStringify(wantedLockfileToStringify, forceSharedFormat) // in most cases the `pnpm-lock.yaml` and `node_modules/.pnpm-lock.yaml` are equal // in those cases the YAML document can be stringified only once for both files