EmbarkStudios / krates

📦 Creates graphs of crates from cargo metadata 🦀
Apache License 2.0
58 stars 18 forks source link

`cargo metadata` incorrectly adds weak dependencies #41

Closed joshtriplett closed 2 years ago

joshtriplett commented 2 years ago

Describe the bug

cargo deny check bans errors out saying a banned package appears in dependencies, even if that package only appears in a weak dependency feature that isn't activated.

To Reproduce Steps to reproduce the behavior:

  1. Create a crate that depends on git2 version 0.14.4, with default-features = false, and features = ["zlib-ng-compat"].
  2. Run cargo tree and observe no libssh2-sys or openssl-sys dependency.
  3. Create a deny.toml that bans openssl-sys.
  4. Run cargo deny check bans, and get this output:
warning[B009]: direct parent 'libssh2-sys = 0.2.23' of banned crate 'openssl-sys = 0.9.73' was not marked as a wrapper
   ┌─ /path/to/project/deny.toml:30:14
   │
30 │     { name = "openssl-sys" },
   │              ^^^^^^^^^^^^^ banned here
   │
   = openssl-sys v0.9.73
     └── libssh2-sys v0.2.23
         └── libgit2-sys v0.13.4+1.4.2
             └── git2 v0.14.4
                 └── project v0.1.0

Expected behavior

cargo deny check bans should understand that there's no dependency on openssl-sys (or libssh2-sys) here.

Jake-Shadle commented 2 years ago

This actually appears to be a cargo bug with weak dependencies (related https://github.com/rust-lang/cargo/issues/10801). cargo-deny relies on the output of cargo metadata to know which dependencies were selected for each package based on the top level features selected, the output for weak dependencies is unfortunately wrong both for metadata and Cargo.lock

# Cargo.toml
[package]
name = "test"

[dependencies]
git2 = { version = "0.14", default-features = false, features = [
    "zlib-ng-compat",
] }
{
    "id": "libgit2-sys 0.13.4+1.4.2 (registry+https://github.com/rust-lang/crates.io-index)",
    "dependencies": [
        "cc 1.0.73 (registry+https://github.com/rust-lang/crates.io-index)",
        "libc 0.2.126 (registry+https://github.com/rust-lang/crates.io-index)",
        "libssh2-sys 0.2.23 (registry+https://github.com/rust-lang/crates.io-index)",
        "libz-sys 1.1.8 (registry+https://github.com/rust-lang/crates.io-index)",
        "pkg-config 0.3.25 (registry+https://github.com/rust-lang/crates.io-index)"
    ],
    "deps": [
        {
            "name": "cc",
            "pkg": "cc 1.0.73 (registry+https://github.com/rust-lang/crates.io-index)",
            "dep_kinds": [
                {
                    "kind": "build",
                    "target": null
                }
            ]
        },
        {
            "name": "libc",
            "pkg": "libc 0.2.126 (registry+https://github.com/rust-lang/crates.io-index)",
            "dep_kinds": [
                {
                    "kind": null,
                    "target": null
                }
            ]
        },
        {
            "name": "libssh2_sys",
            "pkg": "libssh2-sys 0.2.23 (registry+https://github.com/rust-lang/crates.io-index)",
            "dep_kinds": [
                {
                    "kind": null,
                    "target": null
                }
            ]
        },
        {
            "name": "libz_sys",
            "pkg": "libz-sys 1.1.8 (registry+https://github.com/rust-lang/crates.io-index)",
            "dep_kinds": [
                {
                    "kind": null,
                    "target": null
                }
            ]
        },
        {
            "name": "pkg_config",
            "pkg": "pkg-config 0.3.25 (registry+https://github.com/rust-lang/crates.io-index)",
            "dep_kinds": [
                {
                    "kind": "build",
                    "target": null
                }
            ]
        }
    ],
    "features": [
        "zlib-ng-compat"
    ]
},

Here we can see that even though the correct feature is enabled, the libssh2-sys dependency is still included even though it is weak and not explicitly enabled. While (at least according to the above cargo issue) cargo internally does make this an actual weak dependency and thus won't compile the tree rooted at libssh2-sys, and cargo tree prints out the expected tree, the same resolution logic doesn't seem to apply to cargo metadata.

Luckily, this bug can be worked around, but is not specific to cargo-deny itself, but rather the crate it uses to build the crate graph itself, so I'm transferring the issue there.

Jake-Shadle commented 2 years ago

Resolved by #42