stellar / rs-stellar-xdr

Rust lib for Stellar XDR.
Apache License 2.0
20 stars 27 forks source link

`curr` and `next` features are not additive. #316

Closed dnut closed 1 year ago

dnut commented 1 year ago

What version are you using?

20.0.0-rc1

What did you do?

Cargo features are supposed to be additive. See the official docs: https://doc.rust-lang.org/cargo/reference/features.html

A consequence of this is that features should be additive. That is, enabling a feature should not disable functionality, and it should usually be safe to enable any combination of features. A feature should not introduce a SemVer-incompatible change.

These features are not additive because these public glob imports are disabled when a feature is activated:

#[cfg(feature = "curr")]
pub mod curr;
#[cfg(all(feature = "curr", not(feature = "next")))]
pub use curr::*;

#[cfg(feature = "next")]
pub mod next;
#[cfg(all(not(feature = "curr"), feature = "next"))]
pub use next::*;

Cargo is designed with the assumption that all features are additive, likewise that there is no harm in enabling features. If you build projects with cargo, non-additive features are likely to cause problems.

For example, let's say you depend on two separate libraries, one that enables the "next" feature and one that depends on the "curr" feature. If either of those libraries uses the glob imports, the entire thing will fail to compile because the activation of both features at once prevents either of the glob imports from being included.

This is an actual problem I have encountered when implementing something that depends on soroban-client and transitively depends on soroban-sdk. Simply having both of those crates in your dependency tree will cause any attempted compilation to fail. The root cause of this issue is that the curr and next features are not additive.

leighmcculloch commented 1 year ago

Hi @dnut! The code snippet shared looks like it is from an old version of the crate before we removed the behavior described.

In the latest version of the crate, v20.0.0-rc1, the exports are not re-exported at the top level and the code looks like this:

https://github.com/stellar/rs-stellar-xdr/blob/16f4d7ccd37a8f96b1ebb59fc7878a8457fce22e/src/lib.rs#L121-L125

leighmcculloch commented 1 year ago

Note that the stellar-xdr crate is still pretty new and we have not yet released a stable version of the crate.

dnut commented 1 year ago

Nice, thanks for the quick reply. Apologies for failing to look at the master branch. It looks like you've already resolved the issue in the git repository.

FYI, I actually copied that code directly from the latest release available on crates.io, which is 20.0.0-rc1. You can also view the source for any particular version on docs.rs. https://docs.rs/crate/stellar-xdr/20.0.0-rc1/source/src/lib.rs

leighmcculloch commented 1 year ago

Ahh, yes, apologies, I thought we had made that change before the last release but I was incorrect. The next release slated for sometime next month should have this change.

leighmcculloch commented 1 year ago

Closing as this has been fixed in the main branch slated for next release.