Remove support for serde - #212
Merged
Merged
Conversation
Cargo unifies features per package across the whole dependency graph, but a proc-macro crate emits its code into every crate that calls it. The `serde` feature therefore never stayed local to the crate that enabled it, and any crate sharing this instance received `serde::` paths whether or not it depended on serde. That is the defect that broke labelflair, where one crate enabling the feature stopped an unrelated dependency from compiling. A consumer who wants the derives now writes them at the call site, which the macros already forward, so nothing is lost. The remaining optional dependencies move to dev-dependencies. This crate never used them itself. They existed to gate generated code and to propagate flags such as `uuid?/serde`, which has not reached a consumer since the second version of the feature resolver, so consumers already had to enable those features themselves. The new recipe builds the test crate, which enables no features, against a fully featured build of this crate. Any future feature that changes generated code fails that check rather than somebody else's build.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cargo unifies features per package across the whole dependency graph, but a proc-macro crate emits its code into every crate that calls it. The
serdefeature therefore never stayed local to the crate that enabled it, and any crate sharing this instance receivedserde::paths whether or not it depended on serde.This is the defect behind jdno/labelflair#48.
labelflairenables the feature;clawlessuses this crate without it and has no serde dependency. While the two resolved to semver-incompatible versions Cargo kept them apart, but as soon as both reached 0.6.x they shared one instance andclawlessstopped compiling:I verified against a local checkout of that pull request, patched to this branch:
clawlesscompiles and the whole labelflair suite passes.What replaces it
The derives move to the call site, which the macros already forward:
Nothing is lost. This works for all seven macros, and
secret!takes#[derive(serde::Deserialize)]alone, as before.The optional dependencies
secrecy,ulid,urlanduuidmove to dev-dependencies, and the four remaining features become plain markers that only decide which macros this crate exports. Those dependencies were never used by the macros. They existed to gate generated code and to propagate flags such asuuid?/serde, which has not reached a consumer since the second version of the feature resolver, so consumers already had to enableuuid/serdethemselves. The tests declare them the way any consumer does.This crate now depends on
proc-macro2,quoteandsyn, and on nothing else.The guard
just check-feature-unificationrunscargo check --workspace --all-features, which buildstests/krate(which enables no features) against a fully featured build of this crate. It fails onmaintoday, with the error above, and passes here. CI builds its matrix fromjust --list, so this becomes a job automatically.Notes for review
#[cfg(feature = "uuid")] use ...lines as test blocks and sorted them among the tests, leaving the imports and the type declaration below the tests insecret.rs,ulid.rs,url.rsanduuid.rs. It compiled, so the checks stayed green. Declarations are back at the top. Happy to split this into its own pull request if you would rather review it separately.just check-featurespasses.just pre-commit,just check-featuresandjust check-feature-unificationall pass. I could not runjust check-unused-depslocally becausecargo-udepsis not installed here, so that one is left to CI.