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.

Sign up for GitHub

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

Jump to bottom

Avoid circular dependencies in workspace #3053

Closed
thomaseizinger opened this issue Oct 22, 2022 · 0 comments · Fixed by #3023
Closed

Avoid circular dependencies in workspace #3053

thomaseizinger opened this issue Oct 22, 2022 · 0 comments · Fixed by #3023

Comments

@thomaseizinger
Copy link
Contributor

thomaseizinger commented Oct 22, 2022 •
edited
Loading

Currently we have circular dependencies in our workspace.

This is problematic because:

Getting rid of the circular dependencies is almost easily solvable, except for the NetworkBehaviour custom derive.

That one emits code that assumes that a crate libp2p is in scope where it can import items from. That is an issue because it means we can't use the macro in our workspace without depending on the libp2p meta crate which is a circular dependency again.

The real solution here is being discussed over at rust-lang/rust: rust-lang/rust#54363.

We can't wait for that to be implemented though, so we need to find a workaround.
This was referenced Oct 24, 2022
Merged
Closed
This was referenced Nov 13, 2022
Closed
Merged
@mergify mergify bot closed this as completed in #3023 Dec 12, 2022
mergify bot pushed a commit that referenced this issue Dec 12, 2022
@thomaseizinger
fix: Remove circular dependencies across workspace (#3023)
d7363a5 
Circular dependencies are problematic in several ways:

- They result in cognitive overhead for developers, in trying to figure out what depends on what.
- They present `cargo` with limits in what order the crates can be compiled in.
- They invalidate build caches unnecessarily thus forcing `cargo` to rebuild certain crates.
- They cause problems with tooling such as `release-please`.

To actually break the circular dependencies, this patch inlines the uses of `development_transport` in the examples and tests for all sub-crates. This is only meant to be a short-term fix until #3111 and #2888 are fixed.

To ensure we don't accidentally reintroduce this dependency, we add a basic CI that queries `cargo metadata` using `jq`.

Resolves #3053.
Fixes #3223.
Related: #2918 (comment)
Related: googleapis/release-please#1662
umgefahren pushed a commit to umgefahren/rust-libp2p that referenced this issue Mar 8, 2024
@thomaseizinger
fix: Remove circular dependencies across workspace (libp2p#3023)
05dd63d 
Circular dependencies are problematic in several ways:

- They result in cognitive overhead for developers, in trying to figure out what depends on what.
- They present `cargo` with limits in what order the crates can be compiled in.
- They invalidate build caches unnecessarily thus forcing `cargo` to rebuild certain crates.
- They cause problems with tooling such as `release-please`.

To actually break the circular dependencies, this patch inlines the uses of `development_transport` in the examples and tests for all sub-crates. This is only meant to be a short-term fix until libp2p#3111 and libp2p#2888 are fixed.

To ensure we don't accidentally reintroduce this dependency, we add a basic CI that queries `cargo metadata` using `jq`.

Resolves libp2p#3053.
Fixes libp2p#3223.
Related: libp2p#2918 (comment)
Related: googleapis/release-please#1662
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: Remove circular dependencies across workspace libp2p/rust-libp2p
1 participant
@thomaseizinger