fix: don't build git-hosted dependencies when scripts are ignored (#5897)

close #5876
This commit is contained in:
Zoltan Kochan
2023-01-09 03:59:37 +02:00
committed by GitHub
parent c9d486781e
commit c7b05cd9a4
22 changed files with 182 additions and 14 deletions

View File

@@ -0,0 +1,14 @@
---
"@pnpm/resolve-dependencies": minor
"@pnpm/store-connection-manager": minor
"@pnpm/package-requester": minor
"@pnpm/store-controller-types": minor
"@pnpm/tarball-fetcher": minor
"@pnpm/prepare-package": minor
"@pnpm/git-fetcher": minor
"@pnpm/headless": patch
"@pnpm/client": minor
"@pnpm/core": patch
---
When ignoreScripts=true is passed to the fetcher, do not build git-hosted dependencies.

View File

@@ -0,0 +1,5 @@
---
"pnpm": patch
---
Git-hosted dependencies should not be built, when `ignore-scripts` is set to `true` [#5876](https://github.com/pnpm/pnpm/issues/5876).

View File

@@ -0,0 +1,5 @@
---
"@pnpm/git-fetcher": major
---
Added `@pnpm/logger` to the peer dependencies.

View File

@@ -68,11 +68,6 @@ test('rebuilds dependencies', async () => {
'preinstall',
'install',
'postinstall',
'prepare',
'prepublishOnly',
'preinstall',
'install',
'postinstall',
])
}
})

View File

