rust-lang / cargo

The Rust package manager
https://doc.rust-lang.org/cargo
Apache License 2.0
12.7k stars 2.41k forks source link

Help uncover issues with too few features requested that is masked by feature unification #14021

Open meithecatte opened 4 months ago

meithecatte commented 4 months ago

Problem

Due to the way dependency features get consolidated across the packages that happen to be being built at the same time, cargo check --all can mask problems where insufficient features are specified for a dependency by one of the packages in a workspace.

For the purposes of demonstration, I use a dependency (dep) within the workspace, but I originally observed this issue with a dependency from crates.io.

Steps

  1. Run the following command in an empty directory:

    base64 -d <<EOF | gunzip | tar x
    H4sIAAAAAAAAA+2ZXWvbMBSGfa1foXo3CRRPkiUHUnpR+jOCLxRHSU3rD/yxMcr++47sjIysbRaY
    VLc5z41zJCObHM6r98j3utlVUVcVT4EzGJBIaa98odif1wHBZMBlErNYxozDOOcylgFl7l7pQN92
    uqE0KPQP/dZ9p+Y/KKtaZ496Z1JS6sLQWxpuTB2Sb6Zp86q0MYt4xEJiNnm3HxFM8JCQ1dborm9M
    m5Kq7mBilcLg96p5bGFRWLEwxRrWsTOEAmHfmoaH14dAQJCS9/4PLpm2yb66foat8YVSr9Y/sK9/
    rlRi6x9+sYAq1y9mufD6t/l/ytdR07p7xin9Z2pxlH8uRYL674Mvq2y7m+2V3Io7SHk4T0ndr2lW
    lW1H75a0jwVMSXGDUv3ZGLZkxzvAWfrPY6v/oAOo/z4Y83/vtAs4pf+g9kf5F4rFqP8++Nv/jyb9
    3zoA6BVMuTFlltsuACKYfKa17h7sXVEU0p+4ZUyasf7ddgHn6H/CufV/McgF6r8HDvkvdF66aQNO
    +n95nH8BAeq/D7YltYmfzenzcCyzWe+uZiDky+Xd/Iagen92hlO4Kfl/yQf/n+D5jxfG/L+z/0+S
    o/wL2wag/nvgZf8v/pf/v6a/PxLYrwDD4VKKPcGEGOt/Ov5fKTb6/wXqvw8O+Z+G/x/zD/4f9d8L
    6P8RBEEQ5PL4BSGsH4sAKAAA
    EOF

    This creates the following files:

    Cargo.toml

    [package]
    name = "dep"
    version = "0.1.0"
    edition = "2021"
    
    [features]
    opt = []
    
    [workspace]
    members = [
       "user1",
       "user2",
    ]

    src/lib.rs

    #[cfg(feature = "opt")]
    pub const A: u32 = 42;

    user1/Cargo.toml

    [package]
    name = "user1"
    version = "0.1.0"
    edition = "2021"
    
    [dependencies]
    dep = { path = ".." }

    user2/Cargo.toml

    [package]
    name = "user2"
    version = "0.1.0"
    edition = "2021"
    
    [dependencies]
    dep = { path = "..", features = ["opt"] }

    user*/src/main.rs

    fn main() {
        dbg!(dep::A);
    }
  2. Run cargo check --all and note that it executes successfully. Conclude that all the packages in the workspace compile correctly.

  3. Run cargo check -p user1 and have your expectations subverted – user1 only works because user2 requested the opt feature from dep.

Possible Solution(s)

I am honestly unsure whether it's possible to solve this. Ideally rustc would track whether the items that are being accessed have actually been requested as features by the crate that's accessing them, or if they're compiled in because of a different consumer. Considering that trait implementations can be cfg'd out, I don't think this is practical.

Still, I decided to report this surprising behavior in case someone here has any ideas on this.

Notes

No response

Version

cargo 1.80.0-nightly (7a6fad098 2024-05-31)
release: 1.80.0-nightly
commit-hash: 7a6fad0984d28c8330974636972aa296b67c4513
commit-date: 2024-05-31
host: x86_64-unknown-linux-gnu
libgit2: 1.7.2 (sys:0.18.3 vendored)
libcurl: 8.6.0-DEV (sys:0.4.72+curl-8.6.0 vendored ssl:OpenSSL/1.1.1w)
ssl: OpenSSL 1.1.1w  11 Sep 2023
os: Arch Linux [64-bit]
epage commented 4 months ago

To confirm, this isn't just about feature unification which would make it a duplicate of #4463 but of finding ways to improve the reporting to help catch problems covered up by feature unification, right?

Increasing the interactions between rustc and cargo to help report this is reminiscent of

I believe to do this we'd need the rmeta to track what cfg's were applied to an item and its path and Cargo would need to tell rustc what cfg's would apply to an extern item without unification present.

Maybe I'm missing something but I have the feeling this would be a lot of work to implement, potentially slow things down, and be inaccurate to the point that it couldn't be a rustc lint but a clippy nursery lint.

There are also third party solutions like cargo hack to more exhaustively catch problems like this. I wonder if continuing to recommend those is the best path forward.