Skip to content

Commit e4d3851

Browse files
David-OConnorDavid-OConnor
committed
Fixed a bug in version ordering which caused big failures
1 parent 9d60435 commit e4d3851

File tree

7 files changed

+248
-207
lines changed

7 files changed

+248
-207
lines changed

‎README.md‎

Lines changed: 74 additions & 151 deletions
Large diffs are not rendered by default.

‎install_pyflow.sh‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
#!/bin/sh
2+
set -eu
3+
4+
# This file installs the app into the user's home directory and makes sure it is on PATH.
5+
6+
NAME_UPPER="Pyflow"
7+
NAME="pyflow"
8+
9+
APP_DIR="$HOME/$NAME"
10+
BIN_DIR="$HOME/.local/bin"
11+
TARGET_PATH="$BIN_DIR/$NAME"
12+
13+
if [ ! -f "./$NAME" ]; then
14+
printf "Error: expected executable ./%s in the current directory.\n" "$NAME" >&2
15+
exit 1
16+
fi
17+
18+
chmod +x "./$NAME"
19+
20+
mkdir -p "$APP_DIR"
21+
mkdir -p "$BIN_DIR"
22+
23+
cp "./$NAME" "$APP_DIR/$NAME"
24+
cp "./$NAME" "$TARGET_PATH"
25+
chmod +x "$APP_DIR/$NAME" "$TARGET_PATH"
26+
27+
add_path_line() {
28+
RC_FILE="$1"
29+
LINE='export PATH="$HOME/.local/bin:$PATH"'
30+
31+
if [ -f "$RC_FILE" ]; then
32+
if grep -Fqx "$LINE" "$RC_FILE"; then
33+
return
34+
fi
35+
fi
36+
37+
{
38+
printf '\n# Added by Pyflow installer\n'
39+
printf '%s\n' "$LINE"
40+
} >> "$RC_FILE"
41+
}
42+
43+
add_path_line "$HOME/.profile"
44+
45+
if [ -n "${BASH_VERSION:-}" ]; then
46+
add_path_line "$HOME/.bashrc"
47+
fi
48+
49+
if [ -n "${ZSH_VERSION:-}" ]; then
50+
add_path_line "$HOME/.zshrc"
51+
fi
52+
53+
if [ -f "$HOME/.bashrc" ]; then
54+
add_path_line "$HOME/.bashrc"
55+
fi
56+
57+
if [ -f "$HOME/.zshrc" ]; then
58+
add_path_line "$HOME/.zshrc"
59+
fi
60+
61+
case ":$PATH:" in
62+
*":$BIN_DIR:"*)
63+
PATH_ALREADY_SET="yes"
64+
;;
65+
*)
66+
PATH_ALREADY_SET="no"
67+
;;
68+
esac
69+
70+
printf "\nInstalled %s to:\n %s\n" "$NAME_UPPER" "$APP_DIR/$NAME"
71+
printf "\nCommand installed to:\n %s\n" "$TARGET_PATH"
72+
73+
if [ "$PATH_ALREADY_SET" = "yes" ]; then
74+
printf "\n%s is already available on PATH in this shell.\n" "$NAME"
75+
printf "You can launch it now with:\n %s\n" "$NAME"
76+
else
77+
printf "\nAdded %s to your shell startup files.\n" "$BIN_DIR"
78+
printf "After restarting your terminal, you can launch it with:\n %s\n" "$NAME"
79+
printf "\nTo use it immediately in this shell, run:\n export PATH=\"%s:\$PATH\"\n" "$BIN_DIR"
80+
fi

‎src/dep_parser.rs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,8 @@ fn parse_wh_py_ver(input: &str) -> IResult<&str, Constraint> {
109109
alt((tag("2"), tag("3"), tag("4"))),
110110
opt(digit1),
111111
),
112-
|(prefix, major, minor): (&str, &str, Option<&str>)| { // <-- Capture `prefix`
112+
|(prefix, major, minor): (&str, &str, Option<&str>)| {
113+
// <-- Capture `prefix`
113114
let major: u32 = major.parse().unwrap();
114115

115116
// Determine constraint type based on the wheel tag prefix
@@ -145,7 +146,7 @@ fn parse_wh_py_ver(input: &str) -> IResult<&str, Constraint> {
145146
}
146147
},
147148
)
148-
.parse(input)
149+
.parse(input)
149150
}
150151