@@ -16,13 +16,15 @@ const PREPUBLISH_SCRIPTS = [
]
export interface PreparePackageOptions {
ignoreScripts?: boolean
rawConfig: object
unsafePerm?: boolean
}
export async function preparePackage (opts: PreparePackageOptions, pkgDir: string) {
export async function preparePackage (opts: PreparePackageOptions, pkgDir: string): Promise<boolean> {
const manifest = await safeReadPackageJsonFromDir(pkgDir)
if (manifest?.scripts == null || !packageShouldBeBuilt(manifest, pkgDir)) return
if (manifest?.scripts == null || !packageShouldBeBuilt(manifest, pkgDir)) return false
if (opts.ignoreScripts) return true
const pm = (await preferredPM(pkgDir))?.name ?? 'npm'
const execOpts: RunLifecycleHookOptions = {
depPath: `${manifest.name}@${manifest.version}`,
@@ -46,6 +48,7 @@ export async function preparePackage (opts: PreparePackageOptions, pkgDir: strin
throw err
}
await rimraf(path.join(pkgDir, 'node_modules'))
return true
}
function packageShouldBeBuilt (manifest: PackageManifest, pkgDir: string): boolean {

View File

@@ -29,6 +29,9 @@
"url": "https://github.com/pnpm/pnpm/issues"
},
"homepage": "https://github.com/pnpm/pnpm/blob/main/fetching/git-fetcher#readme",
"peerDependencies": {
"@pnpm/logger": "^5.0.0"
},
"dependencies": {
"@pnpm/fetcher-base": "workspace:*",
"@pnpm/prepare-package": "workspace:*",

View File

@@ -1,5 +1,6 @@
import path from 'path'
import type { GitFetcher } from '@pnpm/fetcher-base'
import { globalWarn } from '@pnpm/logger'
import { preparePackage } from '@pnpm/prepare-package'
import rimraf from '@zkochan/rimraf'
import execa from 'execa'
@@ -9,11 +10,17 @@ export interface CreateGitFetcherOptions {
gitShallowHosts?: string[]
rawConfig: object
unsafePerm?: boolean
ignoreScripts?: boolean
}
export function createGitFetcher (createOpts: CreateGitFetcherOptions) {
const allowedHosts = new Set(createOpts?.gitShallowHosts ?? [])
const preparePkg = preparePackage.bind(null, { rawConfig: createOpts.rawConfig, unsafePerm: createOpts.unsafePerm })
const ignoreScripts = createOpts.ignoreScripts ?? false
const preparePkg = preparePackage.bind(null, {
ignoreScripts: createOpts.ignoreScripts,
rawConfig: createOpts.rawConfig,
unsafePerm: createOpts.unsafePerm,
})
const gitFetcher: GitFetcher = async (cafs, resolution, opts) => {
const tempLocation = await cafs.tempDir()
@@ -26,7 +33,10 @@ export function createGitFetcher (createOpts: CreateGitFetcherOptions) {
}
await execGit(['checkout', resolution.commit], { cwd: tempLocation })
try {
await preparePkg(tempLocation)
const shouldBeBuilt = await preparePkg(tempLocation)
if (ignoreScripts && shouldBeBuilt) {
globalWarn(`The git-hosted package fetched from "${resolution.repo}" has to be built but the build scripts were ignored.`)
}
} catch (err: any) { // eslint-disable-line
err.message = `Failed to prepare git-hosted package fetched from "${resolution.repo}": ${err.message}` // eslint-disable-line
throw err

View File

@@ -2,6 +2,7 @@
import path from 'path'
import { createCafsStore } from '@pnpm/create-cafs-store'
import { createGitFetcher } from '@pnpm/git-fetcher'
import { globalWarn } from '@pnpm/logger'
import { DependencyManifest } from '@pnpm/types'
import pDefer from 'p-defer'
import tempy from 'tempy'
@@ -16,8 +17,17 @@ jest.mock('execa', () => {
}
})
jest.mock('@pnpm/logger', () => {
const originalModule = jest.requireActual('@pnpm/logger')
return {
...originalModule,
globalWarn: jest.fn(),
}
})
beforeEach(() => {
(execa as jest.Mock).mockClear()
;(execa as jest.Mock).mockClear()
;(globalWarn as jest.Mock).mockClear()
})
test('fetch', async () => {
@@ -138,6 +148,21 @@ test('fail when preparing a git-hosted package', async () => {
).rejects.toThrow('Failed to prepare git-hosted package fetched from "https://github.com/pnpm-e2e/prepare-script-fails.git": @pnpm.e2e/prepare-script-fails@1.0.0 npm-install: `npm install`')
})
test('do not build the package when scripts are ignored', async () => {
const cafsDir = tempy.directory()
const fetch = createGitFetcher({ ignoreScripts: true, rawConfig: {} }).git
const manifest = pDefer<DependencyManifest>()
const { filesIndex } = await fetch(createCafsStore(cafsDir),
{
commit: '55416a9c468806a935636c0ad0371a14a64df8c9',
repo: 'https://github.com/pnpm-e2e/prepare-script-works.git',
type: 'git',
}, { manifest })
expect(filesIndex['package.json']).toBeTruthy()
expect(filesIndex['prepare.txt']).toBeFalsy()
expect(globalWarn).toHaveBeenCalledWith('The git-hosted package fetched from "https://github.com/pnpm-e2e/prepare-script-works.git" has to be built but the build scripts were ignored.')
})
function prefixGitArgs (): string[] {
return process.platform === 'win32' ? ['-c', 'core.longpaths=true'] : []
}

View File

@@ -1,5 +1,6 @@
import { FetchFunction, FetchOptions } from '@pnpm/fetcher-base'
import type { Cafs, FilesIndex, PackageFileInfo } from '@pnpm/cafs-types'
import { globalWarn } from '@pnpm/logger'
import { preparePackage } from '@pnpm/prepare-package'
import pMapValues from 'p-map-values'
import omit from 'ramda/src/omit'
@@ -11,6 +12,7 @@ interface Resolution {
}
export interface CreateGitHostedTarballFetcher {
ignoreScripts?: boolean
rawConfig: object
unsafePerm?: boolean
}
@@ -19,7 +21,11 @@ export function createGitHostedTarballFetcher (fetchRemoteTarball: FetchFunction
const fetch = async (cafs: Cafs, resolution: Resolution, opts: FetchOptions) => {
const { filesIndex } = await fetchRemoteTarball(cafs, resolution, opts)
try {
return { filesIndex: await prepareGitHostedPkg(filesIndex as FilesIndex, cafs, fetcherOpts) }
const prepareResult = await prepareGitHostedPkg(filesIndex as FilesIndex, cafs, fetcherOpts)
if (prepareResult.ignoredBuild) {
globalWarn(`The git-hosted package fetched from "${resolution.tarball}" has to be built but the build scripts were ignored.`)
}
return { filesIndex: prepareResult.filesIndex }
} catch (err: any) { // eslint-disable-line
err.message = `Failed to prepare git-hosted package fetched from "${resolution.tarball}": ${err.message}` // eslint-disable-line
throw err
@@ -38,12 +44,15 @@ async function prepareGitHostedPkg (filesIndex: FilesIndex, cafs: Cafs, opts: Cr
},
force: true,
})
await preparePackage(opts, tempLocation)
const shouldBeBuilt = await preparePackage(opts, tempLocation)
const newFilesIndex = await cafs.addFilesFromDir(tempLocation)
// Important! We cannot remove the temp location at this stage.
// Even though we have the index of the package,
// the linking of files to the store is in progress.
return newFilesIndex
return {
filesIndex: newFilesIndex,
ignoredBuild: opts.ignoreScripts && shouldBeBuilt,
}
}
export async function waitForFilesIndex (filesIndex: FilesIndex): Promise<Record<string, PackageFileInfo>> {

View File

@@ -33,6 +33,7 @@ export function createTarballFetcher (
opts: {
rawConfig: object
unsafePerm?: boolean
ignoreScripts?: boolean
timeout?: number
retry?: RetryTimeoutOptions
offline?: boolean

View File

@@ -4,6 +4,7 @@ import path from 'path'
import { FetchError, PnpmError } from '@pnpm/error'
import { createFetchFromRegistry } from '@pnpm/fetch'
import { createCafsStore } from '@pnpm/create-cafs-store'
import { globalWarn } from '@pnpm/logger'
import { fixtures } from '@pnpm/test-fixtures'
import {
createTarballFetcher,
@@ -14,6 +15,18 @@ import nock from 'nock'
import ssri from 'ssri'
import tempy from 'tempy'
jest.mock('@pnpm/logger', () => {
const originalModule = jest.requireActual('@pnpm/logger')
return {
...originalModule,
globalWarn: jest.fn(),
}
})
beforeEach(() => {
;(globalWarn as jest.Mock).mockClear()
})
const cafsDir = tempy.directory()
const cafs = createCafsStore(cafsDir)
@@ -394,3 +407,25 @@ test('fail when extracting a broken tarball', async () => {
)
expect(scope.isDone()).toBeTruthy()
})
test('do not build the package when scripts are ignored', async () => {
process.chdir(tempy.directory())
const tarball = 'https://codeload.github.com/pnpm-e2e/prepare-script-works/tar.gz/55416a9c468806a935636c0ad0371a14a64df8c9'
const resolution = { tarball }
const fetch = createTarballFetcher(fetchFromRegistry, getAuthHeader, {
ignoreScripts: true,
rawConfig: {},
retry: {
maxTimeout: 100,
minTimeout: 0,
retries: 1,
},
})
const { filesIndex } = await fetch.gitHostedTarball(cafs, resolution, { lockfileDir: process.cwd() })
expect(filesIndex).toHaveProperty(['package.json'])
expect(filesIndex).not.toHaveProperty(['prepare.txt'])
expect(globalWarn).toHaveBeenCalledWith(`The git-hosted package fetched from "${tarball}" has to be built but the build scripts were ignored.`)
})

View File

@@ -17,6 +17,7 @@ export { ResolveFunction }
export type ClientOptions = {
authConfig: Record<string, string>
customFetchers?: CustomFetchers
ignoreScripts?: boolean
rawConfig: object
retry?: RetryTimeoutOptions
timeout?: number

View File

@@ -813,6 +813,7 @@ const _installInContext: InstallFunction = async (projects, ctx, opts) => {
engineStrict: opts.engineStrict,
force: opts.force,
forceFullResolution,
ignoreScripts: opts.ignoreScripts,
hooks: {
readPackage: opts.readPackageHook,
},

View File

@@ -5,6 +5,7 @@ import {
addDependenciesToPackage,
install,
} from '@pnpm/core'
import rimraf from '@zkochan/rimraf'
import { isCI } from 'ci-info'
import exists from 'path-exists'
import sinon from 'sinon'
@@ -173,3 +174,47 @@ test('from a github repo the has no package.json file', async () => {
'for-testing.no-package-json': 'github:pnpm/for-testing.no-package-json',
})
})
test('from a github repo that needs to be built. isolated node linker is used', async () => {
const project = prepareEmpty()
const manifest = await addDependenciesToPackage({}, ['pnpm-e2e/prepare-script-works'], await testDefaults({ ignoreScripts: true }, { ignoreScripts: true }))
await project.hasNot('@pnpm.e2e/prepare-script-works/prepare.txt')
await rimraf('node_modules')
await install(manifest, await testDefaults({ preferFrozenLockfile: false }))
await project.has('@pnpm.e2e/prepare-script-works/prepare.txt')
await rimraf('node_modules')
await install(manifest, await testDefaults({ frozenLockfile: true }))
await project.has('@pnpm.e2e/prepare-script-works/prepare.txt')
await rimraf('node_modules')
await install(manifest, await testDefaults({ frozenLockfile: true, ignoreScripts: true }, { ignoreScripts: true }))
await project.hasNot('@pnpm.e2e/prepare-script-works/prepare.txt')
})
test('from a github repo that needs to be built. hoisted node linker is used', async () => {
const project = prepareEmpty()
const manifest = await addDependenciesToPackage(
{},
['pnpm-e2e/prepare-script-works'],
await testDefaults({ ignoreScripts: true, nodeLinker: 'hoisted' }, { ignoreScripts: true })
)
await project.hasNot('@pnpm.e2e/prepare-script-works/prepare.txt')
await rimraf('node_modules')
await install(manifest, await testDefaults({ preferFrozenLockfile: false, nodeLinker: 'hoisted' }))
await project.has('@pnpm.e2e/prepare-script-works/prepare.txt')
await rimraf('node_modules')
await install(manifest, await testDefaults({ frozenLockfile: true, nodeLinker: 'hoisted' }))
await project.has('@pnpm.e2e/prepare-script-works/prepare.txt')
await rimraf('node_modules')
await install(manifest, await testDefaults({ frozenLockfile: true, ignoreScripts: true, nodeLinker: 'hoisted' }, { ignoreScripts: true }))
await project.hasNot('@pnpm.e2e/prepare-script-works/prepare.txt')
})

View File

@@ -56,6 +56,7 @@ export interface LockfileToDepGraphOptions {
force: boolean
importerIds: string[]
include: IncludedDependencies
ignoreScripts: boolean
lockfileDir: string
nodeVersion: string
pnpmVersion: string
@@ -148,6 +149,7 @@ export async function lockfileToDepGraph (
fetchResponse = opts.storeController.fetchPackage({
force: false,
lockfileDir: opts.lockfileDir,
ignoreScripts: opts.ignoreScripts,
pkg: {
id: packageId,
resolution,

View File

@@ -32,6 +32,7 @@ export interface LockfileToHoistedDepGraphOptions {
externalDependencies?: Set<string>
importerIds: string[]
include: IncludedDependencies
ignoreScripts: boolean
currentHoistedLocations?: Record<string, string[]>
lockfileDir: string
nodeVersion: string
@@ -188,6 +189,7 @@ async function fetchDeps (
fetchResponse = opts.storeController.fetchPackage({
force: false,
lockfileDir: opts.lockfileDir,
ignoreScripts: opts.ignoreScripts,
pkg: {
id: packageId,
resolution,

View File

@@ -257,6 +257,7 @@ async function resolveAndFetch (
const fetchResult = ctx.fetchPackageToStore({
fetchRawManifest: true,
force: forceFetch,
ignoreScripts: options.ignoreScripts,
lockfileDir: options.lockfileDir,
pkg: {
...pkg,
@@ -331,7 +332,7 @@ function fetchToStore (
const finishing = pDefer<undefined>()
const filesIndexFile = opts.pkg.resolution['integrity']
? ctx.getFilePathInCafs(opts.pkg.resolution['integrity'], 'index')
: path.join(target, 'integrity.json')
: path.join(target, opts.ignoreScripts ? 'integrity-not-built.json' : 'integrity.json')
doFetchToStore(filesIndexFile, bundledManifest, files, finishing) // eslint-disable-line

View File

@@ -135,6 +135,7 @@ export interface ResolutionContext {
defaultTag: string
dryRun: boolean
forceFullResolution: boolean
ignoreScripts?: boolean
resolvedPackagesByDepPath: ResolvedPackagesByDepPath
outdatedDependencies: { [pkgId: string]: string }
childrenByParentDepPath: ChildrenByParentDepPath
@@ -1044,6 +1045,7 @@ async function resolveDependency (
: undefined,
expectedPkg: currentPkg,
defaultTag: ctx.defaultTag,
ignoreScripts: ctx.ignoreScripts,
publishedBy: options.publishedBy,
pickLowestVersion: options.pickLowestVersion,
downloadPriority: -options.currentDepth,

View File

@@ -67,6 +67,7 @@ export interface ResolveDependenciesOptions {
engineStrict: boolean
force: boolean
forceFullResolution: boolean
ignoreScripts?: boolean
hooks: {
readPackage?: ReadPackageHook
}
@@ -105,6 +106,7 @@ export async function resolveDependencyTree<T> (
engineStrict: opts.engineStrict,
force: opts.force,
forceFullResolution: opts.forceFullResolution,
ignoreScripts: opts.ignoreScripts,
linkWorkspacePackagesDepth: opts.linkWorkspacePackagesDepth ?? -1,
lockfileDir: opts.lockfileDir,
nodeVersion: opts.nodeVersion,

3
pnpm-lock.yaml generated
View File

@@ -1333,6 +1333,9 @@ importers:
'@pnpm/fetcher-base':
specifier: workspace:*
version: link:../fetcher-base
'@pnpm/logger':
specifier: ^5.0.0
version: 5.0.0
'@pnpm/prepare-package':
specifier: workspace:*
version: link:../../exec/prepare-package

View File

@@ -22,6 +22,7 @@ export type CreateNewStoreControllerOptions = CreateResolverOptions & Pick<Confi
| 'nodeVersion'
| 'fetchTimeout'
| 'gitShallowHosts'
| 'ignoreScripts'
| 'hooks'
| 'httpProxy'
| 'httpsProxy'
@@ -60,6 +61,7 @@ export async function createNewStoreController (
filterMetadata: fullMetadata,
httpProxy: opts.httpProxy,
httpsProxy: opts.httpsProxy,
ignoreScripts: opts.ignoreScripts,
key: opts.key,
localAddress: opts.localAddress,
noProxy: opts.noProxy,

View File

@@ -68,6 +68,7 @@ export interface PkgNameVersion {
export interface FetchPackageToStoreOptions {
fetchRawManifest?: boolean
force: boolean
ignoreScripts?: boolean
lockfileDir: string
pkg: PkgNameVersion & {
id: string
@@ -98,6 +99,7 @@ export interface RequestPackageOptions {
pickLowestVersion?: boolean
publishedBy?: Date
downloadPriority: number
ignoreScripts?: boolean
projectDir: string
lockfileDir: string
preferredVersions: PreferredVersions