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

It’s possible to build optional dependencies not listed in Cargo.lock #3629

Open
SimonSapin opened this issue Feb 1, 2017 · 12 comments
Open
Labels
A-features Area: features — conditional compilation A-optional-dependencies Area: dependencies with optional=true C-bug Category: bug S-triage Status: This issue is waiting on initial triage.

Comments

@SimonSapin
Copy link
Contributor

SimonSapin commented Feb 1, 2017

Test case: (run in a temporary directory)

cargo new a
cd a
echo 'b = {path = "b"}' >> Cargo.toml
cargo new b
cd b
echo 'c = {path = "c", optional = true}' >> Cargo.toml
cargo new c
cd c
echo 'd = {path = "d"}' >> Cargo.toml
cargo new d
cd ..
cd ..
cargo build --features b/c --verbose
cat Cargo.lock
cd ..
rm -rf a

Output in cargo 0.17.0-nightly (67e4ef1 2017-01-25)

     Created library `a` project
     Created library `b` project
     Created library `c` project
     Created library `d` project
   Compiling d v0.1.0 (file:///tmp/lol/a/b/c/d)
     Running `rustc --crate-name d b/c/d/src/lib.rs --crate-type lib --emit=dep-info,link -C debuginfo=2 -C metadata=d1f001e6c4277757 -C extra-filename=-d1f001e6c4277757 --out-dir /tmp/lol/a/target/debug/deps -L dependency=/tmp/lol/a/target/debug/deps`
   Compiling c v0.1.0 (file:///tmp/lol/a/b/c)
     Running `rustc --crate-name c b/c/src/lib.rs --crate-type lib --emit=dep-info,link -C debuginfo=2 -C metadata=d3fb71d0c424d062 -C extra-filename=-d3fb71d0c424d062 --out-dir /tmp/lol/a/target/debug/deps -L dependency=/tmp/lol/a/target/debug/deps --extern d=/tmp/lol/a/target/debug/deps/libd-d1f001e6c4277757.rlib`
   Compiling b v0.1.0 (file:///tmp/lol/a/b)
     Running `rustc --crate-name b b/src/lib.rs --crate-type lib --emit=dep-info,link -C debuginfo=2 --cfg 'feature="c"' -C metadata=bf9fdaf735940263 -C extra-filename=-bf9fdaf735940263 --out-dir /tmp/lol/a/target/debug/deps -L dependency=/tmp/lol/a/target/debug/deps --extern c=/tmp/lol/a/target/debug/deps/libc-d3fb71d0c424d062.rlib`
   Compiling a v0.1.0 (file:///tmp/lol/a)
     Running `rustc --crate-name a src/lib.rs --crate-type lib --emit=dep-info,link -C debuginfo=2 --cfg 'feature="b"' -C metadata=50db5a511b950de6 -C extra-filename=-50db5a511b950de6 --out-dir /tmp/lol/a/target/debug/deps -L dependency=/tmp/lol/a/target/debug/deps --extern b=/tmp/lol/a/target/debug/deps/libb-bf9fdaf735940263.rlib`
    Finished dev [unoptimized + debuginfo] target(s) in 2.22 secs
[root]
name = "a"
version = "0.1.0"
dependencies = [
 "b 0.1.0",
]

[[package]]
name = "b"
version = "0.1.0"

Crates c and d were compiled and used, but are not listed in Cargo.lock.

@alexcrichton
Copy link
Member

Definitely looks like a bug! I think that this is only possible from the command line, right?

@alexcrichton alexcrichton added the C-bug Category: bug label Feb 1, 2017
@SimonSapin
Copy link
Contributor Author

Yes, for example adding features = ["c"] to the b dependency in the root Cargo.toml adds c in Cargo.lock.

@alexcrichton
Copy link
Member

Yeah the change here is that you shouldn't be able to activate a feature that wasn't already in the lock file. To that end you shouldn't be able to activate a transitive feature that wasn't already possible to activate via some local feature.

@SimonSapin
Copy link
Contributor Author

For what it’s worth, html5ever uses a custom test harness crate (to support dynamically generated tests on stable Rust) which has optional features for stuff that uses unstable std APIs. It’s been very convenient to use cargo test --features rustc-test/capture. This particular feature doesn’t add any additional crate to the build, but it could. I suppose I could live with adding a local test-capture feature to html5ever.

@alexcrichton
Copy link
Member

Oh yeah this kind of feature should still be supported, but yeah you'll need some sort of feature locally to optionally enable a transitive feature. That'll ensure everything makes its way to the lock file and prevents weird bugs like updating the registry too often and such.

@SimonSapin
Copy link
Contributor Author

So what’s the fix? Forbid --features foo/bar on the command line?

@alexcrichton
Copy link
Member

Nah ---features foo/bar should still be allowed so long as the feature bar in the package foo could be activated via some other means. For example if it's a workspace member or otherwise a crate already has an optional dependency on foo/bar that'd be allowed.

What wouldn't be allowed is activating foo/bar when otherwise it's impossible to activate bar, as that means the feature isn't in Cargo.lock and may not resolve correctly.

@ehuss
Copy link
Contributor

ehuss commented Dec 2, 2023

#13100 raised a variation of this issue where it is possible to [patch] an unused optional dependency. When fixing this, we should pay attention to the patch behavior as well (though presumably disallowing the --feature setting should cover it).

@ehuss
Copy link
Contributor

ehuss commented Dec 2, 2023

Another thought, it would be nice if the solution to this would detect if everything is a path dependency, but not used in a workspace, to suggest using a workspace. For example:

[package]
name = "foo"
version = "1.0.0"
edition = "2021"

[dependencies]
bar = {path = "bar"}

Here, --features bar/baz could fail, but suggest that you add a [workspace] table as a way to fix it.

@BarbossHack
Copy link

BarbossHack commented Dec 2, 2023

#13100 raised a variation of this issue where it is possible to [patch] an unused optional dependency.

This is the opposite, I would like to be able to patch an unused optional dependency (which does not seems to be allowed anymore)

@ehuss
Copy link
Contributor

ehuss commented Dec 2, 2023

#13100 (comment) mentions another recommendation that cargo could make here where it could suggest re-exporting a feature that enables the optional dependency, which tells the resolver to use it as a candidate.

@kpreid
Copy link
Contributor

kpreid commented Sep 20, 2024

--features foo/bar should still be allowed so long as the feature bar in the package foo could be activated via some other means. For example if it's a workspace member or otherwise a crate already has an optional dependency on foo/bar that'd be allowed.

Note that the Cargo documentation currently gives a narrower rule:

If building multiple packages in a [workspace], the package-name/feature-name syntax can be used to specify features for specific workspace members.

I found this issue while trying to determine whether Cargo was working as intended when I found out I could specify an arbitrary feature of an arbitrary package in the dependency graph. It would be nice if this was documented more precisely, including the known issue, because where I started was determining whether I could use package.metadata.docs.rs.features = ["other-package/std"] to enable a feature of a dependency that my package wouldn't otherwise directly enable. I now know that the answer is no, or more precisely, if I did, then it would break when this issue is fixed; but that is not at all obvious.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-features Area: features — conditional compilation A-optional-dependencies Area: dependencies with optional=true C-bug Category: bug S-triage Status: This issue is waiting on initial triage.
Projects
None yet
Development

No branches or pull requests

7 participants