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

close #4822

.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).

packages/build-modules/src/buildSequence.ts

@@ -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

packages/build-modules/src/index.ts

@@ -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'
@@ -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) ?? {},
     }))
   )
 

packages/cafs/package.json

@@ -32,10 +32,12 @@
   },
   "devDependencies": {
     "@pnpm/cafs": "workspace:3.0.16",
+    "@pnpm/types": "workspace:7.10.0",
     "@types/concat-stream": "^1.6.1",
     "@types/node": "^14.18.20",
     "@types/ssri": "^7.1.1",
     "@types/tar-stream": "^2.2.2",
+    "p-defer": "^3.0.0",
     "tempy": "^1.0.1"
   },
   "bugs": {

packages/cafs/src/addFilesFromDir.ts

@@ -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
 }
 

packages/cafs/src/addFilesFromTarball.ts

@@ -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
 }

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, {
@@ -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
@@ -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)
   })
 })
 

packages/cafs/tsconfig.json

@@ -17,6 +17,9 @@
     },
     {
       "path": "../store-controller-types"
+    },
+    {
+      "path": "../types"
     }
   ]
 }

packages/core/test/install/fromRepo.ts

@@ -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',
+  })
+})

packages/fetcher-base/src/index.ts

@@ -45,7 +45,7 @@ export interface FetchOptions {
 }
 
 export interface DeferredManifestPromise {
-  resolve: (manifest: DependencyManifest) => void
+  resolve: (manifest: DependencyManifest | undefined) => void
   reject: (err: Error) => void
 }
 

packages/package-requester/src/packageRequester.ts

@@ -30,6 +30,7 @@ import {
 } from '@pnpm/resolver-base'
 import {
   BundledManifest,
+  BundledManifestFunction,
   FetchPackageToStoreFunction,
   FetchPackageToStoreOptions,
   PackageResponse,
@@ -279,7 +280,7 @@ async function resolveAndFetch (
 }
 
 interface FetchLock {
-  bundledManifest?: Promise<BundledManifest>
+  bundledManifest?: Promise<BundledManifest | undefined>
   files: Promise<PackageFilesResponse>
   filesIndexFile: string
   finishing: Promise<void>
@@ -305,7 +306,7 @@ function fetchToStore (
   },
   opts: FetchPackageToStoreOptions
 ): {
-    bundledManifest?: () => Promise<BundledManifest>
+    bundledManifest?: BundledManifestFunction
     filesIndexFile: string
     files: () => Promise<PackageFilesResponse>
     finishing: () => Promise<void>
@@ -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)
@@ -439,7 +441,7 @@ function fetchToStore (
 
         if ((pkgFilesIndex?.files) != null) {
           const manifest = opts.fetchRawManifest
-            ? safeDeferredPromise<DependencyManifest>()
+            ? safeDeferredPromise<DependencyManifest | undefined>()
             : undefined
           if (
             (
@@ -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)
@@ -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(
@@ -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()

packages/resolve-dependencies/src/index.ts

@@ -244,7 +244,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)

packages/resolve-dependencies/src/resolveDependencies.ts

@@ -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
@@ -717,6 +717,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,
@@ -731,12 +736,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`)
   }
@@ -889,6 +894,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 ?? {})) {

packages/store-controller-types/src/index.ts

@@ -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>
@@ -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