Skip to content

Remove direct path support from language_version #1831

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
j178 wants to merge 2 commits into
base: master
Choose a base branch
from
+75 −205
Open

Remove direct path support from language_version #1831

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1171e04
Remove direct path support from language_version
j178 Mar 19, 2026
37faf13
Update Ruby tests for removed path support
j178 Mar 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions crates/prek/src/languages/bun/installer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ impl BunInstaller {

installed
.find_map(|(v, path)| {
if req.matches(&v, Some(&path)) {
if req.matches(&v) {
Some(BunResult::from_dir(&path).with_version(v))
} else {
None
Expand All @@ -166,7 +166,7 @@ impl BunInstaller {
.context("Failed to list remote versions")?;
let version = versions
.into_iter()
.find(|version| req.matches(version, None))
.find(|version| req.matches(version))
.context("Version not found on remote")?;
Ok(version)
}
Expand Down Expand Up @@ -259,7 +259,7 @@ impl BunInstaller {
match BunResult::from_executable(bun_path).fill_version().await {
Ok(bun_result) => {
// Check if this version matches the request
if bun_request.matches(&bun_result.version, Some(&bun_result.bun)) {
if bun_request.matches(&bun_result.version) {
trace!(
%bun_result,
"Found a matching system bun"
Expand Down
45 changes: 14 additions & 31 deletions crates/prek/src/languages/bun/version.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use std::fmt::Display;
use std::ops::Deref;
use std::path::{Path, PathBuf};
use std::str::FromStr;

use serde::Deserialize;
Expand Down Expand Up @@ -48,14 +47,12 @@ impl FromStr for BunVersion {
/// - `x.y.z` or `bun@x.y.z`: Install the specific version.
/// - `^x.y.z`: Install the latest version that satisfies the semver requirement.
/// Or any other semver compatible version requirement.
/// - `local/path/to/bun`: Use bun executable at the specified path.
#[derive(Debug, Clone, Eq, PartialEq)]
pub(crate) enum BunRequest {
Any,
Major(u64),
MajorMinor(u64, u64),
MajorMinorPatch(u64, u64, u64),
Path(PathBuf),
Range(semver::VersionReq),
}

Expand All @@ -79,20 +76,11 @@ impl FromStr for BunRequest {
return Ok(BunRequest::Any);
}

Self::parse_version_numbers(s, s)
.or_else(|_| {
semver::VersionReq::parse(s)
.map(BunRequest::Range)
.map_err(|_| Error::InvalidVersion(s.to_string()))
})
.or_else(|_| {
let path = PathBuf::from(s);
if path.exists() {
Ok(BunRequest::Path(path))
} else {
Err(Error::InvalidVersion(s.to_string()))
}
})
Self::parse_version_numbers(s, s).or_else(|_| {
semver::VersionReq::parse(s)
.map(BunRequest::Range)
.map_err(|_| Error::InvalidVersion(s.to_string()))
})
}
}

Expand All @@ -118,22 +106,17 @@ impl BunRequest {

pub(crate) fn satisfied_by(&self, install_info: &InstallInfo) -> bool {
let version = &install_info.language_version;
self.matches(
&BunVersion(version.clone()),
Some(install_info.toolchain.as_ref()),
)
self.matches(&BunVersion(version.clone()))
}

pub(crate) fn matches(&self, version: &BunVersion, toolchain: Option<&Path>) -> bool {
pub(crate) fn matches(&self, version: &BunVersion) -> bool {
match self {
Self::Any => true,
Self::Major(major) => version.major == *major,
Self::MajorMinor(major, minor) => version.major == *major && version.minor == *minor,
Self::MajorMinorPatch(major, minor, patch) => {
version.major == *major && version.minor == *minor && version.patch == *patch
}
// FIXME: consider resolving symlinks and normalizing paths before comparison
Self::Path(path) => toolchain.is_some_and(|toolchain_path| toolchain_path == path),
Self::Range(req) => req.matches(version),
}
}
Expand Down Expand Up @@ -204,12 +187,12 @@ mod tests {
fn test_bun_request_matches() {
let version = BunVersion(semver::Version::new(1, 1, 4));

assert!(BunRequest::Any.matches(&version, None));
assert!(BunRequest::Major(1).matches(&version, None));
assert!(!BunRequest::Major(2).matches(&version, None));
assert!(BunRequest::MajorMinor(1, 1).matches(&version, None));
assert!(!BunRequest::MajorMinor(1, 2).matches(&version, None));
assert!(BunRequest::MajorMinorPatch(1, 1, 4).matches(&version, None));
assert!(!BunRequest::MajorMinorPatch(1, 1, 5).matches(&version, None));
assert!(BunRequest::Any.matches(&version));
assert!(BunRequest::Major(1).matches(&version));
assert!(!BunRequest::Major(2).matches(&version));
assert!(BunRequest::MajorMinor(1, 1).matches(&version));
assert!(!BunRequest::MajorMinor(1, 2).matches(&version));
assert!(BunRequest::MajorMinorPatch(1, 1, 4).matches(&version));
assert!(!BunRequest::MajorMinorPatch(1, 1, 5).matches(&version));
}
}
6 changes: 3 additions & 3 deletions crates/prek/src/languages/golang/installer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ impl GoInstaller {

installed
.find_map(|(version, path)| {
if request.matches(&version, Some(&path)) {
if request.matches(&version) {
trace!(%version, "Found matching installed go");
Some(GoResult::from_dir(&path, false).with_version(version))
} else {
Expand Down Expand Up @@ -196,7 +196,7 @@ impl GoInstaller {

let version = versions
.into_iter()
.find(|version| req.matches(version, None))
.find(|version| req.matches(version))
.with_context(|| format!("Version `{req}` not found on remote"))?;
Ok(version)
}
Expand Down Expand Up @@ -263,7 +263,7 @@ impl GoInstaller {
{
Ok(go) => {
// Check if this version matches the request
if go_request.matches(&go.version, Some(&go.path)) {
if go_request.matches(&go.version) {
trace!(
%go,
"Found matching system go"
Expand Down
35 changes: 8 additions & 27 deletions crates/prek/src/languages/golang/version.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use std::fmt::Display;
use std::ops::Deref;
use std::path::{Path, PathBuf};
use std::str::FromStr;

use serde::Deserialize;
Expand Down Expand Up @@ -51,14 +50,12 @@ impl FromStr for GoVersion {
/// `go1.20rc1` or `1.20rc1`
/// `go1.18beta1` or `1.18beta1`
/// `>= 1.20, < 1.22`
/// `local/path/to/go`
#[derive(Debug, Clone, Eq, PartialEq)]
pub(crate) enum GoRequest {
Any,
Major(u64),
MajorMinor(u64, u64),
MajorMinorPatch(u64, u64, u64),
Path(PathBuf),
Range(semver::VersionReq, String),
// TODO: support prerelease versions like `go1.20.0b1`, `go1.20rc1`
// MajorMinorPrerelease(u64, u64, String),
Expand All @@ -73,7 +70,6 @@ impl Display for GoRequest {
GoRequest::MajorMinorPatch(major, minor, patch) => {
write!(f, "go{major}.{minor}.{patch}")
}
GoRequest::Path(path) => write!(f, "path: {}", path.display()),
GoRequest::Range(_, raw) => write!(f, "{raw}"),
}
}
Expand All @@ -96,21 +92,11 @@ impl FromStr for GoRequest {
return Self::parse_version_numbers(version_part, s);
}

Self::parse_version_numbers(s, s)
.or_else(|_| {
semver::VersionReq::parse(s)
.map(|version_req| GoRequest::Range(version_req, s.into()))
.map_err(|_| Error::InvalidVersion(s.to_string()))
})
.or_else(|_| {
let path = PathBuf::from(s);
if path.exists() {
Ok(GoRequest::Path(path))
} else {
// TODO: better error message
Err(Error::InvalidVersion(s.to_string()))
}
})
Self::parse_version_numbers(s, s).or_else(|_| {
semver::VersionReq::parse(s)
.map(|version_req| GoRequest::Range(version_req, s.into()))
.map_err(|_| Error::InvalidVersion(s.to_string()))
})
}
}

Expand All @@ -137,13 +123,10 @@ impl GoRequest {
pub(crate) fn satisfied_by(&self, install_info: &InstallInfo) -> bool {
let version = &install_info.language_version;

self.matches(
&GoVersion(version.clone()),
Some(install_info.toolchain.as_ref()),
)
self.matches(&GoVersion(version.clone()))
}

pub(crate) fn matches(&self, version: &GoVersion, toolchain: Option<&Path>) -> bool {
pub(crate) fn matches(&self, version: &GoVersion) -> bool {
match self {
GoRequest::Any => true,
GoRequest::Major(major) => version.0.major == *major,
Expand All @@ -153,8 +136,6 @@ impl GoRequest {
GoRequest::MajorMinorPatch(major, minor, patch) => {
version.0.major == *major && version.0.minor == *minor && version.0.patch == *patch
}
// FIXME: consider resolving symlinks and normalizing paths before comparison
GoRequest::Path(path) => toolchain.is_some_and(|t| t == path),
GoRequest::Range(req, _) => req.matches(&version.0),
}
}
Expand Down Expand Up @@ -227,7 +208,7 @@ mod tests {
];

for (req, expected) in cases {
let result = req.matches(&version, None);
let result = req.matches(&version);
assert_eq!(result, expected, "Request: {req}");
}
}
Expand Down
6 changes: 3 additions & 3 deletions crates/prek/src/languages/node/installer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ impl NodeInstaller {

installed
.find_map(|(v, path)| {
if req.matches(&v, Some(&path)) {
if req.matches(&v) {
Some(NodeResult::from_dir(&path).with_version(v))
} else {
None
Expand All @@ -175,7 +175,7 @@ impl NodeInstaller {
.context("Failed to list remote versions")?;
let version = versions
.into_iter()
.find(|version| req.matches(version, None))
.find(|version| req.matches(version))
.context("Version not found on remote")?;
Ok(version)
}
Expand Down Expand Up @@ -254,7 +254,7 @@ impl NodeInstaller {
{
Ok(node_result) => {
// Check if this version matches the request
if node_request.matches(&node_result.version, Some(&node_result.node)) {
if node_request.matches(&node_result.version) {
trace!(
%node_result,
"Found a matching system node"
Expand Down
43 changes: 10 additions & 33 deletions crates/prek/src/languages/node/version.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use std::fmt::Display;
use std::path::{Path, PathBuf};
use std::str::FromStr;

use serde::{Deserialize, Deserializer, Serialize};
Expand Down Expand Up @@ -134,14 +133,12 @@ impl NodeVersion {
/// - `^x.y.z`: Install the latest version of node that satisfies the version requirement.
/// Or any other semver compatible version requirement.
/// - `lts/<codename>`: Install the latest version of node with the specified code name.
/// - `local/path/to/node`: Use the node executable at the specified path.
#[derive(Debug, Clone, Eq, PartialEq)]
pub(crate) enum NodeRequest {
Any,
Major(u64),
MajorMinor(u64, u64),
MajorMinorPatch(u64, u64, u64),
Path(PathBuf),
Range(semver::VersionReq),
// A bare `lts` request is interpreted as the latest LTS version.
Lts,
Expand Down Expand Up @@ -174,20 +171,11 @@ impl FromStr for NodeRequest {
Err(Error::InvalidVersion(request.to_string()))
}
} else {
Self::parse_version_numbers(request, request)
.or_else(|_| {
semver::VersionReq::parse(request)
.map(NodeRequest::Range)
.map_err(|_| Error::InvalidVersion(request.to_string()))
})
.or_else(|_| {
let path = PathBuf::from(request);
if path.exists() {
Ok(NodeRequest::Path(path))
} else {
Err(Error::InvalidVersion(request.to_string()))
}
})
Self::parse_version_numbers(request, request).or_else(|_| {
semver::VersionReq::parse(request)
.map(NodeRequest::Range)
.map_err(|_| Error::InvalidVersion(request.to_string()))
})
}
}
}
Expand Down Expand Up @@ -221,16 +209,13 @@ impl NodeRequest {
.and_then(|s| serde_json::from_str(s).ok())
.unwrap_or(Lts::NotLts);

self.matches(
&NodeVersion {
version: version.clone(),
lts: tls,
},
Some(install_info.toolchain.as_ref()),
)
self.matches(&NodeVersion {
version: version.clone(),
lts: tls,
})
}

pub(crate) fn matches(&self, version: &NodeVersion, toolchain: Option<&Path>) -> bool {
pub(crate) fn matches(&self, version: &NodeVersion) -> bool {
match self {
NodeRequest::Any => true,
NodeRequest::Major(major) => version.major() == *major,
Expand All @@ -240,8 +225,6 @@ impl NodeRequest {
NodeRequest::MajorMinorPatch(major, minor, patch) => {
version.major() == *major && version.minor() == *minor && version.patch() == *patch
}
// FIXME: consider resolving symlinks and normalizing paths before comparison
NodeRequest::Path(path) => toolchain.is_some_and(|t| t == path),
NodeRequest::Range(req) => req.matches(version.version()),
NodeRequest::Lts => version.lts.code_name().is_some(),
NodeRequest::CodeName(name) => version
Expand Down Expand Up @@ -338,12 +321,6 @@ mod tests {
let request = NodeRequest::CodeName("Boron".to_string());
assert!(!request.satisfied_by(&install_info));

let request = NodeRequest::Path(PathBuf::from("/usr/bin/node"));
assert!(request.satisfied_by(&install_info));

let request = NodeRequest::Path(PathBuf::from("/usr/bin/nodejs"));
assert!(!request.satisfied_by(&install_info));

let request = NodeRequest::Range(semver::VersionReq::parse(">=12.18").unwrap());
assert!(request.satisfied_by(&install_info));

Expand Down
1 change: 0 additions & 1 deletion crates/prek/src/languages/python/python.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,6 @@ fn to_uv_python_request(request: &LanguageRequest) -> Option<String> {
Some(format!("{major}.{minor}.{patch}"))
}
PythonRequest::Range(_, raw) => Some(raw.clone()),
PythonRequest::Path(path) => Some(path.to_string_lossy().to_string()),
},
_ => unreachable!(),
}
Expand Down
Loading
Loading