mirror of
https://github.com/pnpm/pnpm.git
synced 2026-07-19 04:02:32 -04:00
fix: align git-hosted store readers with the integrity-keyed writer
Once the lockfile pins integrity for git-hosted tarballs, every
post-fetch reader (pkg-finder, store status, FUSE mounter,
skipIfHasSideEffectsCache in building/after-install) looks the package
up under storeIndexKey(integrity, pkgId). Three issues were exposed:
1. pkg-finder ran parse(packageId) and rebuilt name@version, but
packageIdFromSnapshot returns the URL alone for git-hosted (the
`:` in `https://` makes tryGetPackageId strip the `name@`
prefix). parse(URL) returns {} and the lookup key became
`…\tundefined@undefined`. Use packageId directly — that's the
form the writer in package-requester uses.
2. The FUSE handler and the after-install side-effects-cache lookup
used `${name}@${version}`, which doesn't match the resolver's
pkg.id for git-hosted (URL). Use
`nonSemverVersion ?? ${name}@${version}` so the readers compute
the same key the writer uses.
3. On the first install, integrity isn't known when
getFilesIndexFilePath runs, so the writer parks the index under
gitHostedStoreIndexKey. Once the worker computes integrity, the
git-hosted fetcher now also registers the entry under
storeIndexKey(integrity, pkgId), so subsequent integrity-based
lookups (the same install or any later one) resolve to the same
data without a refetch.
The license test that walks the lockfile right after install now
passes locally; that's the case that was failing in CI.
---
Written by an agent (Claude Code, claude-opus-4-7).
This commit is contained in:
@@ -358,7 +358,10 @@ async function _rebuild (
|
||||
}
|
||||
const resolution = (pkgSnapshot.resolution as TarballResolution)
|
||||
let sideEffectsCacheKey: string | undefined
|
||||
const pkgId = `${pkgInfo.name}@${pkgInfo.version}`
|
||||
// Match the resolver-supplied pkg.id used by the writer in
|
||||
// @pnpm/installing.package-requester: that's the tarball URL for
|
||||
// git-hosted packages (nonSemverVersion) and `name@version` otherwise.
|
||||
const pkgId = pkgInfo.nonSemverVersion ?? `${pkgInfo.name}@${pkgInfo.version}`
|
||||
if (opts.skipIfHasSideEffectsCache && resolution.integrity) {
|
||||
const filesIndexFile = storeIndexKey(resolution.integrity!.toString(), pkgId)
|
||||
const pkgFilesIndex = storeIndex!.get(filesIndexFile) as PackageFilesIndex | undefined
|
||||
|
||||
@@ -15,6 +15,13 @@ export interface PkgNameVersion {
|
||||
export interface FetchOptions {
|
||||
allowBuild?: AllowBuild
|
||||
filesIndexFile: string
|
||||
/**
|
||||
* Resolution id of the package being fetched. The git-hosted tarball fetcher
|
||||
* uses this to register a second store index entry under
|
||||
* `storeIndexKey(integrity, pkgId)` once the integrity has been computed,
|
||||
* so subsequent integrity-based lookups can find the package.
|
||||
*/
|
||||
pkgId?: string
|
||||
lockfileDir: string
|
||||
onStart?: (totalSize: number | null, attempt: number) => void
|
||||
onProgress?: (downloaded: number) => void
|
||||
|
||||
@@ -6,7 +6,7 @@ import type { FetchFunction, FetchOptions } from '@pnpm/fetching.fetcher-base'
|
||||
import { packlist } from '@pnpm/fs.packlist'
|
||||
import { globalWarn } from '@pnpm/logger'
|
||||
import type { Cafs, FilesMap } from '@pnpm/store.cafs-types'
|
||||
import type { StoreIndex } from '@pnpm/store.index'
|
||||
import { type StoreIndex, storeIndexKey } from '@pnpm/store.index'
|
||||
import type { BundledManifest } from '@pnpm/types'
|
||||
import { addFilesFromDir } from '@pnpm/worker'
|
||||
|
||||
@@ -38,6 +38,22 @@ export function createGitHostedTarballFetcher (fetchRemoteTarball: FetchFunction
|
||||
if (prepareResult.ignoredBuild) {
|
||||
globalWarn(`The git-hosted package fetched from "${resolution.tarball}" has to be built but the build scripts were ignored.`)
|
||||
}
|
||||
// Register a second index entry under storeIndexKey(integrity, pkgId).
|
||||
// The original filesIndexFile is gitHostedStoreIndexKey-derived (the
|
||||
// resolver doesn't know the integrity yet on a first install), but every
|
||||
// post-fetch reader (pkg-finder, store status, FUSE mounter,
|
||||
// skipIfHasSideEffectsCache in building/after-install) looks the package
|
||||
// up under the integrity key once the lockfile carries integrity. Without
|
||||
// this duplicate, those readers raise ENOENT immediately after install.
|
||||
if (integrity && opts.pkgId) {
|
||||
const integrityKey = storeIndexKey(integrity, opts.pkgId)
|
||||
if (integrityKey !== opts.filesIndexFile) {
|
||||
const data = fetcherOpts.storeIndex.get(opts.filesIndexFile)
|
||||
if (data) {
|
||||
fetcherOpts.storeIndex.set(integrityKey, data)
|
||||
}
|
||||
}
|
||||
}
|
||||
return {
|
||||
filesMap: prepareResult.filesMap,
|
||||
manifest: prepareResult.manifest ?? manifest,
|
||||
|
||||
@@ -554,6 +554,7 @@ function fetchToStore (
|
||||
{
|
||||
allowBuild: opts.allowBuild,
|
||||
filesIndexFile,
|
||||
pkgId: opts.pkg.id,
|
||||
lockfileDir: opts.lockfileDir,
|
||||
readManifest: opts.fetchRawManifest,
|
||||
onProgress: (downloaded) => {
|
||||
|
||||
@@ -176,7 +176,13 @@ export function createFuseHandlersFromLockfile (lockfile: LockfileObject, storeD
|
||||
const pkgSnapshot = lockfile.packages?.[depPath as DepPath]
|
||||
if (pkgSnapshot == null) return undefined
|
||||
const nameVer = nameVerFromPkgSnapshot(depPath, pkgSnapshot)
|
||||
const pkgIndexFilePath = storeIndexKey((pkgSnapshot.resolution as TarballResolution).integrity!, `${nameVer.name}@${nameVer.version}`)
|
||||
// Match the resolver-supplied pkg.id used by the writer in
|
||||
// @pnpm/installing.package-requester: that's the tarball URL for
|
||||
// git-hosted packages (nonSemverVersion) and `name@version` otherwise.
|
||||
const pkgIndexFilePath = storeIndexKey(
|
||||
(pkgSnapshot.resolution as TarballResolution).integrity!,
|
||||
nameVer.nonSemverVersion ?? `${nameVer.name}@${nameVer.version}`
|
||||
)
|
||||
const pkgIndex = storeIndex.get(pkgIndexFilePath) as PackageFilesIndex
|
||||
pkgSnapshotCache.set(depPath, {
|
||||
...nameVer,
|
||||
|
||||
@@ -1,6 +1,5 @@
|
||||
import path from 'node:path'
|
||||
|
||||
import { parse } from '@pnpm/deps.path'
|
||||
import { fetchFromDir } from '@pnpm/fetching.directory-fetcher'
|
||||
import type { Resolution } from '@pnpm/resolving.resolver-base'
|
||||
import { getFilePathByModeInCafs, type PackageFilesIndex } from '@pnpm/store.cafs'
|
||||
@@ -48,10 +47,12 @@ export async function readPackageFileMap (
|
||||
|
||||
let pkgIndexFilePath: string
|
||||
if (isPackageWithIntegrity) {
|
||||
const parsedId = parse(packageId)
|
||||
// The writer in @pnpm/installing.package-requester keys the index file by
|
||||
// the resolution id (`name@version` for npm tarballs, the tarball URL for
|
||||
// git-hosted ones), so we must use the same key here.
|
||||
pkgIndexFilePath = storeIndexKey(
|
||||
packageResolution.integrity as string,
|
||||
parsedId.nonSemverVersion ?? `${parsedId.name}@${parsedId.version}`
|
||||
packageId
|
||||
)
|
||||
} else if (!packageResolution.type && 'tarball' in packageResolution && packageResolution.tarball) {
|
||||
pkgIndexFilePath = gitHostedStoreIndexKey(packageId, { built: true })
|
||||
|
||||
@@ -64,6 +64,29 @@ describe('readPackageFileMap', () => {
|
||||
expect(result!.has('index.js')).toBe(true)
|
||||
})
|
||||
|
||||
it('should resolve git-hosted tarball packages with integrity', async () => {
|
||||
// After we started pinning integrity for git-hosted tarballs, the
|
||||
// packageId here is the URL alone (tryGetPackageId strips the `name@`
|
||||
// because the URL contains `:`). The reader must therefore key by the
|
||||
// URL — the same key the writer used.
|
||||
const integrity = 'sha512-abc123gitHosted'
|
||||
const pkgId = 'https://codeload.github.com/stevemao/left-pad/tar.gz/cafe1234'
|
||||
const key = storeIndexKey(integrity, pkgId)
|
||||
|
||||
storeIndex.set(key, createFilesIndex())
|
||||
|
||||
const resolution: TarballResolution = {
|
||||
integrity,
|
||||
tarball: pkgId,
|
||||
}
|
||||
|
||||
const result = await readPackageFileMap(resolution, pkgId, defaultOpts())
|
||||
|
||||
expect(result).toBeDefined()
|
||||
expect(result!.has('package.json')).toBe(true)
|
||||
expect(result!.has('index.js')).toBe(true)
|
||||
})
|
||||
|
||||
it('should resolve git-hosted tarball packages (no type, has tarball)', async () => {
|
||||
const pkgId = 'left-pad@https://codeload.github.com/stevemao/left-pad/tar.gz/abc123'
|
||||
const key = gitHostedStoreIndexKey(pkgId, { built: true })
|
||||
|
||||
Reference in New Issue
Block a user