Skip to content

Commit

Permalink
Fix non-registry serialization for receipts
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Jul 31, 2024
1 parent f8e2d2f commit 6bafba7
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 21 deletions.
70 changes: 57 additions & 13 deletions crates/pypi-types/src/requirement.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::fmt::{Display, Formatter};
use std::path::{Path, PathBuf};
use std::str::FromStr;

use thiserror::Error;
use url::Url;

Expand Down Expand Up @@ -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)]
Expand Down Expand Up @@ -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<String>,
},
/// Ex) `source = { git = "<https://github.com/astral-test/uv-public-pypackage?rev=0.0.1#0dacfd662c64cb4ceb16e6cf65a157a8b715b979>" }`
Git { git: String },
/// Ex) `source = { url = "<https://example.org/foo-1.0.zip>" }`
Expand All @@ -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<String>,
},
}

impl From<RequirementSource> for RequirementSourceWire {
Expand Down Expand Up @@ -594,7 +594,7 @@ impl TryFrom<RequirementSourceWire> for RequirementSource {
fn try_from(wire: RequirementSourceWire) -> Result<RequirementSource, RequirementError> {
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)?;
Expand All @@ -616,30 +616,30 @@ impl TryFrom<RequirementSourceWire> for RequirementSource {
url.set_query(None);
url.set_fragment(None);

Ok(RequirementSource::Git {
Ok(Self::Git {
repository: url.clone(),
reference,
precise,
subdirectory,
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,
})
}
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,
Expand All @@ -648,7 +648,7 @@ impl TryFrom<RequirementSourceWire> 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,
Expand All @@ -658,3 +658,47 @@ impl TryFrom<RequirementSourceWire> 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);
}
}
7 changes: 1 addition & 6 deletions crates/uv/src/commands/tool/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<Vec<_>>();
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() {
Expand Down
4 changes: 2 additions & 2 deletions crates/uv/tests/tool_install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!({
Expand All @@ -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" },
]
Expand Down

0 comments on commit 6bafba7

Please sign in to comment.