allow to automatically enable corepack (#960)

* allow to automatically enable corepack

* update command docs

* add changeset

* fix clippy

* don't show the value of corepack

* fix test

* test against more shells

* Make exec handle errors more nicely

* nicer i think

* set corepack as debug

* fix windows-style paths in Bash #390

* fix clippy

* run cygpath on `use` validation too

* trim whitespace

* fix test?

* fix test

* add changeset

.changeset/dry-rocks-smash.md

@@ -0,0 +1,5 @@
+---
+"fnm": minor
+---
+
+Add --corepack-enabled flag for automatically enabling corepack on fnm install

.changeset/rare-otters-perform.md

@@ -0,0 +1,5 @@
+---
+"fnm": patch
+---
+
+use cygwinpath to make the path posix-like on Windows Bash usage

docs/commands.md

@@ -13,6 +13,13 @@ OPTIONS:
 
             [env: FNM_ARCH]
 
+        --corepack-enabled
+            Enable corepack support for each new installation. This will make fnm call `corepack
+            enable` on every Node.js installation. For more information about corepack see
+            https://nodejs.org/api/corepack.html
+
+            [env: FNM_COREPACK_ENABLED]
+
         --fnm-dir <BASE_DIR>
             The root directory of fnm installations
 
@@ -101,6 +108,13 @@ OPTIONS:
 
             [env: FNM_ARCH]
 
+        --corepack-enabled
+            Enable corepack support for each new installation. This will make fnm call `corepack
+            enable` on every Node.js installation. For more information about corepack see
+            https://nodejs.org/api/corepack.html
+
+            [env: FNM_COREPACK_ENABLED]
+
         --fnm-dir <BASE_DIR>
             The root directory of fnm installations
 
@@ -151,6 +165,13 @@ OPTIONS:
 
             [env: FNM_ARCH]
 
+        --corepack-enabled
+            Enable corepack support for each new installation. This will make fnm call `corepack
+            enable` on every Node.js installation. For more information about corepack see
+            https://nodejs.org/api/corepack.html
+
+            [env: FNM_COREPACK_ENABLED]
+
         --fnm-dir <BASE_DIR>
             The root directory of fnm installations
 
@@ -204,6 +225,13 @@ OPTIONS:
 
             [env: FNM_ARCH]
 
+        --corepack-enabled
+            Enable corepack support for each new installation. This will make fnm call `corepack
+            enable` on every Node.js installation. For more information about corepack see
+            https://nodejs.org/api/corepack.html
+
+            [env: FNM_COREPACK_ENABLED]
+
         --fnm-dir <BASE_DIR>
             The root directory of fnm installations
 
@@ -260,6 +288,13 @@ OPTIONS:
 
             [env: FNM_ARCH]
 
+        --corepack-enabled
+            Enable corepack support for each new installation. This will make fnm call `corepack
+            enable` on every Node.js installation. For more information about corepack see
+            https://nodejs.org/api/corepack.html
+
+            [env: FNM_COREPACK_ENABLED]
+
         --fnm-dir <BASE_DIR>
             The root directory of fnm installations
 
@@ -317,6 +352,13 @@ OPTIONS:
 
             [env: FNM_ARCH]
 
+        --corepack-enabled
+            Enable corepack support for each new installation. This will make fnm call `corepack
+            enable` on every Node.js installation. For more information about corepack see
+            https://nodejs.org/api/corepack.html
+
+            [env: FNM_COREPACK_ENABLED]
+
         --fnm-dir <BASE_DIR>
             The root directory of fnm installations
 
@@ -387,6 +429,13 @@ OPTIONS:
 
             [env: FNM_ARCH]
 
+        --corepack-enabled
+            Enable corepack support for each new installation. This will make fnm call `corepack
+            enable` on every Node.js installation. For more information about corepack see
+            https://nodejs.org/api/corepack.html
+
+            [env: FNM_COREPACK_ENABLED]
+
         --fnm-dir <BASE_DIR>
             The root directory of fnm installations
 
@@ -450,6 +499,13 @@ OPTIONS:
 
             [env: FNM_ARCH]
 
+        --corepack-enabled
+            Enable corepack support for each new installation. This will make fnm call `corepack
+            enable` on every Node.js installation. For more information about corepack see
+            https://nodejs.org/api/corepack.html
+
+            [env: FNM_COREPACK_ENABLED]
+
         --fnm-dir <BASE_DIR>
             The root directory of fnm installations
 
@@ -506,6 +562,13 @@ OPTIONS:
 
             [env: FNM_ARCH]
 
+        --corepack-enabled
+            Enable corepack support for each new installation. This will make fnm call `corepack
+            enable` on every Node.js installation. For more information about corepack see
+            https://nodejs.org/api/corepack.html
+
+            [env: FNM_COREPACK_ENABLED]
+
         --fnm-dir <BASE_DIR>
             The root directory of fnm installations
 
@@ -556,6 +619,13 @@ OPTIONS:
 
             [env: FNM_ARCH]
 
+        --corepack-enabled
+            Enable corepack support for each new installation. This will make fnm call `corepack
+            enable` on every Node.js installation. For more information about corepack see
+            https://nodejs.org/api/corepack.html
+
+            [env: FNM_COREPACK_ENABLED]
+
         --fnm-dir <BASE_DIR>
             The root directory of fnm installations
 
@@ -610,6 +680,13 @@ OPTIONS:
 
             [env: FNM_ARCH]
 
+        --corepack-enabled
+            Enable corepack support for each new installation. This will make fnm call `corepack
+            enable` on every Node.js installation. For more information about corepack see
+            https://nodejs.org/api/corepack.html
+
+            [env: FNM_COREPACK_ENABLED]
+
         --fnm-dir <BASE_DIR>
             The root directory of fnm installations
 
@@ -667,6 +744,13 @@ OPTIONS:
 
             [env: FNM_ARCH]
 
+        --corepack-enabled
+            Enable corepack support for each new installation. This will make fnm call `corepack
+            enable` on every Node.js installation. For more information about corepack see
+            https://nodejs.org/api/corepack.html
+
+            [env: FNM_COREPACK_ENABLED]
+
         --fnm-dir <BASE_DIR>
             The root directory of fnm installations
 
@@ -721,6 +805,13 @@ OPTIONS:
 
             [env: FNM_ARCH]
 
+        --corepack-enabled
+            Enable corepack support for each new installation. This will make fnm call `corepack
+            enable` on every Node.js installation. For more information about corepack see
+            https://nodejs.org/api/corepack.html
+
+            [env: FNM_COREPACK_ENABLED]
+
         --fnm-dir <BASE_DIR>
             The root directory of fnm installations
 

e2e/__snapshots__/corepack.test.ts.snap

@@ -0,0 +1,28 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`Bash installs corepack: Bash 1`] = `
+"set -e
+eval "$(fnm env --corepack-enabled)"
+fnm install 18
+fnm exec --using=18 node test-pnpm-corepack.js"
+`;
+
+exports[`Fish installs corepack: Fish 1`] = `
+"fnm env --corepack-enabled | source
+fnm install 18
+fnm exec --using=18 node test-pnpm-corepack.js"
+`;
+
+exports[`PowerShell installs corepack: PowerShell 1`] = `
+"$ErrorActionPreference = "Stop"
+fnm env --corepack-enabled | Out-String | Invoke-Expression
+fnm install 18
+fnm exec --using=18 node test-pnpm-corepack.js"
+`;
+
+exports[`Zsh installs corepack: Zsh 1`] = `
+"set -e
+eval "$(fnm env --corepack-enabled)"
+fnm install 18
+fnm exec --using=18 node test-pnpm-corepack.js"
+`;

e2e/corepack.test.ts

@@ -0,0 +1,54 @@
+import fs from "fs"
+import { script } from "./shellcode/script.js"
+import { Bash, Fish, PowerShell, Zsh } from "./shellcode/shells.js"
+import describe from "./describe.js"
+import path from "path"
+import testCwd from "./shellcode/test-cwd.js"
+import { createRequire } from "module"
+
+const require = createRequire(import.meta.url)
+const whichPath = require.resolve("which")
+
+const nodescript = `
+  const which = require(${JSON.stringify(whichPath)});
+  const pnpmBinary = which.sync('pnpm')
+  const nodeBinary = which.sync('node')
+
+  const binPath = require('path').dirname(nodeBinary);
+
+  if (!pnpmBinary.includes(binPath)) {
+    console.log('pnpm not found in current Node.js bin', { binPath, pnpmBinary });
+    process.exit(1);
+  }
+  const scriptContents = require('fs').readFileSync(pnpmBinary, 'utf8');
+  console.log('scriptContents', scriptContents)
+  if (!scriptContents.includes('corepack')) {
+    console.log('corepack not found in pnpm script');
+    process.exit(1);
+  }
+`
+
+for (const shell of [Bash, Fish, PowerShell, Zsh]) {
+  describe(shell, () => {
+    test(`installs corepack`, async () => {
+      const cwd = testCwd()
+      const filepath = path.join(cwd, "test-pnpm-corepack.js")
+      fs.writeFileSync(filepath, nodescript)
+
+      await script(shell)
+        .then(shell.env({ corepackEnabled: true }))
+        .then(shell.call("fnm", ["install", "18"]))
+        .then(
+          shell.call("fnm", [
+            "exec",
+            "--using=18",
+            "node",
+            "test-pnpm-corepack.js",
+          ])
+        )
+        .takeSnapshot(shell)
+        // .addExtraEnvVar("RUST_LOG", "fnm=debug")
+        .execute(shell)
+    })
+  })
+}

e2e/env.test.ts

@@ -26,6 +26,7 @@ for (const shell of [Bash, Zsh, Fish, PowerShell, WinCmd]) {
           FNM_LOGLEVEL: "info",
           FNM_MULTISHELL_PATH: expect.any(String),
           FNM_NODE_DIST_MIRROR: expect.any(String),
+          FNM_COREPACK_ENABLED: "false",
           FNM_VERSION_FILE_STRATEGY: "local",
         })
       }

e2e/shellcode/script.ts

@@ -15,7 +15,8 @@ class Script {
     private readonly config: {
       fnmDir: string
     },
-    private readonly lines: ScriptLine[]
+    private readonly lines: ScriptLine[],
+    private readonly extraEnvVars: Record<string, string> = {}
   ) {}
   then(line: ScriptLine): Script {
     return new Script(this.config, [...this.lines, line])
@@ -28,6 +29,14 @@ class Script {
     return this
   }
 
+  addExtraEnvVar(name: string, value: string): this {
+    return new Script(
+      this.config,
+      this.lines,
+      Object.assign({}, this.extraEnvVars, { [name]: value })
+    ) as this
+  }
+
   async execute(
     shell: Pick<
       Shell,
@@ -54,6 +63,7 @@ class Script {
       cwd: testCwd(),
       env: (() => {
         const newProcessEnv: Record<string, string> = {
+          ...this.extraEnvVars,
           ...removeAllFnmEnvVars(process.env),
           PATH: [testBinDir(), fnmTargetDir(), process.env.PATH]
             .filter(Boolean)
@@ -119,8 +129,8 @@ function streamOutputsAndBuffer(child: ExecaChildProcess) {
 
   if (child.stdout) {
     child.stdout.on("data", (data) => {
-      const line = data.toString().trim()
-      if (line) {
+      const lines = data.toString().trim().split(/\r?\n/)
+      for (const line of lines) {
         process.stdout.write(`${stdoutPrefix}${line}\n`)
       }
       stdout.push(data.toString())
@@ -129,8 +139,8 @@ function streamOutputsAndBuffer(child: ExecaChildProcess) {
 
   if (child.stderr) {
     child.stderr.on("data", (data) => {
-      const line = data.toString().trim()
-      if (line) {
+      const lines = data.toString().trim().split(/\r?\n/)
+      for (const line of lines) {
         process.stdout.write(`${stderrPrefix}${line}\n`)
       }
       stderr.push(data.toString())

e2e/shellcode/shells/cmdEnv.ts

@@ -1,14 +1,19 @@
 import { ScriptLine, define } from "./types.js"
 
-type EnvConfig = { useOnCd: boolean; logLevel: string }
+type EnvConfig = {
+  useOnCd: boolean
+  logLevel: string
+  corepackEnabled: boolean
+}
 export type HasEnv = { env(cfg: Partial<EnvConfig>): ScriptLine }
 
 function stringify(envConfig: Partial<EnvConfig> = {}) {
-  const { useOnCd, logLevel } = envConfig
+  const { useOnCd, logLevel, corepackEnabled } = envConfig
   return [
     `fnm env`,
     useOnCd && "--use-on-cd",
     logLevel && `--log-level=${logLevel}`,
+    corepackEnabled && "--corepack-enabled",
   ]
     .filter(Boolean)
     .join(" ")

package.json

@@ -42,6 +42,7 @@
     "ts-dedent": "^2.2.0",
     "ts-jest": "^29.0.3",
     "typescript": "^4.8.4",
+    "which": "^3.0.1",
     "zod": "^3.19.1"
   },
   "prettier": {

pnpm-lock.yaml

@@ -21,6 +21,7 @@ specifiers:
   ts-dedent: ^2.2.0
   ts-jest: ^29.0.3
   typescript: ^4.8.4
+  which: ^3.0.1
   zod: ^3.19.1
 
 devDependencies:
@@ -44,6 +45,7 @@ devDependencies:
   ts-dedent: 2.2.0
   ts-jest: 29.0.3_4f6uxrzmuwipl5rr3bcogf6k74
   typescript: 4.9.3
+  which: 3.0.1
   zod: 3.19.1
 
 packages:
@@ -5057,6 +5059,14 @@ packages:
       isexe: 2.0.0
     dev: true
 
+  /which/3.0.1:
+    resolution: {integrity: sha512-XA1b62dzQzLfaEOSQFTCOd5KFf/1VSzZo7/7TUjnya6u0vGGKzU96UQBZTAThCb2j4/xjBAyii1OhRLJEivHvg==}
+    engines: {node: ^14.17.0 || ^16.13.0 || >=18.0.0}
+    hasBin: true
+    dependencies:
+      isexe: 2.0.0
+    dev: true
+
   /wrap-ansi/6.2.0:
     resolution: {integrity: sha512-r6lPcBGxZXlIcymEu7InxDMhdW0KDxpLgoFLcguasxCaJ/SOIZwINatK9KY/tf+ZrlywOKU0UDj3ATXUBfxJXA==}
     engines: {node: '>=8'}

src/commands/env.rs

@@ -90,6 +90,10 @@ impl Command for Env {
                 "FNM_NODE_DIST_MIRROR",
                 config.node_dist_mirror.as_str().to_owned(),
             ),
+            (
+                "FNM_COREPACK_ENABLED",
+                config.corepack_enabled().to_string(),
+            ),
             ("FNM_ARCH", config.arch.to_string()),
         ]);
 

src/commands/exec.rs

@@ -23,6 +23,25 @@ pub struct Exec {
     arguments: Vec<String>,
 }
 
+impl Exec {
+    pub(crate) fn new_for_version(
+        version: &crate::version::Version,
+        cmd: &str,
+        arguments: &[&str],
+    ) -> Self {
+        let reader = UserVersionReader::Direct(UserVersion::Full(version.clone()));
+        let args: Vec<_> = std::iter::once(cmd)
+            .chain(arguments.iter().copied())
+            .map(String::from)
+            .collect();
+        Self {
+            version: Some(reader),
+            using_file: false,
+            arguments: args,
+        }
+    }
+}
+
 impl Cmd for Exec {
     type Error = Error;
 
@@ -69,6 +88,8 @@ impl Cmd for Exec {
                 .map_err(|source| Error::CantAddPathToEnvironment { source })?
         };
 
+        log::debug!("Running {} with PATH={:?}", binary, path_env);
+
         let exit_status = Command::new(binary)
             .args(arguments)
             .stdin(Stdio::inherit())
@@ -76,7 +97,10 @@ impl Cmd for Exec {
             .stderr(Stdio::inherit())
             .env("PATH", path_env)
             .spawn()
-            .expect("Can't spawn program")
+            .map_err(|source| Error::CantSpawnProgram {
+                source,
+                binary: binary.to_string(),
+            })?
             .wait()
             .expect("Failed to grab exit code");
 
@@ -87,6 +111,11 @@ impl Cmd for Exec {
 
 #[derive(Debug, Error)]
 pub enum Error {
+    #[error("Can't spawn program: {source}\nMaybe the program {} does not exist on not available in PATH?", binary.bold())]
+    CantSpawnProgram {
+        source: std::io::Error,
+        binary: String,
+    },
     #[error("Can't read path environment variable")]
     CantReadPathVariable,
     #[error("Can't add path to environment variable: {}", source)]

src/commands/install.rs

@@ -1,3 +1,4 @@
+use super::command::Command;
 use crate::alias::create_alias;
 use crate::arch::get_safe_arch;
 use crate::config::FnmConfig;
@@ -49,7 +50,7 @@ impl Install {
     }
 }
 
-impl super::command::Command for Install {
+impl Command for Install {
     type Error = Error;
 
     fn apply(self, config: &FnmConfig) -> Result<(), Self::Error> {
@@ -134,9 +135,15 @@ impl super::command::Command for Install {
             Err(err @ DownloaderError::VersionAlreadyInstalled { .. }) => {
                 outln!(config, Error, "{} {}", "warning:".bold().yellow(), err);
             }
-            other_err => other_err.map_err(|source| Error::DownloadError { source })?,
+            Err(source) => Err(Error::DownloadError { source })?,
+            Ok(_) => {}
         };
 
+        if config.corepack_enabled() {
+            outln!(config, Info, "Enabling corepack for {}", version_str.cyan());
+            enable_corepack(&version, config)?;
+        }
+
         if let UserVersion::Full(Version::Lts(lts_type)) = current_version {
             let alias_name = Version::Lts(lts_type).v_str();
             debug!(
@@ -156,6 +163,19 @@ impl super::command::Command for Install {
     }
 }
 
+fn enable_corepack(version: &Version, config: &FnmConfig) -> Result<(), Error> {
+    let corepack_path = version.installation_path(config);
+    let corepack_path = if cfg!(windows) {
+        corepack_path.join("corepack.cmd")
+    } else {
+        corepack_path.join("bin").join("corepack")
+    };
+    super::exec::Exec::new_for_version(version, corepack_path.to_str().unwrap(), &["enable"])
+        .apply(config)
+        .map_err(|source| Error::CorepackError { source })?;
+    Ok(())
+}
+
 #[derive(Debug, Error)]
 pub enum Error {
     #[error("Can't download the requested binary: {}", source)]
@@ -165,6 +185,11 @@ pub enum Error {
         #[from]
         source: std::io::Error,
     },
+    #[error("Can't enable corepack: {source}")]
+    CorepackError {
+        #[from]
+        source: super::exec::Error,
+    },
     #[error("Can't find version in dotfiles. Please provide a version manually to the command.")]
     CantInferVersion,
     #[error("Having a hard time listing the remote versions: {}", source)]
@@ -186,7 +211,6 @@ pub enum Error {
 
 #[cfg(test)]
 mod tests {
-    use super::super::command::Command;
     use super::*;
     use pretty_assertions::assert_eq;
     use std::str::FromStr;

src/commands/use.rs

@@ -4,6 +4,7 @@ use crate::current_version::current_version;
 use crate::fs;
 use crate::installed_versions;
 use crate::outln;
+use crate::shell;
 use crate::system_version;
 use crate::user_version::UserVersion;
 use crate::version::Version;
@@ -190,8 +191,11 @@ fn warn_if_multishell_path_not_in_path_env_var(
         multishell_path.to_path_buf()
     };
 
+    let fixed_path = bin_path.to_str().and_then(shell::maybe_fix_windows_path);
+    let fixed_path = fixed_path.as_ref().map(|x| &x[..]);
+
     for path in std::env::split_paths(&std::env::var("PATH").unwrap_or_default()) {
-        if bin_path == path {
+        if bin_path == path || fixed_path == path.to_str() {
             return;
         }
     }

src/config.rs

@@ -70,6 +70,17 @@ pub struct FnmConfig {
         hide_env_values = true,
     )]
     version_file_strategy: VersionFileStrategy,
+
+    /// Enable corepack support for each new installation.
+    /// This will make fnm call `corepack enable` on every Node.js installation.
+    /// For more information about corepack see https://nodejs.org/api/corepack.html
+    #[clap(
+        long,
+        env = "FNM_COREPACK_ENABLED",
+        global = true,
+        hide_env_values = true
+    )]
+    corepack_enabled: bool,
 }
 
 impl Default for FnmConfig {
@@ -81,6 +92,7 @@ impl Default for FnmConfig {
             log_level: LogLevel::Info,
             arch: Arch::default(),
             version_file_strategy: VersionFileStrategy::default(),
+            corepack_enabled: false,
         }
     }
 }
@@ -90,6 +102,10 @@ impl FnmConfig {
         &self.version_file_strategy
     }
 
+    pub fn corepack_enabled(&self) -> bool {
+        self.corepack_enabled
+    }
+
     pub fn multishell_path(&self) -> Option<&std::path::Path> {
         match &self.multishell_path {
             None => None,

src/shell/bash.rs

@@ -16,6 +16,8 @@ impl Shell for Bash {
         let path = path
             .to_str()
             .ok_or_else(|| anyhow::anyhow!("Can't convert path to string"))?;
+        let path =
+            super::windows_compat::maybe_fix_windows_path(path).unwrap_or_else(|| path.to_string());
         Ok(format!("export PATH={path:?}:$PATH"))
     }
 

src/shell/fish.rs

@@ -16,6 +16,8 @@ impl Shell for Fish {
         let path = path
             .to_str()
             .ok_or_else(|| anyhow::anyhow!("Can't convert path to string"))?;
+        let path =
+            super::windows_compat::maybe_fix_windows_path(path).unwrap_or_else(|| path.to_string());
         Ok(format!("set -gx PATH {path:?} $PATH;"))
     }
 

src/shell/mod.rs

@@ -7,6 +7,7 @@ mod zsh;
 
 #[allow(clippy::module_inception)]
 mod shell;
+mod windows_compat;
 
 pub use bash::Bash;
 pub use fish::Fish;
@@ -14,4 +15,5 @@ pub use infer::infer_shell;
 pub use powershell::PowerShell;
 pub use shell::{Shell, AVAILABLE_SHELLS};
 pub use windows_cmd::WindowsCmd;
+pub use windows_compat::maybe_fix_windows_path;
 pub use zsh::Zsh;

src/shell/windows_compat.rs

@@ -0,0 +1,21 @@
+/// On Bash for Windows, we need to convert the path from a Windows-style
+/// path to a Unix-style path. This is because Bash for Windows doesn't
+/// understand Windows-style paths. We use `cygpath` to do this conversion.
+/// If `cygpath` fails, we assume we're not on Bash for Windows and just
+/// return the original path.
+pub fn maybe_fix_windows_path(path: &str) -> Option<String> {
+    if !cfg!(windows) {
+        return None;
+    }
+
+    let output = std::process::Command::new("cygpath")
+        .arg(path)
+        .output()
+        .ok()?;
+    if output.status.success() {
+        let output = String::from_utf8(output.stdout).ok()?;
+        Some(output.trim().to_string())
+    } else {
+        None
+    }
+}

src/shell/zsh.rs

@@ -16,6 +16,8 @@ impl Shell for Zsh {
         let path = path
             .to_str()
             .ok_or_else(|| anyhow::anyhow!("Path is not valid UTF-8"))?;
+        let path =
+            super::windows_compat::maybe_fix_windows_path(path).unwrap_or_else(|| path.to_string());
         Ok(format!("export PATH={path:?}:$PATH"))
     }
 

src/version.rs

@@ -77,6 +77,9 @@ impl Version {
     }
 }
 
+// TODO: add a trait called BinPath that &Path and PathBuf implements
+// which adds the `.bin_path()` which works both on windows and unix :)
+
 impl<'de> serde::Deserialize<'de> for Version {
     fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
     where