Support language_version: system (#949)

* .

* Support `language_version: system`

* Fix python downloads

Author: j178

lib/constants/src/env_vars.rs

@@ -25,6 +25,8 @@ impl EnvVars {
     pub const PREK_INTERNAL__SKIP_POST_CHECKOUT: &'static str = "PREK_INTERNAL__SKIP_POST_CHECKOUT";
     pub const PREK_INTERNAL__RUN_ORIGINAL_PRE_COMMIT: &'static str =
         "PREK_INTERNAL__RUN_ORIGINAL_PRE_COMMIT";
+    pub const PREK_INTERNAL__GO_BINARY_NAME: &'static str = "PREK_INTERNAL__GO_BINARY_NAME";
+    pub const PREK_INTERNAL__NODE_BINARY_NAME: &'static str = "PREK_INTERNAL__NODE_BINARY_NAME";
     pub const PREK_GENERATE: &'static str = "PREK_GENERATE";
 
     // UV related

src/languages/golang/golang.rs

@@ -32,13 +32,13 @@ impl LanguageImpl for Golang {
         let go_dir = store.tools_path(crate::store::ToolBucket::Go);
         let installer = GoInstaller::new(go_dir);
 
-        let version = match &hook.language_request {
-            LanguageRequest::Any => &GoRequest::Any,
-            LanguageRequest::Golang(version) => version,
+        let (version, allows_download) = match &hook.language_request {
+            LanguageRequest::Any { system_only } => (&GoRequest::Any, !system_only),
+            LanguageRequest::Golang(version) => (version, true),
             _ => unreachable!(),
         };
         let go = installer
-            .install(store, version)
+            .install(store, version, allows_download)
             .await
             .context("Failed to install go")?;
 

src/languages/golang/installer.rs

@@ -2,8 +2,10 @@ use std::env::consts::EXE_EXTENSION;
 use std::fmt::Display;
 use std::path::{Path, PathBuf};
 use std::str::FromStr;
+use std::sync::LazyLock;
 
 use anyhow::{Context, Result};
+use constants::env_vars::EnvVars;
 use itertools::Itertools;
 use reqwest::Client;
 use target_lexicon::{Architecture, HOST, OperatingSystem};
@@ -31,6 +33,15 @@ impl Display for GoResult {
     }
 }
 
+/// Override the Go binary name for testing.
+static GO_BINARY_NAME: LazyLock<String> = LazyLock::new(|| {
+    if let Ok(name) = EnvVars::var(EnvVars::PREK_INTERNAL__GO_BINARY_NAME) {
+        name
+    } else {
+        "go".to_string()
+    }
+});
+
 impl GoResult {
     fn from_executable(path: PathBuf, from_system: bool) -> Self {
         Self {
@@ -100,7 +111,12 @@ impl GoInstaller {
         }
     }
 
-    pub(crate) async fn install(&self, store: &Store, request: &GoRequest) -> Result<GoResult> {
+    pub(crate) async fn install(
+        &self,
+        store: &Store,
+        request: &GoRequest,
+        allows_download: bool,
+    ) -> Result<GoResult> {
         fs_err::tokio::create_dir_all(&self.root).await?;
 
         let _lock = LockedFile::acquire(self.root.join(".lock"), "go").await?;
@@ -115,6 +131,10 @@ impl GoInstaller {
             return Ok(go);
         }
 
+        if !allows_download {
+            anyhow::bail!("No suitable system Go version found and downloads are disabled");
+        }
+
         let resolved_version = self
             .resolve_version(request)
             .await
@@ -231,7 +251,7 @@ impl GoInstaller {
     }
 
     async fn find_system_go(&self, go_request: &GoRequest) -> Result<Option<GoResult>> {
-        let go_paths = match which::which_all("go") {
+        let go_paths = match which::which_all(&*GO_BINARY_NAME) {
             Ok(paths) => paths,
             Err(e) => {
                 debug!("No go executables found in PATH: {}", e);

src/languages/node/installer.rs

@@ -3,8 +3,10 @@ use std::fmt::Display;
 use std::path::{Path, PathBuf};
 use std::str::FromStr;
 use std::string::ToString;
+use std::sync::LazyLock;
 
 use anyhow::{Context, Result};
+use constants::env_vars::EnvVars;
 use itertools::Itertools;
 use reqwest::Client;
 use target_lexicon::{Architecture, HOST, OperatingSystem};
@@ -31,6 +33,15 @@ impl Display for NodeResult {
     }
 }
 
+/// Override the Node binary name for testing.
+static NODE_BINARY_NAME: LazyLock<String> = LazyLock::new(|| {
+    if let Ok(name) = EnvVars::var(EnvVars::PREK_INTERNAL__NODE_BINARY_NAME) {
+        name
+    } else {
+        "node".to_string()
+    }
+});
+
 impl NodeResult {
     pub(crate) fn from_executables(node: PathBuf, npm: PathBuf) -> Self {
         Self {
@@ -97,7 +108,12 @@ impl NodeInstaller {
     }
 
     /// Install a version of Node.js.
-    pub(crate) async fn install(&self, store: &Store, request: &NodeRequest) -> Result<NodeResult> {
+    pub(crate) async fn install(
+        &self,
+        store: &Store,
+        request: &NodeRequest,
+        allows_download: bool,
+    ) -> Result<NodeResult> {
         fs_err::tokio::create_dir_all(&self.root).await?;
 
         let _lock = LockedFile::acquire(self.root.join(".lock"), "node").await?;
@@ -113,6 +129,10 @@ impl NodeInstaller {
             return Ok(node_result);
         }
 
+        if !allows_download {
+            anyhow::bail!("No suitable system Node version found and downloads are disabled");
+        }
+
         let resolved_version = self.resolve_version(request).await?;
         trace!(version = %resolved_version, "Downloading node");
 
@@ -222,7 +242,7 @@ impl NodeInstaller {
 
     /// Find a suitable system Node.js installation that matches the request.
     async fn find_system_node(&self, node_request: &NodeRequest) -> Result<Option<NodeResult>> {
-        let node_paths = match which::which_all("node") {
+        let node_paths = match which::which_all(&*NODE_BINARY_NAME) {
             Ok(paths) => paths,
             Err(e) => {
                 debug!("No node executables found in PATH: {}", e);

src/languages/node/node.rs

@@ -43,13 +43,13 @@ impl LanguageImpl for Node {
         let node_dir = store.tools_path(ToolBucket::Node);
         let installer = NodeInstaller::new(node_dir);
 
-        let node_request = match &hook.language_request {
-            LanguageRequest::Any => &NodeRequest::Any,
-            LanguageRequest::Node(node_request) => node_request,
+        let (node_request, allows_download) = match &hook.language_request {
+            LanguageRequest::Any { system_only } => (&NodeRequest::Any, !system_only),
+            LanguageRequest::Node(node_request) => (node_request, true),
             _ => unreachable!(),
         };
         let node = installer
-            .install(store, node_request)
+            .install(store, node_request, allows_download)
             .await
             .context("Failed to install node")?;
 

src/languages/python/python.rs

@@ -63,23 +63,6 @@ pub(crate) async fn query_python_info(python: &Path) -> Result<PythonInfo> {
     })
 }
 
-fn to_uv_python_request(request: &LanguageRequest) -> Option<String> {
-    match request {
-        LanguageRequest::Any => None,
-        LanguageRequest::Python(request) => match request {
-            PythonRequest::Any => None,
-            PythonRequest::Major(major) => Some(format!("{major}")),
-            PythonRequest::MajorMinor(major, minor) => Some(format!("{major}.{minor}")),
-            PythonRequest::MajorMinorPatch(major, minor, patch) => {
-                Some(format!("{major}.{minor}.{patch}"))
-            }
-            PythonRequest::Range(_, raw) => Some(raw.clone()),
-            PythonRequest::Path(path) => Some(path.to_string_lossy().to_string()),
-        },
-        _ => unreachable!(),
-    }
-}
-
 impl LanguageImpl for Python {
     async fn install(
         &self,
@@ -102,10 +85,8 @@ impl LanguageImpl for Python {
 
         debug!(%hook, target = %info.env_path.display(), "Installing environment");
 
-        let python_request = to_uv_python_request(&hook.language_request);
-
         // Create venv (auto download Python if needed)
-        Self::create_venv_with_retry(&uv, store, &info, python_request.as_ref())
+        Self::create_venv(&uv, store, &info, &hook.language_request)
             .await
             .context("Failed to create Python virtual environment")?;
 
@@ -223,12 +204,29 @@ impl LanguageImpl for Python {
     }
 }
 
+fn to_uv_python_request(request: &LanguageRequest) -> Option<String> {
+    match request {
+        LanguageRequest::Any { .. } => None,
+        LanguageRequest::Python(request) => match request {
+            PythonRequest::Any => None,
+            PythonRequest::Major(major) => Some(format!("{major}")),
+            PythonRequest::MajorMinor(major, minor) => Some(format!("{major}.{minor}")),
+            PythonRequest::MajorMinorPatch(major, minor, patch) => {
+                Some(format!("{major}.{minor}.{patch}"))
+            }
+            PythonRequest::Range(_, raw) => Some(raw.clone()),
+            PythonRequest::Path(path) => Some(path.to_string_lossy().to_string()),
+        },
+        _ => unreachable!(),
+    }
+}
+
 impl Python {
-    async fn create_venv_with_retry(
+    async fn create_venv(
         uv: &Uv,
         store: &Store,
         info: &InstallInfo,
-        python_request: Option<&String>,
+        python_request: &LanguageRequest,
     ) -> Result<()> {
         // Try creating venv without downloads first
         match Self::create_venv_command(uv, store, info, python_request, false, false)
@@ -246,6 +244,12 @@ impl Python {
             Err(e @ process::Error::Status { .. }) => {
                 // Check if we can retry with downloads
                 if Self::can_retry_with_downloads(&e) {
+                    if !python_request.allows_download() {
+                        anyhow::bail!(
+                            "No suitable system Python version found and downloads are disabled"
+                        );
+                    }
+
                     debug!(
                         "Retrying venv creation with managed Python downloads: `{}`",
                         info.env_path.display()
@@ -270,7 +274,7 @@ impl Python {
         uv: &Uv,
         store: &Store,
         info: &InstallInfo,
-        python_request: Option<&String>,
+        python_request: &LanguageRequest,
         set_install_dir: bool,
         allow_downloads: bool,
     ) -> Cmd {
@@ -297,7 +301,7 @@ impl Python {
             cmd.arg("--no-python-downloads");
         }
 
-        if let Some(python) = python_request {
+        if let Some(python) = to_uv_python_request(python_request) {
             cmd.arg("--python").arg(python);
         }
 

src/languages/python/version.rs

@@ -5,7 +5,7 @@ use std::str::FromStr;
 
 use crate::hook::InstallInfo;
 use crate::languages::version;
-use crate::languages::version::try_into_u64_slice;
+use crate::languages::version::{Error, try_into_u64_slice};
 
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub(crate) enum PythonRequest {
@@ -53,15 +53,15 @@ impl FromStr for PythonRequest {
                     // Try to parse as a VersionReq (like ">= 3.12" or ">=3.8, <3.12")
                     semver::VersionReq::parse(request)
                         .map(|version_req| PythonRequest::Range(version_req, request.into()))
-                        .map_err(|_| version::Error::InvalidVersion(request.to_string()))
+                        .map_err(|_| Error::InvalidVersion(request.to_string()))
                 })
                 .or_else(|_| {
                     // If it doesn't match any known format, treat it as a path
                     let path = PathBuf::from(request);
                     if path.exists() {
                         Ok(PythonRequest::Path(path))
                     } else {
-                        Err(version::Error::InvalidVersion(request.to_string()))
+                        Err(Error::InvalidVersion(request.to_string()))
                     }
                 })
         }
@@ -77,16 +77,16 @@ impl PythonRequest {
     fn parse_version_numbers(
         version_str: &str,
         original_request: &str,
-    ) -> Result<PythonRequest, version::Error> {
+    ) -> Result<PythonRequest, Error> {
         let parts = try_into_u64_slice(version_str)
-            .map_err(|_| version::Error::InvalidVersion(original_request.to_string()))?;
+            .map_err(|_| Error::InvalidVersion(original_request.to_string()))?;
         let parts = split_wheel_tag_version(parts);
 
         match parts[..] {
             [major] => Ok(PythonRequest::Major(major)),
             [major, minor] => Ok(PythonRequest::MajorMinor(major, minor)),
             [major, minor, patch] => Ok(PythonRequest::MajorMinorPatch(major, minor, patch)),
-            _ => Err(version::Error::InvalidVersion(original_request.to_string())),
+            _ => Err(Error::InvalidVersion(original_request.to_string())),
         }
     }
 

src/languages/version.rs

@@ -14,7 +14,7 @@ pub(crate) enum Error {
 
 #[derive(Debug, Clone, Eq, PartialEq)]
 pub(crate) enum LanguageRequest {
-    Any,
+    Any { system_only: bool },
     Python(PythonRequest),
     Node(NodeRequest),
     Golang(GoRequest),
@@ -25,14 +25,26 @@ pub(crate) enum LanguageRequest {
 impl LanguageRequest {
     pub(crate) fn is_any(&self) -> bool {
         match self {
-            LanguageRequest::Any => true,
+            LanguageRequest::Any { .. } => true,
             LanguageRequest::Python(req) => req.is_any(),
             LanguageRequest::Node(req) => req.is_any(),
             LanguageRequest::Golang(req) => req.is_any(),
             LanguageRequest::Semver(_) => false,
         }
     }
 
+    /// Returns true if this request allows downloading a version.
+    ///
+    /// Currently, only `system` disallows downloading. In the future,
+    /// we may add more specific version requests that also disallow downloading.
+    /// For example `language_version: 3.12; system_only`.
+    pub(crate) fn allows_download(&self) -> bool {
+        match self {
+            LanguageRequest::Any { system_only } => !system_only,
+            _ => true,
+        }
+    }
+
     pub(crate) fn parse(lang: Language, request: &str) -> Result<Self, Error> {
         // `pre-commit` support these values in `language_version`:
         // - `default`: substituted by language `get_default_version` function
@@ -44,9 +56,11 @@ impl LanguageRequest {
         // - Node.js version passed down to `nodeenv`
         // - Rust version passed down to `rustup`
 
-        // TODO: support `system`? Does anyone use it?
         if request == "default" || request.is_empty() {
-            return Ok(LanguageRequest::Any);
+            return Ok(LanguageRequest::Any { system_only: false });
+        }
+        if request == "system" {
+            return Ok(LanguageRequest::Any { system_only: true });
         }
 
         Ok(match lang {
@@ -59,7 +73,7 @@ impl LanguageRequest {
 
     pub(crate) fn satisfied_by(&self, install_info: &InstallInfo) -> bool {
         match self {
-            LanguageRequest::Any => true,
+            LanguageRequest::Any { .. } => true,
             LanguageRequest::Python(req) => req.satisfied_by(install_info),
             LanguageRequest::Node(req) => req.satisfied_by(install_info),
             LanguageRequest::Golang(req) => req.satisfied_by(install_info),

tests/common/mod.rs

@@ -390,18 +390,17 @@ macro_rules! cmd_snapshot {
 pub(crate) use cmd_snapshot;
 
 #[allow(clippy::disallowed_methods)]
-pub(crate) fn remove_bin_from_path(bin: &str) -> anyhow::Result<OsString> {
+pub(crate) fn remove_bin_from_path(bin: &str, path: Option<OsString>) -> anyhow::Result<OsString> {
+    let path = path.unwrap_or(std::env::var_os("PATH").expect("PATH not set"));
     let Ok(dirs) = which::which_all(bin) else {
-        return Ok(std::env::var_os("PATH").expect("PATH environment variable is not set"));
+        return Ok(path);
     };
 
     let dirs: FxHashSet<_> = dirs
         .filter_map(|path| path.parent().map(Path::to_path_buf))
         .collect();
 
-    let current_path = std::env::var("PATH").unwrap_or_default();
-
-    let new_path_entries: Vec<_> = std::env::split_paths(&current_path)
+    let new_path_entries: Vec<_> = std::env::split_paths(&path)
         .filter(|path| !dirs.contains(path.as_path()))
         .collect();