Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: resolve manifest promise when manifest is not found #4826

Merged
merged 6 commits into from Jun 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 12 additions & 0 deletions .changeset/seven-shirts-vanish.md
@@ -0,0 +1,12 @@
---
"@pnpm/build-modules": patch
"@pnpm/cafs": patch
"@pnpm/core": patch
"@pnpm/fetcher-base": patch
"@pnpm/package-requester": patch
"@pnpm/resolve-dependencies": patch
"@pnpm/store-controller-types": patch
"pnpm": patch
---

It should be possible to install a git-hosted package that has no `package.json` file [#4822](https://github.com/pnpm/pnpm/issues/4822).
2 changes: 1 addition & 1 deletion packages/build-modules/src/buildSequence.ts
Expand Up @@ -6,7 +6,7 @@ export interface DependenciesGraphNode {
children: {[alias: string]: string}
depPath: string
dir: string
fetchingBundledManifest?: () => Promise<PackageManifest>
fetchingBundledManifest?: () => Promise<PackageManifest | undefined>
filesIndexFile: string
hasBin: boolean
hasBundledDependencies: boolean
Expand Down
4 changes: 2 additions & 2 deletions packages/build-modules/src/index.ts
Expand Up @@ -4,7 +4,7 @@ import { skippedOptionalDependencyLogger } from '@pnpm/core-loggers'
import { runPostinstallHooks } from '@pnpm/lifecycle'
import linkBins, { linkBinsOfPackages } from '@pnpm/link-bins'
import logger from '@pnpm/logger'
import { fromDir as readPackageFromDir } from '@pnpm/read-package-json'
import { fromDir as readPackageFromDir, safeReadPackageFromDir } from '@pnpm/read-package-json'
import { StoreController } from '@pnpm/store-controller-types'
import { DependencyManifest } from '@pnpm/types'
import runGroups from 'run-groups'
Expand Down Expand Up @@ -170,7 +170,7 @@ export async function linkBinsOfDependencies (
const pkgs = await Promise.all(pkgNodes
.map(async (dep) => ({
location: dep.dir,
manifest: await dep.fetchingBundledManifest?.() ?? (await readPackageFromDir(dep.dir) as DependencyManifest),
manifest: await dep.fetchingBundledManifest?.() ?? (await safeReadPackageFromDir(dep.dir) as DependencyManifest) ?? {},
}))
)

Expand Down
2 changes: 2 additions & 0 deletions packages/cafs/package.json
Expand Up @@ -32,10 +32,12 @@
},
"devDependencies": {
"@pnpm/cafs": "workspace:4.0.2",
"@pnpm/types": "workspace:8.0.1",
"@types/concat-stream": "^1.6.0",
"@types/node": "^14.17.32",
"@types/ssri": "^7.1.0",
"@types/tar-stream": "^2.1.0",
"p-defer": "^3.0.0",
"tempy": "^1.0.0"
},
"bugs": {
Expand Down
3 changes: 3 additions & 0 deletions packages/cafs/src/addFilesFromDir.ts
Expand Up @@ -23,6 +23,9 @@ export default async function (
): Promise<FilesIndex> {
const index: FilesIndex = {}
await _retrieveFileIntegrities(cafs, dirname, dirname, index, manifest)
if (manifest && !index['package.json']) {
manifest.resolve(undefined)
}
return index
}

Expand Down
3 changes: 3 additions & 0 deletions packages/cafs/src/addFilesFromTarball.ts
Expand Up @@ -47,5 +47,8 @@ export default async function (
.pipe(decompress() as Duplex)
.on('error', reject).pipe(extract)
})
if (!filesIndex['package.json'] && manifest != null) {
manifest.resolve(undefined)
}
return filesIndex
}
6 changes: 5 additions & 1 deletion packages/cafs/test/index.ts
@@ -1,4 +1,6 @@
import { createReadStream, promises as fs } from 'fs'
import { DependencyManifest } from '@pnpm/types'
import pDefer from 'p-defer'
import path from 'path'
import tempy from 'tempy'
import createCafs, {
Expand All @@ -25,7 +27,8 @@ describe('cafs', () => {
it('replaces an already existing file, if the integrity of it was broken', async () => {
const storeDir = tempy.directory()
const srcDir = path.join(__dirname, 'fixtures/one-file')
const addFiles = async () => createCafs(storeDir).addFilesFromDir(srcDir)
const manifest = pDefer<DependencyManifest>()
const addFiles = async () => createCafs(storeDir).addFilesFromDir(srcDir, manifest)

let filesIndex = await addFiles()
const { integrity } = await filesIndex['foo.txt'].writeResult
Expand All @@ -37,6 +40,7 @@ describe('cafs', () => {
filesIndex = await addFiles()
await filesIndex['foo.txt'].writeResult
expect(await fs.readFile(filePath, 'utf8')).toBe('foo\n')
expect(await manifest.promise).toEqual(undefined)
})
})

Expand Down
3 changes: 3 additions & 0 deletions packages/cafs/tsconfig.json
Expand Up @@ -17,6 +17,9 @@
},
{
"path": "../store-controller-types"
},
{
"path": "../types"
}
]
}
12 changes: 12 additions & 0 deletions packages/core/test/install/fromRepo.ts
Expand Up @@ -162,3 +162,15 @@ test.skip('from a non-github git repo', async () => {
type: 'git',
})
})

