Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix non-registry serialization for receipts #5668

Merged
merged 1 commit into from
Jul 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 62 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,52 @@ 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 path = if cfg!(windows) {
"C:\\home\\ferris\\foo"
} else {
"/home/ferris/foo"
};
let requirement = Requirement {
name: "foo".parse().unwrap(),
extras: vec![],
marker: None,
source: RequirementSource::Directory {
install_path: PathBuf::from(path),
lock_path: PathBuf::from(path),
editable: false,
url: VerbatimUrl::from_path(Path::new(path)).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
Loading