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: run prepublish scripts of packages installed from Git #5837

Merged
merged 14 commits into from
Dec 25, 2022
8 changes: 8 additions & 0 deletions .changeset/fresh-taxis-own.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
"@pnpm/prepare-package": major
"@pnpm/git-fetcher": major
"@pnpm/tarball-fetcher": major
"@pnpm/client": major
---

A new required option added to the prepare package function: rawConfig. It is needed in order to create a proper environment for the package manager executed during the preparation of a git-hosted dependency.
6 changes: 6 additions & 0 deletions .changeset/lucky-pugs-sell.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@pnpm/prepare-package": patch
"pnpm": patch
---

Run the prepublish scripts of packages installed from Git [#5826](https://github.com/pnpm/pnpm/issues/5826).
1 change: 1 addition & 0 deletions env/node.fetcher/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export async function fetchNode (fetch: FetchFromRegistry, version: string, targ
}
const getAuthHeader = () => undefined
const fetchers = createTarballFetcher(fetch, getAuthHeader, {
rawConfig: {}, // This is not needed for fetching Node.js
retry: opts.retry,
timeout: opts.fetchTimeout,
})
Expand Down
14 changes: 10 additions & 4 deletions exec/plugin-commands-rebuild/test/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,16 @@ test('rebuilds dependencies', async () => {

{
const scripts = project.requireModule('test-git-fetch/output.json')
expect(scripts[0]).toBe('preinstall')
expect(scripts[1]).toBe('install')
expect(scripts[2]).toBe('postinstall')
expect(scripts[3]).toBe('prepare')
expect(scripts).toStrictEqual([
'preinstall',
'install',
'postinstall',
'prepare',
'prepublishOnly',
'preinstall',
'install',
'postinstall',
])
}
})

Expand Down
18 changes: 13 additions & 5 deletions exec/prepare-package/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,11 @@
"node": ">=14.6"
},
"scripts": {
"lint": "eslint src/**/*.ts",
"test": "pnpm run compile",
"lint": "eslint src/**/*.ts test/**/*.ts",
"test": "pnpm run compile && pnpm run _test",
"prepublishOnly": "pnpm run compile",
"compile": "tsc --build && pnpm run lint --fix"
"compile": "tsc --build && pnpm run lint --fix",
"_test": "jest"
},
"repository": "https://github.com/pnpm/pnpm/blob/main/exec/prepare-package",
"keywords": [
Expand All @@ -29,14 +30,21 @@
"homepage": "https://github.com/pnpm/pnpm/blob/main/exec/prepare-package#readme",
"dependencies": {
"@pnpm/error": "workspace:*",
"@pnpm/npm-lifecycle": "^2.0.0",
"@pnpm/read-package-json": "workspace:*",
"@zkochan/rimraf": "^2.1.2",
"execa": "npm:safe-execa@^0.1.2",
"preferred-pm": "^3.0.3"
"preferred-pm": "^3.0.3",
"ramda": "npm:@pnpm/ramda@0.28.1"
},
"funding": "https://opencollective.com/pnpm",
"devDependencies": {
"@pnpm/prepare-package": "workspace:*"
"@pnpm/prepare": "workspace:*",
"@pnpm/prepare-package": "workspace:*",
"@pnpm/test-fixtures": "workspace:*",
"@pnpm/types": "workspace:*",
"@types/ramda": "0.28.20",
"load-json-file": "^6.2.0"
},
"exports": {
".": "./lib/index.js"
Expand Down
42 changes: 34 additions & 8 deletions exec/prepare-package/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,45 @@
import path from 'path'
import { PnpmError } from '@pnpm/error'
import lifecycle from '@pnpm/npm-lifecycle'
import { safeReadPackageJsonFromDir } from '@pnpm/read-package-json'
import { PackageScripts } from '@pnpm/types'
import rimraf from '@zkochan/rimraf'
import execa from 'execa'
import preferredPM from 'preferred-pm'
import omit from 'ramda/src/omit'

export async function preparePackage (pkgDir: string) {
const PREPUBLISH_SCRIPTS = [
'prepublish',
'prepublishOnly',
'prepack',
'publish',
'postpublish',
]

export async function preparePackage (opts: { rawConfig: object }, pkgDir: string) {
const manifest = await safeReadPackageJsonFromDir(pkgDir)
if (manifest?.scripts?.prepare != null && manifest.scripts.prepare !== '') {
const pm = (await preferredPM(pkgDir))?.name ?? 'npm'
try {
await execa(pm, ['install'], { cwd: pkgDir })
} catch (err: any) { // eslint-disable-line
throw new PnpmError('PREPARE_PKG_FAILURE', err.shortMessage ?? err.message)
if (manifest?.scripts == null || !packageShouldBeBuilt(manifest.scripts)) return
const pm = (await preferredPM(pkgDir))?.name ?? 'npm'
// We can't prepare a package without running its lifecycle scripts.
// An alternative solution could be to throw an exception.
const config = omit(['ignore-scripts'], opts.rawConfig)
const env = lifecycle.makeEnv(manifest, { config })
const execOpts = { cwd: pkgDir, env, extendEnv: true }
try {
await execa(pm, ['install'], execOpts)
for (const scriptName of PREPUBLISH_SCRIPTS) {
if (manifest.scripts[scriptName] == null || manifest.scripts[scriptName] === '') continue
await execa(pm, ['run', scriptName], execOpts)
}
await rimraf(path.join(pkgDir, 'node_modules'))
} catch (err: any) { // eslint-disable-line
throw new PnpmError('PREPARE_PKG_FAILURE', err.shortMessage ?? err.message)
}
await rimraf(path.join(pkgDir, 'node_modules'))
}

function packageShouldBeBuilt (packageScripts: PackageScripts): boolean {
return [
...PREPUBLISH_SCRIPTS,
'prepare',
].some((scriptName) => packageScripts[scriptName] != null && packageScripts[scriptName] !== '')
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"name": "has-prepublish-script",
"version": "1.0.0",
"scripts": {
"prepublish": "node -e \"process.stdout.write('prepublish')\" | json-append output.json"
},
"devDependencies": {
"json-append": "1.1.1"
}
}
16 changes: 16 additions & 0 deletions exec/prepare-package/test/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import path from 'path'
import { preparePackage } from '@pnpm/prepare-package'
import { tempDir } from '@pnpm/prepare'
import { fixtures } from '@pnpm/test-fixtures'
import { sync as loadJsonFile } from 'load-json-file'

const f = fixtures(__dirname)

test('prepare package runs the prepbublish script', async () => {
const tmp = tempDir()
f.copy('has-prepublish-script', tmp)
await preparePackage({ rawConfig: {} }, tmp)
expect(loadJsonFile(path.join(tmp, 'output.json'))).toStrictEqual([
'prepublish',
])
})
9 changes: 9 additions & 0 deletions exec/prepare-package/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,18 @@
"../../__typings__/**/*.d.ts"
],
"references": [
{
"path": "../../__utils__/prepare"
},
{
"path": "../../__utils__/test-fixtures"
},
{
"path": "../../packages/error"
},
{
"path": "../../packages/types"
},
{
"path": "../../pkg-manifest/read-package-json"
}
Expand Down
5 changes: 3 additions & 2 deletions fetching/git-fetcher/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ import rimraf from '@zkochan/rimraf'
import execa from 'execa'
import { URL } from 'url'

export function createGitFetcher (createOpts?: { gitShallowHosts?: string[] }) {
export function createGitFetcher (createOpts: { gitShallowHosts?: string[], rawConfig: object }) {
const allowedHosts = new Set(createOpts?.gitShallowHosts ?? [])
const preparePkg = preparePackage.bind(null, { rawConfig: createOpts.rawConfig })

const gitFetcher: GitFetcher = async (cafs, resolution, opts) => {
const tempLocation = await cafs.tempDir()
Expand All @@ -18,7 +19,7 @@ export function createGitFetcher (createOpts?: { gitShallowHosts?: string[] }) {
await execGit(['clone', resolution.repo, tempLocation])
}
await execGit(['checkout', resolution.commit], { cwd: tempLocation })
await preparePackage(tempLocation)
await preparePkg(tempLocation)
// removing /.git to make directory integrity calculation faster
await rimraf(path.join(tempLocation, '.git'))
const filesIndex = await cafs.addFilesFromDir(tempLocation, opts.manifest)
Expand Down
10 changes: 5 additions & 5 deletions fetching/git-fetcher/test/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ beforeEach(() => {

test('fetch', async () => {
const cafsDir = tempy.directory()
const fetch = createGitFetcher().git
const fetch = createGitFetcher({ rawConfig: {} }).git
const manifest = pDefer<DependencyManifest>()
const { filesIndex } = await fetch(
createCafsStore(cafsDir),
Expand All @@ -43,7 +43,7 @@ test('fetch', async () => {

test('fetch a package from Git that has a prepare script', async () => {
const cafsDir = tempy.directory()
const fetch = createGitFetcher().git
const fetch = createGitFetcher({ rawConfig: {} }).git
const manifest = pDefer<DependencyManifest>()
const { filesIndex } = await fetch(
createCafsStore(cafsDir),
Expand All @@ -62,7 +62,7 @@ test('fetch a package from Git that has a prepare script', async () => {
// Test case for https://github.com/pnpm/pnpm/issues/1866
test('fetch a package without a package.json', async () => {
const cafsDir = tempy.directory()
const fetch = createGitFetcher().git
const fetch = createGitFetcher({ rawConfig: {} }).git
const manifest = pDefer<DependencyManifest>()
const { filesIndex } = await fetch(
createCafsStore(cafsDir),
Expand All @@ -82,7 +82,7 @@ test('fetch a package without a package.json', async () => {
// Covers the regression reported in https://github.com/pnpm/pnpm/issues/4064
test('fetch a big repository', async () => {
const cafsDir = tempy.directory()
const fetch = createGitFetcher().git
const fetch = createGitFetcher({ rawConfig: {} }).git
const manifest = pDefer<DependencyManifest>()
const { filesIndex } = await fetch(createCafsStore(cafsDir),
{
Expand All @@ -95,7 +95,7 @@ test('fetch a big repository', async () => {

test('still able to shallow fetch for allowed hosts', async () => {
const cafsDir = tempy.directory()
const fetch = createGitFetcher({ gitShallowHosts: ['github.com'] }).git
const fetch = createGitFetcher({ gitShallowHosts: ['github.com'], rawConfig: {} }).git
const manifest = pDefer<DependencyManifest>()
const resolution = {
commit: 'c9b30e71d704cd30fa71f2edd1ecc7dcc4985493',
Expand Down
8 changes: 4 additions & 4 deletions fetching/tarball-fetcher/src/gitHostedTarballFetcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,17 @@ interface Resolution {
tarball: string
}

export function createGitHostedTarballFetcher (fetchRemoteTarball: FetchFunction): FetchFunction {
export function createGitHostedTarballFetcher (fetchRemoteTarball: FetchFunction, rawConfig: object): FetchFunction {
const fetch = async (cafs: Cafs, resolution: Resolution, opts: FetchOptions) => {
const { filesIndex } = await fetchRemoteTarball(cafs, resolution, opts)

return { filesIndex: await prepareGitHostedPkg(filesIndex as FilesIndex, cafs) }
return { filesIndex: await prepareGitHostedPkg(filesIndex as FilesIndex, cafs, rawConfig) }
}

return fetch as FetchFunction
}

async function prepareGitHostedPkg (filesIndex: FilesIndex, cafs: Cafs) {
async function prepareGitHostedPkg (filesIndex: FilesIndex, cafs: Cafs, rawConfig: object) {
const tempLocation = await cafs.tempDir()
await cafs.importPackage(tempLocation, {
filesResponse: {
Expand All @@ -29,7 +29,7 @@ async function prepareGitHostedPkg (filesIndex: FilesIndex, cafs: Cafs) {
},
force: true,
})
await preparePackage(tempLocation)
await preparePackage({ rawConfig }, 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,
Expand Down
3 changes: 2 additions & 1 deletion fetching/tarball-fetcher/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export function createTarballFetcher (
fetchFromRegistry: FetchFromRegistry,
getAuthHeader: GetAuthHeader,
opts: {
rawConfig: object
timeout?: number
retry?: RetryTimeoutOptions
offline?: boolean
Expand All @@ -50,7 +51,7 @@ export function createTarballFetcher (
return {
localTarball: createLocalTarballFetcher(),
remoteTarball: remoteTarballFetcher,
gitHostedTarball: createGitHostedTarballFetcher(remoteTarballFetcher),
gitHostedTarball: createGitHostedTarballFetcher(remoteTarballFetcher, opts.rawConfig),
}
}

Expand Down
4 changes: 4 additions & 0 deletions fetching/tarball-fetcher/test/fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const registry = 'http://example.com/'
const fetchFromRegistry = createFetchFromRegistry({})
const getAuthHeader = () => undefined
const fetch = createTarballFetcher(fetchFromRegistry, getAuthHeader, {
rawConfig: {},
retry: {
maxTimeout: 100,
minTimeout: 0,
Expand Down Expand Up @@ -206,6 +207,7 @@ test("don't fail when fetching a local tarball in offline mode", async () => {

const fetch = createTarballFetcher(fetchFromRegistry, getAuthHeader, {
offline: true,
rawConfig: {},
retry: {
maxTimeout: 100,
minTimeout: 0,
Expand All @@ -230,6 +232,7 @@ test('fail when trying to fetch a non-local tarball in offline mode', async () =

const fetch = createTarballFetcher(fetchFromRegistry, getAuthHeader, {
offline: true,
rawConfig: {},
retry: {
maxTimeout: 100,
minTimeout: 0,
Expand Down Expand Up @@ -321,6 +324,7 @@ test('accessing private packages', async () => {

const getAuthHeader = () => 'Bearer ofjergrg349gj3f2'
const fetch = createTarballFetcher(fetchFromRegistry, getAuthHeader, {
rawConfig: {},
retry: {
maxTimeout: 100,
minTimeout: 0,
Expand Down
3 changes: 2 additions & 1 deletion pkg-manager/client/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export { ResolveFunction }
export type ClientOptions = {
authConfig: Record<string, string>
customFetchers?: CustomFetchers
rawConfig: object
retry?: RetryTimeoutOptions
timeout?: number
userAgent?: string
Expand Down Expand Up @@ -53,7 +54,7 @@ type Fetchers = {
function createFetchers (
fetchFromRegistry: FetchFromRegistry,
getAuthHeader: GetAuthHeader,
opts: Pick<ClientOptions, 'retry' | 'gitShallowHosts' | 'resolveSymlinksInInjectedDirs'>,
opts: Pick<ClientOptions, 'rawConfig' | 'retry' | 'gitShallowHosts' | 'resolveSymlinksInInjectedDirs'>,
customFetchers?: CustomFetchers
): Fetchers {
const defaultFetchers = {
Expand Down
2 changes: 2 additions & 0 deletions pkg-manager/client/test/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ test('createClient()', () => {
const client = createClient({
authConfig: { registry: 'https://registry.npmjs.org/' },
cacheDir: '',
rawConfig: {},
})
expect(typeof client === 'object').toBeTruthy()
})
Expand All @@ -13,6 +14,7 @@ test('createResolver()', () => {
const resolver = createResolver({
authConfig: { registry: 'https://registry.npmjs.org/' },
cacheDir: '',
rawConfig: {},
})
expect(typeof resolver === 'function').toBeTruthy()
})
1 change: 1 addition & 0 deletions pkg-manager/core/test/install/lifecycleScripts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,7 @@ test('run prepare script for git-hosted dependencies', async () => {
'install',
'postinstall',
'prepare',
'prepublishOnly',
'preinstall',
'install',
'postinstall',
Expand Down
1 change: 1 addition & 0 deletions pkg-manager/core/test/utils/testDefaults.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export async function testDefaults<T> (
const cacheDir = path.resolve('cache')
const { resolve, fetchers } = createClient({
authConfig,
rawConfig: {},
retry: retryOpts,
cacheDir,
...resolveOpts,
Expand Down
1 change: 1 addition & 0 deletions pkg-manager/headless/test/utils/testDefaults.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ export async function testDefaults (
const authConfig = { registry }
const { resolve, fetchers } = createClient({
authConfig,
rawConfig: {},
retry: retryOpts,
cacheDir,
...resolveOpts,
Expand Down