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

Specify in root that certain edges in the dependency graph should be "private" #7276

Closed
cbeck88 opened this issue Aug 21, 2019 · 1 comment
Closed
Labels
A-dependency-resolution Area: dependency resolution and the resolver A-features Area: features — conditional compilation C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted`

Comments

@cbeck88
Copy link

cbeck88 commented Aug 21, 2019

Describe the problem you are trying to solve

Cargo works very hard to allow that different versions of the same crate can be part of the same build plan to satisfy the version requirements of other crates in the plan.

However, cargo (by design) unifies features when the version of the crate is the same, across the entire build plan. This because cargo assumes that all features are additive.

Unfortunately, it's very common in the rust ecosystem today that features are not additive.

  • Features may be used to gate optional code that is less portable. If one of the targets in the build plan has stricter portability requirements than another, then feature unification may break the build. (For example one target, a server, may use x86-only instructions, aes-ni, etc., while the corresponding client is more portable. Or one target may be embedded firmware and another may be a server that it needs to talk to.)
  • It is extremely common in the ecosystem that crates follow serde example in providing std and alloc features to determine where they source standard types like string and vec. Serde is subtly non-additive in its use of these features. For example, when std is present, it implements std::error::Error on its generated erorr trait bounds. Since this trait is not available in core sadly, it cannot do this in alloc configuration. This means that users of serde like rmp-serde must expose matching std and alloc features and must be configured in exactly the same way as serde in the global build plan, or the build will fail, because they don't implement the correct trait bound (they cannot use std::error::Error if they are in a no_std configuration!). Note that the feature here is not additive -- serde/std has a stricter trait bound then serde with no features, so crates in an alloc configuration cannot build against serde/std.
  • Cargo unifies features even across build-dependencies and regular dependencies. This means that for instance, if I'm trying to compile mbedtls in an alloc and no_std configuration, but any crate in my tree has a build dependency that uses libc crate (to support some build.rs script), my build will fail, because libc will pick up the libc/std feature from the build dependencies. This issue is described in detail on cargo-xbuild github issues: Build script using std can't compile rust-osdev/cargo-xbuild#10 and is connected to related long-standing cargo issue: Features of dependencies are enabled if they're enabled in build-dependencies; breaks no_std libs #5730

I have read with interest the discussion in this PR: #7216 which looks to be refining the logic in Cargo's "unit dependencies" and resolver etc. This looks like great progress that might improve the situation.

I would like to propose a simpler, "dumber" fix which, like [patch] in root cargo.toml, will allow projects like OP in rust-osdev/cargo-xbuild#10 (and my own projects) to make progress without patching the world. The idea is that the end-user should be able to specify in root / workspace Cargo.toml that a particular edge in the dependency graph should lead to a private subtree separated from everything else. This allows the end-user enormous and fine grained control over the shape of the build plan that results, in a fairly intuitive way -- the reasons they might need to do this are much the same as the reasons that they may need to use [patch], in order to work around some problem. Obviously this would not be allowed in crates that get published.

Describe the solution you'd like

In the same places where [patch] is currently allowed by cargo, a tag [private-version] would be accepted.

[private-version]
mbedtls = { dependency = "libc" }
  • The key is as in [patch], the name of a package. (Alternatively we could consider allowing a Package-id spec as in [replace] but I guess you are trying to move away from that?)
  • The tag dependency names one of the dependencies of the target.

The effect of this block is that cargo will build a separate version of libc used only by mbedtls and whose features are exactly what mbedtls requires, which will not be unified with any other version of libc that is built.

This exactly as-if the user had performed this workaround:

  • Clone the libc repository and change the name arbitrarily
  • Clone mbedtls and patch it so that it's cargo.toml uses the forked version of libc, and imports it as libc so that the code will build without changed
  • Use the patched version of mbedtls as needed

Under the hood what I expect is that cargo will use some name-mangling strategy, so that there is some new libc-for-mbedtls crate in the build-plan which is otherwise the same as as the libc crate, and mbedtls is built against it, but renames it to libc for purpose of source compatibility.

This feature would already be extremely useful and help a lot of people trying to use cargo for cross-platform or embedded development.

We could contemplate an even heavier hammer:

[private-version]
mbedtls = { dependency = "libc", transitive = true }

The idea here would be that not only libc is private to mbedtls, but all of the transitive dependencies of libc down to the sys-root, would be built at separate versions without feature contamination for mbedtls, so that in the build plan, this edge from mbedtls to libc does not lead to any packages which are shared with anything else. This might cause a lot of builds to be unnecessarily slower because sharing would be okay, but it is better than not being able to build at all, or having to patch the world in order to build.

Notes

This allows the user much more control over what build-plan cargo produces -- it allows them to unfold the dependency graph arbitrarily, as fine-grained as they need in order to build, without having to actually patch dependencies' code or Cargo.toml.

For a lot of cutting-edge projects in rust trying to build for exotic targets, this will give them a way to make progress without having to use the essentially infeasible workarounds described in #5730

Note that I'm not saying that #7216 is bad -- far from it!
I'm saying if there is interest from the maintainers it would be great if after we land that we contemplate adding simple tricks like [private-version] which extend the utility of [patch] to help people get things done and work around build problems. I suspect that it might be feasible to implement something like I describe after the UnitDep struct exists.

@cbeck88 cbeck88 added the C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` label Aug 21, 2019
@ehuss ehuss added A-dependency-resolution Area: dependency resolution and the resolver A-features Area: features — conditional compilation labels Sep 21, 2019
@epage
Copy link
Contributor

epage commented Nov 2, 2023

Unfortunately, it's very common in the rust ecosystem today that features are not additive.

Mutually exclusive features are being tracked and discussed in #2980. Let's centralize the conversation there. As this is targeted at that use case, I'm going in favor of that issue and would recommend sharing your thoughts there. If there is a reason we should leave this open, let us know!

btw one proposal is https://internals.rust-lang.org/t/pre-rfc-mutually-excusive-global-features/19618

Cargo unifies features even across build-dependencies and regular dependencies.

This changed with Resolver v2

@epage epage closed this as not planned Won't fix, can't repro, duplicate, stale Nov 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-dependency-resolution Area: dependency resolution and the resolver A-features Area: features — conditional compilation C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted`
Projects
None yet
Development

No branches or pull requests

3 participants