Workspace reduce deps pr #452
Conversation
dan-da
force-pushed
the
workspace_reduce_deps_pr
branch
from
85e43ba to
4c376b6
Compare
This change reduces the number of dependencies in the neptune-core crate.
The logical structure becomes:
neptune-core
├── Cargo.lock
├── Cargo.toml
├── cli
│ ├── Cargo.lock
│ ├── Cargo.toml
│ └── src
│ └── main.rs
├── dashboard
│ ├── Cargo.lock
│ ├── Cargo.toml
│ └── src
│ └── main.rs
└── src
├── lib.rs
└── main.rs
Creates a workspace with the following structure: neptune-cash ├── Cargo.lock ├── Cargo.toml ├── neptune-cli │ ├── Cargo.toml │ └── src │ └── main.rs ├── neptune-core │ ├── Cargo.toml │ └── src │ ├── lib.rs │ └── main.rs ├── neptune-dashboard │ ├── Cargo.toml │ └── src │ └── main.rs └── README.md
dan-da
force-pushed
the
workspace_reduce_deps_pr
branch
from
4c376b6 to
1d5c062
Compare
|
rebased to master. |
|
I'm in favor of this change, thanks for taking it on. 👍 I recommend to use workspace dependencies. If all crates exclusively use Footnotes
|
that's interesting, thx. I will look into it, but I think it could be a follow-up PR. note: if crate (a) depends on crate (b) and (b) is correctly exporting any crate (c) that is used in its public API and if (a) is correctly using (c) via I guess where that falls down and workspace deps can help is if (b) uses (c) internally, and (a) also uses (c). But then isn't cargo supposed to pick a single version of (c) anyway, if possible? edit: something that bothers me about workspace deps is they kind of mix deps across files. In this PR, it is very clear that neptune-core/Cargo.toml defines a set of deps and then neptune-cli/Cargo.toml depends on neptune-core, plus its own set of deps. Everything is self-contained, and pretty much like using a regular non workspace crate. workspace deps change that model and require editing the root Cargo.toml in conjunction when making changes. So I have kind of mixed feelings about it, but that could be because I have no experience with using them. |
There was a problem hiding this comment.
Much cleaner! I like it. That said, I am not familiar enough with the ins and outs of rust project configuration to have an educated opinion.
cargo install --locked --path neptune-core
cargo install --locked --path neptune-cli
cargo install --locked --path neptune-dashboard
Don't forget triton-vm-prover and please update the docs too (installation.md).
it is still part of neptune-core, so installs with it. Actually I didn't even think about breaking it out since it doesn't add any additional deps. I guess it makes some sense for sake of symmetry/consistency. I don't think it even needs to depend on neptune_cash crate/lib. so yeah, I will put it into a separate crate also.
ah, forgot about that one, thx. anyway, right now this PR Is breaking CI. I'm not planning to merge until we get the CI issue(s) worked out. |
And if (b) fails to export (c), or if (a) pulls in (c) manually instead of using (b::c), then workspace dependencies won't be any help, of course.
You are right, it should. If both (a) and (b) are part of the same workspace, it might never pull in multiple versions, and instead report unresolvable dependencies, like so: ❯ cargo build
Updating crates.io index
error: failed to select a version for `bincode`.
... required by package `two v0.1.0 (/home/triton/Code/tmp/two)`
versions that meet the requirements `^1.3.2` are: 1.3.2, 1.3.3
all possible versions conflict with previously selected packages.
previously selected package `bincode v1.2.1`
... which satisfies dependency `bincode = "=1.2.1"` of package `one v0.1.0 (/home/triton/Code/tmp/one)`
failed to select a version for `bincode` which could resolve this conflictThat said, and even though the experiment above indicates that Cargo tries really hard, according to the book: “If crates in the workspace specify incompatible versions of the same dependency, Cargo will resolve each of them, but will still try to resolve as few versions as possible.” Apparently, de-duplication is not guaranteed. I'm not sure how to tweak my experiment to provoke duplicates, though.
I see what you mean. I think I hadn't realized how hard Cargo tries to make sure there's only one version of a given dependency. |
Creates a workspace with the following structure:
This PR came about as an attempt to reduce the number of deps when building neptune-core. neptune-dashboard in particular required extra deps such as ratatui, crossterm, and others.
I made a first commit that simply moves neptune-cli and neptune-dashboard into their own sub-crates with their own deps, but does not create a workspace. This worked, but had the limitation that each crate must be built separately, which nearly tripled build time to build them all.
By introducing a workspace at the top level, all three crates can be bulit together and share a single Cargo.lock. ie
cargo buildandcargo testwork from the root dir.The only limitation I have found is that
cargo install --locked --path .no longer works due to a cargo bug. This is worked around by installing each separately:I updated the README installation instructions to reflect this.
There are some things I'm not sure about and have not tested:
dep removal: cargo tree | wc -l dropped from 824 to 747 in the neptune-core crate.
regardless of deps, I think this is a useful organizational change because it makes it clear which deps are for which binary, whereas before they were all jumbled together.
note that each crate Cargo.toml defines its own version. So when making a release, each would need to be bumped to stay in sync, ideally via a release script.