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

Forced serde_derive dependency #5

Closed
jean-airoldie opened this issue Jul 19, 2019 · 5 comments
Closed

Forced serde_derive dependency #5

jean-airoldie opened this issue Jul 19, 2019 · 5 comments

Comments

@jean-airoldie
Copy link

jean-airoldie commented Jul 19, 2019

Using this crate alongside a serde = { version = "1.0", features = ['derive'] } dependency causes behavior like in #2. This is due to this line that assumes that serde_derive is present. However it cannot be replaced by serde::Serialize since this causes serde-rs/serde#1467 caused by rust-lang/rust#55779.

A sketchy work around is to define a

mod serde_derive {
    pub use serde::{Serialize, Deserialize};
}

in the same module where Serialize_tuple is used.

I think that once rust-lang/rust#55779 is fixed, this crate should depend on serde = { version = "1.0", features = ['derive'] } since, from what I understand, feature flags forces the dependency down the chain, while simply depending on serde_derive (like this crate does currently) does not.

@kardeiz
Copy link
Owner

kardeiz commented Jul 20, 2019

Thanks for the issue, and I think that is a good solution. I'll keep this open until the SO issue is resolved and I update this crate.

Another workaround (definitely not ideal) for your setup would be to copy the serde_derive dependency from Cargo.lock to Cargo.toml. If you make sure to use the same version that serde depends on, nothing else will need to be installed; it just makes serde_derive available to user code.

@jean-airoldie
Copy link
Author

A workaround would be to add a crate helper attribute to the proc macro so that the user can control the name of the dependency crate.

#[derive(Serialize_tuple)]
#[serde_tuple(crate = "serde")]
struct SomeStruct;

@kardeiz
Copy link
Owner

kardeiz commented Jan 7, 2020

@jean-airoldie I think that is a good idea, but I think the name crate could be confusing: I think it would need to be something like crate_that_provides_derive_serialize_and_deserialize, or serde_derive_crate (though I think the latter might also be confusing).

Actually, it looks like your initial suggestion (depend on serde with the derive feature), already works. Could you see if the change I just pushed to master works for you?

Looking back, I'm not sure why this crate was depending on serde and serde_derive, since it doesn't use them directly, but depending on serde with the derive feature does have the desirable feature of not requiring serde_derive in the user's dependencies.

@jean-airoldie
Copy link
Author

The master branch works for me currently. Should we close this?

@kardeiz
Copy link
Owner

kardeiz commented Jan 7, 2020

@jean-airoldie Great! I just went ahead and updated the syn/quote/proc-macro2 dependencies to their 1.x versions and released 0.4.0.

I'll close this now, but let me know if anything doesn't work,

@kardeiz kardeiz closed this as completed Jan 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants