require serde and rkyv everywhere; remove optional serde and rkyv features (#3345)

In *some* places in our crates, `serde` (and `rkyv`) are optional
dependencies. I believe this was done out of reasons of "good sense,"
that is, it follows a Rust ecosystem pattern where serde integration
tends to be an opt-in crate feature. (And similarly for `rkyv`.)

However, ultimately, `uv` itself requires `serde` and `rkyv` to
function. Since our crates are strictly internal, there are limited
consumers for our crates without `serde` (and `rkyv`) enabled. I think
one possibility is that optional `serde` (and `rkyv`) integration means
that someone can do this:

    cargo test -p pep440_rs

And this will run tests _without_ `serde` or `rkyv` enabled. That in
turn could lead to faster iteration time by reducing compile times. But,
I'm not sure this is worth supporting. The iterative compilation times
of
individual crates are probably fast enough in debug mode, even with
`serde` and `rkyv` enabled. Namely, `serde` and `rkyv` themselves
shouldn't need to be re-compiled in most cases. On `main`:

```
from-scratch: `cargo test -p pep440_rs --lib` 0.685
incremental: `cargo test -p pep440_rs --lib` 0.278s
from-scratch: `cargo test -p pep440_rs --features serde,rkyv --lib` 3.948s
incremental: `cargo test -p pep440_rs --features serde,rkyv --lib` 0.321s
```

So while a from-scratch build does take significantly longer, an
incremental build is about the same.

The benefit of doing this change is two-fold:

1. It brings out crates into alignment with "reality." In particular,
   some crates were _implicitly_ relying on `serde` being enabled
   without explicitly declaring it. This technically means that our
   `Cargo.toml`s were wrong in some cases, but it is hard to observe it
   because of feature unification in a Cargo workspace.
2. We no longer need to deal with the cognitive burden of writing
   `#[cfg_attr(feature = "serde", ...)]` everywhere.
This commit is contained in:
Andrew Gallant 2024-05-03 10:21:03 -04:00 committed by GitHub
parent 714adebbd7
commit 1089abda3f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
35 changed files with 170 additions and 277 deletions

View file

@ -1,13 +1,9 @@
use uv_auth::{self, KeyringProvider};
/// Keyring provider type to use for credential lookup.
#[derive(Debug, Default, Clone, Copy, PartialEq, Eq)]
#[derive(Debug, Default, Clone, Copy, PartialEq, Eq, serde::Deserialize)]
#[serde(deny_unknown_fields, rename_all = "kebab-case")]
#[cfg_attr(feature = "clap", derive(clap::ValueEnum))]
#[cfg_attr(feature = "serde", derive(serde::Deserialize))]
#[cfg_attr(
feature = "serde",
serde(deny_unknown_fields, rename_all = "kebab-case")
)]
#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
pub enum KeyringProviderType {
/// Do not use keyring for credential lookup.

View file

@ -196,13 +196,9 @@ impl NoBuild {
}
}
#[derive(Debug, Default, Clone, Copy, Hash, Eq, PartialEq)]
#[derive(Debug, Default, Clone, Copy, Hash, Eq, PartialEq, serde::Deserialize)]
#[serde(deny_unknown_fields, rename_all = "kebab-case")]
#[cfg_attr(feature = "clap", derive(clap::ValueEnum))]
#[cfg_attr(feature = "serde", derive(serde::Deserialize))]
#[cfg_attr(
feature = "serde",
serde(deny_unknown_fields, rename_all = "kebab-case")
)]
#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
pub enum IndexStrategy {
/// Only use results from the first index that returns a match for a given package name.

View file

@ -36,7 +36,6 @@ enum ConfigSettingValue {
List(Vec<String>),
}
#[cfg(feature = "serde")]
impl serde::Serialize for ConfigSettingValue {
fn serialize<S: serde::Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
match self {
@ -46,7 +45,6 @@ impl serde::Serialize for ConfigSettingValue {
}
}
#[cfg(feature = "serde")]
impl<'de> serde::Deserialize<'de> for ConfigSettingValue {
fn deserialize<D: serde::Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
struct Visitor;
@ -84,7 +82,6 @@ impl<'de> serde::Deserialize<'de> for ConfigSettingValue {
/// See: <https://peps.python.org/pep-0517/#config-settings>
#[derive(Debug, Default, Clone)]
#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
#[cfg_attr(not(feature = "serde"), allow(dead_code))]
pub struct ConfigSettings(BTreeMap<String, ConfigSettingValue>);
impl FromIterator<ConfigSettingEntry> for ConfigSettings {
@ -110,7 +107,6 @@ impl FromIterator<ConfigSettingEntry> for ConfigSettings {
}
}
#[cfg(feature = "serde")]
impl ConfigSettings {
/// Convert the settings to a string that can be passed directly to a PEP 517 build backend.
pub fn escape_for_python(&self) -> String {
@ -118,7 +114,6 @@ impl ConfigSettings {
}
}
#[cfg(feature = "serde")]
impl serde::Serialize for ConfigSettings {
fn serialize<S: serde::Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
use serde::ser::SerializeMap;
@ -131,7 +126,6 @@ impl serde::Serialize for ConfigSettings {
}
}
#[cfg(feature = "serde")]
impl<'de> serde::Deserialize<'de> for ConfigSettings {
fn deserialize<D: serde::Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
struct Visitor;
@ -202,7 +196,6 @@ mod tests {
}
#[test]
#[cfg(feature = "serde")]
fn escape_for_python() {
let mut settings = ConfigSettings::default();
settings.0.insert(

View file

@ -21,7 +21,6 @@ impl FromStr for PackageNameSpecifier {
}
}
#[cfg(feature = "serde")]
impl<'de> serde::Deserialize<'de> for PackageNameSpecifier {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where

View file

@ -5,13 +5,9 @@ use platform_tags::{Arch, Os, Platform};
/// system.
///
/// See: <https://doc.rust-lang.org/nightly/rustc/platform-support.html>
#[derive(Debug, Clone, Copy, Eq, PartialEq)]
#[derive(Debug, Clone, Copy, Eq, PartialEq, serde::Deserialize)]
#[serde(deny_unknown_fields, rename_all = "kebab-case")]
#[cfg_attr(feature = "clap", derive(clap::ValueEnum))]
#[cfg_attr(feature = "serde", derive(serde::Deserialize))]
#[cfg_attr(
feature = "serde",
serde(deny_unknown_fields, rename_all = "kebab-case")
)]
#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
pub enum TargetTriple {
/// An alias for `x86_64-pc-windows-msvc`, the default target for Windows.