From 5bc033c4342734a1a8daaeba68d7613cd5b1d4f7 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Tue, 5 May 2020 18:18:57 +0300 Subject: [PATCH] perf: keep all the metadata files in the same dir Reduce the number of directories in the store by keeping all the metadata json files in the same directory. Previously the metadata file of express would be stored at: /registry.npmjs.org/express/index.json Now it will be stored at: /metadata/registry.npmjs.org/express.json ref #2521 PR #2529 --- .changeset/fifty-horses-change.md | 5 +++++ packages/npm-resolver/src/index.ts | 6 +++--- packages/npm-resolver/src/pickPackage.ts | 20 ++++++++++---------- packages/npm-resolver/test/index.ts | 6 +++--- packages/supi/test/install/lockfileOnly.ts | 9 +++++---- 5 files changed, 26 insertions(+), 20 deletions(-) create mode 100644 .changeset/fifty-horses-change.md diff --git a/.changeset/fifty-horses-change.md b/.changeset/fifty-horses-change.md new file mode 100644 index 0000000000..52535436db --- /dev/null +++ b/.changeset/fifty-horses-change.md @@ -0,0 +1,5 @@ +--- +"@pnpm/npm-resolver": major +--- + +Reduce the number of directories in the store by keeping all the metadata json files in the same directory. diff --git a/packages/npm-resolver/src/index.ts b/packages/npm-resolver/src/index.ts index f0e54c8652..3df1149b1b 100644 --- a/packages/npm-resolver/src/index.ts +++ b/packages/npm-resolver/src/index.ts @@ -44,8 +44,8 @@ export { // This file contains meta information // about all the packages published by the same name, not just the manifest // of one package/version -const META_FILENAME = 'index.json' -const FULL_META_FILENAME = 'index-full.json' +const META_DIR = 'metadata' +const FULL_META_DIR = 'metadata-full' export interface ResolverFactoryOptions { rawConfig: object, @@ -111,7 +111,7 @@ export default function createResolver ( pickPackage: pickPackage.bind(null, { fetch, metaCache: opts.metaCache, - metaFileName: opts.fullMetadata ? FULL_META_FILENAME : META_FILENAME, + metaDir: opts.fullMetadata ? FULL_META_DIR : META_DIR, offline: opts.offline, preferOffline: opts.preferOffline, storeDir: opts.storeDir, diff --git a/packages/npm-resolver/src/pickPackage.ts b/packages/npm-resolver/src/pickPackage.ts index 0c535b5edf..6efa33ddcc 100644 --- a/packages/npm-resolver/src/pickPackage.ts +++ b/packages/npm-resolver/src/pickPackage.ts @@ -51,7 +51,7 @@ export type PickPackageOptions = { export default async ( ctx: { fetch: (pkgName: string, registry: string, authHeaderValue?: string) => Promise, - metaFileName: string, + metaDir: string, metaCache: PackageMetaCache, storeDir: string, offline?: boolean, @@ -73,12 +73,12 @@ export default async ( } const registryName = getRegistryName(opts.registry) - const pkgMirror = path.join(ctx.storeDir, registryName, spec.name) + const pkgMirror = path.join(ctx.storeDir, ctx.metaDir, registryName, `${spec.name}.json`) const limit = metafileOperationLimits[pkgMirror] = metafileOperationLimits[pkgMirror] || pLimit(1) let metaCachedInStore: PackageMeta | null | undefined if (ctx.offline || ctx.preferOffline) { - metaCachedInStore = await limit(() => loadMeta(pkgMirror, ctx.metaFileName)) + metaCachedInStore = await limit(() => loadMeta(pkgMirror)) if (ctx.offline) { if (metaCachedInStore) return { @@ -101,7 +101,7 @@ export default async ( } if (spec.type === 'version') { - metaCachedInStore = metaCachedInStore || await limit(() => loadMeta(pkgMirror, ctx.metaFileName)) + metaCachedInStore = metaCachedInStore || await limit(() => loadMeta(pkgMirror)) // use the cached meta only if it has the required package version // otherwise it is probably out of date if (metaCachedInStore?.versions?.[spec.fetchSpec]) { @@ -119,14 +119,14 @@ export default async ( ctx.metaCache.set(spec.name, meta) if (!opts.dryRun) { // tslint:disable-next-line:no-floating-promises - limit(() => saveMeta(pkgMirror, meta, ctx.metaFileName)) + limit(() => saveMeta(pkgMirror, meta)) } return { meta, pickedPackage: pickPackageFromMeta(spec, opts.preferredVersionSelectors, meta), } } catch (err) { - const meta = await loadMeta(pkgMirror, ctx.metaFileName) // TODO: add test for this usecase + const meta = await loadMeta(pkgMirror) // TODO: add test for this usecase if (!meta) throw err logger.error(err, err) logger.debug({ message: `Using cached meta from ${pkgMirror}` }) @@ -137,16 +137,16 @@ export default async ( } } -async function loadMeta (pkgMirror: string, metaFileName: string): Promise { +async function loadMeta (pkgMirror: string): Promise { try { - return await loadJsonFile(path.join(pkgMirror, metaFileName)) + return await loadJsonFile(pkgMirror) } catch (err) { return null } } -function saveMeta (pkgMirror: string, meta: PackageMeta, metaFileName: string): Promise { - return writeJsonFile(path.join(pkgMirror, metaFileName), meta) +function saveMeta (pkgMirror: string, meta: PackageMeta): Promise { + return writeJsonFile(pkgMirror, meta) } function validatePackageName (pkgName: string) { diff --git a/packages/npm-resolver/test/index.ts b/packages/npm-resolver/test/index.ts index ef07ef328f..5bf0a465ef 100644 --- a/packages/npm-resolver/test/index.ts +++ b/packages/npm-resolver/test/index.ts @@ -46,7 +46,7 @@ test('resolveFromNpm()', async t => { // The resolve function does not wait for the package meta cache file to be saved // so we must delay for a bit in order to read it setTimeout(async () => { - const meta = await loadJsonFile(path.join(storeDir, resolveResult!.id, '..', 'index.json')) // tslint:disable-line:no-any + const meta = await loadJsonFile(path.join(storeDir, 'metadata/registry.npmjs.org/is-positive.json')) // tslint:disable-line:no-any t.ok(meta.name) t.ok(meta.versions) t.ok(meta['dist-tags']) @@ -895,7 +895,7 @@ test('resolve when tarball URL is requested from the registry', async t => { // The resolve function does not wait for the package meta cache file to be saved // so we must delay for a bit in order to read it setTimeout(async () => { - const meta = await loadJsonFile(path.join(storeDir, resolveResult!.id, '..', 'index.json')) // tslint:disable-line:no-any + const meta = await loadJsonFile(path.join(storeDir, 'metadata/registry.npmjs.org/is-positive.json')) // tslint:disable-line:no-any t.ok(meta.name) t.ok(meta.versions) t.ok(meta['dist-tags']) @@ -934,7 +934,7 @@ test('resolve when tarball URL is requested from the registry and alias is not s // The resolve function does not wait for the package meta cache file to be saved // so we must delay for a bit in order to read it setTimeout(async () => { - const meta = await loadJsonFile(path.join(storeDir, resolveResult!.id, '..', 'index.json')) // tslint:disable-line:no-any + const meta = await loadJsonFile(path.join(storeDir, 'metadata/registry.npmjs.org/is-positive.json')) // tslint:disable-line:no-any t.ok(meta.name) t.ok(meta.versions) t.ok(meta['dist-tags']) diff --git a/packages/supi/test/install/lockfileOnly.ts b/packages/supi/test/install/lockfileOnly.ts index d23ccaad9c..9f951e2a32 100644 --- a/packages/supi/test/install/lockfileOnly.ts +++ b/packages/supi/test/install/lockfileOnly.ts @@ -4,6 +4,7 @@ import { prepareEmpty } from '@pnpm/prepare' import { REGISTRY_MOCK_PORT } from '@pnpm/registry-mock' import fs = require('mz/fs') import path = require('path') +import exists = require('path-exists') import sinon = require('sinon') import { addDependenciesToPackage, @@ -23,9 +24,9 @@ test('install with lockfileOnly = true', async (t: tape.Test) => { const { cafsHas } = assertStore(t, opts.storeDir) await cafsHas('pkg-with-1-dep', '100.0.0') - t.deepEqual(await fs.readdir(path.join(opts.storeDir, `localhost+${REGISTRY_MOCK_PORT}`, 'pkg-with-1-dep')), ['index.json']) + t.ok(await exists(path.join(opts.storeDir, `metadata/localhost+${REGISTRY_MOCK_PORT}/pkg-with-1-dep.json`))) await cafsHas('dep-of-pkg-with-1-dep', '100.1.0') - t.deepEqual(await fs.readdir(path.join(opts.storeDir, `localhost+${REGISTRY_MOCK_PORT}`, 'dep-of-pkg-with-1-dep')), ['index.json']) + t.ok(await exists(path.join(opts.storeDir, `metadata/localhost+${REGISTRY_MOCK_PORT}/dep-of-pkg-with-1-dep.json`))) await project.hasNot('pkg-with-1-dep') t.ok(manifest.dependencies!['pkg-with-1-dep'], 'the new dependency added to package.json') @@ -42,9 +43,9 @@ test('install with lockfileOnly = true', async (t: tape.Test) => { await install(manifest, opts) await cafsHas('pkg-with-1-dep', '100.0.0') - t.deepEqual(await fs.readdir(path.join(opts.storeDir, `localhost+${REGISTRY_MOCK_PORT}`, 'pkg-with-1-dep')), ['index.json']) + t.ok(await exists(path.join(opts.storeDir, `metadata/localhost+${REGISTRY_MOCK_PORT}/pkg-with-1-dep.json`))) await cafsHas('dep-of-pkg-with-1-dep', '100.1.0') - t.deepEqual(await fs.readdir(path.join(opts.storeDir, `localhost+${REGISTRY_MOCK_PORT}`, 'dep-of-pkg-with-1-dep')), ['index.json']) + t.ok(await exists(path.join(opts.storeDir, `metadata/localhost+${REGISTRY_MOCK_PORT}/dep-of-pkg-with-1-dep.json`))) await project.hasNot('pkg-with-1-dep') t.notOk(await project.readCurrentLockfile(), 'current lockfile not created')