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

serde 1.0.186 always depends on serde_derive, even if the derive feature is not used #2601

Closed
striezel opened this issue Aug 25, 2023 · 5 comments

Comments

@striezel
Copy link
Contributor

When updating serde from v1.0.185 to v1.0.186 in my project, cargo also pulls in serde_derive as new dependency, even though the derive feature is not in use. (See striezel/corona#155.) This should not happen.

I suppose the cause for this is #2588.

@striezel striezel changed the title serde always depends on serde_derive, even if the derive feature is not used serde 1.0.186 always depends on serde_derive, even if the derive feature is not used Aug 25, 2023
@oli-obk
Copy link
Member

oli-obk commented Aug 25, 2023

Is it actually built? I think it only shows up in Cargo.lock and doesn't actually get built, right?

Even so, we could move the lockstep logic to serde_derive's Cargo.toml

@striezel
Copy link
Contributor Author

Is it actually built? I think it only shows up in Cargo.lock and doesn't actually get built, right?

I don't know. I haven't merged and built those changes yet.

@striezel
Copy link
Contributor Author

OK, here are some files for a minimal example:

Cargo.lock

# This file is automatically @generated by Cargo.
# It is not intended for manual editing.
version = 3

[[package]]
name = "serde"
version = "1.0.185"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "be9b6f69f1dfd54c3b568ffa45c310d6973a5e5148fd40cf515acaf38cf5bc31"

[[package]]
name = "serde_example"
version = "0.1.0"
dependencies = [
 "serde",
]

Cargo.toml

[package]
name = "serde_example"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
serde = "1.0.185"

src/main.rs

fn main() {
    println!("Hello, serde!");
}

When I run a cargo update on this small example, the output is:

    Updating crates.io index
      Adding proc-macro2 v1.0.66
      Adding quote v1.0.33
    Updating serde v1.0.185 -> v1.0.186
      Adding serde_derive v1.0.186
      Adding syn v2.0.29
      Adding unicode-ident v1.0.11

... and the Cargo.lock grows from the previous 16 lines to 65 lines:

# This file is automatically @generated by Cargo.
# It is not intended for manual editing.
version = 3

[[package]]
name = "proc-macro2"
version = "1.0.66"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "18fb31db3f9bddb2ea821cde30a9f70117e3f119938b5ee630b7403aa6e2ead9"
dependencies = [
 "unicode-ident",
]

[[package]]
name = "quote"
version = "1.0.33"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "5267fca4496028628a95160fc423a33e8b2e6af8a5302579e322e4b520293cae"
dependencies = [
 "proc-macro2",
]

[[package]]
name = "serde"
version = "1.0.186"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "9f5db24220c009de9bd45e69fb2938f4b6d2df856aa9304ce377b3180f83b7c1"
dependencies = [
 "serde_derive",
]

[[package]]
name = "serde_derive"
version = "1.0.186"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "5ad697f7e0b65af4983a4ce8f56ed5b357e8d3c36651bf6a7e13639c17b8e670"
dependencies = [
 "proc-macro2",
 "quote",
 "syn",
]

[[package]]
name = "serde_example"
version = "0.1.0"
dependencies = [
 "serde",
]

[[package]]
name = "syn"
version = "2.0.29"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "c324c494eba9d92503e6f1ef2e6df781e78f6a7705a0202d9801b198807d518a"
dependencies = [
 "proc-macro2",
 "quote",
 "unicode-ident",
]

[[package]]
name = "unicode-ident"
version = "1.0.11"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "301abaae475aa91687eb82514b328ab47a211a533026cb25fc3e519b86adfc3c"

All of those newly added packages are direct or indirect dependencies of serde_derive.

However, you seem to be right, @oli-obk. None of those additional packages are built when running cargo build. Only serde and the example program are built. Looks like those dependencies only get added to the lockfile and aren't actually built. I guess that's acceptable then, despite being a little bit annoying.

@dtolnay
Copy link
Member

dtolnay commented Aug 25, 2023

we could move the lockstep logic to serde_derive's Cargo.toml

I would have loved to do this. As far as I know, it is not possible, because the dependency in that direction would be circular. Serde_derive depends on serde (on "some" platforms) and serde depends on serde_derive (in some feature combinations) which is enough for Cargo to reject, despite "some" platforms not actually being any platforms.

error: cyclic package dependency: package `serde v1.0.186` depends on itself. Cycle:
package `serde v1.0.186`
    ... which satisfies path dependency `serde` (locked to 1.0.186) of package `serde_derive v1.0.186`
    ... which satisfies path dependency `serde_derive` (locked to 1.0.186) of package `serde v1.0.186`

I think circular optional dependencies are allowed though. So this would only work if serde_derive can know whether serde's "derive" feature is enabled, and only enable its lockstep dependency on serde if serde's "derive" feature is not enabled. As far as I know, Cargo cannot express that. Serde's "derive" feature can turn on a feature in serde_derive to notify it of serde's "derive" feature being on, but serde_derive cannot turn off a dependency in response to that feature, nor can serde turn off a serde_derive feature in response to "derive" being on.

@dtolnay
Copy link
Member

dtolnay commented Aug 25, 2023

Overall I think this is working as intended as currently implemented.

macro-dependencies (rust-lang/rust#54363 (comment)) will allow the lockstep to be expressed in a tidier way (without adding to lockfile) when that becomes available. Serde_derive would then have [macro-dependencies] serde = { version = "=1.0.V", default-features = false }.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants