Skip to content

Commit

Permalink
fix: run prepublish scripts of packages installed from Git (#5837)
Browse files Browse the repository at this point in the history
close #5826
  • Loading branch information
zkochan committed Dec 25, 2022
1 parent 0048e0e commit 339c0a7
Show file tree
Hide file tree
Showing 25 changed files with 166 additions and 37 deletions.
8 changes: 8 additions & 0 deletions .changeset/fresh-taxis-own.md
@@ -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
@@ -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
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
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
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
@@ -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] !== '')
}
@@ -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"
}
}
Empty file.
16 changes: 16 additions & 0 deletions exec/prepare-package/test/index.ts
@@ -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
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
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
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
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
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
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
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
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
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
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
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

0 comments on commit 339c0a7

Please sign in to comment.