Cannot use borsh feature in crates that depend on borsh-derive

[preston-evans98]

Since at least version 1.2, borsh transitively depends on indexmap via toml_edit if the borsh/derive feature is activated. This causes builds to fail with a circular dependency error when using indexmap/borsh and borsh/derive in the same crate.

[package]
name = "example"
version = "0.1.0"
edition = "2021"
resolver = "2"


[dependencies]
borsh = { version = "1", features = ["derive"] }
indexmap = { version = "2.8.0", features = ["borsh"] }

$ cargo check
error: cyclic package dependency: package `borsh v1.5.1` depends on itself. Cycle:
package `borsh v1.5.1`
    ... which satisfies dependency `borsh = "^1.2"` (locked to 1.5.1) of package `indexmap v2.8.0`
    ... which satisfies dependency `indexmap = "^2.3.0"` (locked to 2.8.0) of package `toml_edit v0.22.24`
    ... which satisfies dependency `toml_edit = "^0.22.24"` of package `proc-macro-crate v3.3.0`
    ... which satisfies dependency `proc-macro-crate = "^3"` of package `borsh-derive v1.5.5`
    ... which satisfies dependency `borsh-derive = "~1.5.1"` of package `borsh v1.5.1`

[preston-evans98]

On further thought, I believe this issue needs to be addressed in the borsh-derive crate rather than here: near/borsh-rs#345

[cuviper]

Yeah, #363 added a doc warning about this. I mentioned there that borsh could implement for indexmap itself, but you'd have to be careful that your entire dependency tree doesn't enable borsh/indexmap and indexmap/borsh at the same time, as that's a more obvious cycle.

[preston-evans98]

Thanks for the quick response! I'll see if I can raise a PR to borsh to add indexmap support in-crate or remove the transitive dependency.

[preston-evans98]

PR open here: near/borsh-rs#346

[dj8yfo]

following near/borsh-rs#346 merge, borsh feature probably can be removed or made a dummy one

[preston-evans98]

Thanks @dj8yfo for the quick responses. near/borsh-rs#346 is now merged. @cuviper, let me know if you want a PR to modify or remove the feature. That seems like a potential semver issue to me, but I'm happy to raise one if it's useful

[cuviper]

Yeah, we should only remove it in the next major release. For now, we can update the documentation to recommend using borsh/indexmap instead, calling our feature "deprecated".

Sadly, the real #[deprecated] attribute can't help in this scenario -- rust-lang/rust#39935.