Merge pull request #327 from aminya/venv-mac

fix: check for existence of venv module before installing

.github/workflows/CI.yml

@@ -109,7 +109,7 @@ jobs:
             ~/.pnpm-store
             D:\.pnpm-store
             ./node_modules
-          key: "setupcpp-node_modules-cache-OS:${{ matrix.os }}-${{ hashFiles('./.npmrc', './package.json', '.nvmrc', './packages/*/package.json') }}"
+          key: "setupcpp-node_modules-cache-OS:${{ matrix.os }}-${{ hashFiles('./.npmrc', './package.json', './.nvmrc', './pnpm-*.yaml') }}"
           restore-keys: |
             "setupcpp-node_modules-cache-OS:${{ matrix.os }}-"
 
@@ -174,7 +174,7 @@ jobs:
             ~/.pnpm-store
             D:\.pnpm-store
             ./node_modules
-          key: "setupcpp-node_modules-cache-OS:${{ matrix.os }}-${{ hashFiles('./.npmrc', './package.json', '.nvmrc', './packages/*/package.json') }}"
+          key: "setupcpp-node_modules-cache-OS:${{ matrix.os }}-${{ hashFiles('./.npmrc', './package.json', './.nvmrc', './pnpm-*.yaml') }}"
           restore-keys: |
             "setupcpp-node_modules-cache-OS:${{ matrix.os }}-"
 

src/llvm/__tests__/llvm.test.ts