151152
fn quote(input: &str) -> IResult<&str, &str> {

‎src/dep_resolution.rs‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use std::{cmp::min, collections::HashMap, str::FromStr};
22

3-
use serde::{Deserialize};
3+
use serde::Deserialize;
44
use termcolor::Color;
55

66
use crate::{
@@ -298,7 +298,7 @@ pub(super) mod res {
298298
Ok(resp)
299299
}
300300

301-
/// Find the latest version of a package by querying the warehouse. Also return
301+
/// Find the latest version of a package by querying the warehouse. Also return
302302
/// a vec of the versions found, so we can reuse this later without fetching a second time.
303303
/// Return name to, so we get correct capitalization.
304304
pub fn get_version_info(
@@ -346,13 +346,24 @@ pub(super) mod res {
346346
})
347347
.collect();
348348

349+
// todo temp
350+
// println!("\n\n\nAll compat: {:?}", all_compat);
351+
// for v in &all_compat {
352+
// if v.major == Some(9) && v.minor == Some(7) {
353+
// println!("V: {:?}", v);
354+
// }
355+
// }
356+
349357
// Grab the absolute latest version from the Python-compatible pool
350358
let latest_py_compat_version = all_compat
351359
.iter()
352360
.max()
353361
.unwrap_or_else(|| panic!("Can't find a valid version for {}", name))
354362
.clone();
355363

364+
// todo temp
365+
// println!("\n\n\nlatest compat: {:?}", latest_py_compat_version);
366+
356367
// Return the name, the absolute highest compatible version, and the full pool of options.
357368
// fetch_req_data will use all_compat to evaluate package-specific constraints (like >= 3.1)
358369
Ok((data.info.name, latest_py_compat_version, all_compat))
@@ -489,7 +500,6 @@ pub(super) mod res {
489500
// println!("-V: {:?}", version);
490501
// }
491502

492-
493503
// To minimize request time, only query the latest compatible version.
494504
let best_version = match all_versions
495505
.into_iter()
@@ -568,7 +578,6 @@ pub(super) mod res {
568578
fn make_renamed_packs(
569579
_vers_cache: &HashMap<String, (String, Version, Vec<Version>)>,
570580
deps: &[Dependency],
571-
// all_deps: &[Dependency],
572581
name: &str,
573582
) -> Vec<Package> {
574583
util::print_color(

‎src/dep_types.rs‎

Lines changed: 40 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ impl PartialOrd for VersionModifier {
147147
}
148148

149149
/// An exact, 3-number Semver version. With some possible extras.
150-
#[derive(Clone, Default, Deserialize, Eq)]
150+
#[derive(Clone, Default, Debug, Deserialize, Eq)]
151151
pub struct Version {
152152
pub major: Option<u32>,
153153
pub minor: Option<u32>,
@@ -318,45 +318,43 @@ impl FromStr for Version {
318318

319319
impl Ord for Version {
320320
fn cmp(&self, other: &Self) -> cmp::Ordering {
321-
// None version modifiers should rank highest. Ie 17.0 > 17.0rc1
322-
let self_mod = self.modifier.clone().unwrap_or((VersionModifier::Null, 0));
323-
let other_mod = other.modifier.clone().unwrap_or((VersionModifier::Null, 0));
324-
// Mirror the set field if we have a star present
325-
let cmp_star = |obj: Option<u32>, oth: Option<u32>, star: bool| -> cmp::Ordering {
326-
let none_val = if star {
327-
if obj.is_none() && oth.is_none() {
328-
0
329-
} else if let Some(x) = obj {
330-
x
321+
// 1. Compare numeric components. Treat None as 0 for stability.
322+
let fields = [
323+
(self.major, other.major),
324+
(self.minor, other.minor),
325+
(self.patch, other.patch),
326+
(self.extra_num, other.extra_num),
327+
];
328+
329+
for (s, o) in fields {
330+
let res = s.unwrap_or(0).cmp(&o.unwrap_or(0));
331+
if res != cmp::Ordering::Equal {
332+
return res;
333+
}
334+
}
335+
336+
// 2. Handle Stars: If one is a wildcard and the other isn't,
337+
// define a stable rule (e.g., specific versions are "greater" than wildcards)
338+
if self.star != other.star {
339+
return self.star.cmp(&other.star);
340+
}
341+
342+
// 3. Handle Modifiers
343+
let self_mod = self.modifier.as_ref().map(|(m, v)| (m, *v));
344+
let other_mod = other.modifier.as_ref().map(|(m, v)| (m, *v));
345+
346+
match (self_mod, other_mod) {
347+
(None, Some(_)) => cmp::Ordering::Greater, // None is higher (e.g., 1.0 > 1.0-rc1)
348+
(Some(_), None) => cmp::Ordering::Less,
349+
(Some((m1, v1)), Some((m2, v2))) => {
350+
let res = m1.cmp(m2);
351+
if res == cmp::Ordering::Equal {
352+
v1.cmp(&v2)
331353
} else {
332-
oth.unwrap_or(0)
354+
res
333355
}
334-
} else {
335-
0
336-
};
337-
obj.unwrap_or(none_val).cmp(&oth.unwrap_or(none_val))
338-
};
339-
let star = self.star || other.star;
340-
let maj = cmp_star(self.major, other.major, star);
341-
let min = cmp_star(self.minor, other.minor, star);
342-
let pat = cmp_star(self.patch, other.patch, star);
343-
let ext = cmp_star(self.extra_num, other.extra_num, star);
344-
if !matches!(maj, cmp::Ordering::Equal) {
345-
maj
346-
} else if !matches!(min, cmp::Ordering::Equal) {
347-
min
348-
} else if !matches!(pat, cmp::Ordering::Equal) {
349-
pat
350-
} else if !matches!(ext, cmp::Ordering::Equal) {
351-
ext
352-
} else if !star {
353-
if self_mod.0 == other_mod.0 {
354-
self_mod.1.cmp(&other_mod.1)
355-
} else {
356-
self_mod.0.cmp(&other_mod.0)
357356
}
358-
} else {
359-
cmp::Ordering::Equal
357+
(None, None) => cmp::Ordering::Equal,
360358
}
361359
}
362360
}
@@ -427,11 +425,11 @@ impl fmt::Display for Version {
427425
}
428426
}
429427

430-
impl fmt::Debug for Version {
431-
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
432-
write!(f, "{}", &self.to_string())
433-
}
434-
}
428+
// impl fmt::Debug for Version {
429+
// fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
430+
// write!(f, "{}", &self.to_string())
431+
// }
432+
// }
435433

436434
/// Specify the type of version requirement
437435
#[derive(Clone, Copy, Debug, Deserialize, PartialEq, Serialize)]

‎src/install.rs‎

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,7 @@ pub fn download_and_install_package(
224224
.map(|b| format!("{:02X}", b))
225225
.collect();
226226
if file_digest_str.to_lowercase() != expected_digest.to_lowercase() {
227-
util::print_color(
227+
print_color(
228228
&format!(
229229
"Hash failed for {}. Expected: {}, Actual: {}. Continue with installation anyway? (yes / no)",
230230
filename,
@@ -370,16 +370,45 @@ pub fn download_and_install_package(
370370

371371
replace_distutils(&extracted_parent.join("setup.py"));
372372

373+
Command::new(paths.bin.join("python"))
374+
.args(["-m", "ensurepip", "--upgrade"])
375+
.output()
376+
.unwrap_or_else(|_| {
377+
panic!(
378+
"Problemensuring pip: {:?}. Py path: {:?}",
379+
&extracted_parent,
380+
paths.bin.join("python")
381+
)
382+
});
373383

374-
// Add setuptools using Pip, to bootstrap?
375-
{
384+
Command::new(paths.bin.join("python"))
385+
.args([
386+
"-m",
387+
"pip",
388+
"install",
389+
"--upgrade",
390+
"pip",
391+
"setuptools",
392+
"wheel",
393+
"build",
394+
])
395+
.output()
396+
.unwrap_or_else(|_| {
397+
panic!(
398+
"Problem installing setupjtools, wheel, build: {:?}. Py path: {:?}",
399+
&extracted_parent,
400+
paths.bin.join("python")
401+
)
402+
});
376403

377-
}
404+
// Add setuptools using Pip, to bootstrap?
405+
{}
378406
// #[cfg(target_os = "windows")]
379407
{
380408
let output = Command::new(paths.bin.join("python"))
381409
.current_dir(&extracted_parent)
382-
.args(&["setup.py", "bdist_wheel"])
410+
// .args(&["setup.py", "bdist_wheel"])
411+
.args(&["-m", "build", "--wheel", "--no-isolation"])
383412
.output()
384413
.unwrap_or_else(|_| {
385414
panic!(

‎src/script.rs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use std::{fs, path::Path, str::FromStr};
66

77
use toml::Value;
8+
89
use crate::{
910
commands,
1011
dep_parser::parse_version,
@@ -56,7 +57,7 @@ pub fn run_script(
5657
.expect("Problem reading Python version for this script")
5758
.replace("\n", ""),
5859
)
59-
.expect("Problem parsing version from file");
60+
.expect("Problem parsing version from file");
6061
} else {
6162
cfg_vers = util::prompts::py_vers();
6263
create_or_update_version_file(&py_vers_path, &cfg_vers);
@@ -108,7 +109,7 @@ pub fn run_script(
108109
Extras::new_py(Constraint::new(ReqType::Exact, py_vers.clone())),
109110
)),
110111
)
111-
.unwrap_or_else(|_| panic!("Problem getting version info for {}", &name));
112+
.unwrap_or_else(|_| panic!("Problem getting version info for {}", &name));
112113
(vinfo.0, vinfo.1)
113114
};
114115

@@ -391,4 +392,4 @@ mod tests {
391392

392393
assert_eq!(expected, actual);
393394
}
394-
}
395+
}

0 commit comments

Comments
 (0)