uv-resolver: make PubGrubPackage orderable

It turns out that we use PubGrubPackage as the key in hashmaps in a fair
few places. And when we iterate over hashmaps, the order is unspecified.
This can in turn result in changes in output as a result of changes in
the PubGrubPackage definition, purely as a function of its changing
hash. This is confusing as there should be no semantic difference.

Thus, this is a precursor to introducing some more determinism to places
I found in the error reporting whose output depending on hashmap
iteration order.
This commit is contained in:
Andrew Gallant 2024-05-20 13:11:07 -04:00 committed by Andrew Gallant
parent 9f109f243c
commit 976bc9ba0e
5 changed files with 21 additions and 10 deletions

View file

@ -23,7 +23,7 @@ pub enum ParsedUrlError {
UrlParse(String, #[source] url::ParseError), UrlParse(String, #[source] url::ParseError),
} }
#[derive(Debug, Clone, Hash, PartialEq, Eq)] #[derive(Debug, Clone, Hash, PartialEq, PartialOrd, Eq, Ord)]
pub struct VerbatimParsedUrl { pub struct VerbatimParsedUrl {
pub parsed_url: ParsedUrl, pub parsed_url: ParsedUrl,
pub verbatim: VerbatimUrl, pub verbatim: VerbatimUrl,
@ -36,7 +36,7 @@ pub struct VerbatimParsedUrl {
/// * A remote archive (`https://`), optional with a subdirectory (source dist only). /// * A remote archive (`https://`), optional with a subdirectory (source dist only).
/// ///
/// A URL in a requirement `foo @ <url>` must be one of the above. /// A URL in a requirement `foo @ <url>` must be one of the above.
#[derive(Debug, Clone, Eq, PartialEq, Hash)] #[derive(Debug, Clone, Eq, PartialEq, PartialOrd, Hash, Ord)]
pub enum ParsedUrl { pub enum ParsedUrl {
/// The direct URL is a path to a local directory or file. /// The direct URL is a path to a local directory or file.
Path(ParsedPathUrl), Path(ParsedPathUrl),
@ -51,7 +51,7 @@ pub enum ParsedUrl {
/// ///
/// Examples: /// Examples:
/// * `file:///home/ferris/my_project` /// * `file:///home/ferris/my_project`
#[derive(Debug, Clone, Eq, PartialEq, Hash)] #[derive(Debug, Clone, Eq, PartialEq, PartialOrd, Hash, Ord)]
pub struct ParsedPathUrl { pub struct ParsedPathUrl {
pub url: Url, pub url: Url,
pub path: PathBuf, pub path: PathBuf,
@ -63,7 +63,7 @@ pub struct ParsedPathUrl {
/// Examples: /// Examples:
/// * `git+https://git.example.com/MyProject.git` /// * `git+https://git.example.com/MyProject.git`
/// * `git+https://git.example.com/MyProject.git@v1.0#egg=pkg&subdirectory=pkg_dir` /// * `git+https://git.example.com/MyProject.git@v1.0#egg=pkg&subdirectory=pkg_dir`
#[derive(Debug, Clone, Eq, PartialEq, Hash)] #[derive(Debug, Clone, Eq, PartialEq, PartialOrd, Hash, Ord)]
pub struct ParsedGitUrl { pub struct ParsedGitUrl {
pub url: GitUrl, pub url: GitUrl,
pub subdirectory: Option<PathBuf>, pub subdirectory: Option<PathBuf>,
@ -96,7 +96,7 @@ impl TryFrom<Url> for ParsedGitUrl {
/// * A built distribution: `https://files.pythonhosted.org/packages/62/06/d5604a70d160f6a6ca5fd2ba25597c24abd5c5ca5f437263d177ac242308/tqdm-4.66.1-py2.py3-none-any.whl` /// * A built distribution: `https://files.pythonhosted.org/packages/62/06/d5604a70d160f6a6ca5fd2ba25597c24abd5c5ca5f437263d177ac242308/tqdm-4.66.1-py2.py3-none-any.whl`
/// * A source distribution with a valid name: `https://files.pythonhosted.org/packages/62/06/d5604a70d160f6a6ca5fd2ba25597c24abd5c5ca5f437263d177ac242308/tqdm-4.66.1.tar.gz` /// * A source distribution with a valid name: `https://files.pythonhosted.org/packages/62/06/d5604a70d160f6a6ca5fd2ba25597c24abd5c5ca5f437263d177ac242308/tqdm-4.66.1.tar.gz`
/// * A source dist with a recognizable extension but invalid name: `https://github.com/foo-labs/foo/archive/master.zip#egg=pkg&subdirectory=packages/bar` /// * A source dist with a recognizable extension but invalid name: `https://github.com/foo-labs/foo/archive/master.zip#egg=pkg&subdirectory=packages/bar`
#[derive(Debug, Clone, Eq, PartialEq, Hash)] #[derive(Debug, Clone, Eq, PartialEq, Hash, PartialOrd, Ord)]
pub struct ParsedArchiveUrl { pub struct ParsedArchiveUrl {
pub url: Url, pub url: Url,
pub subdirectory: Option<PathBuf>, pub subdirectory: Option<PathBuf>,

View file

@ -14,7 +14,7 @@ use crate::Pep508Url;
/// A wrapper around [`Url`] that preserves the original string. /// A wrapper around [`Url`] that preserves the original string.
#[derive(Debug, Clone, Eq, derivative::Derivative, serde::Deserialize, serde::Serialize)] #[derive(Debug, Clone, Eq, derivative::Derivative, serde::Deserialize, serde::Serialize)]
#[derivative(PartialEq, Hash)] #[derivative(PartialEq, Hash, Ord)]
pub struct VerbatimUrl { pub struct VerbatimUrl {
/// The parsed URL. /// The parsed URL.
#[serde( #[serde(
@ -24,6 +24,7 @@ pub struct VerbatimUrl {
url: Url, url: Url,
/// The URL as it was provided by the user. /// The URL as it was provided by the user.
#[derivative(PartialEq = "ignore")] #[derivative(PartialEq = "ignore")]
#[derivative(Ord = "ignore")]
#[derivative(Hash = "ignore")] #[derivative(Hash = "ignore")]
given: Option<String>, given: Option<String>,
} }
@ -163,6 +164,16 @@ impl VerbatimUrl {
} }
} }
// This impl is written out because the `derive` doesn't seem to get it right.
// Or does it in a way where Clippy complains about non-canonical `PartialOrd`
// impls. So we just do it by hand by deferring to the derived `Ord` impl. This
// guarantees they are consistent.
impl PartialOrd for VerbatimUrl {
fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
Some(self.cmp(other))
}
}
impl std::str::FromStr for VerbatimUrl { impl std::str::FromStr for VerbatimUrl {
type Err = VerbatimUrlError; type Err = VerbatimUrlError;

View file

@ -23,7 +23,7 @@ use crate::FetchStrategy;
const CHECKOUT_READY_LOCK: &str = ".ok"; const CHECKOUT_READY_LOCK: &str = ".ok";
/// A reference to commit or commit-ish. /// A reference to commit or commit-ish.
#[derive(Debug, Clone, PartialEq, Eq, Hash)] #[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
pub enum GitReference { pub enum GitReference {
/// A specific branch. /// A specific branch.
Branch(String), Branch(String),

View file

@ -12,7 +12,7 @@ mod source;
mod util; mod util;
/// A URL reference to a Git repository. /// A URL reference to a Git repository.
#[derive(Debug, Clone, PartialEq, Eq, Hash)] #[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Hash, Ord)]
pub struct GitUrl { pub struct GitUrl {
/// The URL of the Git repository, with any query parameters, fragments, and leading `git+` /// The URL of the Git repository, with any query parameters, fragments, and leading `git+`
/// removed. /// removed.

View file

@ -10,7 +10,7 @@ use crate::resolver::Urls;
/// 2. Uses the same strategy as pip and posy to handle extras: for each extra, we create a virtual /// 2. Uses the same strategy as pip and posy to handle extras: for each extra, we create a virtual
/// package (e.g., `black[colorama]`), and mark it as a dependency of the real package (e.g., /// package (e.g., `black[colorama]`), and mark it as a dependency of the real package (e.g.,
/// `black`). We then discard the virtual packages at the end of the resolution process. /// `black`). We then discard the virtual packages at the end of the resolution process.
#[derive(Debug, Clone, Eq, Hash, PartialEq)] #[derive(Debug, Clone, Eq, Hash, PartialEq, PartialOrd, Ord)]
pub enum PubGrubPackage { pub enum PubGrubPackage {
/// The root package, which is used to start the resolution process. /// The root package, which is used to start the resolution process.
Root(Option<PackageName>), Root(Option<PackageName>),
@ -90,7 +90,7 @@ impl PubGrubPackage {
} }
} }
#[derive(Debug, Clone, Eq, PartialEq, Hash)] #[derive(Debug, Clone, Eq, PartialEq, PartialOrd, Hash, Ord)]
pub enum PubGrubPython { pub enum PubGrubPython {
/// The Python version installed in the current environment. /// The Python version installed in the current environment.
Installed, Installed,