From cfdb491e56bd6a3e26aa0f4bf1f21aa9dab9bf55 Mon Sep 17 00:00:00 2001 From: Takumi Ishikawa <20138086+polyomino24@users.noreply.github.com> Date: Mon, 10 Nov 2025 15:56:42 +0900 Subject: [PATCH 1/6] fix: Revert to readlink in generatePosixLink to avoid unnecessary unlinks --- sources/commands/Enable.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sources/commands/Enable.ts b/sources/commands/Enable.ts index 978059072..6a2a58413 100644 --- a/sources/commands/Enable.ts +++ b/sources/commands/Enable.ts @@ -83,9 +83,9 @@ export class EnableCommand extends Command { const symlink = path.relative(installDirectory, path.join(distFolder, `${binName}.js`)); if (fs.existsSync(file)) { - const currentSymlink = await fs.promises.realpath(file); + const currentSymlink = await fs.promises.readlink(file); - if (binName.includes(`yarn`) && corepackUtils.isYarnSwitchPath(currentSymlink)) { + if (binName.includes(`yarn`) && corepackUtils.isYarnSwitchPath(await fs.promises.realpath(file))) { console.warn(`${binName} is already installed in ${file} and points to a Yarn Switch install - skipping`); return; } From 426e7f04c37095811f21b0b571f0be85f6784b1e Mon Sep 17 00:00:00 2001 From: Takumi Ishikawa <20138086+polyomino24@users.noreply.github.com> Date: Mon, 10 Nov 2025 19:46:49 +0900 Subject: [PATCH 2/6] fix: Use lstat and readlink to safely update symlinks --- sources/commands/Enable.ts | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/sources/commands/Enable.ts b/sources/commands/Enable.ts index 6a2a58413..3e72aa55c 100644 --- a/sources/commands/Enable.ts +++ b/sources/commands/Enable.ts @@ -83,17 +83,23 @@ export class EnableCommand extends Command { const symlink = path.relative(installDirectory, path.join(distFolder, `${binName}.js`)); if (fs.existsSync(file)) { - const currentSymlink = await fs.promises.readlink(file); + const stats = await fs.promises.lstat(file); - if (binName.includes(`yarn`) && corepackUtils.isYarnSwitchPath(await fs.promises.realpath(file))) { - console.warn(`${binName} is already installed in ${file} and points to a Yarn Switch install - skipping`); - return; - } + if (stats.isSymbolicLink()) { + const currentSymlink = await fs.promises.readlink(file); - if (currentSymlink !== symlink) { - await fs.promises.unlink(file); + if (binName.includes(`yarn`) && corepackUtils.isYarnSwitchPath(await fs.promises.realpath(file))) { + console.warn(`${binName} is already installed in ${file} and points to a Yarn Switch install - skipping`); + return; + } + + if (currentSymlink !== symlink) { + await fs.promises.unlink(file); + } else { + return; + } } else { - return; + await fs.promises.unlink(file); } } From 4c8e5dae52e3c1bb0575984057c25af862d6c9c8 Mon Sep 17 00:00:00 2001 From: Takumi Ishikawa <20138086+polyomino24@users.noreply.github.com> Date: Mon, 10 Nov 2025 20:19:32 +0900 Subject: [PATCH 3/6] test: Add tests for generatePosixLink --- tests/Enable.test.ts | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/tests/Enable.test.ts b/tests/Enable.test.ts index 7ca77cbee..90132d4c0 100644 --- a/tests/Enable.test.ts +++ b/tests/Enable.test.ts @@ -125,4 +125,44 @@ describe(`EnableCommand`, () => { expect(file).toBe(`hello`); }); }); + + test.skipIf(process.platform === `win32`)(`should not re-link if binaries are already correct`, async () => { + await xfs.mktempPromise(async cwd => { + await makeBin(cwd, `corepack` as Filename); + + process.env.PATH = `${npath.fromPortablePath(cwd)}${delimiter}${process.env.PATH}`; + + await expect(runCli(cwd, [`enable`])).resolves.toMatchObject({ + exitCode: 0, + }); + const yarnStat1 = await xfs.lstatPromise(ppath.join(cwd, `yarn`)); + + await new Promise(resolve => setTimeout(resolve, 10)); + + await expect(runCli(cwd, [`enable`])).resolves.toMatchObject({ + exitCode: 0, + }); + const yarnStat2 = await xfs.lstatPromise(ppath.join(cwd, `yarn`)); + + expect(yarnStat2.mtimeMs).toBe(yarnStat1.mtimeMs); + }); + }); + + test.skipIf(process.platform === `win32`)(`should overwrite existing symlinks if they are incorrect`, async () => { + await xfs.mktempPromise(async cwd => { + await makeBin(cwd, `corepack` as Filename); + + await xfs.writeFilePromise(ppath.join(cwd, `dummy-target`), `hello`); + await xfs.symlinkPromise(ppath.join(cwd, `dummy-target`), ppath.join(cwd, `yarn`)); + + process.env.PATH = `${npath.fromPortablePath(cwd)}${delimiter}${process.env.PATH}`; + + await expect(runCli(cwd, [`enable`])).resolves.toMatchObject({ + exitCode: 0, + }); + + const newLink = await xfs.readlinkPromise(ppath.join(cwd, `yarn`)); + expect(newLink).toContain(`yarn.js`); + }); + }); }); From b664e26c9d70a2e494cad67c5796c7b34569dfae Mon Sep 17 00:00:00 2001 From: Takumi Ishikawa <20138086+polyomino24@users.noreply.github.com> Date: Mon, 10 Nov 2025 21:27:51 +0900 Subject: [PATCH 4/6] refactor: Simplify symlink update logic --- sources/commands/Enable.ts | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/sources/commands/Enable.ts b/sources/commands/Enable.ts index 3e72aa55c..595aefe53 100644 --- a/sources/commands/Enable.ts +++ b/sources/commands/Enable.ts @@ -81,10 +81,9 @@ export class EnableCommand extends Command { async generatePosixLink(installDirectory: string, distFolder: string, binName: string) { const file = path.join(installDirectory, binName); const symlink = path.relative(installDirectory, path.join(distFolder, `${binName}.js`)); + const stats = fs.lstatSync(file, {throwIfNoEntry: false}); - if (fs.existsSync(file)) { - const stats = await fs.promises.lstat(file); - + if (stats) { if (stats.isSymbolicLink()) { const currentSymlink = await fs.promises.readlink(file); @@ -93,14 +92,11 @@ export class EnableCommand extends Command { return; } - if (currentSymlink !== symlink) { - await fs.promises.unlink(file); - } else { + if (currentSymlink === symlink) { return; } - } else { - await fs.promises.unlink(file); } + await fs.promises.unlink(file); } await fs.promises.symlink(symlink, file); From b745ef52cd42ef121deef4e6c2a8535cf5fec430 Mon Sep 17 00:00:00 2001 From: Takumi Ishikawa <20138086+polyomino24@users.noreply.github.com> Date: Tue, 11 Nov 2025 10:26:02 +0900 Subject: [PATCH 5/6] test: Update EnableCommand tests - add expected stdout & stderr - use Node.js async setTimeout - add --install-directory option --- tests/Enable.test.ts | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/tests/Enable.test.ts b/tests/Enable.test.ts index 90132d4c0..b767c7364 100644 --- a/tests/Enable.test.ts +++ b/tests/Enable.test.ts @@ -1,6 +1,7 @@ import {Filename, ppath, xfs, npath} from '@yarnpkg/fslib'; import {delimiter} from 'node:path'; import process from 'node:process'; +import {setTimeout} from 'node:timers/promises'; import {describe, beforeEach, it, expect, test} from 'vitest'; import {Engine} from '../sources/Engine'; @@ -132,14 +133,18 @@ describe(`EnableCommand`, () => { process.env.PATH = `${npath.fromPortablePath(cwd)}${delimiter}${process.env.PATH}`; - await expect(runCli(cwd, [`enable`])).resolves.toMatchObject({ + await expect(runCli(cwd, [`enable`, `--install-directory`, npath.fromPortablePath(cwd)])).resolves.toMatchObject({ + stdout: ``, + stderr: ``, exitCode: 0, }); const yarnStat1 = await xfs.lstatPromise(ppath.join(cwd, `yarn`)); - await new Promise(resolve => setTimeout(resolve, 10)); + await setTimeout(10); - await expect(runCli(cwd, [`enable`])).resolves.toMatchObject({ + await expect(runCli(cwd, [`enable`, `--install-directory`, npath.fromPortablePath(cwd)])).resolves.toMatchObject({ + stdout: ``, + stderr: ``, exitCode: 0, }); const yarnStat2 = await xfs.lstatPromise(ppath.join(cwd, `yarn`)); @@ -157,7 +162,9 @@ describe(`EnableCommand`, () => { process.env.PATH = `${npath.fromPortablePath(cwd)}${delimiter}${process.env.PATH}`; - await expect(runCli(cwd, [`enable`])).resolves.toMatchObject({ + await expect(runCli(cwd, [`enable`, `--install-directory`, npath.fromPortablePath(cwd)])).resolves.toMatchObject({ + stdout: ``, + stderr: ``, exitCode: 0, }); From 7bfbd2ca3a64530fc9e440bd274768070786e5bf Mon Sep 17 00:00:00 2001 From: Takumi Ishikawa <20138086+polyomino24@users.noreply.github.com> Date: Fri, 14 Nov 2025 17:51:30 +0900 Subject: [PATCH 6/6] fix(test): remove unnecessary PATH setting --- tests/Enable.test.ts | 6 ------ 1 file changed, 6 deletions(-) diff --git a/tests/Enable.test.ts b/tests/Enable.test.ts index b767c7364..8d559c6b9 100644 --- a/tests/Enable.test.ts +++ b/tests/Enable.test.ts @@ -93,7 +93,6 @@ describe(`EnableCommand`, () => { await xfs.mktempPromise(async cwd => { await xfs.writeFilePromise(ppath.join(cwd, `yarn`), `hello`); - process.env.PATH = `${npath.fromPortablePath(cwd)}${delimiter}${process.env.PATH}`; await expect(runCli(cwd, [`enable`, `--install-directory`, npath.fromPortablePath(cwd)])).resolves.toMatchObject({ stdout: ``, stderr: ``, @@ -115,7 +114,6 @@ describe(`EnableCommand`, () => { ppath.join(cwd, `yarn`), ); - process.env.PATH = `${npath.fromPortablePath(cwd)}${delimiter}${process.env.PATH}`; await expect(runCli(cwd, [`enable`, `--install-directory`, npath.fromPortablePath(cwd)])).resolves.toMatchObject({ stdout: ``, stderr: expect.stringMatching(/^yarn is already installed in .+ and points to a Yarn Switch install - skipping\n$/), @@ -131,8 +129,6 @@ describe(`EnableCommand`, () => { await xfs.mktempPromise(async cwd => { await makeBin(cwd, `corepack` as Filename); - process.env.PATH = `${npath.fromPortablePath(cwd)}${delimiter}${process.env.PATH}`; - await expect(runCli(cwd, [`enable`, `--install-directory`, npath.fromPortablePath(cwd)])).resolves.toMatchObject({ stdout: ``, stderr: ``, @@ -160,8 +156,6 @@ describe(`EnableCommand`, () => { await xfs.writeFilePromise(ppath.join(cwd, `dummy-target`), `hello`); await xfs.symlinkPromise(ppath.join(cwd, `dummy-target`), ppath.join(cwd, `yarn`)); - process.env.PATH = `${npath.fromPortablePath(cwd)}${delimiter}${process.env.PATH}`; - await expect(runCli(cwd, [`enable`, `--install-directory`, npath.fromPortablePath(cwd)])).resolves.toMatchObject({ stdout: ``, stderr: ``,