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

feat: prefer-symlinked-executables #5048

Merged
merged 3 commits into from
Jul 18, 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
16 changes: 16 additions & 0 deletions .changeset/dirty-mirrors-occur.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
---
"@pnpm/build-modules": minor
"@pnpm/config": minor
"@pnpm/core": minor
"@pnpm/headless": minor
"@pnpm/hoist": minor
"@pnpm/link-bins": minor
"pnpm": minor
---

A new setting supported: `prefer-symlinked-executables`. When `true`, pnpm will create symlinks to executables in
`node_modules/.bin` instead of command shims (but on POSIX systems only).

This setting is `true` by default when `node-linker` is set to `hoisted`.

Related issue: [#4782](https://github.com/pnpm/pnpm/issues/4782).
14 changes: 12 additions & 2 deletions packages/build-modules/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export default async (
ignoreScripts?: boolean
lockfileDir: string
optional: boolean
preferSymlinkedExecutables?: boolean
rawConfig: object
unsafePerm: boolean
userAgent: string
Expand Down Expand Up @@ -70,6 +71,7 @@ async function buildDependency (
ignoreScripts?: boolean
lockfileDir: string
optional: boolean
preferSymlinkedExecutables?: boolean
rawConfig: object
rootModulesDir: string
scriptsPrependNodePath?: boolean | 'warn-only'
Expand Down Expand Up @@ -168,6 +170,7 @@ export async function linkBinsOfDependencies (
opts: {
extraNodePaths?: string[]
optional: boolean
preferSymlinkedExecutables?: boolean
warn: (message: string) => void
}
) {
Expand Down Expand Up @@ -204,11 +207,18 @@ export async function linkBinsOfDependencies (
}))
)

await linkBinsOfPackages(pkgs, binPath, { extraNodePaths: opts.extraNodePaths })
await linkBinsOfPackages(pkgs, binPath, {
extraNodePaths: opts.extraNodePaths,
preferSymlinkedExecutables: opts.preferSymlinkedExecutables,
})

// link also the bundled dependencies` bins
if (depNode.hasBundledDependencies) {
const bundledModules = path.join(depNode.dir, 'node_modules')
await linkBins(bundledModules, binPath, { extraNodePaths: opts.extraNodePaths, warn: opts.warn })
await linkBins(bundledModules, binPath, {
extraNodePaths: opts.extraNodePaths,
preferSymlinkedExecutables: opts.preferSymlinkedExecutables,
warn: opts.warn,
})
}
}
1 change: 1 addition & 0 deletions packages/config/src/Config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ export interface Config {
useNodeVersion?: string
useStderr?: boolean
nodeLinker?: 'hoisted' | 'isolated' | 'pnp'
preferSymlinkedExecutables?: boolean

// proxy
httpProxy?: string
Expand Down
12 changes: 11 additions & 1 deletion packages/config/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ export const types = Object.assign({
pnpmfile: String,
'prefer-frozen-lockfile': Boolean,
'prefer-offline': Boolean,
'prefer-symlinked-executables': Boolean,
'prefer-workspace-packages': Boolean,
production: [null, true],
'public-hoist-pattern': Array,
Expand Down Expand Up @@ -458,7 +459,16 @@ export default async (
if (!pnpmConfig.noProxy) {
pnpmConfig.noProxy = pnpmConfig['noproxy'] ?? getProcessEnv('no_proxy')
}
pnpmConfig.enablePnp = pnpmConfig.nodeLinker === 'pnp'
switch (pnpmConfig.nodeLinker) {
case 'pnp':
pnpmConfig.enablePnp = pnpmConfig.nodeLinker === 'pnp'
break
case 'hoisted':
if (pnpmConfig.preferSymlinkedExecutables == null) {
pnpmConfig.preferSymlinkedExecutables = true
}
break
}
if (!pnpmConfig.userConfig) {
pnpmConfig.userConfig = npmConfig.sources.user?.data
}
Expand Down
15 changes: 15 additions & 0 deletions packages/config/test/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -897,3 +897,18 @@ test('getConfig() sets merge-git-branch-lockfiles when branch matches merge-git-
expect(config.mergeGitBranchLockfiles).toBe(true)
}
})

test('preferSymlinkedExecutables should be true when nodeLinker is hoisted', async () => {
prepareEmpty()

const { config } = await getConfig({
cliOptions: {
'node-linker': 'hoisted',
},
packageManager: {
name: 'pnpm',
version: '1.0.0',
},
})
expect(config.preferSymlinkedExecutables).toBeTruthy()
})
1 change: 1 addition & 0 deletions packages/core/src/install/extendInstallOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ export interface StrictInstallOptions {
modulesCacheMaxAge: number
peerDependencyRules: PeerDependencyRules
allowedDeprecatedVersions: AllowedDeprecatedVersions
preferSymlinkedExecutables: boolean

publicHoistPattern: string[] | undefined
hoistPattern: string[] | undefined
Expand Down
4 changes: 4 additions & 0 deletions packages/core/src/install/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -937,6 +937,7 @@ const _installInContext: InstallFunction = async (projects, ctx, opts) => {
ignoreScripts: opts.ignoreScripts,
lockfileDir: ctx.lockfileDir,
optional: opts.include.optionalDependencies,
preferSymlinkedExecutables: opts.preferSymlinkedExecutables,
rawConfig: opts.rawConfig,
rootModulesDir: ctx.virtualStoreDir,
scriptsPrependNodePath: opts.scriptsPrependNodePath,
Expand Down Expand Up @@ -972,6 +973,7 @@ const _installInContext: InstallFunction = async (projects, ctx, opts) => {
}, {})
linkedPackages = await linkBins(project.modulesDir, project.binsDir, {
allowExoticManifests: true,
preferSymlinkedExecutables: opts.preferSymlinkedExecutables,
projectManifest: project.manifest,
nodeExecPathByAlias,
extraNodePaths: ctx.extraNodePaths,
Expand Down Expand Up @@ -1009,6 +1011,7 @@ const _installInContext: InstallFunction = async (projects, ctx, opts) => {
project.binsDir,
{
extraNodePaths: ctx.extraNodePaths,
preferSymlinkedExecutables: opts.preferSymlinkedExecutables,
}
)
}
Expand Down Expand Up @@ -1170,6 +1173,7 @@ async function linkAllBins (
depGraph: DependenciesGraph,
opts: {
extraNodePaths?: string[]
preferSymlinkedExecutables?: boolean
optional: boolean
warn: (message: string) => void
}
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/link/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ export default async function link (
const linkToBin = maybeOpts?.linkToBin ?? path.join(destModules, '.bin')
await linkBinsOfPackages(linkedPkgs.map((p) => ({ manifest: p.manifest, location: p.path })), linkToBin, {
extraNodePaths: ctx.extraNodePaths,
preferSymlinkedExecutables: opts.preferSymlinkedExecutables,
})

let newPkg!: ProjectManifest
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/link/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ interface StrictLinkOptions {
reporter: ReporterFunction
targetDependenciesField?: DependenciesField
dir: string
preferSymlinkedExecutables: boolean

hoistPattern: string[] | undefined
forceHoistPattern: boolean
Expand Down
14 changes: 14 additions & 0 deletions packages/core/test/install/misc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,20 @@ test('bin specified in the directories property linked to .bin folder', async ()
await project.isExecutable('.bin/pkg-with-directories-bin')
})

test('bin specified in the directories property symlinked to .bin folder when prefer-symlinked-executables is true on POSIX', async () => {
const project = prepareEmpty()

const opts = await testDefaults({ fastUnpack: false, preferSymlinkedExecutables: true })
await addDependenciesToPackage({}, ['pkg-with-directories-bin'], opts)

await project.isExecutable('.bin/pkg-with-directories-bin')

if (!isWindows()) {
const link = await fs.readlink('node_modules/.bin/pkg-with-directories-bin')
expect(link).toBeTruthy()
}
})

testOnNonWindows('building native addons', async () => {
const project = prepareEmpty()

Expand Down
32 changes: 27 additions & 5 deletions packages/headless/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ export interface HeadlessOptions {
engineStrict: boolean
extraBinPaths?: string[]
extraNodePaths?: string[]
preferSymlinkedExecutables?: boolean
hoistingLimits?: HoistingLimits
ignoreScripts: boolean
ignorePackageManifest?: boolean
Expand Down Expand Up @@ -304,6 +305,7 @@ export default async (opts: HeadlessOptions) => {
force: opts.force,
ignoreScripts: opts.ignoreScripts,
lockfileDir: opts.lockfileDir,
preferSymlinkedExecutables: opts.preferSymlinkedExecutables,
sideEffectsCacheRead: opts.sideEffectsCacheRead,
})
stageLogger.debug({
Expand Down Expand Up @@ -355,6 +357,7 @@ export default async (opts: HeadlessOptions) => {
extraNodePath: opts.extraNodePaths,
lockfile: hoistLockfile,
importerIds,
preferSymlinkedExecutables: opts.preferSymlinkedExecutables,
privateHoistedModulesDir: hoistedModulesDir,
privateHoistPattern: opts.hoistPattern ?? [],
publicHoistedModulesDir,
Expand All @@ -368,6 +371,7 @@ export default async (opts: HeadlessOptions) => {
await linkAllBins(graph, {
extraNodePaths: opts.extraNodePaths,
optional: opts.include.optionalDependencies,
preferSymlinkedExecutables: opts.preferSymlinkedExecutables,
warn,
})

Expand Down Expand Up @@ -433,6 +437,7 @@ export default async (opts: HeadlessOptions) => {
ignoreScripts: opts.ignoreScripts,
lockfileDir,
optional: opts.include.optionalDependencies,
preferSymlinkedExecutables: opts.preferSymlinkedExecutables,
rawConfig: opts.rawConfig,
rootModulesDir: virtualStoreDir,
scriptsPrependNodePath: opts.scriptsPrependNodePath,
Expand All @@ -455,7 +460,10 @@ export default async (opts: HeadlessOptions) => {
if (!opts.ignorePackageManifest) {
await Promise.all(opts.projects.map(async (project) => {
if (opts.publicHoistPattern?.length && path.relative(opts.lockfileDir, project.rootDir) === '') {
await linkBinsOfImporter(project, { extraNodePaths: opts.extraNodePaths })
await linkBinsOfImporter(project, {
extraNodePaths: opts.extraNodePaths,
preferSymlinkedExecutables: opts.preferSymlinkedExecutables,
})
} else {
const directPkgDirs = Object.values(directDependenciesByImporterId[project.id])
await linkBinsOfPackages(
Expand All @@ -471,6 +479,7 @@ export default async (opts: HeadlessOptions) => {
project.binsDir,
{
extraNodePaths: opts.extraNodePaths,
preferSymlinkedExecutables: opts.preferSymlinkedExecutables,
}
)
}
Expand Down Expand Up @@ -568,12 +577,13 @@ async function linkBinsOfImporter (
modulesDir: string
rootDir: string
},
{ extraNodePaths }: { extraNodePaths?: string[] } = {}
{ extraNodePaths, preferSymlinkedExecutables }: { extraNodePaths?: string[], preferSymlinkedExecutables?: boolean } = {}
) {
const warn = (message: string) => logger.info({ message, prefix: rootDir })
return linkBins(modulesDir, binsDir, {
extraNodePaths,
allowExoticManifests: true,
preferSymlinkedExecutables,
projectManifest: manifest,
warn,
})
Expand Down Expand Up @@ -725,6 +735,7 @@ async function linkAllBins (
opts: {
extraNodePaths?: string[]
optional: boolean
preferSymlinkedExecutables?: boolean
warn: (message: string) => void
}
) {
Expand All @@ -745,7 +756,11 @@ async function linkAllBins (
const pkgSnapshots = props<string, DependenciesGraphNode>(Object.values(childrenToLink), depGraph)

if (pkgSnapshots.includes(undefined as any)) { // eslint-disable-line
await linkBins(depNode.modules, binPath, { extraNodePaths: opts.extraNodePaths, warn: opts.warn })
await linkBins(depNode.modules, binPath, {
extraNodePaths: opts.extraNodePaths,
preferSymlinkedExecutables: opts.preferSymlinkedExecutables,
warn: opts.warn,
})
} else {
const pkgs = await Promise.all(
pkgSnapshots
Expand All @@ -756,13 +771,20 @@ async function linkAllBins (
}))
)

await linkBinsOfPackages(pkgs, binPath, { extraNodePaths: opts.extraNodePaths })
await linkBinsOfPackages(pkgs, binPath, {
extraNodePaths: opts.extraNodePaths,
preferSymlinkedExecutables: opts.preferSymlinkedExecutables,
})
}

// link also the bundled dependencies` bins
if (depNode.hasBundledDependencies) {
const bundledModules = path.join(depNode.dir, 'node_modules')
await linkBins(bundledModules, binPath, { extraNodePaths: opts.extraNodePaths, warn: opts.warn })
await linkBins(bundledModules, binPath, {
extraNodePaths: opts.extraNodePaths,
preferSymlinkedExecutables: opts.preferSymlinkedExecutables,
warn: opts.warn,
})
}
}))
)
Expand Down
3 changes: 3 additions & 0 deletions packages/headless/src/linkHoistedModules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export default async function linkHoistedModules (
force: boolean
ignoreScripts: boolean
lockfileDir: string
preferSymlinkedExecutables?: boolean
sideEffectsCacheRead: boolean
}
): Promise<void> {
Expand Down Expand Up @@ -85,6 +86,7 @@ async function linkAllPkgsInOrder (
force: boolean
ignoreScripts: boolean
lockfileDir: string
preferSymlinkedExecutables?: boolean
sideEffectsCacheRead: boolean
warn: (message: string) => void
}
Expand Down Expand Up @@ -130,6 +132,7 @@ async function linkAllPkgsInOrder (
const binsDir = path.join(modulesDir, '.bin')
await linkBins(modulesDir, binsDir, {
allowExoticManifests: true,
preferSymlinkedExecutables: opts.preferSymlinkedExecutables,
warn: opts.warn,
})
}
14 changes: 12 additions & 2 deletions packages/hoist/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const hoistLogger = logger('hoist')
export default async function hoistByLockfile (
opts: {
extraNodePath?: string[]
preferSymlinkedExecutables?: boolean
lockfile: Lockfile
importerIds?: string[]
privateHoistPattern: string[]
Expand Down Expand Up @@ -65,7 +66,10 @@ export default async function hoistByLockfile (
// the bins of the project's direct dependencies.
// This is possible because the publicly hoisted modules
// are in the same directory as the regular dependencies.
await linkAllBins(opts.privateHoistedModulesDir, { extraNodePaths: opts.extraNodePath })
await linkAllBins(opts.privateHoistedModulesDir, {
extraNodePaths: opts.extraNodePath,
preferSymlinkedExecutables: opts.preferSymlinkedExecutables,
})

return hoistedDependencies
}
Expand All @@ -85,7 +89,12 @@ function createGetAliasHoistType (
}
}

async function linkAllBins (modulesDir: string, opts: { extraNodePaths?: string[] }) {
interface LinkAllBinsOptions {
extraNodePaths?: string[]
preferSymlinkedExecutables?: boolean
}

async function linkAllBins (modulesDir: string, opts: LinkAllBinsOptions) {
const bin = path.join(modulesDir, '.bin')
const warn: WarnFunction = (message, code) => {
if (code === 'BINARIES_CONFLICT') return
Expand All @@ -95,6 +104,7 @@ async function linkAllBins (modulesDir: string, opts: { extraNodePaths?: string[
await linkBins(modulesDir, bin, {
allowExoticManifests: true,
extraNodePaths: opts.extraNodePaths,
preferSymlinkedExecutables: opts.preferSymlinkedExecutables,
warn,
})
} catch (err: any) { // eslint-disable-line
Expand Down
3 changes: 2 additions & 1 deletion packages/link-bins/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@
"is-windows": "^1.0.2",
"normalize-path": "^3.0.0",
"p-settle": "^4.1.1",
"ramda": "^0.28.0"
"ramda": "^0.28.0",
"symlink-dir": "^5.0.1"
},
"devDependencies": {
"@pnpm/link-bins": "workspace:*",
Expand Down