Skip to content

Commit 0870b45

Browse files
committed
fix: probe expected build-details.json paths and use deterministic fallback (PR #452)
Address Copilot review: avoid relying on fs::read_dir iteration order. When a (major, minor) hint is supplied, directly probe lib/{python|pypy}M.m{,t}/build-details.json. When no hint is given, enumerate but pick the highest (major, minor) tuple.
1 parent 54f87b4 commit 0870b45

1 file changed

Lines changed: 62 additions & 19 deletions

File tree

crates/pet-python-utils/src/build_details.rs

Lines changed: 62 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -125,14 +125,28 @@ fn find_file(prefix: &Path, pyver: Option<(u64, u64)>) -> Option<PathBuf> {
125125
return Some(win_path);
126126
}
127127

128-
// Unix-style: <prefix>/lib/python<X.Y>[t]/build-details.json
129-
// (also catches pypy<X.Y> and any other implementation that follows the
130-
// same convention).
131128
let lib_dir = prefix.join("lib");
132-
let entries = fs::read_dir(&lib_dir).ok()?;
133129

134-
let mut preferred: Option<PathBuf> = None;
135-
let mut fallback: Option<PathBuf> = None;
130+
// Fast path: if we have a (major, minor) hint, probe the expected paths
131+
// directly and skip the directory scan entirely.
132+
if let Some((major, minor)) = pyver {
133+
for impl_prefix in ["python", "pypy"] {
134+
for suffix in ["", "t"] {
135+
let candidate = lib_dir
136+
.join(format!("{impl_prefix}{major}.{minor}{suffix}"))
137+
.join(BUILD_DETAILS_FILE);
138+
if candidate.is_file() {
139+
return Some(candidate);
140+
}
141+
}
142+
}
143+
}
144+
145+
// Slow path: enumerate `lib/` and pick deterministically (highest
146+
// `(major, minor)`) so multi-version prefixes don't depend on `read_dir`
147+
// iteration order.
148+
let entries = fs::read_dir(&lib_dir).ok()?;
149+
let mut best: Option<(u64, u64, PathBuf)> = None;
136150
for entry in entries.filter_map(Result::ok) {
137151
let path = entry.path();
138152
if !path.is_dir() {
@@ -150,21 +164,18 @@ fn find_file(prefix: &Path, pyver: Option<(u64, u64)>) -> Option<PathBuf> {
150164
if !candidate.is_file() {
151165
continue;
152166
}
153-
if let Some((want_major, want_minor)) = pyver {
154-
// `unwrap()`s here are safe: the regex guarantees both groups are
155-
// present and contain only digits, so `parse::<u64>` cannot fail.
156-
let major: u64 = captures[2].parse().unwrap();
157-
let minor: u64 = captures[3].parse().unwrap();
158-
if major == want_major && minor == want_minor {
159-
preferred = Some(candidate);
160-
break;
161-
}
162-
}
163-
if fallback.is_none() {
164-
fallback = Some(candidate);
167+
// `unwrap()`s here are safe: the regex guarantees both groups are
168+
// present and contain only digits, so `parse::<u64>` cannot fail.
169+
let major: u64 = captures[2].parse().unwrap();
170+
let minor: u64 = captures[3].parse().unwrap();
171+
if best
172+
.as_ref()
173+
.is_none_or(|(bm, bn, _)| (major, minor) > (*bm, *bn))
174+
{
175+
best = Some((major, minor, candidate));
165176
}
166177
}
167-
preferred.or(fallback)
178+
best.map(|(_, _, path)| path)
168179
}
169180

170181
fn strip_bin(prefix: &Path) -> PathBuf {
@@ -430,6 +441,38 @@ mod tests {
430441
assert_eq!(bd.version_string(), "3.10.0");
431442
}
432443

444+
#[test]
445+
fn no_hint_picks_highest_version_deterministically() {
446+
// Multiple stdlib dirs side by side: pick the highest (major, minor)
447+
// regardless of `read_dir` ordering.
448+
let dir = tempdir().unwrap();
449+
let prefix = dir.path();
450+
write(
451+
&prefix
452+
.join("lib")
453+
.join("python3.10")
454+
.join("build-details.json"),
455+
&sample_with_minor(10),
456+
);
457+
write(
458+
&prefix
459+
.join("lib")
460+
.join("python3.15")
461+
.join("build-details.json"),
462+
&sample_with_minor(15),
463+
);
464+
write(
465+
&prefix
466+
.join("lib")
467+
.join("python3.14")
468+
.join("build-details.json"),
469+
&sample_with_minor(14),
470+
);
471+
472+
let bd = BuildDetails::find(prefix).unwrap();
473+
assert_eq!(bd.version_string(), "3.15.0");
474+
}
475+
433476
#[test]
434477
fn rejects_unsupported_schema_version() {
435478
let dir = tempdir().unwrap();

0 commit comments

Comments
 (0)