test('from a github repo the has no package.json file', async () => {
const project = prepareEmpty()

const manifest = await addDependenciesToPackage({}, ['pnpm/for-testing.no-package-json'], await testDefaults())

await project.has('for-testing.no-package-json')

expect(manifest.dependencies).toStrictEqual({
'for-testing.no-package-json': 'github:pnpm/for-testing.no-package-json',
})
})
2 changes: 1 addition & 1 deletion packages/fetcher-base/src/index.ts
Expand Up @@ -45,7 +45,7 @@ export interface FetchOptions {
}

export interface DeferredManifestPromise {
resolve: (manifest: DependencyManifest) => void
resolve: (manifest: DependencyManifest | undefined) => void
reject: (err: Error) => void
}

Expand Down
16 changes: 9 additions & 7 deletions packages/package-requester/src/packageRequester.ts
Expand Up @@ -30,6 +30,7 @@ import {
} from '@pnpm/resolver-base'
import {
BundledManifest,
BundledManifestFunction,
FetchPackageToStoreFunction,
FetchPackageToStoreOptions,
PackageResponse,
Expand Down Expand Up @@ -279,7 +280,7 @@ async function resolveAndFetch (
}

interface FetchLock {
bundledManifest?: Promise<BundledManifest>
bundledManifest?: Promise<BundledManifest | undefined>
files: Promise<PackageFilesResponse>
filesIndexFile: string
finishing: Promise<void>
Expand All @@ -305,7 +306,7 @@ function fetchToStore (
},
opts: FetchPackageToStoreOptions
): {
bundledManifest?: () => Promise<BundledManifest>
bundledManifest?: BundledManifestFunction
filesIndexFile: string
files: () => Promise<PackageFilesResponse>
finishing: () => Promise<void>
Expand Down Expand Up @@ -385,6 +386,7 @@ function fetchToStore (
if (opts.fetchRawManifest && (result.bundledManifest == null)) {
result.bundledManifest = removeKeyOnFail(
result.files.then(async (filesResult) => {
if (!filesResult.filesIndex['package.json']) return undefined
if (!filesResult.local) {
const { integrity, mode } = filesResult.filesIndex['package.json']
const manifestPath = ctx.getFilePathByModeInCafs(integrity, mode)
Expand Down Expand Up @@ -439,7 +441,7 @@ function fetchToStore (

if ((pkgFilesIndex?.files) != null) {
const manifest = opts.fetchRawManifest
? safeDeferredPromise<DependencyManifest>()
? safeDeferredPromise<DependencyManifest | undefined>()
: undefined
if (
(
Expand Down Expand Up @@ -473,7 +475,7 @@ Actual package in the store by the given integrity: ${pkgFilesIndex.name}@${pkgF
})
if (manifest != null) {
manifest()
.then((manifest) => bundledManifest.resolve(normalizeBundledManifest(manifest)))
.then((manifest) => bundledManifest.resolve(manifest == null ? manifest : normalizeBundledManifest(manifest)))
.catch(bundledManifest.reject)
}
finishing.resolve(undefined)
Expand All @@ -496,11 +498,11 @@ Actual package in the store by the given integrity: ${pkgFilesIndex.name}@${pkgF
const priority = (++ctx.requestsQueue['counter'] % ctx.requestsQueue['concurrency'] === 0 ? -1 : 1) * 1000 // eslint-disable-line

const fetchManifest = opts.fetchRawManifest
? safeDeferredPromise<DependencyManifest>()
? safeDeferredPromise<DependencyManifest | undefined>()
: undefined
if (fetchManifest != null) {
fetchManifest()
.then((manifest) => bundledManifest.resolve(normalizeBundledManifest(manifest)))
.then((manifest) => bundledManifest.resolve(manifest == null ? manifest : normalizeBundledManifest(manifest)))
.catch(bundledManifest.reject)
}
const fetchedPackage = await ctx.requestsQueue.add(async () => ctx.fetch(
Expand Down Expand Up @@ -561,7 +563,7 @@ Actual package in the store by the given integrity: ${pkgFilesIndex.name}@${pkgF
/* eslint-disable @typescript-eslint/no-floating-promises */
bundledManifest.promise
.then((manifest) => writeFilesIndexFile(filesIndexFile, {
pkg: manifest,
pkg: manifest ?? {},
files: integrity,
}))
.catch()
Expand Down
2 changes: 1 addition & 1 deletion packages/resolve-dependencies/src/index.ts
Expand Up @@ -248,7 +248,7 @@ async function finishLockfileUpdates (
// The npm team suggests to always read the package.json for deciding whether the package has lifecycle scripts
const pkgJson = await depNode.fetchingBundledManifest()
depNode.requiresBuild = Boolean(
pkgJson.scripts != null && (
pkgJson?.scripts != null && (
Boolean(pkgJson.scripts.preinstall) ||
Boolean(pkgJson.scripts.install) ||
Boolean(pkgJson.scripts.postinstall)
Expand Down
27 changes: 22 additions & 5 deletions packages/resolve-dependencies/src/resolveDependencies.ts
Expand Up @@ -177,7 +177,7 @@ export interface ResolvedPackage {
dev: boolean
optional: boolean
fetchingFiles: () => Promise<PackageFilesResponse>
fetchingBundledManifest?: () => Promise<DependencyManifest>
fetchingBundledManifest?: () => Promise<DependencyManifest | undefined>
filesIndexFile: string
finishing: () => Promise<void>
name: string
Expand Down Expand Up @@ -718,6 +718,11 @@ async function resolveDependency (

if (pkgResponse.body.isLocal) {
const manifest = pkgResponse.body.manifest ?? await pkgResponse.bundledManifest!() // eslint-disable-line @typescript-eslint/dot-notation
if (!manifest) {
// This should actually never happen because the local-resolver returns a manifest
// even if no real manifest exists in the filesystem.
throw new PnpmError('MISSING_PACKAGE_JSON', `Can't install ${wantedDependency.pref}: Missing package.json file`)
}
return {
alias: wantedDependency.alias || manifest.name,
depPath: pkgResponse.body.id,
Expand All @@ -732,12 +737,12 @@ async function resolveDependency (
}
}

let pkg: PackageManifest
let prepare!: boolean
let hasBin!: boolean
pkg = (ctx.readPackageHook != null)
? await ctx.readPackageHook(pkgResponse.body.manifest ?? await pkgResponse.bundledManifest!())
: pkgResponse.body.manifest ?? await pkgResponse.bundledManifest!()
let pkg: PackageManifest = await getManifestFromResponse(pkgResponse, wantedDependency)
if (ctx.readPackageHook != null) {
pkg = await ctx.readPackageHook(pkg)
}
if (!pkg.name) { // TODO: don't fail on optional dependencies
throw new PnpmError('MISSING_PACKAGE_NAME', `Can't install ${wantedDependency.pref}: Missing package name`)
}
Expand Down Expand Up @@ -890,6 +895,18 @@ async function resolveDependency (
}
}

async function getManifestFromResponse (
pkgResponse: PackageResponse,
wantedDependency: WantedDependency
): Promise<PackageManifest> {
const pkg = pkgResponse.body.manifest ?? await pkgResponse.bundledManifest!()
if (pkg) return pkg
return {
name: wantedDependency.pref.split('/').pop()!,
version: '0.0.0',
}
}

function getMissingPeers (pkg: PackageManifest, parentPkgAliases: ParentPkgAliases): Record<string, string> {
const missingPeers = {} as Record<string, string>
for (const [peerName, peerVersion] of Object.entries(pkg.peerDependencies ?? {})) {
Expand Down
6 changes: 4 additions & 2 deletions packages/store-controller-types/src/index.ts
Expand Up @@ -47,7 +47,7 @@ export interface StoreController {
export type FetchPackageToStoreFunction = (
opts: FetchPackageToStoreOptions
) => {
bundledManifest?: () => Promise<BundledManifest>
bundledManifest?: BundledManifestFunction
filesIndexFile: string
files: () => Promise<PackageFilesResponse>
finishing: () => Promise<void>
Expand Down Expand Up @@ -100,8 +100,10 @@ export interface RequestPackageOptions {
workspacePackages?: WorkspacePackages
}

export type BundledManifestFunction = () => Promise<BundledManifest | undefined>

export interface PackageResponse {
bundledManifest?: () => Promise<BundledManifest>
bundledManifest?: BundledManifestFunction
files?: () => Promise<PackageFilesResponse>
filesIndexFile?: string
finishing?: () => Promise<void> // a package request is finished once its integrity is generated and saved
Expand Down
4 changes: 4 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.