From 6bafba7cb6c9d0b7119f1726d0d37841f3dc8fe7 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 31 Jul 2024 15:26:56 -0400 Subject: [PATCH] Fix non-registry serialization for receipts --- crates/pypi-types/src/requirement.rs | 70 +++++++++++++++++++++----- crates/uv/src/commands/tool/install.rs | 7 +-- crates/uv/tests/tool_install.rs | 4 +- 3 files changed, 60 insertions(+), 21 deletions(-) diff --git a/crates/pypi-types/src/requirement.rs b/crates/pypi-types/src/requirement.rs index 41546d1987ca..a37dda7e0a65 100644 --- a/crates/pypi-types/src/requirement.rs +++ b/crates/pypi-types/src/requirement.rs @@ -1,6 +1,7 @@ use std::fmt::{Display, Formatter}; use std::path::{Path, PathBuf}; use std::str::FromStr; + use thiserror::Error; use url::Url; @@ -29,7 +30,6 @@ pub enum RequirementError { /// The main change is using [`RequirementSource`] to represent all supported package sources over /// [`VersionOrUrl`], which collapses all URL sources into a single stringly type. #[derive(Hash, Debug, Clone, Eq, PartialEq, serde::Serialize, serde::Deserialize)] -#[serde(rename_all = "kebab-case")] pub struct Requirement { pub name: PackageName, #[serde(skip_serializing_if = "Vec::is_empty", default)] @@ -476,12 +476,6 @@ impl Display for RequirementSource { #[derive(Clone, Debug, serde::Serialize, serde::Deserialize)] #[serde(untagged)] enum RequirementSourceWire { - /// Ex) `source = { specifier = "foo >1,<2" }` - Registry { - #[serde(skip_serializing_if = "VersionSpecifiers::is_empty", default)] - specifier: VersionSpecifiers, - index: Option, - }, /// Ex) `source = { git = "" }` Git { git: String }, /// Ex) `source = { url = "" }` @@ -495,6 +489,12 @@ enum RequirementSourceWire { Directory { directory: PortablePathBuf }, /// Ex) `source = { editable = "/home/ferris/iniconfig" }` Editable { editable: PortablePathBuf }, + /// Ex) `source = { specifier = "foo >1,<2" }` + Registry { + #[serde(skip_serializing_if = "VersionSpecifiers::is_empty", default)] + specifier: VersionSpecifiers, + index: Option, + }, } impl From for RequirementSourceWire { @@ -594,7 +594,7 @@ impl TryFrom for RequirementSource { fn try_from(wire: RequirementSourceWire) -> Result { match wire { RequirementSourceWire::Registry { specifier, index } => { - Ok(RequirementSource::Registry { specifier, index }) + Ok(Self::Registry { specifier, index }) } RequirementSourceWire::Git { git } => { let mut url = Url::parse(&git)?; @@ -616,7 +616,7 @@ impl TryFrom for RequirementSource { url.set_query(None); url.set_fragment(None); - Ok(RequirementSource::Git { + Ok(Self::Git { repository: url.clone(), reference, precise, @@ -624,14 +624,14 @@ impl TryFrom for RequirementSource { url: VerbatimUrl::from_url(url), }) } - RequirementSourceWire::Direct { url, subdirectory } => Ok(RequirementSource::Url { + RequirementSourceWire::Direct { url, subdirectory } => Ok(Self::Url { url: VerbatimUrl::from_url(url.clone()), subdirectory: subdirectory.map(PathBuf::from), location: url.clone(), }), RequirementSourceWire::Path { path } => { let path = PathBuf::from(path); - Ok(RequirementSource::Path { + Ok(Self::Path { url: VerbatimUrl::from_path(path.as_path())?, install_path: path.clone(), lock_path: path, @@ -639,7 +639,7 @@ impl TryFrom for RequirementSource { } RequirementSourceWire::Directory { directory } => { let directory = PathBuf::from(directory); - Ok(RequirementSource::Directory { + Ok(Self::Directory { url: VerbatimUrl::from_path(directory.as_path())?, install_path: directory.clone(), lock_path: directory, @@ -648,7 +648,7 @@ impl TryFrom for RequirementSource { } RequirementSourceWire::Editable { editable } => { let editable = PathBuf::from(editable); - Ok(RequirementSource::Directory { + Ok(Self::Directory { url: VerbatimUrl::from_path(editable.as_path())?, install_path: editable.clone(), lock_path: editable, @@ -658,3 +658,47 @@ impl TryFrom for RequirementSource { } } } + +#[cfg(test)] +mod tests { + use std::path::{Path, PathBuf}; + + use pep508_rs::VerbatimUrl; + + use crate::{Requirement, RequirementSource}; + + #[test] + fn roundtrip() { + let requirement = Requirement { + name: "foo".parse().unwrap(), + extras: vec![], + marker: None, + source: RequirementSource::Registry { + specifier: ">1,<2".parse().unwrap(), + index: None, + }, + origin: None, + }; + + let raw = toml::to_string(&requirement).unwrap(); + let deserialized: Requirement = toml::from_str(&raw).unwrap(); + assert_eq!(requirement, deserialized); + + let requirement = Requirement { + name: "foo".parse().unwrap(), + extras: vec![], + marker: None, + source: RequirementSource::Directory { + install_path: PathBuf::from("/home/ferris/foo"), + lock_path: PathBuf::from("/home/ferris/foo"), + editable: false, + url: VerbatimUrl::from_url("file:///home/ferris/foo".parse().unwrap()), + }, + origin: None, + }; + + let raw = toml::to_string(&requirement).unwrap(); + let deserialized: Requirement = toml::from_str(&raw).unwrap(); + assert_eq!(requirement, deserialized); + } +} diff --git a/crates/uv/src/commands/tool/install.rs b/crates/uv/src/commands/tool/install.rs index 0f9f4e521e5e..3dd0d8815e6f 100644 --- a/crates/uv/src/commands/tool/install.rs +++ b/crates/uv/src/commands/tool/install.rs @@ -240,12 +240,7 @@ pub(crate) async fn install( // If the requested and receipt requirements are the same... if existing_environment.is_some() { if let Some(tool_receipt) = existing_tool_receipt.as_ref() { - let receipt = tool_receipt - .requirements() - .iter() - .cloned() - .map(Requirement::from) - .collect::>(); + let receipt = tool_receipt.requirements().to_vec(); if requirements == receipt { // And the user didn't request a reinstall or upgrade... if !force && settings.reinstall.is_none() && settings.upgrade.is_none() { diff --git a/crates/uv/tests/tool_install.rs b/crates/uv/tests/tool_install.rs index 3ca3259c901a..4d70b9fb5032 100644 --- a/crates/uv/tests/tool_install.rs +++ b/crates/uv/tests/tool_install.rs @@ -408,7 +408,7 @@ fn tool_install_editable() { ----- stderr ----- warning: `uv tool install` is experimental and may change without warning - `black` is already installed + Installed 1 executable: black "###); insta::with_settings!({ @@ -417,7 +417,7 @@ fn tool_install_editable() { // We should have a tool receipt assert_snapshot!(fs_err::read_to_string(tool_dir.join("black").join("uv-receipt.toml")).unwrap(), @r###" [tool] - requirements = [{ name = "black", editable = "[WORKSPACE]/scripts/packages/black_editable" }] + requirements = [{ name = "black" }] entrypoints = [ { name = "black", install-path = "[TEMP_DIR]/bin/black" }, ]