Allow cyclic dependencies between dependency graphs where possible (coming from the new resolver)

[mehcode]

Describe the problem you are trying to solve

Given two crates foo and foo-macro, where foo-macro has a dependency on foo, foo should be able to optionally depend on foo-macro via a cargo feature (as long as foo-macro does not activate the feature).

Describe the solution you'd like

See above.

Notes

With the new v2 dependency resolver (amazing stuff by the way, this solves a lot of other various wrinkles I've stumbled across), it feels like this should be fairly straight forward to solve for someone familiar with the cargo internals. My guess would be the checker for cyclic dependencies simply now has false positives and adjusting the check to recognize the above scenarios would simply work.

---

For a more concrete use case, SQLx currently has three primary crates: sqlx, sqlx-core, and sqlx-macros. All the sqlx crate does is forward everything from sqlx-core and sqlx-macros into a single crate. sqlx-macros needs to use sqlx-core. This feature request would remove the need to have the sqlx-core crate.

[Eh2406]

I do remember discussing moving the cycle check to the "feature resolver" from the "dependency resolver" with @ehuss. I remember we thought it would cause problems, but at the moment I don't remember precisely what. One difficulty is that the output of the "dependency resolver" is currently a DAG, and much of Cargo therefore does not need checks for loops. If the only code that looks at the output of the "dependency resolver" is "feature resolver" then this is not much of a problem.

[jhpratt]

I would love to have this for the time crate. I have currently just stripped down the code from the main crate and reused it in the macro crate, but this is needlessly wasteful. Having this would also make it easier to implement the macro itself, for what it's worth.

[lausek]

I just want to summarize my research into this topic so far. The point of failure seems to be at check_cycles:1026. This algorithm simply does not recognize different feature configurations of packages even though it should (?). As far as I understand, the current implementation also evaluates optional dependencies regardless of specified features.

My fork of cargo was extended by a test case that should pass after this feature was implemented.

cargo build
     +
     |
     v
   parent +-> child +-> parent
     1)         2)        3)

1. Is the starting point of the process. parent includes child as default feature.
2. Child gets checked. It specifies parent as dependency, setting default-features to false therefore not including child.
3. parent should not have dependencies now.

[epage]

Besides anything else, a challenge with allowing cycles is publishing. We allow some cycles between packages today when there are no cycles in the build-unit graph and #4242 is the issue for dealing with publishing these. This is worked around today by stripping non-registry dev-dependencies on publish.

I also feel there is a strong benefit for keeping separate -core and -macros packages.

• Build time:
  • splitting them in clap allowed a heavy crate to build in parallel to the proc-macro machinery
  • splitting them in serde allows packages that don't need the proc-macro (e.g. toml) to not be blocked on the proc-macro machiner
• Dependencies: proc-macros generally generate code against their calling crate, making them dependent on them. Proc-macros easily overlook this. The easiest workaround is to have the top-level crate have a = dependency on the proc-macro. Alternatively, when you split up the packages, the proc-macro could have a target.'cfg(false)'.dependencies dependency on the -core crate.

For these reasons, I'm going to propose that we close this.

[epage]

Unsure if rust-lang/rfcs#3826 would be of interest here.

[epage]

Another reason to consider closing this is this relies on breaking cycles via features but that would only work with non-additive features.

#9738 exists for cycles avoided by target dependencies.