huak icon indicating copy to clipboard operation
huak copied to clipboard

Python version parsing from interpreter paths isn't reliable

Open cnpryer opened this issue 1 year ago • 2 comments

The way this is currently done just expects the file name to meet certain requirements:

  • Windows
    • Starts with "python"
    • Ends with ".exe"
  • Unix
    • Starts with "python"
    • Is at least "python3.0" in length

The Windows implementation won't parse a version number unless, for some reason, there is a version number to parse in the exe name. IME there isn't.

The unix implementation doesn't account for interpreters like "python311" (showed up on ubuntu-latest here; see below).

 failures:

---- ops::tests::test_use_python stdout ----
thread 'ops::tests::test_use_python' panicked at 'assertion failed: `(left == right)`
  left: `[3]`,
 right: `[311]`', src/huak/ops.rs:1564:9
// ...
assert_eq!(
    venv.python_version().unwrap().release[..version.release.len()],
    version.release
);

At the moment the pep440_rs Version struct is used to parse version numbers from strings and is done for unix using

Version::from_str(&file_name["python".len()..])

So the path would have been .../python311. right: [311] is Version.release, a vector of usize corresponding to the major.minor.max numbers. So this would effectively be "311.0.0" if interpreted as a major.

cnpryer avatar Mar 30 '23 21:03 cnpryer

Tagging as a good first issue if scope is limited to something like (1) parse version slice of filename, (2) check if its a valid Python interpreter version, (3a) if its not fallback to pulling it using command method, (3b) otherwise convert to version and use.

This should be mainly limited to the python use subcommand, which gives this even less scope.

cnpryer avatar Apr 11 '23 01:04 cnpryer

cc #599

cnpryer avatar Nov 16 '23 18:11 cnpryer