diff --git a/crates/prek/src/config/hook.rs b/crates/prek/src/config/hook.rs index 15ea060fd..41aa7527d 100644 --- a/crates/prek/src/config/hook.rs +++ b/crates/prek/src/config/hook.rs @@ -306,6 +306,163 @@ pub(crate) enum Shell { Cmd, } +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Deserialize, Serialize, strum::Display)] +#[serde(rename_all = "kebab-case")] +#[strum(serialize_all = "kebab-case")] +#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] +pub(crate) enum ToolchainPreference { + OnlyManaged, + #[default] + Managed, + System, + OnlySystem, +} + +#[derive(Deserialize)] +#[serde(untagged, deny_unknown_fields)] +#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] +#[cfg_attr(feature = "schemars", schemars(rename = "LanguageVersion"))] +enum LanguageVersionWire { + Request(String), + Options { + /// A language-specific version request. Defaults to `default`. + request: Option, + /// Which toolchain sources to prefer or require. Defaults to `managed`. + preference: Option, + }, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +enum ConfiguredVersionRequest { + Unspecified, + Default, + Explicit(String), +} + +#[derive(Debug, Clone, PartialEq, Eq, Deserialize)] +#[serde(from = "LanguageVersionWire")] +#[cfg_attr( + feature = "schemars", + derive(schemars::JsonSchema), + schemars(with = "LanguageVersionWire") +)] +pub(crate) struct LanguageVersion { + request: ConfiguredVersionRequest, + preference: Option, + allows_download: bool, +} + +impl LanguageVersion { + pub(crate) fn request(&self) -> Option<&str> { + match &self.request { + ConfiguredVersionRequest::Explicit(request) => Some(request), + ConfiguredVersionRequest::Unspecified | ConfiguredVersionRequest::Default => None, + } + } + + pub(crate) fn preference(&self) -> ToolchainPreference { + self.preference.unwrap_or_default() + } + + pub(crate) fn allows_download(&self) -> bool { + self.allows_download && self.preference() != ToolchainPreference::OnlySystem + } + + pub(crate) fn update(&mut self, other: &Self) { + if other.request != ConfiguredVersionRequest::Unspecified { + self.request.clone_from(&other.request); + self.allows_download = other.allows_download; + } + if other.preference.is_some() { + self.preference = other.preference; + } + } + + pub(crate) fn apply_defaults(&mut self, defaults: &Self) { + if self.request == ConfiguredVersionRequest::Unspecified { + self.request.clone_from(&defaults.request); + self.allows_download = defaults.allows_download; + } + if self.preference.is_none() { + self.preference = defaults.preference; + } + } + + pub(crate) fn is_default(&self) -> bool { + self.preference.is_none() + && matches!( + self.request, + ConfiguredVersionRequest::Unspecified | ConfiguredVersionRequest::Default + ) + && self.allows_download + } +} + +impl From for LanguageVersion { + fn from(request: String) -> Self { + Self::from(LanguageVersionWire::Request(request)) + } +} + +impl From<&str> for LanguageVersion { + fn from(request: &str) -> Self { + Self::from(request.to_string()) + } +} + +impl From for LanguageVersion { + fn from(wire: LanguageVersionWire) -> Self { + let (request, preference) = match wire { + LanguageVersionWire::Request(request) => (Some(request), None), + LanguageVersionWire::Options { + request, + preference, + } => (request, preference), + }; + let (request, allows_download) = match request { + Some(request) if request == "default" => (ConfiguredVersionRequest::Default, true), + Some(request) if request == "system" => (ConfiguredVersionRequest::Default, false), + Some(request) => (ConfiguredVersionRequest::Explicit(request), true), + None => (ConfiguredVersionRequest::Unspecified, true), + }; + + Self { + request, + preference, + allows_download, + } + } +} + +impl Display for LanguageVersion { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let request = match &self.request { + ConfiguredVersionRequest::Unspecified => None, + ConfiguredVersionRequest::Default if self.allows_download => Some("default"), + ConfiguredVersionRequest::Default => Some("system"), + ConfiguredVersionRequest::Explicit(request) => Some(request.as_str()), + }; + + if self.preference.is_none() + && let Some(request) = request + { + return f.write_str(request); + } + + f.write_str("{")?; + if let Some(request) = request { + write!(f, " request: {request}")?; + } + if let Some(preference) = self.preference { + if request.is_some() { + f.write_str(",")?; + } + write!(f, " preference: {preference}")?; + } + f.write_str(" }") + } +} + /// Common hook options. #[derive(Debug, Clone, Default)] #[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] @@ -343,10 +500,10 @@ pub(crate) struct HookOptions { pub pass_filenames: Option, /// A description of the hook. pub description: Option, - /// Run the hook on a specific version of the language. + /// Select a language version and optionally configure the toolchain source preference. /// Default is `default`. /// See . - pub language_version: Option, + pub language_version: Option, /// Write the output of the hook to a file when the hook fails or verbose is enabled. pub log_file: Option, /// Run the hook entry through a predefined shell adapter. @@ -393,7 +550,6 @@ impl HookOptions { fail_fast, pass_filenames, description, - language_version, log_file, shell, require_serial, @@ -402,6 +558,14 @@ impl HookOptions { minimum_prek_version, ); + if let Some(other_language_version) = &other.language_version { + if let Some(language_version) = &mut self.language_version { + language_version.update(other_language_version); + } else { + self.language_version.clone_from(&other.language_version); + } + } + // Merge environment variables. if let Some(other_env) = &other.env { if let Some(self_env) = &mut self.env { @@ -439,7 +603,7 @@ struct HookWire { fail_fast: Option, pass_filenames: Option, description: Option, - language_version: Option, + language_version: Option, log_file: Option, shell: Option, require_serial: Option, @@ -770,6 +934,132 @@ mod tests { use super::*; use clap::ValueEnum; + fn parse_language_version(yaml: &str) -> LanguageVersion { + serde_saphyr::from_str(yaml).expect("language version should parse") + } + + #[test] + fn language_version_accepts_request_and_options_forms() { + let scalar = parse_language_version("'3.12'\n"); + assert_eq!(scalar.request(), Some("3.12")); + assert_eq!(scalar.preference(), ToolchainPreference::Managed); + + let options = + parse_language_version("request: '>=3.12, <3.13'\npreference: only-managed\n"); + assert_eq!(options.request(), Some(">=3.12, <3.13")); + assert_eq!(options.preference(), ToolchainPreference::OnlyManaged); + + let options = parse_language_version("request: '3.13'\n"); + assert_eq!(options.preference(), ToolchainPreference::Managed); + } + + #[test] + fn language_version_special_requests_clear_request_and_set_download_option() { + for (wire, allows_download) in [ + ("default", true), + ("request: default\n", true), + ("system", false), + ("request: system\n", false), + ] { + let language_version = parse_language_version(wire); + assert_eq!( + language_version.request, + ConfiguredVersionRequest::Default, + "{wire}" + ); + assert_eq!(language_version.request(), None, "{wire}"); + assert_eq!( + language_version.allows_download(), + allows_download, + "{wire}" + ); + assert_eq!(language_version.is_default(), allows_download, "{wire}"); + } + } + + #[test] + fn language_version_update_merges_fields() { + let mut manifest = HookOptions { + language_version: Some(LanguageVersion::from("system")), + ..Default::default() + }; + let override_options = HookOptions { + language_version: Some(parse_language_version("preference: only-managed\n")), + ..Default::default() + }; + + manifest.update(&override_options); + + let language_version = manifest.language_version.as_ref().unwrap(); + assert_eq!(language_version.request(), None); + assert_eq!( + language_version.preference(), + ToolchainPreference::OnlyManaged + ); + assert!(!language_version.allows_download()); + + let mut manifest = HookOptions { + language_version: Some(parse_language_version("preference: only-managed\n")), + ..Default::default() + }; + let override_options = HookOptions { + language_version: Some(LanguageVersion::from("3.13")), + ..Default::default() + }; + + manifest.update(&override_options); + + let language_version = manifest.language_version.as_ref().unwrap(); + assert_eq!(language_version.request(), Some("3.13")); + assert_eq!( + language_version.preference(), + ToolchainPreference::OnlyManaged + ); + assert!(language_version.allows_download()); + } + + #[test] + fn language_version_special_requests_override_explicit_versions() { + for (request, allows_download) in [("default", true), ("system", false)] { + let mut manifest = HookOptions { + language_version: Some(LanguageVersion::from("3.12")), + ..Default::default() + }; + let override_options = HookOptions { + language_version: Some(LanguageVersion::from(request)), + ..Default::default() + }; + + manifest.update(&override_options); + + let language_version = manifest.language_version.as_ref().unwrap(); + assert_eq!(language_version.request(), None, "{request}"); + assert_eq!( + language_version.allows_download(), + allows_download, + "{request}" + ); + } + } + + #[test] + fn language_version_defaults_fill_missing_fields() { + let defaults = parse_language_version("request: system\npreference: only-managed\n"); + let mut language_version = parse_language_version("preference: system\n"); + + language_version.apply_defaults(&defaults); + + assert_eq!(language_version.request(), None); + assert_eq!(language_version.preference(), ToolchainPreference::System); + assert!(!language_version.allows_download()); + } + + #[test] + fn language_version_rejects_unknown_option_fields() { + let result = serde_saphyr::from_str::("toolchain: system\n"); + assert!(result.is_err()); + } + #[test] fn stages_deserialize_empty_as_empty() { #[derive(Debug, Deserialize)] diff --git a/crates/prek/src/config/mod.rs b/crates/prek/src/config/mod.rs index ec6f8abac..21515f70d 100644 --- a/crates/prek/src/config/mod.rs +++ b/crates/prek/src/config/mod.rs @@ -16,8 +16,8 @@ use serde::{Deserialize, Deserializer}; pub(crate) use hook::Language; pub(crate) use hook::{ - BuiltinHook, HookOptions, HookType, LocalHook, Manifest, ManifestHook, MetaHook, PassFilenames, - RemoteHook, Shell, Stage, Stages, + BuiltinHook, HookOptions, HookType, LanguageVersion, LocalHook, Manifest, ManifestHook, + MetaHook, PassFilenames, RemoteHook, Shell, Stage, Stages, ToolchainPreference, }; pub(crate) use pattern::{FilePattern, GlobPatterns}; pub(crate) use priority::{Priority, PriorityAlias, validate_name}; @@ -55,7 +55,7 @@ pub(crate) struct Config { /// Default is `[pre-commit]`. pub default_install_hook_types: Option>, /// A mapping from language to the default `language_version`. - pub default_language_version: Option>, + pub default_language_version: Option>, /// A configuration-wide default for the stages property of hooks. /// Default to all stages. pub default_stages: Option, @@ -760,6 +760,40 @@ mod tests { insta::assert_debug_snapshot!(result); } + #[test] + fn language_version_options_toml() -> Result<()> { + let config: Config = toml::from_str(indoc::indoc! {r#" + [[repos]] + repo = "local" + + [[repos.hooks]] + id = "hook" + name = "hook" + entry = "python -m hook" + language = "python" + language_version = { request = ">=3.12, <3.13", preference = "only-managed" } + "#}) + .expect("config should parse"); + let Some(Repo::Local(repo)) = config.repos.first() else { + anyhow::bail!("expected local repo"); + }; + let Some(language_version) = repo + .hooks + .first() + .and_then(|hook| hook.options.language_version.as_ref()) + else { + anyhow::bail!("expected language version"); + }; + + assert_eq!(language_version.request(), Some(">=3.12, <3.13")); + assert_eq!( + language_version.preference(), + ToolchainPreference::OnlyManaged + ); + assert!(language_version.allows_download()); + Ok(()) + } + #[test] fn parse_update_options() { let yaml = indoc::indoc! {r#" diff --git a/crates/prek/src/config/snapshots/prek__config__tests__language_version.snap b/crates/prek/src/config/snapshots/prek__config__tests__language_version.snap index d1a1376d1..c2301803a 100644 --- a/crates/prek/src/config/snapshots/prek__config__tests__language_version.snap +++ b/crates/prek/src/config/snapshots/prek__config__tests__language_version.snap @@ -33,7 +33,11 @@ Ok( pass_filenames: None, description: None, language_version: Some( - "default", + LanguageVersion { + request: Default, + preference: None, + allows_download: true, + }, ), log_file: None, shell: None, @@ -66,7 +70,11 @@ Ok( pass_filenames: None, description: None, language_version: Some( - "system", + LanguageVersion { + request: Default, + preference: None, + allows_download: false, + }, ), log_file: None, shell: None, @@ -99,7 +107,13 @@ Ok( pass_filenames: None, description: None, language_version: Some( - "3.8", + LanguageVersion { + request: Explicit( + "3.8", + ), + preference: None, + allows_download: true, + }, ), log_file: None, shell: None, diff --git a/crates/prek/src/hook.rs b/crates/prek/src/hook.rs index 0702b7515..bc2ee6add 100644 --- a/crates/prek/src/hook.rs +++ b/crates/prek/src/hook.rs @@ -89,11 +89,16 @@ impl HookSpec { fn with_project_defaults(mut self, config: &Config) -> Self { let language = self.language; - if self.options.language_version.is_none() { - self.options.language_version = config - .default_language_version - .as_ref() - .and_then(|v| v.get(&language).cloned()); + if let Some(default) = config + .default_language_version + .as_ref() + .and_then(|versions| versions.get(&language)) + { + if let Some(language_version) = &mut self.options.language_version { + language_version.apply_defaults(default); + } else { + self.options.language_version = Some(default.clone()); + } } if self @@ -141,7 +146,7 @@ impl HookSpec { if !language.supports_language_version() && let Some(language_version) = &self.options.language_version - && language_version != "default" + && !language_version.is_default() { bail!( "Hook specified `language_version: {language_version}` but the language `{language}` does not support toolchain installation for now" @@ -444,8 +449,7 @@ impl Hook { _unused_keys: _, } = options; - let language_version = language_version.unwrap_or_default(); - let language_request = LanguageRequest::parse(language, &language_version) + let language_request = LanguageRequest::from_config(language, language_version.as_ref()) .map_err(|error| hook_error(&id, error))?; let entry = HookEntry::new(id.clone(), raw_entry, shell); @@ -573,13 +577,9 @@ impl HookEnvRequirement { return Ok(None); } - let language_version = hook_spec - .options - .language_version - .as_deref() - .unwrap_or_default(); - let language_request = LanguageRequest::parse(language, language_version) - .with_context(|| format!("Invalid language_version for hook `{}`", hook_spec.id))?; + let language_request = + LanguageRequest::from_config(language, hook_spec.options.language_version.as_ref()) + .with_context(|| format!("Invalid language_version for hook `{}`", hook_spec.id))?; let dependencies = hook_spec .options .additional_dependencies @@ -814,8 +814,8 @@ mod tests { use serde_json::json; use crate::config::{ - Config, HookOptions, Language, ManifestHook, PassFilenames, Priority, RemoteHook, Shell, - Stage, Stages, + Config, HookOptions, Language, LanguageVersion, ManifestHook, PassFilenames, Priority, + RemoteHook, Shell, Stage, Stages, }; use crate::hook::HookSpec; use crate::hooks::check_fast_path; @@ -909,7 +909,13 @@ mod tests { default_install_hook_types: None, default_language_version: Some( { - Python: "python3.12", + Python: LanguageVersion { + request: Explicit( + "python3.12", + ), + preference: None, + allows_download: true, + }, }, ), default_stages: Some( @@ -974,6 +980,7 @@ mod tests { 12, ), ), + preference: Managed, allows_download: true, }, log_file: None, @@ -1077,6 +1084,41 @@ mod tests { assert_eq!(hook_spec.options.stages, Some(Stages::ALL)); } + #[test] + fn hook_spec_with_project_defaults_merges_language_version_fields() { + let config: Config = serde_saphyr::from_str(indoc::indoc! {r" + repos: [] + default_language_version: + python: + request: '3.12' + preference: only-managed + "}) + .expect("config should parse"); + let language_version: LanguageVersion = + serde_saphyr::from_str("preference: system\n").expect("version should parse"); + let hook_spec = HookSpec { + id: "test-hook".to_string(), + name: "test-hook".to_string(), + entry: "python -m test".to_string(), + language: Language::Python, + language_overridden: false, + priority: None, + groups: None, + options: HookOptions { + language_version: Some(language_version), + ..Default::default() + }, + } + .with_project_defaults(&config); + + let language_version = hook_spec.options.language_version.as_ref().unwrap(); + assert_eq!(language_version.request(), Some("3.12")); + assert_eq!( + language_version.preference(), + crate::config::ToolchainPreference::System + ); + } + #[tokio::test] async fn hook_from_spec_preserves_explicit_empty_default_stages() -> Result<()> { let temp = tempfile::tempdir()?; @@ -1287,6 +1329,36 @@ mod tests { Ok(()) } + #[test] + fn toolchain_preferences_do_not_affect_existing_environment_reuse() -> Result<()> { + let dependencies = Vec::new(); + let install_info: InstallInfo = serde_json::from_value(json!({ + "schema_version": INSTALL_INFO_SCHEMA_VERSION, + "language": "python", + "language_version": "3.12.0", + "dependencies": [], + "env_path": "/tmp/python-env", + "toolchain": "/usr/bin/python3", + "extra": {}, + }))?; + + for preference in ["only-managed", "managed", "system", "only-system"] { + let language_version: LanguageVersion = + serde_saphyr::from_str(&format!("preference: {preference}\n"))?; + let language_request = + LanguageRequest::from_config(Language::Python, Some(&language_version))?; + let requirement = HookEnvRequirementRef { + language: Language::Python, + repo: None, + dependencies: &dependencies, + language_request: &language_request, + }; + + assert!(requirement.is_satisfied_by(&install_info), "{preference}"); + } + Ok(()) + } + /// Set up a temporary directory with a minimal `.pre-commit-config.yaml` /// and a `remote-repo` subdirectory. fn setup_hook_test() -> Result<(tempfile::TempDir, Arc)> { @@ -1329,7 +1401,7 @@ mod tests { priority: None, groups: None, options: HookOptions { - language_version: language_version.map(str::to_string), + language_version: language_version.map(LanguageVersion::from), ..Default::default() }, }; @@ -1434,7 +1506,7 @@ mod tests { hook.language_request.version_request(), expected.version_request() ); - assert!(!hook.language_request.allows_download()); + assert!(!hook.language_request.toolchain_policy().allows_download()); Ok(()) } @@ -1492,7 +1564,7 @@ mod tests { hook.language_request.version_request(), expected.version_request() ); - assert!(!hook.language_request.allows_download()); + assert!(!hook.language_request.toolchain_policy().allows_download()); Ok(()) } } diff --git a/crates/prek/src/languages/bun/bun.rs b/crates/prek/src/languages/bun/bun.rs index f728dde42..67ff3b9ba 100644 --- a/crates/prek/src/languages/bun/bun.rs +++ b/crates/prek/src/languages/bun/bun.rs @@ -42,7 +42,7 @@ impl LanguageBackend for Bun { let bun_request: &BunRequest = hook.language_request.version(); let bun = installer - .install(store, bun_request, hook.language_request.allows_download()) + .install(store, bun_request, hook.language_request.toolchain_policy()) .await .context("Failed to install bun")?; diff --git a/crates/prek/src/languages/bun/installer.rs b/crates/prek/src/languages/bun/installer.rs index 9b90658c0..6f2da8584 100644 --- a/crates/prek/src/languages/bun/installer.rs +++ b/crates/prek/src/languages/bun/installer.rs @@ -17,6 +17,7 @@ use crate::git; use crate::http::{REQWEST_CLIENT, download_artifact}; use crate::languages::bun::BunRequest; use crate::languages::bun::version::BunVersion; +use crate::languages::version::{ToolchainPolicy, ToolchainSource, find_system_executables}; use crate::process::Cmd; use crate::store::Store; @@ -82,25 +83,25 @@ impl BunInstaller { &self, store: &Store, request: &BunRequest, - allows_download: bool, + policy: ToolchainPolicy, ) -> Result { fs_err::tokio::create_dir_all(&self.root).await?; let _lock = LockedFile::acquire(self.root.join(".lock"), "bun").await?; - if let Ok(bun_result) = self.find_installed(request) { - trace!(%bun_result, "Found installed bun"); - return Ok(bun_result); - } - - // Find all bun executables in PATH and check their versions - if let Some(bun_result) = self.find_system_bun(request).await? { - trace!(%bun_result, "Using system bun"); - return Ok(bun_result); + for &source in policy.search_order() { + let result = match source { + ToolchainSource::Managed => self.find_installed(request).ok(), + ToolchainSource::System => self.find_system_bun(request).await?, + }; + if let Some(result) = result { + trace!(%result, ?source, "Found bun"); + return Ok(result); + } } - if !allows_download { - anyhow::bail!("No suitable system Bun version found and downloads are disabled"); + if !policy.allows_download() { + anyhow::bail!("No suitable Bun version found for toolchain policy: {policy}"); } let resolved_version = self.resolve_version(request).await?; @@ -253,7 +254,7 @@ impl BunInstaller { /// Find a suitable system Bun installation that matches the request. async fn find_system_bun(&self, bun_request: &BunRequest) -> Result> { - let bun_paths = match which::which_all(&*BUN_BINARY_NAME) { + let bun_paths = match find_system_executables(&*BUN_BINARY_NAME, &self.root) { Ok(paths) => paths, Err(e) => { debug!("No bun executables found in PATH: {}", e); diff --git a/crates/prek/src/languages/bun/version.rs b/crates/prek/src/languages/bun/version.rs index 2a9bca275..04aac3b12 100644 --- a/crates/prek/src/languages/bun/version.rs +++ b/crates/prek/src/languages/bun/version.rs @@ -33,10 +33,8 @@ impl FromStr for BunVersion { } } -/// `language_version` field of bun can be one of the following: -/// - `default`: Find system installed bun, or download the latest version. -/// - `system`: Find system installed bun, or error if not found. -/// - `bun` or `bun@latest`: Same as `default`. +/// A parsed Bun version request: +/// - `bun` or `bun@latest`: Accept any version. /// - `x.y` or `bun@x.y`: Install the latest version with the same major and minor version. /// - `x.y.z` or `bun@x.y.z`: Install the specific version. /// - `^x.y.z`: Install the latest version that satisfies the semver requirement. diff --git a/crates/prek/src/languages/deno/deno.rs b/crates/prek/src/languages/deno/deno.rs index 3f6bf38a1..34f7a7e50 100644 --- a/crates/prek/src/languages/deno/deno.rs +++ b/crates/prek/src/languages/deno/deno.rs @@ -67,7 +67,11 @@ impl LanguageBackend for Deno { let deno_request: &DenoRequest = hook.language_request.version(); let deno = installer - .install(store, deno_request, hook.language_request.allows_download()) + .install( + store, + deno_request, + hook.language_request.toolchain_policy(), + ) .await .context("Failed to install deno")?; diff --git a/crates/prek/src/languages/deno/installer.rs b/crates/prek/src/languages/deno/installer.rs index 2fa9d3800..4676476db 100644 --- a/crates/prek/src/languages/deno/installer.rs +++ b/crates/prek/src/languages/deno/installer.rs @@ -17,6 +17,7 @@ use crate::fs::LockedFile; use crate::http::{REQWEST_CLIENT, download_artifact}; use crate::languages::deno::DenoRequest; use crate::languages::deno::version::DenoVersion; +use crate::languages::version::{ToolchainPolicy, ToolchainSource, find_system_executables}; use crate::process::Cmd; use crate::store::Store; @@ -94,25 +95,25 @@ impl DenoInstaller { &self, store: &Store, request: &DenoRequest, - allows_download: bool, + policy: ToolchainPolicy, ) -> Result { fs_err::tokio::create_dir_all(&self.root).await?; let _lock = LockedFile::acquire(self.root.join(".lock"), "deno").await?; - if let Ok(deno_result) = self.find_installed(request) { - trace!(%deno_result, "Found installed deno"); - return Ok(deno_result); - } - - // Find all deno executables in PATH and check their versions - if let Some(deno_result) = self.find_system_deno(request).await? { - trace!(%deno_result, "Using system deno"); - return Ok(deno_result); + for &source in policy.search_order() { + let result = match source { + ToolchainSource::Managed => self.find_installed(request).ok(), + ToolchainSource::System => self.find_system_deno(request).await?, + }; + if let Some(result) = result { + trace!(%result, ?source, "Found deno"); + return Ok(result); + } } - if !allows_download { - anyhow::bail!("No suitable system Deno version found and downloads are disabled"); + if !policy.allows_download() { + anyhow::bail!("No suitable Deno version found for toolchain policy: {policy}"); } let resolved_version = self.resolve_version(request).await?; @@ -280,7 +281,7 @@ impl DenoInstaller { /// Find a suitable system Deno installation that matches the request. async fn find_system_deno(&self, deno_request: &DenoRequest) -> Result> { - let deno_paths = match which::which_all(&*DENO_BINARY_NAME) { + let deno_paths = match find_system_executables(&*DENO_BINARY_NAME, &self.root) { Ok(paths) => paths, Err(e) => { debug!("No deno executables found in PATH: {}", e); diff --git a/crates/prek/src/languages/deno/version.rs b/crates/prek/src/languages/deno/version.rs index a7112c885..3ac9ebb84 100644 --- a/crates/prek/src/languages/deno/version.rs +++ b/crates/prek/src/languages/deno/version.rs @@ -34,10 +34,8 @@ impl FromStr for DenoVersion { } } -/// `language_version` field of deno can be one of the following: -/// - `default`: Find system installed deno, or download the latest version. -/// - `system`: Find system installed deno, or error if not found. -/// - `deno` or `deno@latest`: Same as `default`. +/// A parsed Deno version request: +/// - `deno` or `deno@latest`: Accept any version. /// - `x.y` or `deno@x.y`: Install the latest version with the same major and minor version. /// - `x.y.z` or `deno@x.y.z`: Install the specific version. /// - `^x.y.z`: Install the latest version that satisfies the semver requirement. diff --git a/crates/prek/src/languages/dotnet/dotnet.rs b/crates/prek/src/languages/dotnet/dotnet.rs index 4dd9a223d..dda1443cf 100644 --- a/crates/prek/src/languages/dotnet/dotnet.rs +++ b/crates/prek/src/languages/dotnet/dotnet.rs @@ -47,7 +47,7 @@ impl LanguageBackend for Dotnet { let installer = DotnetInstaller::new(store.tools_path(ToolBucket::Dotnet)); let request: &DotnetRequest = hook.language_request.version(); let dotnet = installer - .install(request, hook.language_request.allows_download()) + .install(request, hook.language_request.toolchain_policy()) .await .context("Failed to install dotnet SDK")?; diff --git a/crates/prek/src/languages/dotnet/installer.rs b/crates/prek/src/languages/dotnet/installer.rs index 47f9c91ac..b9cdceb2e 100644 --- a/crates/prek/src/languages/dotnet/installer.rs +++ b/crates/prek/src/languages/dotnet/installer.rs @@ -13,6 +13,7 @@ use super::version::DotnetVersion; use crate::fs::LockedFile; use crate::http::REQWEST_CLIENT; use crate::languages::dotnet::DotnetRequest; +use crate::languages::version::{ToolchainPolicy, ToolchainSource, find_system_executables}; use crate::process::Cmd; static DOTNET_BINARY_NAME: LazyLock = LazyLock::new(|| { @@ -92,23 +93,24 @@ impl DotnetInstaller { pub(crate) async fn install( &self, request: &DotnetRequest, - allows_download: bool, + policy: ToolchainPolicy, ) -> Result { fs_err::tokio::create_dir_all(&self.root).await?; let _lock = LockedFile::acquire(self.root.join(".lock"), "dotnet").await?; - if let Ok(result) = self.find_installed(request) { - debug!(%result, "Using existing managed dotnet"); - return Ok(result); - } - - if let Some(result) = self.find_system_dotnet(request).await? { - debug!(%result, "Using system dotnet"); - return Ok(result); + for &source in policy.search_order() { + let result = match source { + ToolchainSource::Managed => self.find_installed(request).ok(), + ToolchainSource::System => self.find_system_dotnet(request).await?, + }; + if let Some(result) = result { + debug!(%result, ?source, "Found dotnet"); + return Ok(result); + } } - if !allows_download { - bail!("No suitable dotnet version found and downloads are disabled"); + if !policy.allows_download() { + bail!("No suitable dotnet version found for toolchain policy: {policy}"); } self.install_managed(request).await @@ -149,7 +151,7 @@ impl DotnetInstaller { /// Finds the first system `dotnet` executable in `PATH` that satisfies the request. async fn find_system_dotnet(&self, request: &DotnetRequest) -> Result> { - let dotnet_paths = match which::which_all(&*DOTNET_BINARY_NAME) { + let dotnet_paths = match find_system_executables(&*DOTNET_BINARY_NAME, &self.root) { Ok(paths) => paths, Err(err) => { debug!("No dotnet executables found in PATH: {err}"); diff --git a/crates/prek/src/languages/golang/golang.rs b/crates/prek/src/languages/golang/golang.rs index ab6a317a5..dd8252734 100644 --- a/crates/prek/src/languages/golang/golang.rs +++ b/crates/prek/src/languages/golang/golang.rs @@ -33,7 +33,7 @@ impl LanguageBackend for Golang { let version: &GoRequest = hook.language_request.version(); let go = installer - .install(store, version, hook.language_request.allows_download()) + .install(store, version, hook.language_request.toolchain_policy()) .await .context("Failed to install go")?; diff --git a/crates/prek/src/languages/golang/installer.rs b/crates/prek/src/languages/golang/installer.rs index 97b4a9e7a..ceabf78f4 100644 --- a/crates/prek/src/languages/golang/installer.rs +++ b/crates/prek/src/languages/golang/installer.rs @@ -19,6 +19,7 @@ use crate::http::{REQWEST_CLIENT, download_artifact}; use crate::languages::golang::GoRequest; use crate::languages::golang::golang::bin_dir; use crate::languages::golang::version::GoVersion; +use crate::languages::version::{ToolchainPolicy, ToolchainSource, find_system_executables}; use crate::process::Cmd; use crate::store::Store; @@ -119,24 +120,25 @@ impl GoInstaller { &self, store: &Store, request: &GoRequest, - allows_download: bool, + policy: ToolchainPolicy, ) -> Result { fs_err::tokio::create_dir_all(&self.root).await?; let _lock = LockedFile::acquire(self.root.join(".lock"), "go").await?; - if let Ok(go) = self.find_installed(request) { - trace!(%go, "Found installed go"); - return Ok(go); - } - - if let Some(go) = self.find_system_go(request).await? { - trace!(%go, "Using system go"); - return Ok(go); + for &source in policy.search_order() { + let result = match source { + ToolchainSource::Managed => self.find_installed(request).ok(), + ToolchainSource::System => self.find_system_go(request).await?, + }; + if let Some(result) = result { + trace!(%result, ?source, "Found go"); + return Ok(result); + } } - if !allows_download { - anyhow::bail!("No suitable system Go version found and downloads are disabled"); + if !policy.allows_download() { + anyhow::bail!("No suitable Go version found for toolchain policy: {policy}"); } let resolved_version = self @@ -274,7 +276,7 @@ impl GoInstaller { } async fn find_system_go(&self, go_request: &GoRequest) -> Result> { - let go_paths = match which::which_all(&*GO_BINARY_NAME) { + let go_paths = match find_system_executables(&*GO_BINARY_NAME, &self.root) { Ok(paths) => paths, Err(e) => { debug!("No go executables found in PATH: {}", e); diff --git a/crates/prek/src/languages/golang/version.rs b/crates/prek/src/languages/golang/version.rs index e0bebab8a..0aae21bba 100644 --- a/crates/prek/src/languages/golang/version.rs +++ b/crates/prek/src/languages/golang/version.rs @@ -34,9 +34,7 @@ impl FromStr for GoVersion { } } -/// `language_version` field of golang can be one of the following: -/// `default` -/// `system` +/// A parsed Go version request: /// `go` /// `go1.20` or `1.20` /// `go1.20.3` or `1.20.3` diff --git a/crates/prek/src/languages/mise/installer.rs b/crates/prek/src/languages/mise/installer.rs index e079d5671..bfbb45ace 100644 --- a/crates/prek/src/languages/mise/installer.rs +++ b/crates/prek/src/languages/mise/installer.rs @@ -16,7 +16,9 @@ use crate::checksum::{Sha256Digest, digest_from_sha256sums}; use crate::fs::{LockedFile, is_executable}; use crate::git; use crate::http::{REQWEST_CLIENT, download_artifact}; -use crate::languages::version::SemverRequest; +use crate::languages::version::{ + SemverRequest, ToolchainPolicy, ToolchainSource, find_system_executables, +}; use crate::process::Cmd; use crate::store::Store; @@ -112,23 +114,24 @@ impl MiseInstaller { &self, store: &Store, request: &SemverRequest, - allows_download: bool, + policy: ToolchainPolicy, ) -> Result { fs_err::tokio::create_dir_all(&self.root).await?; let _lock = LockedFile::acquire(self.root.join(".lock"), "mise").await?; - if let Some(result) = self.find_installed(request).await { - trace!(%result, "Found managed mise"); - return Ok(result); - } - - if let Some(result) = self.find_system_mise(request).await { - trace!(%result, "Using system mise"); - return Ok(result); + for &source in policy.search_order() { + let result = match source { + ToolchainSource::Managed => self.find_installed(request).await, + ToolchainSource::System => self.find_system_mise(request).await, + }; + if let Some(result) = result { + trace!(%result, ?source, "Found mise"); + return Ok(result); + } } - if !allows_download { - anyhow::bail!("No compatible mise executable found and downloads are disabled"); + if !policy.allows_download() { + anyhow::bail!("No compatible mise executable found for toolchain policy: {policy}"); } let version = self.resolve_version(request).await?; @@ -179,7 +182,7 @@ impl MiseInstaller { } async fn find_system_mise(&self, request: &SemverRequest) -> Option { - let paths = match which::which_all(&*MISE_BINARY_NAME) { + let paths = match find_system_executables(&*MISE_BINARY_NAME, &self.root) { Ok(paths) => paths, Err(err) => { debug!(%err, "No mise executable found in PATH"); diff --git a/crates/prek/src/languages/mise/mise.rs b/crates/prek/src/languages/mise/mise.rs index d5276321b..0e9a09699 100644 --- a/crates/prek/src/languages/mise/mise.rs +++ b/crates/prek/src/languages/mise/mise.rs @@ -83,7 +83,7 @@ impl LanguageBackend for Mise { let installer = MiseInstaller::new(store.tools_path(ToolBucket::Mise)); let request: &SemverRequest = hook.language_request.version(); let mise = installer - .install(store, request, hook.language_request.allows_download()) + .install(store, request, hook.language_request.toolchain_policy()) .await .context("Failed to install mise")?; diff --git a/crates/prek/src/languages/node/installer.rs b/crates/prek/src/languages/node/installer.rs index c7ecaa266..460bb11f0 100644 --- a/crates/prek/src/languages/node/installer.rs +++ b/crates/prek/src/languages/node/installer.rs @@ -19,6 +19,7 @@ use crate::fs::{LockedFile, is_executable}; use crate::http::{REQWEST_CLIENT, download_artifact}; use crate::languages::node::NodeRequest; use crate::languages::node::version::NodeVersion; +use crate::languages::version::{ToolchainPolicy, ToolchainSource, find_system_executables}; use crate::process::Cmd; use crate::store::Store; @@ -115,25 +116,25 @@ impl NodeInstaller { &self, store: &Store, request: &NodeRequest, - allows_download: bool, + policy: ToolchainPolicy, ) -> Result { fs_err::tokio::create_dir_all(&self.root).await?; let _lock = LockedFile::acquire(self.root.join(".lock"), "node").await?; - if let Ok(node_result) = self.find_installed(request) { - trace!(%node_result, "Found installed node"); - return Ok(node_result); - } - - // Find all node and npm executables in PATH and check their versions - if let Some(node_result) = self.find_system_node(request).await? { - trace!(%node_result, "Using system node"); - return Ok(node_result); + for &source in policy.search_order() { + let result = match source { + ToolchainSource::Managed => self.find_installed(request).ok(), + ToolchainSource::System => self.find_system_node(request).await?, + }; + if let Some(result) = result { + trace!(%result, ?source, "Found node"); + return Ok(result); + } } - if !allows_download { - anyhow::bail!("No suitable system Node version found and downloads are disabled"); + if !policy.allows_download() { + anyhow::bail!("No suitable Node version found for toolchain policy: {policy}"); } let resolved_version = self.resolve_version(request).await?; @@ -269,7 +270,7 @@ impl NodeInstaller { /// Find a suitable system Node.js installation that matches the request. async fn find_system_node(&self, node_request: &NodeRequest) -> Result> { - let node_paths = match which::which_all(&*NODE_BINARY_NAME) { + let node_paths = match find_system_executables(&*NODE_BINARY_NAME, &self.root) { Ok(paths) => paths, Err(e) => { debug!("No node executables found in PATH: {}", e); diff --git a/crates/prek/src/languages/node/node.rs b/crates/prek/src/languages/node/node.rs index 6b11070a6..8be2aa9ea 100644 --- a/crates/prek/src/languages/node/node.rs +++ b/crates/prek/src/languages/node/node.rs @@ -66,7 +66,11 @@ impl LanguageBackend for Node { let node_request: &NodeRequest = hook.language_request.version(); let node = installer - .install(store, node_request, hook.language_request.allows_download()) + .install( + store, + node_request, + hook.language_request.toolchain_policy(), + ) .await .context("Failed to install node")?; diff --git a/crates/prek/src/languages/node/version.rs b/crates/prek/src/languages/node/version.rs index 18bbf9aca..60556ac58 100644 --- a/crates/prek/src/languages/node/version.rs +++ b/crates/prek/src/languages/node/version.rs @@ -115,9 +115,8 @@ impl NodeVersion { } } -/// The `language_version` field of node language, can be one of the following: -/// - `default`: Find the system installed node, or download the latest version. -/// - `system`: Find the system installed node, or return an error if not found. +/// A parsed Node.js version request: +/// - `node`: Accept any version. /// - `x.y.z`: Install the specific version of node. /// - `x.y`: Install the latest version of node with the same major and minor version. /// - `x`: Install the latest version of node with the same major version. diff --git a/crates/prek/src/languages/python/python.rs b/crates/prek/src/languages/python/python.rs index 49433cf12..2732c6681 100644 --- a/crates/prek/src/languages/python/python.rs +++ b/crates/prek/src/languages/python/python.rs @@ -16,7 +16,7 @@ use crate::hook::InstalledHook; use crate::hook::{Hook, InstallInfo}; use crate::languages::python::PythonRequest; use crate::languages::python::uv::Uv; -use crate::languages::version::LanguageRequest; +use crate::languages::version::{LanguageRequest, ToolchainSource}; use crate::languages::{ExecutionEnvironment, LanguageBackend}; use crate::process; use crate::process::Cmd; @@ -242,70 +242,64 @@ impl Python { info: &InstallInfo, python_request: &LanguageRequest, ) -> Result<()> { - // Prefer Python installations already managed by prek. - match Self::create_venv_command(uv, store, info, python_request, VenvAttempt::PrekManaged) - .check(true) - .output() - .await - { - Ok(_) => { - debug!( - "Venv created with prek-managed Python: `{}`", - info.env_path.display() - ); - return Ok(()); - } - Err(process::Error::Status { .. }) => {} - Err(e) => { - return Err(e.into()); - } - } - - // Next, use uv's normal discovery outside prek's managed store. - match Self::create_venv_command(uv, store, info, python_request, VenvAttempt::External) - .check(true) - .output() - .await - { - Ok(_) => { - debug!( - "Venv created with Python discovered outside prek's managed store: `{}`", - info.env_path.display() - ); - Ok(()) - } - Err(e @ process::Error::Status { .. }) => { - if Self::can_retry_with_downloads(&e) { - if !python_request.allows_download() { - anyhow::bail!( - "No suitable system Python version found and downloads are disabled" - ); - } - + let policy = python_request.toolchain_policy(); + let mut last_error = None; + + for &source in policy.search_order() { + let attempt = match source { + ToolchainSource::Managed => VenvAttempt::PrekManaged, + ToolchainSource::System => VenvAttempt::External, + }; + match Self::try_create_venv(uv, store, info, python_request, attempt).await { + Ok(()) => return Ok(()), + Err(error @ process::Error::Status { .. }) => { + last_error = Some((source, error)); + } + Err(error) => { debug!( - "Downloading Python into prek's managed store: `{}`", + "Failed to create venv `{}`: {error}", info.env_path.display() ); - Self::create_venv_command( - uv, - store, - info, - python_request, - VenvAttempt::Download, - ) - .check(true) - .output() - .await?; - return Ok(()); + return Err(error.into()); } - // If we can't retry, return the original error - Err(e.into()) - } - Err(e) => { - debug!("Failed to create venv `{}`: {e}", info.env_path.display()); - Err(e.into()) } } + + if let Some((ToolchainSource::System, error)) = last_error + && !Self::can_retry_with_downloads(&error) + { + return Err(error.into()); + } + + if policy.allows_download() { + debug!( + "Downloading Python into prek's managed store: `{}`", + info.env_path.display() + ); + Self::try_create_venv(uv, store, info, python_request, VenvAttempt::Download).await?; + return Ok(()); + } + + anyhow::bail!("No suitable Python version found for toolchain policy: {policy}") + } + + async fn try_create_venv( + uv: &Uv, + store: &Store, + info: &InstallInfo, + python_request: &LanguageRequest, + attempt: VenvAttempt, + ) -> std::result::Result<(), process::Error> { + Self::create_venv_command(uv, store, info, python_request, attempt) + .check(true) + .output() + .await?; + debug!( + ?attempt, + "Created Python virtual environment: `{}`", + info.env_path.display() + ); + Ok(()) } fn create_venv_command( diff --git a/crates/prek/src/languages/ruby/installer.rs b/crates/prek/src/languages/ruby/installer.rs index 312567dfe..303036a32 100644 --- a/crates/prek/src/languages/ruby/installer.rs +++ b/crates/prek/src/languages/ruby/installer.rs @@ -14,6 +14,7 @@ use crate::checksum::{Sha256Digest, digest_from_sha256sums}; use crate::fs::LockedFile; use crate::http::{DownloadChecksumPolicy, REQWEST_CLIENT, download_artifact_with}; use crate::languages::ruby::RubyRequest; +use crate::languages::version::{ToolchainPolicy, ToolchainSource, find_system_executables}; use crate::process::Cmd; use crate::store::Store; @@ -196,38 +197,31 @@ impl RubyInstaller { &self, store: &Store, request: &RubyRequest, - allows_download: bool, + policy: ToolchainPolicy, ) -> Result { fs_err::tokio::create_dir_all(&self.root).await?; let _lock = LockedFile::acquire(self.root.join(".lock"), "ruby").await?; - // 1. Check previously downloaded rubies - if let Some(ruby) = self.find_installed(request) { - trace!( - "Using managed Ruby: {} at {}", - ruby.version(), - ruby.ruby_bin().display() - ); - return Ok(ruby); - } - - // 2. Check system Ruby (PATH + version managers) - if let Some(ruby) = self.find_system_ruby(request).await? { - trace!( - "Using system Ruby: {} at {}", - ruby.version(), - ruby.ruby_bin().display() - ); - return Ok(ruby); + for &source in policy.search_order() { + let result = match source { + ToolchainSource::Managed => self.find_installed(request), + ToolchainSource::System => self.find_system_ruby(request).await?, + }; + if let Some(result) = result { + trace!( + ?source, + version = %result.version(), + path = %result.ruby_bin().display(), + "Found Ruby" + ); + return Ok(result); + } } - // 3. Download if allowed and platform is supported - if !allows_download { + if !policy.allows_download() { anyhow::bail!(ruby_not_found_error( request, - // allows_download can only be false if the original request was - // for any version of ruby, but system-only. - "Automatic installation is disabled (language_version: system)." + &format!("No suitable Ruby was found for toolchain policy: {policy}.") )); } @@ -407,7 +401,7 @@ impl RubyInstaller { /// Find Ruby in the system PATH async fn find_system_ruby(&self, request: &RubyRequest) -> Result> { // Try all rubies in PATH first - if let Ok(ruby_paths) = which::which_all("ruby") { + if let Ok(ruby_paths) = find_system_executables("ruby", &self.root) { for ruby_path in ruby_paths { if let Some(result) = try_ruby_path(&ruby_path, request).await { return Ok(Some(result)); diff --git a/crates/prek/src/languages/ruby/ruby.rs b/crates/prek/src/languages/ruby/ruby.rs index 0cb9a3171..59b4f723b 100644 --- a/crates/prek/src/languages/ruby/ruby.rs +++ b/crates/prek/src/languages/ruby/ruby.rs @@ -35,7 +35,7 @@ impl LanguageBackend for Ruby { let request: &RubyRequest = hook.language_request.version(); let ruby = installer - .install(store, request, hook.language_request.allows_download()) + .install(store, request, hook.language_request.toolchain_policy()) .await .context("Failed to install Ruby")?; diff --git a/crates/prek/src/languages/rust/installer.rs b/crates/prek/src/languages/rust/installer.rs index a6e357ce3..e4099b733 100644 --- a/crates/prek/src/languages/rust/installer.rs +++ b/crates/prek/src/languages/rust/installer.rs @@ -11,6 +11,7 @@ use crate::fs::LockedFile; use crate::languages::rust::RustRequest; use crate::languages::rust::rustup::{Rustup, ToolchainInfo}; use crate::languages::rust::version::{Channel, RustVersion}; +use crate::languages::version::{ToolchainPolicy, ToolchainSource}; use crate::process::Cmd; pub(crate) struct RustResult { @@ -80,26 +81,25 @@ impl RustInstaller { pub(crate) async fn install( &self, request: &RustRequest, - allows_download: bool, + policy: ToolchainPolicy, ) -> Result { let rustup_home = self.rustup.rustup_home(); fs_err::tokio::create_dir_all(rustup_home).await?; let _lock = LockedFile::acquire(rustup_home.join(".lock"), "rustup").await?; - // Check installed - if let Ok(rust) = self.find_installed(request).await { - trace!(%rust, "Found installed rust"); - return Ok(rust); - } - - // Check system rust - if let Some(rust) = self.find_system_rust(request).await? { - trace!(%rust, "Using system rust"); - return Ok(rust); + for &source in policy.search_order() { + let result = match source { + ToolchainSource::Managed => self.find_installed(request).await.ok(), + ToolchainSource::System => self.find_system_rust(request).await?, + }; + if let Some(result) = result { + trace!(%result, ?source, "Found rust"); + return Ok(result); + } } - if !allows_download { - anyhow::bail!("No suitable system Rust version found and downloads are disabled"); + if !policy.allows_download() { + anyhow::bail!("No suitable Rust version found for toolchain policy: {policy}"); } // Install new toolchain diff --git a/crates/prek/src/languages/rust/rust.rs b/crates/prek/src/languages/rust/rust.rs index 49fc3c575..281a3a008 100644 --- a/crates/prek/src/languages/rust/rust.rs +++ b/crates/prek/src/languages/rust/rust.rs @@ -407,7 +407,7 @@ impl LanguageBackend for Rust { let version: &RustRequest = hook.language_request.version(); let rust = installer - .install(version, hook.language_request.allows_download()) + .install(version, hook.language_request.toolchain_policy()) .await .context("Failed to install rust")?; let rustc_bin = bin_dir(rust.toolchain()); diff --git a/crates/prek/src/languages/rust/version.rs b/crates/prek/src/languages/rust/version.rs index 05121f9e9..0f2013cfb 100644 --- a/crates/prek/src/languages/rust/version.rs +++ b/crates/prek/src/languages/rust/version.rs @@ -112,9 +112,7 @@ impl RustVersion { } } -/// `language_version` field of rust can be one of the following: -/// `default` -/// `system` +/// A parsed Rust version request: /// `stable` /// `nightly` /// `beta` diff --git a/crates/prek/src/languages/version.rs b/crates/prek/src/languages/version.rs index ca8c44704..9a2953fbf 100644 --- a/crates/prek/src/languages/version.rs +++ b/crates/prek/src/languages/version.rs @@ -1,6 +1,9 @@ +use std::ffi::OsStr; +use std::fmt::{self, Display}; +use std::path::{Path, PathBuf}; use std::str::FromStr; -use crate::config::Language; +use crate::config::{Language, LanguageVersion, ToolchainPreference}; use crate::hook::InstallInfo; use crate::languages::bun::BunRequest; use crate::languages::deno::DenoRequest; @@ -21,9 +24,62 @@ pub(crate) enum Error { #[derive(Debug, Clone, Eq, PartialEq)] pub(crate) struct LanguageRequest { version: VersionRequest, + preference: ToolchainPreference, allows_download: bool, } +#[derive(Debug, Clone, Copy, Eq, PartialEq)] +pub(crate) enum ToolchainSource { + Managed, + System, +} + +#[derive(Debug, Clone, Copy, Eq, PartialEq)] +pub(crate) struct ToolchainPolicy { + preference: ToolchainPreference, + allows_download: bool, +} + +impl ToolchainPolicy { + pub(crate) fn search_order(self) -> &'static [ToolchainSource] { + match self.preference { + ToolchainPreference::OnlyManaged => &[ToolchainSource::Managed], + ToolchainPreference::Managed => &[ToolchainSource::Managed, ToolchainSource::System], + ToolchainPreference::System => &[ToolchainSource::System, ToolchainSource::Managed], + ToolchainPreference::OnlySystem => &[ToolchainSource::System], + } + } + + pub(crate) fn allows_download(self) -> bool { + self.allows_download + } +} + +pub(crate) fn find_system_executables( + binary_name: impl AsRef, + managed_root: &Path, +) -> which::Result> { + let managed_root = + dunce::canonicalize(managed_root).unwrap_or_else(|_| managed_root.to_path_buf()); + Ok(which::which_all(binary_name)? + .filter(|path| { + let path = dunce::canonicalize(path).unwrap_or_else(|_| path.clone()); + !path.starts_with(&managed_root) + }) + .collect()) +} + +impl Display for ToolchainPolicy { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let downloads = if self.allows_download { + "enabled" + } else { + "disabled" + }; + write!(f, "{} (downloads {downloads})", self.preference) + } +} + #[derive(Debug, Clone, Eq, PartialEq)] pub(crate) enum VersionRequest { Bun(BunRequest), @@ -70,9 +126,11 @@ impl LanguageRequest { self.version.is_any() } - /// Returns true if this request allows downloading a version. - pub(crate) fn allows_download(&self) -> bool { - self.allows_download + pub(crate) fn toolchain_policy(&self) -> ToolchainPolicy { + ToolchainPolicy { + preference: self.preference, + allows_download: self.allows_download, + } } pub(crate) fn version_request(&self) -> &VersionRequest { @@ -89,19 +147,27 @@ impl LanguageRequest { } pub(crate) fn parse(lang: Language, request: &str) -> Result { - // `pre-commit` support these values in `language_version`: - // - `default`: substituted by language `get_default_version` function - // In `get_default_version`, if a system version is available, it will return `system`. - // For Python, it will find from sys.executable, `pythonX.Y`, or versions `py` can find. - // Otherwise, it will still return `default`. - // - `system`: use a locally available version without downloading - // - Python version passed down to `virtualenv`, e.g. `python`, `python3`, `python3.8` - // - Node.js version passed down to `nodeenv` - // - Rust version passed down to `rustup` + let language_version = LanguageVersion::from(request); + Self::from_config(lang, Some(&language_version)) + } + + pub(crate) fn from_config( + lang: Language, + language_version: Option<&LanguageVersion>, + ) -> Result { + let (request, preference, allows_download) = match language_version { + Some(language_version) => ( + language_version.request().unwrap_or_default(), + language_version.preference(), + language_version.allows_download(), + ), + None => ("", ToolchainPreference::default(), true), + }; Ok(Self { version: VersionRequest::parse(lang, request)?, - allows_download: request != "system", + preference, + allows_download, }) } @@ -112,11 +178,6 @@ impl LanguageRequest { impl VersionRequest { pub(crate) fn parse(lang: Language, request: &str) -> Result { - let request = match request { - "default" | "system" => "", - request => request, - }; - Ok(match lang { Language::Bun => Self::Bun(request.parse()?), Language::Dotnet => Self::Dotnet(request.parse()?), @@ -221,8 +282,11 @@ pub(crate) fn try_into_u64_slice(version: &str) -> Result, std::num::Pa #[cfg(test)] mod tests { - use super::{LanguageRequest, SemverRequest, VersionRequest}; - use crate::config::Language; + use super::{ + LanguageRequest, SemverRequest, ToolchainSource, VersionRequest, find_system_executables, + }; + use crate::config::{Language, LanguageVersion}; + use crate::fs::make_executable; use crate::languages::python::PythonRequest; #[test] @@ -254,4 +318,99 @@ mod tests { assert!(!exact.matches(&newer)); assert!(compatible.matches(&newer)); } + + #[test] + fn structured_preferences_produce_expected_policies() { + let cases = [ + ("only-managed", true, &[ToolchainSource::Managed][..]), + ( + "managed", + true, + &[ToolchainSource::Managed, ToolchainSource::System][..], + ), + ( + "system", + true, + &[ToolchainSource::System, ToolchainSource::Managed][..], + ), + ("only-system", false, &[ToolchainSource::System][..]), + ]; + + for (preference, download, search_order) in cases { + let language_version: LanguageVersion = + serde_saphyr::from_str(&format!("request: '>=3.12'\npreference: {preference}\n")) + .unwrap(); + let request = + LanguageRequest::from_config(Language::Python, Some(&language_version)).unwrap(); + let policy = request.toolchain_policy(); + + assert_eq!(policy.allows_download(), download, "{preference}"); + assert_eq!(policy.search_order(), search_order, "{preference}"); + } + } + + #[test] + fn legacy_system_request_keeps_managed_first_fallback_without_downloads() { + let request = LanguageRequest::parse(Language::Python, "system").unwrap(); + let policy = request.toolchain_policy(); + + assert_eq!( + policy.search_order(), + &[ToolchainSource::Managed, ToolchainSource::System] + ); + assert!(!policy.allows_download()); + } + + #[test] + fn system_executables_exclude_managed_binaries() { + let temp = tempfile::tempdir().unwrap(); + let managed_root = temp.path().join("managed"); + let binary = managed_root + .join("bin") + .join("tool") + .with_extension(std::env::consts::EXE_EXTENSION); + fs_err::create_dir_all(binary.parent().unwrap()).unwrap(); + fs_err::write(&binary, "").unwrap(); + make_executable(&binary).unwrap(); + + let executables = find_system_executables(&binary, &managed_root).unwrap(); + + assert!(executables.is_empty()); + } + + #[test] + fn system_executables_include_binaries_outside_managed_root() { + let temp = tempfile::tempdir().unwrap(); + let managed_root = temp.path().join("managed"); + fs_err::create_dir_all(&managed_root).unwrap(); + let binary = temp + .path() + .join("system") + .join("tool") + .with_extension(std::env::consts::EXE_EXTENSION); + fs_err::create_dir_all(binary.parent().unwrap()).unwrap(); + fs_err::write(&binary, "").unwrap(); + make_executable(&binary).unwrap(); + + let executables = find_system_executables(&binary, &managed_root).unwrap(); + + assert_eq!(executables, vec![binary]); + } + + #[cfg(unix)] + #[test] + fn system_executables_exclude_symlinks_to_managed_binaries() { + let temp = tempfile::tempdir().unwrap(); + let managed_root = temp.path().join("managed"); + let binary = managed_root.join("bin/tool"); + fs_err::create_dir_all(binary.parent().unwrap()).unwrap(); + fs_err::write(&binary, "").unwrap(); + make_executable(&binary).unwrap(); + let link = temp.path().join("tool"); + std::os::unix::fs::symlink(binary, &link).unwrap(); + + let executables = find_system_executables(link, &managed_root).unwrap(); + + assert!(executables.is_empty()); + } } diff --git a/crates/prek/tests/run.rs b/crates/prek/tests/run.rs index ff6df5c1f..d7fe63154 100644 --- a/crates/prek/tests/run.rs +++ b/crates/prek/tests/run.rs @@ -4199,7 +4199,7 @@ fn system_language_version() { cmd_snapshot!(context, context.run() .arg("system-node") - .env(EnvVars::PREK_INTERNAL__NODE_BINARY_NAME, "node-never-exist"), @r" + .env(EnvVars::PREK_INTERNAL__NODE_BINARY_NAME, "node-never-exist"), @r#" success: false exit_code: 2 ----- stdout ----- @@ -4207,13 +4207,13 @@ fn system_language_version() { ----- stderr ----- error: Failed to install hook `system-node` caused by: Failed to install node - caused by: No suitable system Node version found and downloads are disabled - "); + caused by: No suitable Node version found for toolchain policy: managed (downloads disabled) + "#); cmd_snapshot!(context, context.run() .arg("system-go") - .env(EnvVars::PREK_INTERNAL__GO_BINARY_NAME, "go-never-exist"), @r" + .env(EnvVars::PREK_INTERNAL__GO_BINARY_NAME, "go-never-exist"), @r#" success: false exit_code: 2 ----- stdout ----- @@ -4221,13 +4221,13 @@ fn system_language_version() { ----- stderr ----- error: Failed to install hook `system-go` caused by: Failed to install go - caused by: No suitable system Go version found and downloads are disabled - "); + caused by: No suitable Go version found for toolchain policy: managed (downloads disabled) + "#); cmd_snapshot!(context, context.run() .arg("system-bun") - .env(EnvVars::PREK_INTERNAL__BUN_BINARY_NAME, "bun-never-exist"), @r" + .env(EnvVars::PREK_INTERNAL__BUN_BINARY_NAME, "bun-never-exist"), @r#" success: false exit_code: 2 ----- stdout ----- @@ -4235,13 +4235,13 @@ fn system_language_version() { ----- stderr ----- error: Failed to install hook `system-bun` caused by: Failed to install bun - caused by: No suitable system Bun version found and downloads are disabled - "); + caused by: No suitable Bun version found for toolchain policy: managed (downloads disabled) + "#); cmd_snapshot!(context, context.run() .arg("system-dotnet") - .env(EnvVars::PREK_INTERNAL__DOTNET_BINARY_NAME, "dotnet-never-exist"), @" + .env(EnvVars::PREK_INTERNAL__DOTNET_BINARY_NAME, "dotnet-never-exist"), @r#" success: false exit_code: 2 ----- stdout ----- @@ -4249,8 +4249,8 @@ fn system_language_version() { ----- stderr ----- error: Failed to install hook `system-dotnet` caused by: Failed to install dotnet SDK - caused by: No suitable dotnet version found and downloads are disabled - "); + caused by: No suitable dotnet version found for toolchain policy: managed (downloads disabled) + "#); } /// Tests that empty `entry` field. diff --git a/docs/authoring-hooks.md b/docs/authoring-hooks.md index b5aa4d526..3d44a80cd 100644 --- a/docs/authoring-hooks.md +++ b/docs/authoring-hooks.md @@ -35,7 +35,7 @@ each manifest hook: | `fail_fast` | No | No | boolean | Stop the run immediately if this hook fails. | | `pass_filenames` | No | No | boolean or positive integer | Control whether, or how many, matching filenames are passed. | | `description` | No | No | string | Free-form metadata shown in listings; its first line is also shown with run details. | -| `language_version` | No | No | string | Language/toolchain version request. | +| `language_version` | No | No | string or map | Language/toolchain version request and source preference. | | `log_file` | No | No | string path | Write hook output to a file when the hook fails or is verbose. | | `require_serial` | No | No | boolean | Avoid concurrent invocations of this hook. | | `stages` | No | No | list of stage names | Git hook stages where this hook is eligible to run. | @@ -59,6 +59,10 @@ manifest semantics. For the upstream reference, see: `pass_filenames: n` with a positive integer is also a `prek` extension. Upstream `pre-commit` only accepts a boolean value. + The `language_version` options map with `request` and `preference` fields is + a `prek` extension. Use the string form when a manifest must also work with + upstream `pre-commit`. + When `shell` is set, `entry` is treated as shell source. Hook `args` and filenames are passed as script arguments, so POSIX shell entries should read them with `"$@"`. `shell` is supported only for language backends that use diff --git a/docs/languages.md b/docs/languages.md index 261983426..74502536e 100644 --- a/docs/languages.md +++ b/docs/languages.md @@ -13,10 +13,39 @@ For `repo: local` hooks, `language` is required. For remote hooks, it is read fr ## Toolchain management and `language_version` -prek resolves toolchains in two stages: +`language_version` can be a version request string or an options object with +`request` and `preference` fields. The string form remains shorthand for a +request with the default `managed` preference. -1. **Reuse a compatible local toolchain**, including toolchains already managed by prek and system installations. -2. **Download a toolchain** when the language supports it, downloads are allowed, and the request cannot be satisfied locally. +The preferences are: + +- `only-managed`: use a compatible toolchain already managed by prek, or download one. +- `managed` (default): prefer an existing prek-managed toolchain, then search outside prek's managed store, then download one. +- `system`: prefer a toolchain outside prek's managed store, then try an existing prek-managed toolchain, then download one. +- `only-system`: only use a toolchain outside prek's managed store and never download one. + +Here, “system” means any toolchain discovered outside prek's managed store. It +includes executables installed by the operating system, a package manager, or a +version manager. + +The preference controls toolchain selection when a hook environment must be +created. It does not affect reuse of an existing compatible hook environment. + +For example: + +=== "prek.toml" + + ```toml + language_version = { request = ">=3.12, <3.13", preference = "only-managed" } + ``` + +=== ".pre-commit-config.yaml" + + ```yaml + language_version: + request: ">=3.12, <3.13" + preference: only-managed + ``` If `language_version` is `system`, prek does not download a new toolchain. It may still reuse a compatible toolchain already managed by prek before searching system installations. Version constraints derived from project metadata, such as Python’s `requires-python` or Go’s `go` directive, still apply without re-enabling downloads. @@ -32,6 +61,7 @@ Languages with managed toolchain downloads in prek today: - [Node](#node) - [Bun](#bun) - [Deno](#deno) +- [.NET](#dotnet) - [Golang](#golang) - [mise](#mise) - [Rust](#rust) diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index eb2ae90f0..a6abfe36d 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -253,7 +253,7 @@ This is a global default; individual hooks can also set `fail_fast`. ### `default_language_version` -Map a language name to the default [`language_version`](#language_version) used by hooks of that language. +Map a language name to the default [`language_version`](#language_version) used by hooks of that language. Each value can be a request string or an options map. - Type: map - Default: none (hooks fall back to `language_version: default`) @@ -264,7 +264,7 @@ Example: ```toml default_language_version.python = "3.12" - default_language_version.node = "20" + default_language_version.node = { request = "20", preference = "only-managed" } ``` === ".pre-commit-config.yaml" @@ -272,11 +272,16 @@ Example: ```yaml default_language_version: python: "3.12" - node: "20" + node: + request: "20" + preference: only-managed ``` `prek` treats [`language_version`](#language_version) as a version request (often a semver-like selector) and may install toolchains automatically. See [Difference from pre-commit](../diff.md). +Defaults are applied field by field. For example, a hook can override only the +`request` while retaining the default `preference` for its language. + ### `default_stages` Default [`stages`](#stages) used when a hook does not specify its own. @@ -1407,9 +1412,9 @@ verbose run details. ### `language_version` -Choose the language/toolchain version request for this hook. +Choose the language/toolchain version request and source preference for this hook. -- Type: string +- Type: string or map - Default: `default` If not set, `prek` may use [`default_language_version`](#default_language_version) for the hook’s language. @@ -1423,6 +1428,24 @@ If not set, `prek` may use [`default_language_version`](#default_language_versio - `default`: use the language’s default resolution logic. - `system`: do not download a new toolchain. A compatible toolchain already managed by prek may still be reused, and project metadata can further constrain the required version. + The map form accepts these fields: + + - `request`: the same version request accepted by the string form. It defaults to `default`. + - `preference`: controls which toolchain sources prek searches and in what order. It defaults to `managed`. + + `preference` accepts: + + - `only-managed`: use an existing prek-managed toolchain, or download one. + - `managed`: prefer an existing prek-managed toolchain, then search outside prek's managed store, then download one. + - `system`: prefer a toolchain outside prek's managed store, then try an existing prek-managed toolchain, then download one. + - `only-system`: only search outside prek's managed store and never download a toolchain. + + A “system” toolchain is any toolchain discovered outside prek's managed store, + including one installed by an operating-system package manager or version manager. + + The preference affects toolchain selection when a hook environment must be + created. It does not affect reuse of an existing compatible hook environment. + Language-specific behavior: - Python: passed to the Python resolver (for example `python3` or `python3.12`). May trigger toolchain download. @@ -1438,7 +1461,7 @@ If not set, `prek` may use [`default_language_version`](#default_language_versio ```toml hooks = [ { id = "ruff", language = "python", language_version = "3.12" }, - { id = "eslint", language = "node", language_version = "20" }, + { id = "eslint", language = "node", language_version = { request = "20", preference = "only-managed" } }, { id = "cargo-fmt", language = "rust", language_version = "stable" }, { id = "my-tool", language = "system", language_version = "system" }, ] @@ -1454,7 +1477,9 @@ If not set, `prek` may use [`default_language_version`](#default_language_versio - id: eslint language: node - language_version: "20" + language_version: + request: "20" + preference: only-managed - id: cargo-fmt language: rust diff --git a/prek.schema.json b/prek.schema.json index 2c8f6c2a5..96fbca7f5 100644 --- a/prek.schema.json +++ b/prek.schema.json @@ -38,79 +38,79 @@ "type": "object", "properties": { "bun": { - "type": "string" + "$ref": "#/definitions/LanguageVersion" }, "conda": { - "type": "string" + "$ref": "#/definitions/LanguageVersion" }, "coursier": { - "type": "string" + "$ref": "#/definitions/LanguageVersion" }, "dart": { - "type": "string" + "$ref": "#/definitions/LanguageVersion" }, "deno": { - "type": "string" + "$ref": "#/definitions/LanguageVersion" }, "docker": { - "type": "string" + "$ref": "#/definitions/LanguageVersion" }, "docker_image": { - "type": "string" + "$ref": "#/definitions/LanguageVersion" }, "dotnet": { - "type": "string" + "$ref": "#/definitions/LanguageVersion" }, "fail": { - "type": "string" + "$ref": "#/definitions/LanguageVersion" }, "golang": { - "type": "string" + "$ref": "#/definitions/LanguageVersion" }, "haskell": { - "type": "string" + "$ref": "#/definitions/LanguageVersion" }, "julia": { - "type": "string" + "$ref": "#/definitions/LanguageVersion" }, "lua": { - "type": "string" + "$ref": "#/definitions/LanguageVersion" }, "mise": { - "type": "string" + "$ref": "#/definitions/LanguageVersion" }, "node": { - "type": "string" + "$ref": "#/definitions/LanguageVersion" }, "perl": { - "type": "string" + "$ref": "#/definitions/LanguageVersion" }, "php": { - "type": "string" + "$ref": "#/definitions/LanguageVersion" }, "pygrep": { - "type": "string" + "$ref": "#/definitions/LanguageVersion" }, "python": { - "type": "string" + "$ref": "#/definitions/LanguageVersion" }, "r": { - "type": "string" + "$ref": "#/definitions/LanguageVersion" }, "ruby": { - "type": "string" + "$ref": "#/definitions/LanguageVersion" }, "rust": { - "type": "string" + "$ref": "#/definitions/LanguageVersion" }, "script": { - "type": "string" + "$ref": "#/definitions/LanguageVersion" }, "swift": { - "type": "string" + "$ref": "#/definitions/LanguageVersion" }, "system": { - "type": "string" + "$ref": "#/definitions/LanguageVersion" } }, "additionalProperties": false @@ -370,8 +370,8 @@ "type": "string" }, "language_version": { - "description": "Run the hook on a specific version of the language.\nDefault is `default`.\nSee .", - "type": "string" + "description": "Select a language version and optionally configure the toolchain source preference.\nDefault is `default`.\nSee .", + "$ref": "#/definitions/LanguageVersion" }, "log_file": { "description": "Write the output of the hook to a file when the hook fails or verbose is enabled.", @@ -495,6 +495,36 @@ } ] }, + "LanguageVersion": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "object", + "properties": { + "request": { + "description": "A language-specific version request. Defaults to `default`.", + "type": "string" + }, + "preference": { + "description": "Which toolchain sources to prefer or require. Defaults to `managed`.", + "$ref": "#/definitions/ToolchainPreference" + } + }, + "additionalProperties": false + } + ] + }, + "ToolchainPreference": { + "type": "string", + "enum": [ + "only-managed", + "managed", + "system", + "only-system" + ] + }, "Shell": { "description": "A predefined shell adapter used to run hook entries as shell source.", "type": "string", @@ -650,8 +680,8 @@ "type": "string" }, "language_version": { - "description": "Run the hook on a specific version of the language.\nDefault is `default`.\nSee .", - "type": "string" + "description": "Select a language version and optionally configure the toolchain source preference.\nDefault is `default`.\nSee .", + "$ref": "#/definitions/LanguageVersion" }, "log_file": { "description": "Write the output of the hook to a file when the hook fails or verbose is enabled.", @@ -819,8 +849,8 @@ "type": "string" }, "language_version": { - "description": "Run the hook on a specific version of the language.\nDefault is `default`.\nSee .", - "type": "string" + "description": "Select a language version and optionally configure the toolchain source preference.\nDefault is `default`.\nSee .", + "$ref": "#/definitions/LanguageVersion" }, "log_file": { "description": "Write the output of the hook to a file when the hook fails or verbose is enabled.", @@ -993,8 +1023,8 @@ "type": "string" }, "language_version": { - "description": "Run the hook on a specific version of the language.\nDefault is `default`.\nSee .", - "type": "string" + "description": "Select a language version and optionally configure the toolchain source preference.\nDefault is `default`.\nSee .", + "$ref": "#/definitions/LanguageVersion" }, "log_file": { "description": "Write the output of the hook to a file when the hook fails or verbose is enabled.",