@@ -64,20 +64,22 @@ describe("setup-llvm", () => {
     const directory = await setupTmpDir("llvm")
 
     const osVersion = await ubuntuVersion()
-    const { binDir } = await setupLLVM(getVersion("llvm", "true", osVersion), directory, process.arch)
-    await testBin("clang++", ["--version"], binDir)
+    {
+      const { binDir } = await setupLLVM(getVersion("llvm", "true", osVersion), directory, process.arch)
+      await testBin("clang++", ["--version"], binDir)
 
-    expect(process.env.CC?.includes("clang")).toBeTruthy()
-    expect(process.env.CXX?.includes("clang++")).toBeTruthy()
+      expect(process.env.CC?.includes("clang")).toBeTruthy()
+      expect(process.env.CXX?.includes("clang++")).toBeTruthy()
 
-    // test compilation
-    const file = join(dirname, "main.cpp")
-    const main_exe = join(dirname, addExeExt("main"))
-    await execa("clang++", [file, "-o", main_exe], { cwd: dirname })
-    if (process.platform !== "win32") {
-      await chmod(main_exe, "755")
+      // test compilation
+      const file = join(dirname, "main.cpp")
+      const main_exe = join(dirname, addExeExt("main"))
+      await execa("clang++", [file, "-o", main_exe], { cwd: dirname })
+      if (process.platform !== "win32") {
+        await chmod(main_exe, "755")
+      }
+      await execa(main_exe, { cwd: dirname, stdio: "inherit" })
     }
-    await execa(main_exe, { cwd: dirname, stdio: "inherit" })
 
     {
       const { binDir } = await setupClangFormat(getVersion("llvm", "true", osVersion), directory, process.arch)

src/python/python.ts

@@ -22,7 +22,12 @@ import type { InstallationInfo } from "../utils/setup/setupBin.js"
 import { setupChocoPack } from "../utils/setup/setupChocoPack.js"
 import { setupDnfPack } from "../utils/setup/setupDnfPack.js"
 import { setupPacmanPack } from "../utils/setup/setupPacmanPack.js"
-import { hasPipx, setupPipPackSystem, setupPipPackWithPython } from "../utils/setup/setupPipPack.js"
+import {
+  hasPipxBinary,
+  hasPipxModule,
+  setupPipPackSystem,
+  setupPipPackWithPython,
+} from "../utils/setup/setupPipPack.js"
 import { isBinUptoDate } from "../utils/setup/version.js"
 import { unique } from "../utils/std/index.js"
 import { getVersionDefault, isMinVersion } from "../versions/versions.js"
@@ -36,6 +41,9 @@ export async function setupPython(
   assert(installInfo.bin !== undefined)
   const foundPython = installInfo.bin
 
+  // setup venv
+  await setupVenv(foundPython)
+
   // setup pip
   const foundPip = await findOrSetupPip(foundPython)
   if (foundPip === undefined) {
@@ -43,7 +51,6 @@ export async function setupPython(
   }
 
   await setupPipx(foundPython)
-
   await setupWheel(foundPython)
 
   return installInfo as InstallationInfo & { bin: string }
@@ -51,30 +58,55 @@ export async function setupPython(
 
 async function setupPipx(foundPython: string) {
   try {
-    if (!(await hasPipx(foundPython))) {
+    if (!(await hasPipxModule(foundPython))) {
       try {
+        // first try with the system-wide pipx
+        await setupPipPackSystem("pipx", isArch())
+        // then install with the system-wide pipx
         await setupPipPackWithPython(foundPython, "pipx", undefined, { upgrade: true, usePipx: false })
       } catch (err) {
-        if (setupPipPackSystem("pipx", false) === null) {
-          throw new Error(`pipx was not installed correctly ${err}`)
-        }
+        throw new Error(`pipx was not installed completely: ${err}`)
       }
     }
-    await execa(foundPython, ["-m", "pipx", "ensurepath"], { stdio: "inherit" })
-    await setupVenv(foundPython)
+    if (await hasPipxModule(foundPython)) {
+      await execa(foundPython, ["-m", "pipx", "ensurepath"], { stdio: "inherit" })
+      return
+    } else if (await hasPipxBinary()) {
+      notice("pipx module not found. Trying to install with pipx binary...")
+      await execa("pipx", ["ensurepath"], { stdio: "inherit" })
+      return
+    } else {
+      throw new Error("pipx module or pipx binary not found. Corrput pipx installation.")
+    }
   } catch (err) {
     notice(`Failed to install pipx: ${(err as Error).toString()}. Ignoring...`)
   }
 }
 
 async function setupVenv(foundPython: string) {
+  if (await hasVenv(foundPython)) {
+    info("venv module already installed.")
+    return
+  }
+
   try {
-    await setupPipPackWithPython(foundPython, "venv", undefined, { upgrade: false, usePipx: false })
+    await setupPipPackSystem("venv")
   } catch (err) {
     info(`Failed to install venv: ${(err as Error).toString()}. Ignoring...`)
   }
 }
 
+async function hasVenv(foundPython: string): Promise<boolean> {
+  try {
+    // check if venv module exits
+    await execa(foundPython, ["-m", "venv", "-h"], { stdio: "ignore" })
+    return true
+  } catch {
+    // if module not found, continue
+  }
+  return false
+}
+
 /** Setup wheel and setuptools */
 async function setupWheel(foundPython: string) {
   try {

src/utils/setup/setupPipPack.ts

@@ -51,7 +51,7 @@ export async function setupPipPackWithPython(
 ): Promise<InstallationInfo> {
   const { usePipx = true, user = true, upgrade = false, isLibrary = false } = options
 
-  const isPipx = usePipx && !isLibrary && (await hasPipx(givenPython))
+  const isPipx = usePipx && !isLibrary && (await hasPipxModule(givenPython))
 
   const pip = isPipx ? "pipx" : "pip"
 
@@ -120,7 +120,11 @@ async function finishPipPackageInstall(givenPython: string, name: string) {
   return binDir
 }
 
-export async function hasPipx(givenPython: string) {
+export async function hasPipxBinary() {
+  return (await which("pipx", { nothrow: true })) !== null
+}
+
+export async function hasPipxModule(givenPython: string) {
   const res = await execa(givenPython, ["-m", "pipx", "--help"], { stdio: "ignore", reject: false })
   return res.exitCode === 0
 }
@@ -282,6 +286,7 @@ export function setupPipPackSystem(name: string, addPythonPrefix = true) {
       return installAptPack([{ name: addPythonPrefix ? `python3-${name}` : name }])
     }
   } else if (process.platform === "darwin") {
+    // brew doesn't have venv
     if (["venv"].includes(name)) {
       return null
     }