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

Mention cyclic dependency in main rustdocs #363

Merged
merged 1 commit into from
Dec 16, 2024

Conversation

Fraser999
Copy link
Contributor

If a project has the following in its manifest:

[dependencies]
borsh = { version = "1.5.3", features = ["derive"] }
indexmap = { version = "2.7.0", features = ["borsh"] }

then the following error is returned by cargo:

error: cyclic package dependency: package `borsh v1.5.3` depends on itself. Cycle:
package `borsh v1.5.3`
    ... which satisfies dependency `borsh = "^1.2"` (locked to 1.5.3) of package `indexmap v2.7.0`
    ... which satisfies dependency `indexmap = "^2.3.0"` (locked to 2.7.0) of package `toml_edit v0.22.22`
    ... which satisfies dependency `toml_edit = "^0.22.20"` (locked to 0.22.22) of package `proc-macro-crate v3.2.0`
    ... which satisfies dependency `proc-macro-crate = "^3"` (locked to 3.2.0) of package `borsh-derive v1.5.3`
    ... which satisfies dependency `borsh-derive = "~1.5.3"` (locked to 1.5.3) of package `borsh v1.5.3`

The cycle can be broken by not enabling the derive feature of borsh, and instead just directly specifying the borsh-derive crate, e.g.

[dependencies]
borsh = "1.5.3"
borsh-derive = "1.5.3"
indexmap = { version = "2.7.0", features = ["borsh"] }

This only works as long as no other dependency (direct or transient) enables the derive feature of borsh. In that case, I think the only way to break the cycle is to not enable the borsh feature of indexmap, likely requiring implementing the borsh traits by hand on the types holding indexmap members.

@cuviper
Copy link
Member

cuviper commented Dec 16, 2024

Hmm, that's unfortunate. I'd expect this to be avoided by cargo's resolver v2:

Features enabled on build-dependencies or proc-macros will not be unified when those same dependencies are used as a normal dependency.

In this case, indexmap is a regular dependency of the root crate, and also an indirect dependency of a proc-macro. I think in theory these should be distinctly resolvable, enabling indexmap/borsh for the root but not for the proc-macro, so there's no actual cycle in the build plan.

@ehuss, can you explain the cargo perspective?

@ehuss
Copy link

ehuss commented Dec 16, 2024

Yea, this is just something that is not yet supported. The dependency resolver doesn't really know about host-versus-target dependencies, and thus isn't able to see that there isn't a cycle. Cycle detection would need to be pushed to a later stage, but there is a fair bit of complexity and risk with that. There's a little bit more information in rust-lang/cargo#8734 and rust-lang/cargo#9738.

@cuviper
Copy link
Member

cuviper commented Dec 16, 2024

Thanks, those issues are definitely on point.

I guess it would be avoided if borsh implemented its traits for indexmap instead, but I don't know if there's any way to make that transition without a breaking change. For now, a doc warning is better than nothing...

@cuviper cuviper enabled auto-merge December 16, 2024 21:53
@cuviper cuviper added this pull request to the merge queue Dec 16, 2024
Merged via the queue into indexmap-rs:master with commit 31c9862 Dec 16, 2024
16 checks passed
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

Successfully merging this pull request may close these issues.

3 participants