From 32c5c86bdbb72fd14aa0a293180ae52ef4d926f2 Mon Sep 17 00:00:00 2001 From: Priyambada Roul Date: Thu, 7 Aug 2025 23:22:00 +0530 Subject: [PATCH 1/3] Detect Swiftly Installation --- src/toolchain/swiftly.ts | 14 +++++++- src/ui/ToolchainSelection.ts | 2 +- test/unit-tests/toolchain/swiftly.test.ts | 44 +++++++++++++++++++++++ 3 files changed, 58 insertions(+), 2 deletions(-) diff --git a/src/toolchain/swiftly.ts b/src/toolchain/swiftly.ts index 554811ab8..833a882ac 100644 --- a/src/toolchain/swiftly.ts +++ b/src/toolchain/swiftly.ts @@ -148,7 +148,7 @@ export class Swiftly { } } - private static isSupported() { + public static isSupported() { return process.platform === "linux" || process.platform === "darwin"; } @@ -235,4 +235,16 @@ export class Swiftly { ); return JSON.parse(swiftlyConfigRaw); } + + public static async isInstalled(): Promise { + if (!this.isSupported()) { + return false; + } + try { + const { stdout } = await execFile("which", ["swiftly"]); + return stdout.trim().length > 0; + } catch (error) { + return false; + } + } } diff --git a/src/ui/ToolchainSelection.ts b/src/ui/ToolchainSelection.ts index d465ee61e..e42b4f410 100644 --- a/src/ui/ToolchainSelection.ts +++ b/src/ui/ToolchainSelection.ts @@ -259,7 +259,7 @@ async function getQuickPickItems( } // Various actions that the user can perform (e.g. to install new toolchains) const actionItems: ActionItem[] = []; - if (process.platform === "linux" || process.platform === "darwin") { + if (Swiftly.isSupported() && !(await Swiftly.isInstalled())) { const platformName = process.platform === "linux" ? "Linux" : "macOS"; actionItems.push({ type: "action", diff --git a/test/unit-tests/toolchain/swiftly.test.ts b/test/unit-tests/toolchain/swiftly.test.ts index fa48bea81..12943ce67 100644 --- a/test/unit-tests/toolchain/swiftly.test.ts +++ b/test/unit-tests/toolchain/swiftly.test.ts @@ -102,4 +102,48 @@ suite("Swiftly Unit Tests", () => { expect(mockUtilities.execFile).not.have.been.called; }); }); + + suite("isInstalled", () => { + test("should return true when swiftly is found", async () => { + mockUtilities.execFile.withArgs("which", ["swiftly"]).resolves({ + stdout: "/usr/local/bin/swiftly\n", + stderr: "", + }); + + const result = await Swiftly.isInstalled(); + + expect(result).to.be.true; + expect(mockUtilities.execFile).to.have.been.calledWith("which", ["swiftly"]); + }); + + test("should return false when swiftly is not found", async () => { + mockUtilities.execFile.withArgs("which", ["swiftly"]).rejects(new Error("not found")); + + const result = await Swiftly.isInstalled(); + + expect(result).to.be.false; + expect(mockUtilities.execFile).to.have.been.calledWith("which", ["swiftly"]); + }); + + test("should return false when which returns empty output", async () => { + mockUtilities.execFile.withArgs("which", ["swiftly"]).resolves({ + stdout: "", + stderr: "", + }); + + const result = await Swiftly.isInstalled(); + + expect(result).to.be.false; + expect(mockUtilities.execFile).to.have.been.calledWith("which", ["swiftly"]); + }); + + test("should return false when platform is not supported", async () => { + mockedPlatform.setValue("win32"); + + const result = await Swiftly.isInstalled(); + + expect(result).to.be.false; + expect(mockUtilities.execFile).not.to.have.been.called; + }); + }); }); From 3bc4ae0ef283308ef8cec3e45e0d2e47a987a61d Mon Sep 17 00:00:00 2001 From: Priyambada Roul Date: Sun, 10 Aug 2025 20:09:24 +0530 Subject: [PATCH 2/3] Refactor binary path detection with new shell utility function --- src/toolchain/swiftly.ts | 5 +++-- src/toolchain/toolchain.ts | 14 ++---------- src/utilities/shell.ts | 16 ++++++++++++++ test/unit-tests/toolchain/swiftly.test.ts | 27 ++++++----------------- test/unit-tests/utilities/shell.test.ts | 21 ++++++++++++++++++ 5 files changed, 49 insertions(+), 34 deletions(-) create mode 100644 src/utilities/shell.ts create mode 100644 test/unit-tests/utilities/shell.test.ts diff --git a/src/toolchain/swiftly.ts b/src/toolchain/swiftly.ts index 833a882ac..d7bd97c1f 100644 --- a/src/toolchain/swiftly.ts +++ b/src/toolchain/swiftly.ts @@ -20,6 +20,7 @@ import * as vscode from "vscode"; import { Version } from "../utilities/version"; import { z } from "zod/v4/mini"; import { SwiftLogger } from "../logging/SwiftLogger"; +import { findBinaryPath } from "../utilities/shell"; const ListResult = z.object({ toolchains: z.array( @@ -241,8 +242,8 @@ export class Swiftly { return false; } try { - const { stdout } = await execFile("which", ["swiftly"]); - return stdout.trim().length > 0; + await findBinaryPath("swiftly"); + return true; } catch (error) { return false; } diff --git a/src/toolchain/toolchain.ts b/src/toolchain/toolchain.ts index e96dc2ca1..ddda1382e 100644 --- a/src/toolchain/toolchain.ts +++ b/src/toolchain/toolchain.ts @@ -26,6 +26,7 @@ import { Sanitizer } from "./Sanitizer"; import { lineBreakRegex } from "../utilities/tasks"; import { Swiftly } from "./swiftly"; import { SwiftLogger } from "../logging/SwiftLogger"; +import { findBinaryPath } from "../utilities/shell"; /** * Contents of **Info.plist** on Windows. */ @@ -550,18 +551,7 @@ export class SwiftToolchain { // use `type swift` to find `swift`. Run inside /bin/sh to ensure // we get consistent output as different shells output a different // format. Tried running with `-p` but that is not available in /bin/sh - const { stdout, stderr } = await execFile("/bin/sh", [ - "-c", - "LC_MESSAGES=C type swift", - ]); - const swiftMatch = /^swift is (.*)$/.exec(stdout.trimEnd()); - if (swiftMatch) { - swift = swiftMatch[1]; - } else { - throw Error( - `/bin/sh -c LC_MESSAGES=C type swift: stdout: ${stdout}, stderr: ${stderr}` - ); - } + swift = await findBinaryPath("swift"); break; } } diff --git a/src/utilities/shell.ts b/src/utilities/shell.ts new file mode 100644 index 000000000..a72500d27 --- /dev/null +++ b/src/utilities/shell.ts @@ -0,0 +1,16 @@ +import { execFile } from "./utilities"; + +export async function findBinaryPath(binaryName: string): Promise { + const { stdout, stderr } = await execFile("/bin/sh", [ + "-c", + `LC_MESSAGES=C type ${binaryName}`, + ]); + const binaryNameMatch = new RegExp(`^${binaryName} is (.*)$`).exec(stdout.trimEnd()); + if (binaryNameMatch) { + return binaryNameMatch[1]; + } else { + throw Error( + `/bin/sh -c LC_MESSAGES=C type ${binaryName}: stdout: ${stdout}, stderr: ${stderr}` + ); + } +} diff --git a/test/unit-tests/toolchain/swiftly.test.ts b/test/unit-tests/toolchain/swiftly.test.ts index 12943ce67..41e11de10 100644 --- a/test/unit-tests/toolchain/swiftly.test.ts +++ b/test/unit-tests/toolchain/swiftly.test.ts @@ -15,10 +15,12 @@ import { expect } from "chai"; import { Swiftly } from "../../../src/toolchain/swiftly"; import * as utilities from "../../../src/utilities/utilities"; +import * as shell from "../../../src/utilities/shell"; import { mockGlobalModule, mockGlobalValue } from "../../MockUtils"; suite("Swiftly Unit Tests", () => { const mockUtilities = mockGlobalModule(utilities); + const mockShell = mockGlobalModule(shell); const mockedPlatform = mockGlobalValue(process, "platform"); setup(() => { @@ -105,36 +107,21 @@ suite("Swiftly Unit Tests", () => { suite("isInstalled", () => { test("should return true when swiftly is found", async () => { - mockUtilities.execFile.withArgs("which", ["swiftly"]).resolves({ - stdout: "/usr/local/bin/swiftly\n", - stderr: "", - }); + mockShell.findBinaryPath.withArgs("swiftly").resolves("/usr/local/bin/swiftly"); const result = await Swiftly.isInstalled(); expect(result).to.be.true; - expect(mockUtilities.execFile).to.have.been.calledWith("which", ["swiftly"]); + expect(mockShell.findBinaryPath).to.have.been.calledWith("swiftly"); }); test("should return false when swiftly is not found", async () => { - mockUtilities.execFile.withArgs("which", ["swiftly"]).rejects(new Error("not found")); - - const result = await Swiftly.isInstalled(); - - expect(result).to.be.false; - expect(mockUtilities.execFile).to.have.been.calledWith("which", ["swiftly"]); - }); - - test("should return false when which returns empty output", async () => { - mockUtilities.execFile.withArgs("which", ["swiftly"]).resolves({ - stdout: "", - stderr: "", - }); + mockShell.findBinaryPath.withArgs("swiftly").rejects(new Error("not found")); const result = await Swiftly.isInstalled(); expect(result).to.be.false; - expect(mockUtilities.execFile).to.have.been.calledWith("which", ["swiftly"]); + expect(mockShell.findBinaryPath).to.have.been.calledWith("swiftly"); }); test("should return false when platform is not supported", async () => { @@ -143,7 +130,7 @@ suite("Swiftly Unit Tests", () => { const result = await Swiftly.isInstalled(); expect(result).to.be.false; - expect(mockUtilities.execFile).not.to.have.been.called; + expect(mockShell.findBinaryPath).not.to.have.been.called; }); }); }); diff --git a/test/unit-tests/utilities/shell.test.ts b/test/unit-tests/utilities/shell.test.ts new file mode 100644 index 000000000..b9aa54c3f --- /dev/null +++ b/test/unit-tests/utilities/shell.test.ts @@ -0,0 +1,21 @@ +import { expect } from "chai"; +import { findBinaryPath } from "../../../src/utilities/shell"; + +suite("Shell Unit Test Suite", () => { + suite("findBinaryPath", () => { + test("returns the path to a binary in the PATH", async () => { + const binaryPath = await findBinaryPath("node"); + expect(binaryPath).to.be.a("string"); + expect(binaryPath).to.include("node"); + }); + + test("throws for a non-existent binary", async () => { + try { + await findBinaryPath("nonexistentbinary"); + expect.fail("Expected an error to be thrown for a non-existent binary"); + } catch (error) { + expect(error).to.be.an("error"); + } + }); + }); +}); From 1b4804233ca29e51aa78b7eff2d8406d76ff8cd0 Mon Sep 17 00:00:00 2001 From: Priyambada Roul Date: Wed, 13 Aug 2025 23:13:24 +0530 Subject: [PATCH 3/3] Added missing copyright headers --- src/toolchain/toolchain.ts | 3 -- src/utilities/shell.ts | 17 ++++++++++ test/unit-tests/utilities/shell.test.ts | 45 +++++++++++++++++++++++-- 3 files changed, 60 insertions(+), 5 deletions(-) diff --git a/src/toolchain/toolchain.ts b/src/toolchain/toolchain.ts index ddda1382e..e50afe8d0 100644 --- a/src/toolchain/toolchain.ts +++ b/src/toolchain/toolchain.ts @@ -548,9 +548,6 @@ export class SwiftToolchain { break; } default: { - // use `type swift` to find `swift`. Run inside /bin/sh to ensure - // we get consistent output as different shells output a different - // format. Tried running with `-p` but that is not available in /bin/sh swift = await findBinaryPath("swift"); break; } diff --git a/src/utilities/shell.ts b/src/utilities/shell.ts index a72500d27..77a6aa295 100644 --- a/src/utilities/shell.ts +++ b/src/utilities/shell.ts @@ -1,5 +1,22 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the VS Code Swift open source project +// +// Copyright (c) 2025 the VS Code Swift project authors +// Licensed under Apache License v2.0 +// +// See LICENSE.txt for license information +// See CONTRIBUTORS.txt for the list of VS Code Swift project authors +// +// SPDX-License-Identifier: Apache-2.0 +// +//===----------------------------------------------------------------------===// + import { execFile } from "./utilities"; +// use `type swift` to find `swift`. Run inside /bin/sh to ensure +// we get consistent output as different shells output a different +// format. Tried running with `-p` but that is not available in /bin/sh export async function findBinaryPath(binaryName: string): Promise { const { stdout, stderr } = await execFile("/bin/sh", [ "-c", diff --git a/test/unit-tests/utilities/shell.test.ts b/test/unit-tests/utilities/shell.test.ts index b9aa54c3f..cbbd2a5e6 100644 --- a/test/unit-tests/utilities/shell.test.ts +++ b/test/unit-tests/utilities/shell.test.ts @@ -1,20 +1,61 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the VS Code Swift open source project +// +// Copyright (c) 2025 the VS Code Swift project authors +// Licensed under Apache License v2.0 +// +// See LICENSE.txt for license information +// See CONTRIBUTORS.txt for the list of VS Code Swift project authors +// +// SPDX-License-Identifier: Apache-2.0 +// +//===----------------------------------------------------------------------===// + +import { beforeEach, afterEach } from "mocha"; import { expect } from "chai"; +import * as sinon from "sinon"; import { findBinaryPath } from "../../../src/utilities/shell"; +import * as utilities from "../../../src/utilities/utilities"; suite("Shell Unit Test Suite", () => { + let execFileStub: sinon.SinonStub; + + beforeEach(() => { + execFileStub = sinon.stub(utilities, "execFile"); + }); + + afterEach(() => { + sinon.restore(); + }); + suite("findBinaryPath", () => { test("returns the path to a binary in the PATH", async () => { + execFileStub.resolves({ + stdout: "node is /usr/local/bin/node\n", + stderr: "", + }); + const binaryPath = await findBinaryPath("node"); - expect(binaryPath).to.be.a("string"); - expect(binaryPath).to.include("node"); + expect(binaryPath).to.equal("/usr/local/bin/node"); + expect(execFileStub).to.have.been.calledWith("/bin/sh", [ + "-c", + "LC_MESSAGES=C type node", + ]); }); test("throws for a non-existent binary", async () => { + execFileStub.resolves({ + stdout: "", + stderr: "sh: type: nonexistentbinary: not found\n", + }); + try { await findBinaryPath("nonexistentbinary"); expect.fail("Expected an error to be thrown for a non-existent binary"); } catch (error) { expect(error).to.be.an("error"); + expect((error as Error).message).to.include("nonexistentbinary"); } }); });