From 751af41ccf3c68c7fe8b81eba024b1d86fdfaf7f Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Sun, 27 Sep 2026 01:19:09 +0200 Subject: [PATCH] fix(path-extender-posix): keep lines between duplicate pnpm sections (#16227) * fix(path-extender-posix): keep lines between duplicate pnpm sections `@pnpm/os.env.path-extender-posix` replaced the pnpm section with a greedy regex, so a shell config holding two `# pnpm` ... `# pnpm end` blocks lost everything between the first opening marker and the last closing one. pnpm/pnpm#15321 already fixed this for `pnpm setup`, but it did so in a copy of the package inside `@pnpm/engine.pm.commands`, because the package still lived in pnpm/components. Now that the package is in this repository, move the fixed code and its tests back into it and let `pnpm setup` use the package again. The copy is deleted. The regression test for the published API is adapted from pnpm/components#29 by Zelys, which proposed a non-greedy regex for the same bug. Closes https://github.com/pnpm/pnpm/issues/12282 Co-authored-by: Zelys Co-Authored-By: Claude Opus 5.5 * fix(path-extender-posix): prefer the block that sets proxyVarName The duplicate-section selection looked for `
_HOME`, which misses the generated block when a caller's `proxyVarName` differs from its section name. Pass `proxyVarName` through to `findSection`. Also declare the section helpers after `updateShellConfig`, document the `null` result, and reword the changeset. Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Zelys Co-authored-by: Claude Opus 5.5 --- ...nder-posix-keeps-lines-between-sections.md | 6 + pnpm-lock.yaml | 12 +- pnpm11/engine/pm/commands/package.json | 6 +- .../pm/commands/src/setup/pathExtender.ts | 20 - .../commands/src/setup/pathExtenderPosix.ts | 346 ------------------ pnpm11/engine/pm/commands/src/setup/setup.ts | 10 +- .../pm/commands/test/setup/setup.test.ts | 4 +- .../os/env/path-extender-posix/package.json | 6 +- .../src/path-extender-posix.ts | 146 ++++++-- .../test/path-extender-posix.test.ts | 34 ++ .../test/sections.test.ts} | 24 +- 11 files changed, 202 insertions(+), 412 deletions(-) create mode 100644 .changeset/path-extender-posix-keeps-lines-between-sections.md delete mode 100644 pnpm11/engine/pm/commands/src/setup/pathExtender.ts delete mode 100644 pnpm11/engine/pm/commands/src/setup/pathExtenderPosix.ts rename pnpm11/{engine/pm/commands/test/setup/pathExtenderPosix.test.ts => os/env/path-extender-posix/test/sections.test.ts} (92%) diff --git a/.changeset/path-extender-posix-keeps-lines-between-sections.md b/.changeset/path-extender-posix-keeps-lines-between-sections.md new file mode 100644 index 0000000000..41fae394c0 --- /dev/null +++ b/.changeset/path-extender-posix-keeps-lines-between-sections.md @@ -0,0 +1,6 @@ +--- +"@pnpm/os.env.path-extender-posix": patch +"@pnpm/os.env.path-extender": patch +--- + +`addDirToPosixEnvPath` now keeps the lines between two `# pnpm` sections of a shell config file when it overwrites the pnpm section [#12282](https://github.com/pnpm/pnpm/issues/12282). diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 78915e05d6..031945e815 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -4178,9 +4178,6 @@ importers: symlink-dir: specifier: 'catalog:' version: 10.0.3 - write-file-atomic: - specifier: 'catalog:' - version: 7.0.1 devDependencies: '@jest/globals': specifier: 'catalog:' @@ -4209,9 +4206,6 @@ importers: '@types/semver': specifier: 'catalog:' version: 7.8.0 - '@types/write-file-atomic': - specifier: 'catalog:' - version: 4.0.3 pnpm11/engine/runtime/bun-resolver: dependencies: @@ -8000,6 +7994,9 @@ importers: '@pnpm/error': specifier: workspace:* version: link:../../../core/error + write-file-atomic: + specifier: 'catalog:' + version: 7.0.1 devDependencies: '@jest/globals': specifier: 'catalog:' @@ -8010,6 +8007,9 @@ importers: '@pnpm/prepare': specifier: workspace:* version: link:../../../__utils__/prepare + '@types/write-file-atomic': + specifier: 'catalog:' + version: 4.0.3 pnpm11/os/env/path-extender-windows: dependencies: diff --git a/pnpm11/engine/pm/commands/package.json b/pnpm11/engine/pm/commands/package.json index 9a8f938653..97030c84f4 100644 --- a/pnpm11/engine/pm/commands/package.json +++ b/pnpm11/engine/pm/commands/package.json @@ -66,8 +66,7 @@ "ramda": "catalog:", "render-help": "catalog:", "semver": "catalog:", - "symlink-dir": "catalog:", - "write-file-atomic": "catalog:" + "symlink-dir": "catalog:" }, "peerDependencies": { "@pnpm/logger": "catalog:" @@ -82,8 +81,7 @@ "@pnpm/testing.mock-agent": "workspace:*", "@types/cross-spawn": "catalog:", "@types/ramda": "catalog:", - "@types/semver": "catalog:", - "@types/write-file-atomic": "catalog:" + "@types/semver": "catalog:" }, "engines": { "node": ">=22.13" diff --git a/pnpm11/engine/pm/commands/src/setup/pathExtender.ts b/pnpm11/engine/pm/commands/src/setup/pathExtender.ts deleted file mode 100644 index 6c1059fc80..0000000000 --- a/pnpm11/engine/pm/commands/src/setup/pathExtender.ts +++ /dev/null @@ -1,20 +0,0 @@ -import { - addDirToEnvPath as addDirToEnvPathExt, - type AddDirToEnvPathOpts, - type ConfigReport, - type PathExtenderReport, -} from '@pnpm/os.env.path-extender' - -import { addDirToPosixEnvPath } from './pathExtenderPosix.js' - -export type { ConfigReport, PathExtenderReport } - -export async function addDirToEnvPath ( - dir: string, - opts: AddDirToEnvPathOpts -): Promise { - if (process.platform === 'win32') { - return addDirToEnvPathExt(dir, opts) - } - return addDirToPosixEnvPath(dir, opts) -} diff --git a/pnpm11/engine/pm/commands/src/setup/pathExtenderPosix.ts b/pnpm11/engine/pm/commands/src/setup/pathExtenderPosix.ts deleted file mode 100644 index 2f869fa753..0000000000 --- a/pnpm11/engine/pm/commands/src/setup/pathExtenderPosix.ts +++ /dev/null @@ -1,346 +0,0 @@ -// cspell:ignore ZDOTDIR esep -import fs from 'node:fs' -import os from 'node:os' -import path from 'node:path' -import util from 'node:util' - -import { PnpmError } from '@pnpm/error' -import writeFileAtomic from 'write-file-atomic' - -export class BadShellSectionError extends PnpmError { - public current: string - public wanted: string - constructor (opts: { configSectionName: string, wanted: string, current: string, configFile: string }) { - super('BAD_SHELL_SECTION', `The config file at "${opts.configFile}" already contains a ${opts.configSectionName} section but with other configuration`) - this.current = opts.current - this.wanted = opts.wanted - } -} - -export type AddingPosition = 'start' | 'end' - -export interface AddDirToPosixEnvPathOpts { - proxyVarName?: string - proxyVarSubDir?: string - overwrite?: boolean - position?: AddingPosition - configSectionName: string -} - -export type ConfigFileChangeType = 'skipped' | 'appended' | 'modified' | 'created' - -export interface ConfigReport { - path: string - changeType: ConfigFileChangeType -} - -export interface PathExtenderPosixReport { - configFile: ConfigReport - oldSettings: string - newSettings: string -} - -export async function addDirToPosixEnvPath ( - dir: string, - opts: AddDirToPosixEnvPathOpts -): Promise { - const currentShell = detectCurrentShell() - return updateShell(currentShell, dir, opts) -} - -function detectCurrentShell (): string | null { - if (process.env.ZSH_VERSION) return 'zsh' - if (process.env.BASH_VERSION) return 'bash' - if (process.env.FISH_VERSION) return 'fish' - if (process.env.NU_VERSION) return 'nu' - return typeof process.env.SHELL === 'string' ? path.basename(process.env.SHELL) : null -} - -async function updateShell ( - currentShell: string | null, - pnpmHomeDir: string, - opts: AddDirToPosixEnvPathOpts -): Promise { - switch (currentShell) { - case 'bash': - case 'zsh': - case 'ksh': - case 'dash': - case 'sh': { - return setupShell(currentShell, pnpmHomeDir, opts) - } - case 'fish': { - return setupFishShell(pnpmHomeDir, opts) - } - case 'nu': { - return setupNuShell(pnpmHomeDir, opts) - } - } - const supportedShellsMsg = 'Supported shell languages are bash, zsh, fish, ksh, dash, sh, and nushell.' - if (!currentShell) { - throw new PnpmError('UNKNOWN_SHELL', 'Could not infer shell type.', { - hint: `Set the SHELL environment variable to your active shell.\n${supportedShellsMsg}`, - }) - } - throw new PnpmError('UNSUPPORTED_SHELL', `Can't setup configuration for "${currentShell}" shell`, { - hint: supportedShellsMsg, - }) -} - -async function setupShell ( - shell: 'bash' | 'zsh' | 'ksh' | 'dash' | 'sh', - dir: string, - opts: AddDirToPosixEnvPathOpts -): Promise { - const configFile = getConfigFilePath(shell) - let newSettings: string - const _createPathValue = createPathValue.bind(null, opts.position ?? 'start') - if (opts.proxyVarName) { - const pathRef = opts.proxyVarSubDir ? `$${opts.proxyVarName}/${opts.proxyVarSubDir}` : `$${opts.proxyVarName}` - newSettings = `export ${opts.proxyVarName}="${dir}" -case ":$PATH:" in - *":${pathRef}:"*) ;; - *) export PATH="${_createPathValue(pathRef)}" ;; -esac` - } else { - newSettings = `case ":$PATH:" in - *":${dir}:"*) ;; - *) export PATH="${_createPathValue(dir)}" ;; -esac` - } - const content = wrapSettings(opts.configSectionName, newSettings) - const { changeType, oldSettings } = await updateShellConfig(configFile, content, opts) - return { - configFile: { - path: configFile, - changeType, - }, - oldSettings, - newSettings, - } -} - -function getConfigFilePath (shell: 'bash' | 'zsh' | 'ksh' | 'dash' | 'sh'): string { - switch (shell) { - case 'zsh': return path.join((process.env.ZDOTDIR || os.homedir()), `.${shell}rc`) - case 'dash': - case 'sh': { - if (!process.env.ENV) { - throw new PnpmError('NO_SHELL_CONFIG', `Cannot find a config file for ${shell}. The ENV environment variable is not set.`) - } - return process.env.ENV - } - default: return path.join(os.homedir(), `.${shell}rc`) - } -} - -function createPathValue (position: AddingPosition, dir: string): string { - return position === 'start' - ? `${dir}:$PATH` - : `$PATH:${dir}` -} - -async function setupFishShell (dir: string, opts: AddDirToPosixEnvPathOpts): Promise { - const configFile = path.join(os.homedir(), '.config/fish/config.fish') - let newSettings: string - const _createPathValue = createFishPathValue.bind(null, opts.position ?? 'start') - if (opts.proxyVarName) { - const pathRef = opts.proxyVarSubDir ? `$${opts.proxyVarName}/${opts.proxyVarSubDir}` : `$${opts.proxyVarName}` - const matchPattern = opts.proxyVarSubDir ? `"${pathRef}"` : pathRef - newSettings = `set -gx ${opts.proxyVarName} "${dir}" -if not string match -q -- ${matchPattern} $PATH - set -gx PATH ${_createPathValue(pathRef)} -end` - } else { - newSettings = `if not string match -q -- "${dir}" $PATH - set -gx PATH ${_createPathValue(dir)} -end` - } - const content = wrapSettings(opts.configSectionName, newSettings) - const { changeType, oldSettings } = await updateShellConfig(configFile, content, opts) - return { - configFile: { - path: configFile, - changeType, - }, - oldSettings, - newSettings, - } -} - -async function setupNuShell (dir: string, opts: AddDirToPosixEnvPathOpts): Promise { - const configFile = path.join(os.homedir(), '.config/nushell/env.nu') - let newSettings: string - const addingCommand = (opts.position ?? 'start') === 'start' ? 'prepend' : 'append' - if (opts.proxyVarName) { - const pathRef = opts.proxyVarSubDir - ? `($env.${opts.proxyVarName} | path join "${opts.proxyVarSubDir}")` - : `$env.${opts.proxyVarName}` - newSettings = `$env.${opts.proxyVarName} = "${dir}" -$env.PATH = ($env.PATH | split row (char esep) | ${addingCommand} ${pathRef} )` - } else { - newSettings = `$env.PATH = ($env.PATH | split row (char esep) | ${addingCommand} ${dir} )` - } - const content = wrapSettings(opts.configSectionName, newSettings) - const { changeType, oldSettings } = await updateShellConfig(configFile, content, opts) - return { - configFile: { - path: configFile, - changeType, - }, - oldSettings, - newSettings, - } -} - -export function wrapSettings (sectionName: string, settings: string): string { - return `# ${sectionName} -${settings} -# ${sectionName} end` -} - -function createFishPathValue (position: AddingPosition, dir: string): string { - return position === 'start' - ? `"${dir}" $PATH` - : `$PATH "${dir}"` -} - -interface UpdateShellResult { - changeType: ConfigFileChangeType - oldSettings: string -} - -export interface FoundSection { - start: number - end: number - inner: string - fullMatch: string -} - -/** - * Locate the `#
` ... `#
end` block. - * - * A valid section is bounded by an opening `#
` line and a closing - * `#
end` line with no intermediate `#
` or `#
end` - * markers. If several valid sections exist, the last one whose non-comment - * lines reference both `PATH` and `
_HOME` wins, then the last one - * referencing `
_HOME`, then the last one referencing `PATH`, then the - * last section. - */ -export function findSection (content: string, section: string): FoundSection | null { - if (!content) return null - const startMarker = `# ${section}` - const endMarker = `# ${section} end` - - const sections: FoundSection[] = [] - let lastStart: { lineStart: number, innerStart: number } | null = null - let offset = 0 - - const lines = content.split('\n') - for (let i = 0; i < lines.length; i++) { - const line = lines[i] - const lineStart = offset - const lineLengthWithNewline = i < lines.length - 1 ? line.length + 1 : line.length - offset += lineLengthWithNewline - - const trimmed = line.replace(/[\r \t]+$/, '') - if (trimmed === startMarker) { - lastStart = { - lineStart, - innerStart: offset, - } - } else if (trimmed === endMarker) { - if (lastStart) { - const { lineStart: startOffset, innerStart } = lastStart - lastStart = null - - const inner = content.slice(innerStart, lineStart).replace(/[\r\n]+$/, '') - const markerLen = line.replace(/[\r\n]+$/, '').length - const rangeEnd = lineStart + markerLen - sections.push({ - start: startOffset, - end: rangeEnd, - inner, - fullMatch: content.slice(startOffset, rangeEnd), - }) - } - } - } - - if (sections.length === 0) return null - if (sections.length === 1) return sections[0] - - const homeVar = `${section.toUpperCase()}_HOME` - const settings = sections.map(({ inner }) => stripComments(inner)) - const predicates: Array<(text: string) => boolean> = [ - (text) => text.includes('PATH') && text.includes(homeVar), - (text) => text.includes(homeVar), - (text) => text.includes('PATH'), - ] - for (const predicate of predicates) { - for (let i = sections.length - 1; i >= 0; i--) { - if (predicate(settings[i])) return sections[i] - } - } - return sections[sections.length - 1] -} - -function stripComments (settings: string): string { - return settings.split('\n').filter((line) => !line.trimStart().startsWith('#')).join('\n') -} - -export function replaceSection (originalContent: string, newSection: string, sectionName: string): string { - const section = findSection(originalContent, sectionName) - if (!section) return originalContent - return originalContent.slice(0, section.start) + newSection + originalContent.slice(section.end) -} - -export async function updateShellConfig ( - configFile: string, - newContent: string, - opts: AddDirToPosixEnvPathOpts -): Promise { - await fs.promises.mkdir(path.dirname(configFile), { recursive: true }) - try { - await fs.promises.writeFile(configFile, `${newContent}\n`, { encoding: 'utf8', flag: 'wx' }) - return { - changeType: 'created', - oldSettings: '', - } - } catch (err: unknown) { - if (!util.types.isNativeError(err) || !('code' in err) || err.code !== 'EEXIST') { - throw err - } - } - const configContent = await fs.promises.readFile(configFile, 'utf8') - const section = findSection(configContent, opts.configSectionName) - if (!section) { - await fs.promises.appendFile(configFile, `\n${newContent}\n`, 'utf8') - return { - changeType: 'appended', - oldSettings: '', - } - } - const oldSettings = section.inner - const normalizedFullMatch = section.fullMatch.replace(/\r\n/g, '\n') - if (normalizedFullMatch !== newContent) { - if (!opts.overwrite) { - throw new BadShellSectionError({ - configSectionName: opts.configSectionName, - current: section.fullMatch, - wanted: newContent, - configFile, - }) - } - const newConfigContent = configContent.slice(0, section.start) + newContent + configContent.slice(section.end) - await writeFileAtomic(configFile, newConfigContent, 'utf8') - return { - changeType: 'modified', - oldSettings, - } - } - return { - changeType: 'skipped', - oldSettings, - } -} diff --git a/pnpm11/engine/pm/commands/src/setup/setup.ts b/pnpm11/engine/pm/commands/src/setup/setup.ts index 08df0f3ebd..39b92c5291 100644 --- a/pnpm11/engine/pm/commands/src/setup/setup.ts +++ b/pnpm11/engine/pm/commands/src/setup/setup.ts @@ -5,6 +5,11 @@ import path from 'node:path' import { detectIfCurrentPkgIsExecutable, packageManager } from '@pnpm/cli.meta' import { docsUrl } from '@pnpm/cli.utils' import { logger } from '@pnpm/logger' +import { + addDirToEnvPath, + type ConfigReport, + type PathExtenderReport, +} from '@pnpm/os.env.path-extender' import PATH from 'path-name' import { renderHelp } from 'render-help' @@ -12,11 +17,6 @@ import { validateGHActionsEnvFileValues, writeGHActionsEnvFiles, } from './ghActionsEnv.js' -import { - addDirToEnvPath, - type ConfigReport, - type PathExtenderReport, -} from './pathExtender.js' export const rcOptionsTypes = (): Record => ({}) diff --git a/pnpm11/engine/pm/commands/test/setup/setup.test.ts b/pnpm11/engine/pm/commands/test/setup/setup.test.ts index 9583d05d7a..8e3635e60e 100644 --- a/pnpm11/engine/pm/commands/test/setup/setup.test.ts +++ b/pnpm11/engine/pm/commands/test/setup/setup.test.ts @@ -5,7 +5,7 @@ import { afterEach, beforeEach, expect, jest, test } from '@jest/globals' import { PnpmError } from '@pnpm/error' import type { PathExtenderReport } from '@pnpm/os.env.path-extender' -jest.unstable_mockModule('../../lib/setup/pathExtender.js', () => ({ +jest.unstable_mockModule('@pnpm/os.env.path-extender', () => ({ addDirToEnvPath: jest.fn(), })) @@ -33,7 +33,7 @@ jest.unstable_mockModule('fs', () => { } }) -const { addDirToEnvPath } = await import('../../lib/setup/pathExtender.js') +const { addDirToEnvPath } = await import('@pnpm/os.env.path-extender') const { detectIfCurrentPkgIsExecutable } = await import('@pnpm/cli.meta') const { spawnSync } = await import('node:child_process') const { setup, LEGACY_HOME_DIR_SHIM_NAMES } = await import('@pnpm/engine.pm.commands') diff --git a/pnpm11/os/env/path-extender-posix/package.json b/pnpm11/os/env/path-extender-posix/package.json index 7645fb0468..33c0e8d050 100644 --- a/pnpm11/os/env/path-extender-posix/package.json +++ b/pnpm11/os/env/path-extender-posix/package.json @@ -36,12 +36,14 @@ "prepublishOnly": "tsgo --build" }, "dependencies": { - "@pnpm/error": "workspace:*" + "@pnpm/error": "workspace:*", + "write-file-atomic": "catalog:" }, "devDependencies": { "@jest/globals": "catalog:", "@pnpm/os.env.path-extender-posix": "workspace:*", - "@pnpm/prepare": "workspace:*" + "@pnpm/prepare": "workspace:*", + "@types/write-file-atomic": "catalog:" }, "engines": { "node": ">=22.13" diff --git a/pnpm11/os/env/path-extender-posix/src/path-extender-posix.ts b/pnpm11/os/env/path-extender-posix/src/path-extender-posix.ts index e724f62558..157ad5d8ef 100644 --- a/pnpm11/os/env/path-extender-posix/src/path-extender-posix.ts +++ b/pnpm11/os/env/path-extender-posix/src/path-extender-posix.ts @@ -1,14 +1,17 @@ +// cspell:ignore ZDOTDIR esep import fs from 'node:fs' import os from 'node:os' import path from 'node:path' +import util from 'node:util' import { PnpmError } from '@pnpm/error' +import writeFileAtomic from 'write-file-atomic' -class BadShellSectionError extends PnpmError { +export class BadShellSectionError extends PnpmError { public current: string public wanted: string constructor (opts: { configSectionName: string, wanted: string, current: string, configFile: string }) { - super('BAD_SHELL_SECTION', `The config file at "${opts.configFile} already contains a ${opts.configSectionName} section but with other configuration`) + super('BAD_SHELL_SECTION', `The config file at "${opts.configFile}" already contains a ${opts.configSectionName} section but with other configuration`) this.current = opts.current this.wanted = opts.wanted } @@ -47,7 +50,7 @@ export async function addDirToPosixEnvPath ( return updateShell(currentShell, dir, opts) } -function detectCurrentShell () { +function detectCurrentShell (): string | null { if (process.env.ZSH_VERSION) return 'zsh' if (process.env.BASH_VERSION) return 'bash' if (process.env.FISH_VERSION) return 'fish' @@ -76,10 +79,11 @@ async function updateShell ( } } const supportedShellsMsg = 'Supported shell languages are bash, zsh, fish, ksh, dash, sh, and nushell.' - if (!currentShell) throw new PnpmError('UNKNOWN_SHELL', 'Could not infer shell type.', { - hint: `Set the SHELL environment variable to your active shell. -${supportedShellsMsg}`, - }) + if (!currentShell) { + throw new PnpmError('UNKNOWN_SHELL', 'Could not infer shell type.', { + hint: `Set the SHELL environment variable to your active shell.\n${supportedShellsMsg}`, + }) + } throw new PnpmError('UNSUPPORTED_SHELL', `Can't setup configuration for "${currentShell}" shell`, { hint: supportedShellsMsg, }) @@ -91,7 +95,7 @@ async function setupShell ( opts: AddDirToPosixEnvPathOpts ): Promise { const configFile = getConfigFilePath(shell) - let newSettings!: string + let newSettings: string const _createPathValue = createPathValue.bind(null, opts.position ?? 'start') if (opts.proxyVarName) { const pathRef = opts.proxyVarSubDir ? `$${opts.proxyVarName}/${opts.proxyVarSubDir}` : `$${opts.proxyVarName}` @@ -132,7 +136,7 @@ function getConfigFilePath (shell: 'bash' | 'zsh' | 'ksh' | 'dash' | 'sh'): stri } } -function createPathValue (position: AddingPosition, dir: string) { +function createPathValue (position: AddingPosition, dir: string): string { return position === 'start' ? `${dir}:$PATH` : `$PATH:${dir}` @@ -140,7 +144,7 @@ function createPathValue (position: AddingPosition, dir: string) { async function setupFishShell (dir: string, opts: AddDirToPosixEnvPathOpts): Promise { const configFile = path.join(os.homedir(), '.config/fish/config.fish') - let newSettings!: string + let newSettings: string const _createPathValue = createFishPathValue.bind(null, opts.position ?? 'start') if (opts.proxyVarName) { const pathRef = opts.proxyVarSubDir ? `$${opts.proxyVarName}/${opts.proxyVarSubDir}` : `$${opts.proxyVarName}` @@ -168,7 +172,7 @@ end` async function setupNuShell (dir: string, opts: AddDirToPosixEnvPathOpts): Promise { const configFile = path.join(os.homedir(), '.config/nushell/env.nu') - let newSettings!: string + let newSettings: string const addingCommand = (opts.position ?? 'start') === 'start' ? 'prepend' : 'append' if (opts.proxyVarName) { const pathRef = opts.proxyVarSubDir @@ -190,13 +194,14 @@ $env.PATH = ($env.PATH | split row (char esep) | ${addingCommand} ${pathRef} )` newSettings, } } -function wrapSettings (sectionName: string, settings: string): string { + +export function wrapSettings (sectionName: string, settings: string): string { return `# ${sectionName} ${settings} # ${sectionName} end` } -function createFishPathValue (position: AddingPosition, dir: string) { +function createFishPathValue (position: AddingPosition, dir: string): string { return position === 'start' ? `"${dir}" $PATH` : `$PATH "${dir}"` @@ -207,40 +212,45 @@ interface UpdateShellResult { oldSettings: string } -async function updateShellConfig ( +export async function updateShellConfig ( configFile: string, newContent: string, opts: AddDirToPosixEnvPathOpts ): Promise { - if (!fs.existsSync(configFile)) { - await fs.promises.mkdir(path.dirname(configFile), { recursive: true }) - await fs.promises.writeFile(configFile, `${newContent}\n`, 'utf8') + await fs.promises.mkdir(path.dirname(configFile), { recursive: true }) + try { + await fs.promises.writeFile(configFile, `${newContent}\n`, { encoding: 'utf8', flag: 'wx' }) return { changeType: 'created', oldSettings: '', } + } catch (err: unknown) { + if (!util.types.isNativeError(err) || !('code' in err) || err.code !== 'EEXIST') { + throw err + } } const configContent = await fs.promises.readFile(configFile, 'utf8') - const match = new RegExp(`# ${opts.configSectionName}\n([\\s\\S]*)\n# ${opts.configSectionName} end`, 'g').exec(configContent) - if (!match) { + const section = findSection(configContent, opts.configSectionName, opts.proxyVarName) + if (!section) { await fs.promises.appendFile(configFile, `\n${newContent}\n`, 'utf8') return { changeType: 'appended', oldSettings: '', } } - const oldSettings = match[1] - if (match[0] !== newContent) { + const oldSettings = section.inner + const normalizedFullMatch = section.fullMatch.replace(/\r\n/g, '\n') + if (normalizedFullMatch !== newContent) { if (!opts.overwrite) { throw new BadShellSectionError({ configSectionName: opts.configSectionName, - current: match[0], + current: section.fullMatch, wanted: newContent, configFile, }) } - const newConfigContent = replaceSection(configContent, newContent, opts.configSectionName) - await fs.promises.writeFile(configFile, newConfigContent, 'utf8') + const newConfigContent = configContent.slice(0, section.start) + newContent + configContent.slice(section.end) + await writeFileAtomic(configFile, newConfigContent, 'utf8') return { changeType: 'modified', oldSettings, @@ -252,6 +262,90 @@ async function updateShellConfig ( } } -function replaceSection (originalContent: string, newSection: string, sectionName: string): string { - return originalContent.replace(new RegExp(`# ${sectionName}[\\s\\S]*# ${sectionName} end`, 'g'), newSection) +export interface FoundSection { + start: number + end: number + inner: string + fullMatch: string +} + +/** + * Locate the `#
` ... `#
end` block. + * + * A valid section is bounded by an opening `#
` line and a closing + * `#
end` line with no intermediate `#
` or `#
end` + * markers. If several valid sections exist, the last one whose non-comment + * lines reference both `PATH` and `homeVar` wins, then the last one + * referencing `homeVar`, then the last one referencing `PATH`, then the last + * section. Returns `null` when the content holds no complete section. + */ +export function findSection ( + content: string, + section: string, + homeVar = `${section.toUpperCase()}_HOME` +): FoundSection | null { + if (!content) return null + const startMarker = `# ${section}` + const endMarker = `# ${section} end` + + const sections: FoundSection[] = [] + let lastStart: { lineStart: number, innerStart: number } | null = null + let offset = 0 + + const lines = content.split('\n') + for (let i = 0; i < lines.length; i++) { + const line = lines[i] + const lineStart = offset + const lineLengthWithNewline = i < lines.length - 1 ? line.length + 1 : line.length + offset += lineLengthWithNewline + + const trimmed = line.replace(/[\r \t]+$/, '') + if (trimmed === startMarker) { + lastStart = { + lineStart, + innerStart: offset, + } + } else if (trimmed === endMarker) { + if (lastStart) { + const { lineStart: startOffset, innerStart } = lastStart + lastStart = null + + const inner = content.slice(innerStart, lineStart).replace(/[\r\n]+$/, '') + const markerLen = line.replace(/[\r\n]+$/, '').length + const rangeEnd = lineStart + markerLen + sections.push({ + start: startOffset, + end: rangeEnd, + inner, + fullMatch: content.slice(startOffset, rangeEnd), + }) + } + } + } + + if (sections.length === 0) return null + if (sections.length === 1) return sections[0] + + const settings = sections.map(({ inner }) => stripComments(inner)) + const predicates: Array<(text: string) => boolean> = [ + (text) => text.includes('PATH') && text.includes(homeVar), + (text) => text.includes(homeVar), + (text) => text.includes('PATH'), + ] + for (const predicate of predicates) { + for (let i = sections.length - 1; i >= 0; i--) { + if (predicate(settings[i])) return sections[i] + } + } + return sections[sections.length - 1] +} + +function stripComments (settings: string): string { + return settings.split('\n').filter((line) => !line.trimStart().startsWith('#')).join('\n') +} + +export function replaceSection (originalContent: string, newSection: string, sectionName: string): string { + const section = findSection(originalContent, sectionName) + if (!section) return originalContent + return originalContent.slice(0, section.start) + newSection + originalContent.slice(section.end) } diff --git a/pnpm11/os/env/path-extender-posix/test/path-extender-posix.test.ts b/pnpm11/os/env/path-extender-posix/test/path-extender-posix.test.ts index 10eb9f024b..36a507ade4 100644 --- a/pnpm11/os/env/path-extender-posix/test/path-extender-posix.test.ts +++ b/pnpm11/os/env/path-extender-posix/test/path-extender-posix.test.ts @@ -267,6 +267,40 @@ case ":$PATH:" in esac # pnpm end`) }) + it('should keep the lines between two pnpm sections when overwriting (pnpm/pnpm#12282)', async () => { + const duplicateSection = `# pnpm +export PNPM_HOME="duplicate_block" +# pnpm end` + fs.writeFileSync(configFile, `# user config +# pnpm +export PNPM_HOME="old_home" +case ":$PATH:" in + *":$PNPM_HOME:"*) ;; + *) export PATH="$PNPM_HOME:$PATH" ;; +esac +# pnpm end +# content between sections that must not be deleted +${duplicateSection} +# after sections`, 'utf8') + const report = await addDirToPosixEnvPath(pnpmHomeDir, { + proxyVarName: 'PNPM_HOME', + overwrite: true, + configSectionName: 'pnpm', + }) + expect(report.configFile?.changeType).toBe('modified') + const configContent = fs.readFileSync(configFile, 'utf8') + expect(configContent).toBe(`# user config +# pnpm +export PNPM_HOME="${pnpmHomeDir}" +case ":$PATH:" in + *":$PNPM_HOME:"*) ;; + *) export PATH="$PNPM_HOME:$PATH" ;; +esac +# pnpm end +# content between sections that must not be deleted +${duplicateSection} +# after sections`) + }) }) describe('Zsh', () => { diff --git a/pnpm11/engine/pm/commands/test/setup/pathExtenderPosix.test.ts b/pnpm11/os/env/path-extender-posix/test/sections.test.ts similarity index 92% rename from pnpm11/engine/pm/commands/test/setup/pathExtenderPosix.test.ts rename to pnpm11/os/env/path-extender-posix/test/sections.test.ts index f8a800b50b..561ce78d51 100644 --- a/pnpm11/engine/pm/commands/test/setup/pathExtenderPosix.test.ts +++ b/pnpm11/os/env/path-extender-posix/test/sections.test.ts @@ -11,7 +11,7 @@ import { replaceSection, updateShellConfig, wrapSettings, -} from '../../src/setup/pathExtenderPosix.js' +} from '../src/path-extender-posix.js' function opts (overwrite: boolean): AddDirToPosixEnvPathOpts { return { @@ -242,6 +242,28 @@ esac`) } }) + test('prioritizes the block that sets proxyVarName when it differs from the section name', async () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'pnpm-posix-test-')) + try { + const configFile = path.join(dir, '.zshrc') + const generatedEnvBlock = wrapSettings('tool', 'export MY_HOME=/old\nexport PATH=$MY_HOME:$PATH') + const customPathBlock = wrapSettings('tool', 'export PATH=/custom/bin:$PATH') + fs.writeFileSync(configFile, `${generatedEnvBlock}\n\n${customPathBlock}\n`) + + const newEnvBlock = wrapSettings('tool', 'export MY_HOME=/new\nexport PATH=$MY_HOME:$PATH') + const result = await updateShellConfig(configFile, newEnvBlock, { + configSectionName: 'tool', + proxyVarName: 'MY_HOME', + overwrite: true, + }) + + expect(result.oldSettings).toBe('export MY_HOME=/old\nexport PATH=$MY_HOME:$PATH') + expect(fs.readFileSync(configFile, 'utf8')).toBe(`${newEnvBlock}\n\n${customPathBlock}\n`) + } finally { + fs.rmSync(dir, { recursive: true, force: true }) + } + }) + test('prioritizes generated env block over later custom path block', async () => { const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'pnpm-posix-test-')) try {