fix(dlx): don't fail when running dlx from project with patched deps (#7201)

close #7198
close #7183
close #7200

.changeset/poor-items-breathe.md

@@ -0,0 +1,7 @@
+---
+"@pnpm/plugin-commands-patching": major
+"@pnpm/core": major
+"@pnpm/config": major
+---
+
+The paths in patchedDependencies passed to `@pnpm/core` are absolute.

.changeset/witty-bikes-sin.md

@@ -0,0 +1,6 @@
+---
+"pnpm": patch
+"@pnpm/plugin-commands-script-runners": patch
+---
+
+`pnpm dlx` should ignore any settings that are in a `package.json` file found in the current working directory [#7198](https://github.com/pnpm/pnpm/issues/7198).

config/config/src/getOptionsFromRootManifest.ts

@@ -38,7 +38,14 @@ export function getOptionsFromRootManifest (manifestDir: string, manifest: Proje
   const peerDependencyRules = manifest.pnpm?.peerDependencyRules
   const allowedDeprecatedVersions = manifest.pnpm?.allowedDeprecatedVersions
   const allowNonAppliedPatches = manifest.pnpm?.allowNonAppliedPatches
-  const patchedDependencies = manifest.pnpm?.patchedDependencies
+  let patchedDependencies = manifest.pnpm?.patchedDependencies
+  if (patchedDependencies) {
+    patchedDependencies = { ...patchedDependencies }
+    for (const [dep, patchFile] of Object.entries(patchedDependencies)) {
+      if (path.isAbsolute(patchFile)) continue
+      patchedDependencies[dep] = path.join(manifestDir, patchFile)
+    }
+  }
   const settings: OptionsFromRootManifest = {
     allowedDeprecatedVersions,
     allowNonAppliedPatches,

exec/plugin-commands-script-runners/src/dlx.ts

@@ -88,7 +88,9 @@ export async function handler (
   const pkgs = opts.package ?? [command]
   const env = makeEnv({ userAgent: opts.userAgent, prependPaths: [binsDir] })
   await add.handler({
-    ...omit(['workspaceDir'], opts),
+    // Ideally the config reader should ignore these settings when the dlx command is executed.
+    // This is a temporary solution until "@pnpm/config" is refactored.
+    ...omit(['workspaceDir', 'rootProjectManifest', 'rootProjectManifestDir'], opts),
     bin: binsDir,
     dir: prefix,
     lockfileDir: prefix,

patching/plugin-commands-patching/src/patchCommit.ts

@@ -42,7 +42,7 @@ export function help () {
   })
 }
 
-export async function handler (opts: install.InstallCommandOptions & Pick<Config, 'patchesDir' | 'rootProjectManifest'>, params: string[]) {
+export async function handler (opts: install.InstallCommandOptions & Pick<Config, 'patchesDir' | 'rootProjectManifest' | 'rootProjectManifestDir'>, params: string[]) {
   const userDir = params[0]
   const lockfileDir = opts.lockfileDir ?? opts.dir ?? process.cwd()
   const patchesDirName = normalizePath(path.normalize(opts.patchesDir ?? 'patches'))

patching/plugin-commands-patching/test/patch.test.ts

@@ -77,6 +77,7 @@ describe('patch and commit', () => {
       ...DEFAULT_OPTS,
       cacheDir,
       dir: process.cwd(),
+      rootProjectManifestDir: process.cwd(),
       frozenLockfile: false,
       fixLockfile: true,
       storeDir,
@@ -115,6 +116,7 @@ describe('patch and commit', () => {
       ...DEFAULT_OPTS,
       cacheDir,
       dir: process.cwd(),
+      rootProjectManifestDir: process.cwd(),
       frozenLockfile: false,
       fixLockfile: true,
       storeDir,
@@ -138,6 +140,7 @@ describe('patch and commit', () => {
       ...DEFAULT_OPTS,
       cacheDir,
       dir: process.cwd(),
+      rootProjectManifestDir: process.cwd(),
       frozenLockfile: false,
       fixLockfile: true,
       storeDir,
@@ -160,6 +163,7 @@ describe('patch and commit', () => {
       ...DEFAULT_OPTS,
       cacheDir,
       dir: process.cwd(),
+      rootProjectManifestDir: process.cwd(),
       frozenLockfile: false,
       fixLockfile: true,
       patchesDir,
@@ -198,6 +202,7 @@ describe('patch and commit', () => {
       ...DEFAULT_OPTS,
       cacheDir,
       dir: process.cwd(),
+      rootProjectManifestDir: process.cwd(),
       frozenLockfile: false,
       fixLockfile: true,
       storeDir,
@@ -217,6 +222,7 @@ describe('patch and commit', () => {
       ...DEFAULT_OPTS,
       cacheDir,
       dir: process.cwd(),
+      rootProjectManifestDir: process.cwd(),
       frozenLockfile: false,
       fixLockfile: true,
       storeDir,
@@ -266,6 +272,7 @@ describe('patch and commit', () => {
       ...DEFAULT_OPTS,
       cacheDir,
       dir: process.cwd(),
+      rootProjectManifestDir: process.cwd(),
       frozenLockfile: false,
       fixLockfile: true,
       storeDir,
@@ -317,6 +324,7 @@ describe('patch and commit', () => {
       ...DEFAULT_OPTS,
       cacheDir,
       dir: process.cwd(),
+      rootProjectManifestDir: process.cwd(),
       frozenLockfile: false,
       fixLockfile: true,
       storeDir,
@@ -377,6 +385,7 @@ describe('prompt to choose version', () => {
       ...DEFAULT_OPTS,
       cacheDir,
       dir: process.cwd(),
+      rootProjectManifestDir: process.cwd(),
       frozenLockfile: false,
       fixLockfile: true,
       storeDir,
@@ -435,6 +444,7 @@ describe('patching should work when there is a no EOL in the patched file', () =
     await patchCommit.handler({
       ...DEFAULT_OPTS,
       dir: process.cwd(),
+      rootProjectManifestDir: process.cwd(),
       frozenLockfile: false,
       fixLockfile: true,
     }, [userPatchDir])
@@ -463,6 +473,7 @@ describe('patching should work when there is a no EOL in the patched file', () =
     await patchCommit.handler({
       ...DEFAULT_OPTS,
       dir: process.cwd(),
+      rootProjectManifestDir: process.cwd(),
       frozenLockfile: false,
       fixLockfile: true,
     }, [userPatchDir])
@@ -551,6 +562,7 @@ describe('patch and commit in workspaces', () => {
       allProjectsGraph,
       selectedProjectsGraph,
       dir: process.cwd(),
+      rootProjectManifestDir: process.cwd(),
       cacheDir,
       storeDir,
       lockfileDir: process.cwd(),
@@ -611,6 +623,7 @@ describe('patch and commit in workspaces', () => {
       allProjectsGraph,
       selectedProjectsGraph,
       dir: process.cwd(),
+      rootProjectManifestDir: process.cwd(),
       cacheDir,
       storeDir,
       lockfileDir: process.cwd(),
@@ -685,6 +698,7 @@ describe('patch with custom modules-dir and virtual-store-dir', () => {
     await patchCommit.handler({
       ...DEFAULT_OPTS,
       dir: customModulesDirFixture,
+      rootProjectManifestDir: customModulesDirFixture,
       saveLockfile: true,
       frozenLockfile: false,
       fixLockfile: true,

pkg-manager/core/src/install/index.ts

@@ -698,11 +698,10 @@ Note that in CI environments, this setting is enabled by default.`,
 }
 
 async function calcPatchHashes (patches: Record<string, string>, lockfileDir: string) {
-  return pMapValues(async (patchFileRelativePath) => {
-    const patchFilePath = path.join(lockfileDir, patchFileRelativePath)
+  return pMapValues(async (patchFilePath) => {
     return {
       hash: await createBase32HashFromFile(patchFilePath),
-      path: patchFileRelativePath,
+      path: path.relative(lockfileDir, patchFilePath).replaceAll('\\', '/'),
     }
   }, patches)
 }

pkg-manager/core/test/install/patch.ts

@@ -16,7 +16,7 @@ test('patch package', async () => {
   const patchPath = path.join(f.find('patch-pkg'), 'is-positive@1.0.0.patch')
 
   const patchedDependencies = {
-    'is-positive@1.0.0': path.relative(process.cwd(), patchPath),
+    'is-positive@1.0.0': patchPath,
   }
   const opts = await testDefaults({
     fastUnpack: false,
@@ -36,7 +36,7 @@ test('patch package', async () => {
   const lockfile = await project.readLockfile()
   expect(lockfile.patchedDependencies).toStrictEqual({
     'is-positive@1.0.0': {
-      path: patchedDependencies['is-positive@1.0.0'],
+      path: path.relative(process.cwd(), patchedDependencies['is-positive@1.0.0']).replaceAll('\\', '/'),
       hash: patchFileHash,
     },
   })
@@ -102,8 +102,8 @@ test('patch package reports warning if not all patches are applied and allowNonA
   const patchPath = path.join(f.find('patch-pkg'), 'is-positive@1.0.0.patch')
 
   const patchedDependencies = {
-    'is-positive@1.0.0': path.relative(process.cwd(), patchPath),
-    'is-negative@1.0.0': path.relative(process.cwd(), patchPath),
+    'is-positive@1.0.0': patchPath,
+    'is-negative@1.0.0': patchPath,
   }
   const opts = await testDefaults({
     fastUnpack: false,
@@ -131,8 +131,8 @@ test('patch package throws an exception if not all patches are applied', async (
   const patchPath = path.join(f.find('patch-pkg'), 'is-positive@1.0.0.patch')
 
   const patchedDependencies = {
-    'is-positive@1.0.0': path.relative(process.cwd(), patchPath),
-    'is-negative@1.0.0': path.relative(process.cwd(), patchPath),
+    'is-positive@1.0.0': patchPath,
+    'is-negative@1.0.0': patchPath,
   }
   const opts = await testDefaults({
     fastUnpack: false,
@@ -155,7 +155,7 @@ test('the patched package is updated if the patch is modified', async () => {
   const patchPath = path.resolve('patches', 'is-positive@1.0.0.patch')
 
   const patchedDependencies = {
-    'is-positive@1.0.0': path.relative(process.cwd(), patchPath),
+    'is-positive@1.0.0': patchPath,
   }
   const opts = await testDefaults({
     fastUnpack: false,
@@ -182,7 +182,7 @@ test('patch package when scripts are ignored', async () => {
   const patchPath = path.join(f.find('patch-pkg'), 'is-positive@1.0.0.patch')
 
   const patchedDependencies = {
-    'is-positive@1.0.0': path.relative(process.cwd(), patchPath),
+    'is-positive@1.0.0': patchPath,
   }
   const opts = await testDefaults({
     fastUnpack: false,
@@ -203,7 +203,7 @@ test('patch package when scripts are ignored', async () => {
   const lockfile = await project.readLockfile()
   expect(lockfile.patchedDependencies).toStrictEqual({
     'is-positive@1.0.0': {
-      path: patchedDependencies['is-positive@1.0.0'],
+      path: path.relative(process.cwd(), patchedDependencies['is-positive@1.0.0']).replaceAll('\\', '/'),
       hash: patchFileHash,
     },
   })
@@ -269,7 +269,7 @@ test('patch package when the package is not in onlyBuiltDependencies list', asyn
   const patchPath = path.join(f.find('patch-pkg'), 'is-positive@1.0.0.patch')
 
   const patchedDependencies = {
-    'is-positive@1.0.0': path.relative(process.cwd(), patchPath),
+    'is-positive@1.0.0': patchPath,
   }
   const opts = await testDefaults({
     fastUnpack: false,
@@ -290,7 +290,7 @@ test('patch package when the package is not in onlyBuiltDependencies list', asyn
   const lockfile = await project.readLockfile()
   expect(lockfile.patchedDependencies).toStrictEqual({
     'is-positive@1.0.0': {
-      path: patchedDependencies['is-positive@1.0.0'],
+      path: path.relative(process.cwd(), patchedDependencies['is-positive@1.0.0']).replaceAll('\\', '/'),
       hash: patchFileHash,
     },
   })
@@ -356,7 +356,7 @@ test('patch package when the patched package has no dependencies and appears mul
   const patchPath = path.join(f.find('patch-pkg'), 'is-positive@1.0.0.patch')
 
   const patchedDependencies = {
-    'is-positive@1.0.0': path.relative(process.cwd(), patchPath),
+    'is-positive@1.0.0': patchPath,
   }
   const opts = await testDefaults({
     fastUnpack: false,
@@ -388,7 +388,7 @@ test('patch package should fail when the patch could not be applied', async () =
   const patchPath = path.join(f.find('patch-pkg'), 'is-positive@1.0.0.patch')
 
   const patchedDependencies = {
-    'is-positive@3.1.0': path.relative(process.cwd(), patchPath),
+    'is-positive@3.1.0': patchPath,
   }
   const opts = await testDefaults({
     fastUnpack: false,

pnpm/test/run.ts

@@ -141,6 +141,29 @@ test('silent dlx prints the output of the child process only', async () => {
   expect(result.stdout.toString().trim()).toBe('hi')
 })
 
+test('dlx ignores configuration in current project package.json', async () => {
+  prepare({
+    pnpm: {
+      patchedDependencies: {
+        'shx@0.3.4': 'this_doesnt_exist',
+      },
+    },
+  })
+  const global = path.resolve('..', 'global')
+  const pnpmHome = path.join(global, 'pnpm')
+  mkdirSync(global)
+
+  const env = {
+    [PATH_NAME]: `${pnpmHome}${path.delimiter}${process.env[PATH_NAME]}`,
+    PNPM_HOME: pnpmHome,
+    XDG_DATA_HOME: global,
+  }
+
+  const result = execPnpmSync(['dlx', 'shx@0.3.4', 'echo', 'hi'], { env })
+  // It didn't try to use the patch that doesn't exist, so it did not fail
+  expect(result.status).toBe(0)
+})
+
 testOnPosix('pnpm run with preferSymlinkedExecutables true', async () => {
   prepare({
     scripts: {