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

Use workspace dependencies for local crates #1425

Open
quasiyoke opened this issue Feb 7, 2025 · 1 comment
Open

Use workspace dependencies for local crates #1425

quasiyoke opened this issue Feb 7, 2025 · 1 comment

Comments

@quasiyoke
Copy link
Contributor

quasiyoke commented Feb 7, 2025

Observed

We specify specific versions and locations of external dependencies in the root (workspace) Cargo.toml. For local dependencies (crates within the same repository), we specify the path to them in the Cargo.toml of a specific dependent crate.

Question

Maybe we should specify all (repeated?) dependency descriptions in the workspace Cargo.toml?

Note

I think it's right that crates "know" which of their dependencies are local. It's good that local dependencies are specified in a separate group.

Context: #664

@MOZGIII
Copy link
Contributor

MOZGIII commented Feb 7, 2025

Maybe, but byt what are the benefits? To elaborate, the way it works, historically, is we start from specifying all deps locally, and only then moving some deps to workspace deps, after it is justified.

With external deps, we first started with substrate/frontier git deps specifically to avoid redecoaring them everywhere, which gave us huge savings during the updates (way less files to edit, and simpler mental model - so harder to miss anything).

Then we figured other external deps could also use workspace deps - as we want to have just one version for each of them, and it easier to do if we have them specified just once. It is, of course, doesn't work entiraly because of the deep dependencies, but at least it is better thab the alternative where we could have multiple versions specified accidentally in different crates.

So far, there was no use-case to move local deps to workspace deps that would be potent enough...
If anything, having local crates only depend on each other locally makes it arguably better cleanup wise - as when the crate is gone, then also gone are all the mentions if it. With workspace deps we could have dangling refs. And there's not much benefit to having them global either - with local deps, the workspace deps specification becomes yet another non-trivial place where you could set default features, which needs to be thought through how to use.

So, as you can see, moving to global workspace deps for locsl crates has a couple issues, and the obvious benefits not necessarily outweight them - and that's why were kind of reluctant to go forward with this so far.

I am not fundamentally opposed to it though, just seems to me we're beffer of without it so far.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants