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

Support unifying transative path dependencies after patching #121

Open
eopb opened this issue Sep 5, 2024 · 1 comment
Open

Support unifying transative path dependencies after patching #121

eopb opened this issue Sep 5, 2024 · 1 comment

Comments

@eopb
Copy link
Owner

eopb commented Sep 5, 2024

Problem

To make the issue clear, let's consider an example. We're working on project foo which has two dependencies from crates.io, a and b:

[dependencies]
a = "*"
b = "*"

Dependencies a and b both come from a monorepo. as dependencies look like this:

[dependencies]
b = { version = "*", path = "../b" }

As a result, we have a dependency graph that looks like this (where arrows point to the dependency):

stateDiagram-v2
    foo --> b
    foo --> a
    a --> b
Loading

Now let's consider what happens when we patch foo with a local copy of a using cargo override --path ../monorepo/a

[dependencies]
a = "*"
b = "*"

[patch.crates-io]
a = { path = "../monorepo/a" }
stateDiagram-v2
    pa: ../monorepo/a
    pb: ../monorepo/b
    foo --> pa
    foo --> b
    pa --> pb
Loading

The resolver is now selecting two copies of b, the path dependency specified in a's Cargo.toml as well as the version of b from crates.io.

If a exposes types from b in its public interface, and foo attempts to pass types from its copy of b into a or vice versa, this patch will cause foo to stop compiling. This is because types from as copy of b and foos copy of b can not be considered the same type.

Solution

Adding a second patch to foo solves the issue

[dependencies]
a = "*"
b = "*"

[patch.crates-io]
a = { path = "../monorepo/a" }
b = { path = "../monorepo/b" }

There are multiple levels of automation that we could use here:

  1. cargo override could emit a note if a patch contains direct path dependencies.

    The note could suggest running subsequent cargo override commands to patch these.

    If these dependencies have path dependencies of their own, notes for those will come up in the subsequent calls to cargo override.

  2. Rather than emit notes, cargo override could automatically apply the additional patches.

  3. cargo override could apply these additional patches only for public dependencies

This issue was originally discussed over on zulip: https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/Plugin.20.60cargo.20override.60/near/466959784. Thanks to @epage for bringing it to my attention

@epage
Copy link
Contributor

epage commented Sep 5, 2024

This also ties a little bit into #107

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

No branches or pull requests

2 participants