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

Resolver does not lock disabled nested optional dependencies. #7792

Closed
ehuss opened this issue Jan 11, 2020 · 2 comments
Closed

Resolver does not lock disabled nested optional dependencies. #7792

ehuss opened this issue Jan 11, 2020 · 2 comments
Labels
A-dependency-resolution Area: dependency resolution and the resolver A-optional-dependencies Area: dependencies with optional=true C-bug Category: bug

Comments

@ehuss
Copy link
Contributor

ehuss commented Jan 11, 2020

If you have a dependency with an optional dependency that is not enabled by default, then that dependency will not get recorded in Cargo.lock. However, it is possible to build it with the --features flag. I can't tell if this is intentional or not.

Example: a → b →(optional) c
In a, run cargo build --features b/c

cargo new --lib a
cd a
cargo new --lib b
cd b
cargo new --lib c
echo 'c = {path="c", optional=true}' >> Cargo.toml
cd ..
echo 'b = {path="b"}' >> Cargo.toml
cargo build --features b/c
# Notice that c is missing from Cargo.lock.

@Eh2406 Do you happen to know if this is intentional? If not, how feasible would it be to change?

For resolving features independently, I was hoping I could build the resolve graph with --all-features, but that doesn't work for cases like this (c will be missing from the result).

As a follow-up question, how crazy would it be to resolve the entire graph assuming all optional dependencies are enabled? I assume that could cause problems since that could add more constraints to existing projects that might create conflicts.

NOTE: #2851 (comment) seems kinda relevant. Support was added in #2876.

@ehuss ehuss added C-bug Category: bug A-dependency-resolution Area: dependency resolution and the resolver A-optional-dependencies Area: dependencies with optional=true labels Jan 11, 2020
@Eh2406
Copy link
Contributor

Eh2406 commented Jan 12, 2020

I did not know that --features b/c was a thing that you can do. WIth your edit, that you can do that at all appears to be intentional. I do agree with this #2851 (comment) that it is odd functionality to have.

So the existing behavior for when a feature is assumed activated for generating a lockfile is somewhat odd. The competing constraints are that we want to not call for unnecessary dependencies (that may limit what versions are acceptable in unexpected ways) and on the other hand we don't want to be generating different lockfile depending on the commands invoked.

The current behavior is:

  • If it is the root package we generate the lockfile as if it had --all-features, here for example. This makes sure that builds with different --features do not trample on each others lockfile.
  • When resolving dependencies we only activate the needed features, here. This ensures that we don't add constraints/dependencies/crates for unneeded things.

I have no idea how --features b/c ends up building a thing that is not in the lockfile! It does meat the constraints, but it breaks the point of a lockfile!

how crazy would it be to resolve the entire graph assuming all optional dependencies are enabled?

You mean recursively for all transitive dependencies? This line becomes ResolveOpts::everything(). It is easy to construct cases where this will make dependencies unresolvable that work under todays rules, but I don't know if it really happens that mutch.

For example you can NOT build w, but you can depend on w as long as only one of x or y is enabled.

w:
 x = {optional=true}
 y = {optional=true}

x:
  z = '=1.0.1'

y:
  z = '=1.0.0'

r:
 w = '*'

Now we can build r, but --features w/x --features w/y will not work (I hope). If we add your c to the lockfile then I don't see how we can let r build.

I don't know how realistic this is.

@ehuss
Copy link
Contributor Author

ehuss commented Jan 15, 2020

This is essentially a duplicate of #3629, so I'm going to close in favor of that (where Alex suggests forbidding passing these kinds of unlocked features).

@ehuss ehuss closed this as completed Jan 15, 2020
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-optional-dependencies Area: dependencies with optional=true C-bug Category: bug
Projects
None yet
Development

No branches or pull requests

2 participants