Failure to build without also explicitly adding serde as dependency

[Borketh]

Every invocation of bit_struct! and enums! fails because of this:

error[E0463]: can't find crate for `serde`
  --> src/leaf6.rs:6:1
   |
6  | / bit_struct! {
7  | |
8  | |     pub struct ThermalFeatures(u32) {
9  | |         DTS: bool,
...  |
27 | |     }
28 | | }
   | |_^ can't find crate
   |
   = note: this error originates in the derive macro `$crate::serde::Deserialize` which comes from the expansion of the macro `bit_struct` (in Nightly builds, run with -Z macro-backtrace for more info)

[andrewgazelka]

I think this is an issue with serde—not our code. Do you have any suggestions?

[Borketh]

I don't think so - I can compile when I add serde as a dependency to my own code, but when I remove it (because I don't use serde in any way) it does not compile. Can you make the serde compatibility an optional feature in bit-struct?

[JarredAllen]

It looks to me like some serde trait implementations are manually implemented, so we could also switch to manually implementing the rest of the traits (replacing the derive macros) to resolve this issue. IMO we should also make it an optional feature like @AstroFloof requested, so crates won't have to compile serde as an unused transitive dependency, but this would be necessary to avoid crates which want the serde derives to have to import it in their Cargo.toml (and would also get around this compile error).

In general, there is afaik no way to write derive macros generally so they work in crates that don't depend upon the crate providing the macro, which is why we're stuck in this predicament.

[Borketh]

Isn't there a way to specify optional dependencies based on features? That would mean this package can be compiled with the serde feature enabled (automatically compiling serde) or disabled (not compiling serde).

[JarredAllen]

The problem is that there's no way to unify the serde feature in this crate with whether or not the crate depending on this crate explicitly adds serde as a dependency. I don't think the difference matters to you, since you seem to not want serde at all, but someone who makes a library containing just bitstructs and enums that wants to export serde implementations of those, would hit the same error as you're getting right now unless they also add serde as an explicit dependency (which imo isn't expected, since their code doesn't reference serde explicitly).

[JarredAllen]

@AstroFloof I believe this recently-merged PR should resolve your issue, please let me know if it doesn't

[Borketh]

@JarredAllen @andrewgazelka No, it doesn't, and I wish you had consulted me before merging and releasing that PR. I could have tested by setting my dependency to point to the forked repo before this happened. As it is, serde still needs to be specified as a dependency on my end. Additionally, you do not define the feature serde anywhere in the Cargo.toml, so please refer to the documentation about features.

[Borketh]

I will attempt to PR the fix.

[JarredAllen]

Additionally, you do not define the feature serde anywhere in the Cargo.toml, so please refer to the documentation about features.

I'm unclear what you're seeing that makes you think there is no serde feature. By default, Rust creates an implicit feature with the name of the optional dependency, which is what my PR relies on. Also, docs.rs lists a serde features. Can you elaborate on what you're seeing that makes you conclude that there is no serde feature?

[Borketh]

Hmm, it appears the egg is on my face. I was unaware of the implicit feature creation, and when I went to test with 0.3.2 my code threw the same errors as before. I made my change locally and it stopped throwing those errors, so I assumed that was the issue. Now it seems that 0.3.2 did fix the issue and there was some cached-compile problem or something similar that was causing the error still to occur initially. Apologies for the extraneous commentary and PR.

However, I should point out that closing an issue before checking with its author if the issue is resolved is somewhat problematic.

[andrewgazelka]

Hmm, it appears the egg is on my face. I was unaware of the implicit feature creation, and when I went to test with 0.3.2 my code threw the same errors as before. I made my change locally and it stopped throwing those errors, so I assumed that was the issue. Now it seems that 0.3.2 did fix the issue and there was some cached-compile problem or something similar that was causing the error still to occur initially. Apologies for the extraneous commentary and PR.

However, I should point out that closing an issue before checking with its author if the issue is resolved is somewhat problematic.

Probably could have handled it better. Kind of easy to do with GitHub auto closing of issues that are marked as closed in a PR.

[JarredAllen]

Hmm, it appears the egg is on my face. I was unaware of the implicit feature creation, and when I went to test with 0.3.2 my code threw the same errors as before. I made my change locally and it stopped throwing those errors, so I assumed that was the issue. Now it seems that 0.3.2 did fix the issue and there was some cached-compile problem or something similar that was causing the error still to occur initially. Apologies for the extraneous commentary and PR.

I'm glad it fixed your issue!

However, I should point out that closing an issue before checking with its author if the issue is resolved is somewhat problematic.

Sorry about that, I'm relatively inexperienced with these sorts of public github repos and have mostly worked in spaces where the standard behavior was to close the issue and let the author re-open if it isn't fixed (which is why I did that on my PR). I'll keep that in mind going forward, thanks for the tip.

[Borketh]

Probably could have handled it better. Kind of easy to do with GitHub auto closing of issues that are marked as closed in a PR.

Yeah, the merging could have waited until I could test.

the standard behavior was to close the issue and let the author re-open if it isn't fixed

That doesn't seem to be something I can do, at least not on this repo.

Anyway, thank you both!