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

Remove serde::Serialize implementations for rkyv-able structs #7663

Merged
merged 1 commit into from
Sep 24, 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
8 changes: 2 additions & 6 deletions crates/distribution-types/src/file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,7 @@ pub enum FileConversionError {
}

/// Internal analog to [`pypi_types::File`].
#[derive(
Debug, Clone, Hash, Serialize, Deserialize, rkyv::Archive, rkyv::Deserialize, rkyv::Serialize,
)]
#[derive(Debug, Clone, Hash, rkyv::Archive, rkyv::Deserialize, rkyv::Serialize)]
#[rkyv(derive(Debug))]
pub struct File {
pub dist_info_metadata: bool,
Expand Down Expand Up @@ -68,9 +66,7 @@ impl File {
}

/// While a registry file is generally a remote URL, it can also be a file if it comes from a directory flat indexes.
#[derive(
Debug, Clone, Hash, Serialize, Deserialize, rkyv::Archive, rkyv::Deserialize, rkyv::Serialize,
)]
#[derive(Debug, Clone, Hash, rkyv::Archive, rkyv::Deserialize, rkyv::Serialize)]
#[rkyv(derive(Debug))]
pub enum FileLocation {
/// URL relative to the base URL.
Expand Down
1 change: 0 additions & 1 deletion crates/pypi-types/src/simple_json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ impl CoreMetadata {
PartialEq,
Eq,
Hash,
Serialize,
Deserialize,
rkyv::Archive,
rkyv::Deserialize,
Expand Down
2 changes: 1 addition & 1 deletion crates/uv-cache/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -765,7 +765,7 @@ impl CacheBucket {
fn to_str(self) -> &'static str {
match self {
Self::SourceDistributions => "sdists-v4",
Self::FlatIndex => "flat-index-v0",
Self::FlatIndex => "flat-index-v1",
Self::Git => "git-v0",
Self::Interpreter => "interpreter-v2",
// Note that when bumping this, you'll also need to bump it
Expand Down
14 changes: 9 additions & 5 deletions crates/uv-client/src/flat_index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use uv_cache::{Cache, CacheBucket};

use crate::cached_client::{CacheControl, CachedClientError};
use crate::html::SimpleHtml;
use crate::{Connectivity, Error, ErrorKind, RegistryClient};
use crate::{Connectivity, Error, ErrorKind, OwnedArchive, RegistryClient};

#[derive(Debug, thiserror::Error)]
pub enum FlatIndexError {
Expand Down Expand Up @@ -170,7 +170,7 @@ impl<'a> FlatIndexClient<'a> {
let SimpleHtml { base, files } = SimpleHtml::parse(&text, &url)
.map_err(|err| Error::from_html_err(err, url.clone()))?;

let files: Vec<File> = files
let unarchived: Vec<File> = files
.into_iter()
.filter_map(|file| {
match File::try_from(file, base.as_url()) {
Expand All @@ -183,15 +183,15 @@ impl<'a> FlatIndexClient<'a> {
}
})
.collect();
Ok::<Vec<File>, CachedClientError<Error>>(files)
OwnedArchive::from_unarchived(&unarchived)
}
.boxed_local()
.instrument(info_span!("parse_flat_index_html", url = % url))
};
let response = self
.client
.cached_client()
.get_serde(
.get_cacheable(
flat_index_request,
&cache_entry,
cache_control,
Expand All @@ -201,7 +201,11 @@ impl<'a> FlatIndexClient<'a> {
match response {
Ok(files) => {
let files = files
.into_iter()
.iter()
.map(|file| {
rkyv::deserialize::<File, rkyv::rancor::Error>(file)
.expect("archived version always deserializes")
})
.filter_map(|file| {
Some((
DistFilename::try_from_normalized_filename(&file.filename)?,
Expand Down
15 changes: 5 additions & 10 deletions crates/uv-client/src/registry_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ use futures::{FutureExt, TryStreamExt};
use http::HeaderMap;
use reqwest::{Client, Response, StatusCode};
use reqwest_middleware::ClientWithMiddleware;
use serde::{Deserialize, Serialize};
use tracing::{info_span, instrument, trace, warn, Instrument};
use url::Url;

Expand Down Expand Up @@ -722,9 +721,7 @@ impl RegistryClient {
}
}

#[derive(
Default, Debug, Serialize, Deserialize, rkyv::Archive, rkyv::Deserialize, rkyv::Serialize,
)]
#[derive(Default, Debug, rkyv::Archive, rkyv::Deserialize, rkyv::Serialize)]
#[rkyv(derive(Debug))]
pub struct VersionFiles {
pub wheels: Vec<VersionWheel>,
Expand Down Expand Up @@ -755,27 +752,25 @@ impl VersionFiles {
}
}

#[derive(Debug, Serialize, Deserialize, rkyv::Archive, rkyv::Deserialize, rkyv::Serialize)]
#[derive(Debug, rkyv::Archive, rkyv::Deserialize, rkyv::Serialize)]
#[rkyv(derive(Debug))]
pub struct VersionWheel {
pub name: WheelFilename,
pub file: File,
}

#[derive(Debug, Serialize, Deserialize, rkyv::Archive, rkyv::Deserialize, rkyv::Serialize)]
#[derive(Debug, rkyv::Archive, rkyv::Deserialize, rkyv::Serialize)]
#[rkyv(derive(Debug))]
pub struct VersionSourceDist {
pub name: SourceDistFilename,
pub file: File,
}

#[derive(
Default, Debug, Serialize, Deserialize, rkyv::Archive, rkyv::Deserialize, rkyv::Serialize,
)]
#[derive(Default, Debug, rkyv::Archive, rkyv::Deserialize, rkyv::Serialize)]
#[rkyv(derive(Debug))]
pub struct SimpleMetadata(Vec<SimpleMetadatum>);

#[derive(Debug, Serialize, Deserialize, rkyv::Archive, rkyv::Deserialize, rkyv::Serialize)]
#[derive(Debug, rkyv::Archive, rkyv::Deserialize, rkyv::Serialize)]
#[rkyv(derive(Debug))]
pub struct SimpleMetadatum {
pub version: Version,
Expand Down
4 changes: 2 additions & 2 deletions crates/uv-normalize/src/group_name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::fmt::{Display, Formatter};
use std::str::FromStr;
use std::sync::LazyLock;

use serde::{Deserialize, Deserializer, Serialize};
use serde::{Deserialize, Deserializer};

use crate::{validate_and_normalize_owned, validate_and_normalize_ref, InvalidNameError};

Expand All @@ -12,7 +12,7 @@ use crate::{validate_and_normalize_owned, validate_and_normalize_ref, InvalidNam
/// See:
/// - <https://peps.python.org/pep-0735/>
/// - <https://packaging.python.org/en/latest/specifications/name-normalization/>
#[derive(Debug, Default, Clone, PartialEq, Eq, Hash, PartialOrd, Ord, Serialize)]
#[derive(Debug, Default, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
pub struct GroupName(String);

Expand Down
19 changes: 12 additions & 7 deletions crates/uv-workspace/src/pyproject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ pub enum PyprojectTomlError {
}

/// A `pyproject.toml` as specified in PEP 517.
#[derive(Serialize, Deserialize, Debug, Clone)]
#[derive(Deserialize, Debug, Clone)]
#[cfg_attr(test, derive(Serialize))]
#[serde(rename_all = "kebab-case")]
pub struct PyProjectToml {
/// PEP 621-compliant project metadata.
Expand Down Expand Up @@ -111,7 +112,8 @@ impl AsRef<[u8]> for PyProjectToml {
/// PEP 621 project metadata (`project`).
///
/// See <https://packaging.python.org/en/latest/specifications/pyproject-toml>.
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)]
#[derive(Deserialize, Debug, Clone, PartialEq)]
#[cfg_attr(test, derive(Serialize))]
#[serde(rename_all = "kebab-case")]
pub struct Project {
/// The name of the project
Expand All @@ -133,15 +135,17 @@ pub struct Project {
pub(crate) scripts: Option<serde::de::IgnoredAny>,
}

#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq)]
#[derive(Deserialize, Debug, Clone, PartialEq, Eq)]
#[cfg_attr(test, derive(Serialize))]
#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
pub struct Tool {
pub uv: Option<ToolUv>,
}

// NOTE(charlie): When adding fields to this struct, mark them as ignored on `Options` in
// `crates/uv-settings/src/settings.rs`.
#[derive(Serialize, Deserialize, OptionsMetadata, Debug, Clone, PartialEq, Eq)]
#[derive(Deserialize, OptionsMetadata, Debug, Clone, PartialEq, Eq)]
#[cfg_attr(test, derive(Serialize))]
#[serde(rename_all = "kebab-case")]
#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
pub struct ToolUv {
Expand Down Expand Up @@ -287,9 +291,9 @@ pub struct ToolUv {
pub constraint_dependencies: Option<Vec<pep508_rs::Requirement<VerbatimParsedUrl>>>,
}

#[derive(Serialize, Default, Debug, Clone, PartialEq, Eq)]
#[derive(Default, Debug, Clone, PartialEq, Eq)]
#[cfg_attr(test, derive(Serialize))]
#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
#[serde(rename_all = "kebab-case", deny_unknown_fields)]
pub struct ToolUvSources(BTreeMap<PackageName, Source>);

impl ToolUvSources {
Expand Down Expand Up @@ -346,7 +350,8 @@ impl<'de> serde::de::Deserialize<'de> for ToolUvSources {
}
}

#[derive(Serialize, Deserialize, OptionsMetadata, Default, Debug, Clone, PartialEq, Eq)]
#[derive(Deserialize, OptionsMetadata, Default, Debug, Clone, PartialEq, Eq)]
#[cfg_attr(test, derive(Serialize))]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sigh, these need Serialize for testing. Wonder if we should just use the debug repr though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Debug repr for tests seems fine to me.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's kind of bad because we have glob patterns and the debug repr is very verbose.

#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
#[serde(rename_all = "kebab-case", deny_unknown_fields)]
pub struct ToolUvWorkspace {
Expand Down
Loading