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

Cargo check also checks path depencies #124851

Closed
vringar opened this issue May 7, 2024 · 7 comments
Closed

Cargo check also checks path depencies #124851

vringar opened this issue May 7, 2024 · 7 comments
Labels
T-cargo Relevant to the cargo team, which will review and decide on the PR/issue.

Comments

@vringar
Copy link

vringar commented May 7, 2024

I have a workspace with multiple crates that depends on another set of crates via path dependencies
The <project root>/fuzzer/Cargo.toml looks like this:

[workspace]
members = [
    "amd_sp",
    "libasp"
]
resolver = "2"

[workspace.dependencies]
libafl = { path = "../LibAFL/libafl/", default-features = false, features = [
    "prelude",
    "fork"
] }
libafl_bolts = { path = "../LibAFL/libafl_bolts/"}
libafl_qemu = { path = "../LibAFL/libafl_qemu/", features = [
    "arm",
    "systemmode",
], default-features = false }

[profile.dev]
debug = true
debug-assertions = true
[profile.release]
debug = true
debug-assertions = true

When I run cargo check I expect to only get warnings for my workspace members, however my output is flooded with warnings from the library like this:

warning: unexpected `cfg` condition name: `emulation_mode`
   --> <project root>/LibAFL/libafl_qemu/src/command.rs:648:23
    |
648 |                 #[cfg(emulation_mode = "usermode")]
    |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: consider using a Cargo feature instead or adding `println!("cargo::rustc-check-cfg=cfg(emulation_mode, values(\"usermode\"))");` to the top of the `build.rs`
    = note: see <https://doc.rust-lang.org/nightly/cargo/reference/build-scripts.html#rustc-check-cfg> for more information about checking conditional configuration

Meta

rustc --version --verbose:

rustc 1.80.0-nightly (7d83a4c13 2024-05-06)
binary: rustc
commit-hash: 7d83a4c131ab9ae81a74c6fd825c827d74a2881d
commit-date: 2024-05-06
host: x86_64-unknown-linux-gnu
release: 1.80.0-nightly
LLVM version: 18.1.4

I'm sorry that this is not a minimal example. If you have trouble reproducing it, I will attempt to reduce it and publish that as a repo.

@vringar vringar added the C-bug Category: This is a bug. label May 7, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label May 7, 2024
@ehuss
Copy link
Contributor

ehuss commented May 7, 2024

path dependencies are automatically added as workspace members. More information can be found at https://doc.rust-lang.org/cargo/reference/workspaces.html#the-members-and-exclude-fields.

@vringar
Copy link
Author

vringar commented May 7, 2024

But it's also not excludable via cargo check --workspace --exclude libafl_qemu. Should I report that separately?

Also the path I specified is not "residing in the workspace directory" right? Because it starts with ..?

@kpreid
Copy link
Contributor

kpreid commented May 7, 2024

Workspaces are irrelevant here; what's going on is that Cargo only suppresses warnings from registry/git dependencies. I couldn't quickly find a good canonical documentation of this behavior, but:

As for dependents, Cargo suppresses lints from non-path dependencies with features like --cap-lints.

https://doc.rust-lang.org/cargo/reference/manifest.html#the-lints-section

I believe the rationale is that if you are using a path dependency, either you're developing that code and are interested in warnings from it as much as you are the main package, or you're going to ignore all warning output because you're just building a binary.

If your goal is to depend on code that hasn't been published to crates.io, consider using a git dependency instead of a path dependency.

@vringar
Copy link
Author

vringar commented May 7, 2024

But then why does --exclude require a workspace? Does it just remove the entry point but still warns on it as part of a transitive dependency?

@kpreid
Copy link
Contributor

kpreid commented May 7, 2024

But then why does --exclude require a workspace? Does it just remove the entry point but still warns on it as part of a transitive dependency?

If I follow your meaning right, then yes —--exclude is about modifying the set of packages you asked to operate on to contain fewer packages than the whole workspace, but after that set has been chosen, the operation itself still does whatever it does with dependencies.

@vringar
Copy link
Author

vringar commented May 7, 2024

Okay, so this is working as expected and should be closed?
Or does this also work as a "user story" of this is unexpected behavior and should be explained in a hint/be configurable?

@saethlin saethlin added T-cargo Relevant to the cargo team, which will review and decide on the PR/issue. and removed C-bug Category: This is a bug. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels May 10, 2024
@weihanglo
Copy link
Member

Documentation for package selections (incl. --workspace and --exclude flags`) can be found here: https://doc.rust-lang.org/nightly/cargo/commands/cargo-check.html#package-selection.

Since ehuss and kpreid explained the behavior pretty well, if no objection I lean toward closing this.

@vringar vringar closed this as completed May 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-cargo Relevant to the cargo team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants