revert: "feat: use-experimental-npmjs-files-index option (#7177)"

This reverts commit a2bf6a0e96.
This commit is contained in:
Zoltan Kochan
2023-11-22 02:01:27 +02:00
parent 23039a6d60
commit 3db0db8d12
19 changed files with 16 additions and 122 deletions

View File

@@ -1,10 +0,0 @@
---
"@pnpm/config": minor
"@pnpm/core": minor
"@pnpm/exec.files-include-install-scripts": patch
"@pnpm/plugin-commands-installation": minor
"@pnpm/resolve-dependencies": minor
"pnpm": minor
---
(EXPERIMENTAL) When the `use-experimental-npmjs-files-index` option is set to `true` and `--lockfile-only` installation is performed, package tarballs are not downloaded. npm's beta file index feature is used to populate the lockfile [#7117](https://github.com/pnpm/pnpm/pull/7177).

View File

@@ -162,7 +162,6 @@ export interface Config {
ignoreWorkspaceCycles?: boolean
disallowWorkspaceCycles?: boolean
packGzipLevel?: number
useExperimentalNpmjsFilesIndex?: boolean
registries: Registries
ignoreWorkspaceRootCheck: boolean

View File

@@ -114,7 +114,6 @@ export const types = Object.assign({
'resolve-peers-from-workspace-root': Boolean,
'aggregate-output': Boolean,
'reporter-hide-prefix': Boolean,
'use-experimental-npmjs-files-index': Boolean,
'save-peer': Boolean,
'save-workspace-protocol': Boolean,
'script-shell': String,
@@ -243,7 +242,6 @@ export async function getConfig (
registry: npmDefaults.registry,
'resolution-mode': 'highest',
'resolve-peers-from-workspace-root': true,
'use-experimental-npmjs-files-index': false,
'save-peer': false,
'save-workspace-protocol': 'rolling',
'scripts-prepend-node-path': false,

View File

@@ -254,6 +254,6 @@
"xmarw",
"zkochan",
"zoli",
"zoltan"
"zoltan"
]
}

View File

@@ -1,5 +1,4 @@
export function filesIncludeInstallScripts (filesIndex: Record<string, unknown>): boolean {
return filesIndex['binding.gyp'] != null ||
filesIndex['/binding.gyp'] != null ||
Object.keys(filesIndex).some((filename) => !(filename.match(/^[/]?[.]hooks[\\/]/) == null)) // TODO: optimize this
Object.keys(filesIndex).some((filename) => !(filename.match(/^[.]hooks[\\/]/) == null)) // TODO: optimize this
}

View File

@@ -17,7 +17,6 @@ export type ListMissingPeersOptions = Partial<GetContextOptions>
| 'preferWorkspacePackages'
| 'saveWorkspaceProtocol'
| 'storeController'
| 'useExperimentalNpmjsFilesIndex'
| 'useGitBranchLockfile'
| 'workspacePackages'
>
@@ -83,7 +82,6 @@ export async function getPeerDependencyIssues (
saveWorkspaceProtocol: false, // this doesn't matter in our case. We won't write changes to package.json files
storeController: opts.storeController,
tag: 'latest',
useExperimentalNpmjsFilesIndex: opts.useExperimentalNpmjsFilesIndex ?? false,
virtualStoreDir: ctx.virtualStoreDir,
wantedLockfile: ctx.wantedLockfile,
workspacePackages: opts.workspacePackages ?? {},

View File

@@ -48,7 +48,6 @@ export interface StrictInstallOptions {
lockfileIncludeTarballUrl: boolean
preferWorkspacePackages: boolean
preserveWorkspaceProtocol: boolean
useExperimentalNpmjsFilesIndex: boolean
scriptsPrependNodePath: boolean | 'warn-only'
scriptShell?: string
shellEmulator: boolean
@@ -200,7 +199,6 @@ const defaults = (opts: InstallOptions) => {
resolutionMode: 'lowest-direct',
saveWorkspaceProtocol: 'rolling',
lockfileIncludeTarballUrl: false,
useExperimentalNpmjsFilesIndex: false,
scriptsPrependNodePath: false,
shamefullyHoist: false,
shellEmulator: false,

View File

@@ -1036,7 +1036,6 @@ const _installInContext: InstallFunction = async (projects, ctx, opts) => {
wantedLockfile: ctx.wantedLockfile,
workspacePackages: opts.workspacePackages,
patchedDependencies: opts.patchedDependencies,
useExperimentalNpmjsFilesIndex: opts.lockfileOnly && opts.useExperimentalNpmjsFilesIndex,
lockfileIncludeTarballUrl: opts.lockfileIncludeTarballUrl,
resolvePeersFromWorkspaceRoot: opts.resolvePeersFromWorkspaceRoot,
supportedArchitectures: opts.supportedArchitectures,

View File

@@ -1,5 +1,4 @@
import path from 'path'
import { promises as fs } from 'fs'
import { assertStore } from '@pnpm/assert-store'
import { WANTED_LOCKFILE } from '@pnpm/constants'
import { prepareEmpty } from '@pnpm/prepare'
@@ -90,12 +89,3 @@ test('do not update the lockfile when lockfileOnly and frozenLockfile are both u
frozenLockfile: true,
}))).rejects.toThrow(/is not up to date/)
})
test('a lockfile only update with the useExperimentalNpmjsFilesIndex flag resolves without packages being fetched', async () => {
const project = prepareEmpty()
const opts = await testDefaults({ useExperimentalNpmjsFilesIndex: true, lockfileOnly: true })
await addDependenciesToPackage({}, ['nodecv@1.1.2'], opts)
const lockfile = await project.readLockfile()
expect(lockfile.packages!['/nodecv@1.1.2'].requiresBuild).toBeTruthy()
expect(await fs.readdir(opts.storeDir)).toStrictEqual([])
})

View File

@@ -270,7 +270,6 @@ export type InstallCommandOptions = Pick<Config,
| 'preferFrozenLockfile'
| 'production'
| 'registries'
| 'useExperimentalNpmjsFilesIndex'
| 'rootProjectManifest'
| 'rootProjectManifestDir'
| 'save'

View File

@@ -57,7 +57,6 @@ export type InstallDepsOptions = Pick<Config,
| 'production'
| 'rawLocalConfig'
| 'registries'
| 'useExperimentalNpmjsFilesIndex'
| 'rootProjectManifestDir'
| 'rootProjectManifest'
| 'save'

View File

@@ -68,7 +68,6 @@ type RecursiveOptions = CreateStoreControllerOptions & Pick<Config,
| 'saveProd'
| 'saveWorkspaceProtocol'
| 'lockfileIncludeTarballUrl'
| 'useExperimentalNpmjsFilesIndex'
| 'sharedWorkspaceLockfile'
| 'tag'
> & {

View File

@@ -33,7 +33,6 @@
"@pnpm/core-loggers": "workspace:*",
"@pnpm/dependency-path": "workspace:*",
"@pnpm/error": "workspace:*",
"@pnpm/fetch": "workspace:*",
"@pnpm/exec.files-include-install-scripts": "workspace:*",
"@pnpm/lockfile-types": "workspace:*",
"@pnpm/lockfile-utils": "workspace:*",

View File

@@ -1,68 +0,0 @@
import { PnpmError } from '@pnpm/error'
import { filesIncludeInstallScripts } from '@pnpm/exec.files-include-install-scripts'
import { fetch } from '@pnpm/fetch'
import { type ProjectManifest } from '@pnpm/types'
interface RegistryFileFields {
size: number
type: 'File'
path: string
contentType: string
hex: string
isBinary: boolean
linesCount: number
}
interface RegistryFileList {
files: {
[path: string]: RegistryFileFields
}
}
export async function checkViaRegistryIfPkgRequiresBuild (pkgName: string, pkgVersion: string): Promise<boolean> {
try {
const regFS = await fetchPkgIndex(pkgName, pkgVersion)
const hasInstall = filesIncludeInstallScripts(regFS.files)
if (hasInstall) return true
const pkgJsonHex = regFS.files['/package.json'].hex
const pkgJsonContent = await fetchSpecificFileFromRegistryFS(pkgName, pkgVersion, pkgJsonHex)
const pkgJson = JSON.parse(pkgJsonContent) as ProjectManifest
return pkgManifestHasInstallScripts(pkgJson)
} catch (err) {
if (err instanceof Error) {
throw new PnpmError('REG_FS_PARSE', `Failed to fetch ${pkgName}@${pkgVersion}: ${err.message}`)
}
}
return false
}
async function fetchPkgIndex (pkgName: string, pkgVersion: string): Promise<RegistryFileList> {
const url = `https://npmjs.com/package/${pkgName}/v/${pkgVersion}/index`
const fetchResult = await fetch(url)
if (!fetchResult.ok) {
throw new PnpmError('PKG_INDEX_FETCH', `Failed to fetch ${pkgName}@${pkgVersion} Status: ${fetchResult.statusText}`)
}
const fetchJSON = await fetchResult.json() as RegistryFileList
if (fetchJSON.files == null) {
throw new PnpmError('PKG_INDEX_FILE_PARSE', `Unable to parse file list for ${pkgName}@${pkgVersion} Status: ${fetchResult.statusText}`)
}
return fetchJSON
}
async function fetchSpecificFileFromRegistryFS (pkgName: string, pkgVersion: string, fileHex: string): Promise<string> {
const url = `https://npmjs.com/package/${pkgName}/file/${fileHex}`
const fetchResult = await fetch(url)
if (!fetchResult.ok) {
throw new PnpmError('REG_FS_ERROR', `Failed to fetch ${pkgName}@${pkgVersion}, file: ${fileHex} Status: ${fetchResult.statusText}`)
}
return fetchResult.text()
}
export function pkgManifestHasInstallScripts (manifest: ProjectManifest | undefined): boolean {
if (!manifest?.scripts) return false
return Boolean(manifest.scripts.preinstall) ||
Boolean(manifest.scripts.install) ||
Boolean(manifest.scripts.postinstall)
}

View File

@@ -26,7 +26,6 @@ import promiseShare from 'promise-share'
import difference from 'ramda/src/difference'
import zipWith from 'ramda/src/zipWith'
import isSubdir from 'is-subdir'
import { checkViaRegistryIfPkgRequiresBuild, pkgManifestHasInstallScripts } from './checkViaRegistryIfPkgRequiresBuild'
import { getWantedDependencies, type WantedDependency } from './getWantedDependencies'
import { depPathToRef } from './depPathToRef'
import { createNodeIdForLinkedLocalPkg, type UpdateMatchingFunction } from './resolveDependencies'
@@ -101,7 +100,6 @@ export async function resolveDependencies (
saveWorkspaceProtocol: 'rolling' | boolean
lockfileIncludeTarballUrl?: boolean
allowNonAppliedPatches?: boolean
useExperimentalNpmjsFilesIndex?: boolean
}
) {
const _toResolveImporter = toResolveImporter.bind(null, {
@@ -288,7 +286,7 @@ export async function resolveDependencies (
return {
dependenciesByProjectId,
dependenciesGraph,
finishLockfileUpdates: promiseShare(finishLockfileUpdates(dependenciesGraph, pendingRequiresBuilds, newLockfile, opts.useExperimentalNpmjsFilesIndex)),
finishLockfileUpdates: promiseShare(finishLockfileUpdates(dependenciesGraph, pendingRequiresBuilds, newLockfile)),
outdatedDependencies,
linkedDependenciesByProjectId,
newLockfile,
@@ -324,8 +322,7 @@ function verifyPatches (
async function finishLockfileUpdates (
dependenciesGraph: DependenciesGraph,
pendingRequiresBuilds: string[],
newLockfile: Lockfile,
useExperimentalNpmjsFilesIndex: boolean
newLockfile: Lockfile
) {
return Promise.all(pendingRequiresBuilds.map(async (depPath) => {
const depNode = dependenciesGraph[depPath]
@@ -336,12 +333,17 @@ async function finishLockfileUpdates (
// We assume that all optional dependencies have to be built.
// Optional dependencies are not always downloaded, so there is no way to know whether they need to be built or not.
requiresBuild = true
} else if (useExperimentalNpmjsFilesIndex) {
requiresBuild = await checkViaRegistryIfPkgRequiresBuild(depNode.name, depNode.version)
} else {
// The npm team suggests to always read the package.json for deciding whether the package has lifecycle scripts
const { files, bundledManifest: pkgJson } = await depNode.fetching()
requiresBuild = pkgManifestHasInstallScripts(pkgJson) || filesIncludeInstallScripts(files.filesIndex)
requiresBuild = Boolean(
pkgJson?.scripts != null && (
Boolean(pkgJson.scripts.preinstall) ||
Boolean(pkgJson.scripts.install) ||
Boolean(pkgJson.scripts.postinstall)
) ||
filesIncludeInstallScripts(files.filesIndex)
)
}
if (typeof depNode.requiresBuild === 'function') {
depNode.requiresBuild['resolve'](requiresBuild)

View File

@@ -158,7 +158,6 @@ export interface ResolutionContext {
pnpmVersion: string
registries: Registries
resolutionMode?: 'highest' | 'time-based' | 'lowest-direct'
useExperimentalNpmjsFilesIndex: boolean
virtualStoreDir: string
workspacePackages?: WorkspacePackages
missingPeersOfChildrenByPkgId: Record<string, { parentImporterId: string, missingPeersOfChildren: MissingPeersOfChildren }>
@@ -1087,8 +1086,9 @@ async function resolveDependency (
? ctx.lockfileDir
: options.parentPkg.rootDir,
registry: wantedDependency.alias && pickRegistryForPackage(ctx.registries, wantedDependency.alias, wantedDependency.pref) || ctx.registries.default,
// we can skip fetching only if we use the NPMJS Files Index, and we don't need the full tarball
skipFetch: !!ctx.useExperimentalNpmjsFilesIndex,
// Unfortunately, even when run with --lockfile-only, we need the *real* package.json
// so fetching of the tarball cannot be ever avoided. Related issue: https://github.com/pnpm/pnpm/issues/1176
skipFetch: false,
update: options.update,
workspacePackages: ctx.workspacePackages,
supportedArchitectures: options.supportedArchitectures,
@@ -1247,6 +1247,7 @@ async function resolveDependency (
pkg.deprecated = currentPkg.dependencyLockfile.deprecated
}
hasBin = Boolean((pkg.bin && !(pkg.bin === '' || Object.keys(pkg.bin).length === 0)) ?? pkg.directories?.bin)
/* eslint-enable @typescript-eslint/dot-notation */
}
if (options.currentDepth === 0 && pkgResponse.body.latest && pkgResponse.body.latest !== pkg.version) {
ctx.outdatedDependencies[pkgResponse.body.id] = pkgResponse.body.latest

View File

@@ -85,7 +85,6 @@ export interface ResolveDependenciesOptions {
lockfileDir: string
storeController: StoreController
tag: string
useExperimentalNpmjsFilesIndex: boolean
virtualStoreDir: string
wantedLockfile: Lockfile
workspacePackages: WorkspacePackages
@@ -124,7 +123,6 @@ export async function resolveDependencyTree<T> (
resolutionMode: opts.resolutionMode,
skipped: wantedToBeSkippedPackageIds,
storeController: opts.storeController,
useExperimentalNpmjsFilesIndex: opts.useExperimentalNpmjsFilesIndex,
virtualStoreDir: opts.virtualStoreDir,
wantedLockfile: opts.wantedLockfile,
appliedPatches: new Set<string>(),

View File

@@ -27,9 +27,6 @@
{
"path": "../../lockfile/prune-lockfile"
},
{
"path": "../../network/fetch"
},
{
"path": "../../packages/constants"
},

3
pnpm-lock.yaml generated
View File

@@ -4065,9 +4065,6 @@ importers:
'@pnpm/error':
specifier: workspace:*
version: link:../../packages/error
'@pnpm/fetch':
specifier: workspace:*
version: link:../../network/fetch
'@pnpm/exec.files-include-install-scripts':
specifier: workspace:*
version: link:../../exec/files-include-install-scripts