Skip to content

More precise dependency list #210

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

Draft
wants to merge 22 commits into
base: master
Choose a base branch
from
Draft

More precise dependency list #210

wants to merge 22 commits into from

Conversation

Shnatsel
Copy link
Member

cargo metadata's feature unification across all dependency types, including dev-dependencies, causes it to over-report the dependency graph in certain cases. This PR works around that, fixing the long-standing issue #66

Comment on lines +170 to +180
fn parse_cargo_tree_line(line: &str) -> CargoTreePkg {
let parts: Vec<&str> = line.split_ascii_whitespace().collect();
let name = parts[0].to_owned();
let version_str = parts[1];
let version_str = version_str
.strip_prefix("v")
.expect("Failed to parse version string \"{version_str}\": does not start with 'v'");
// Technically this can fail because crates.io didn't always enforce semver validity, and crates with invalid semver exist.
// But since we're relying on the cargo_metadata crate that also parses semver, in those cases we're screwed regardless.
let version = cargo_metadata::semver::Version::parse(version_str).unwrap();
CargoTreePkg { name, version }
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To better prepare for a LTS Linux distribution having an old cargo-auditable (which is mentioned in another fn docstring in this PR), it might be worthwhile adding a few ways to avoid problems if cargo tree .. output changes in a way that could break cargo-auditable .

  1. Don't use cargo tree for binaries that are not in a workspace, as I assume that is the only time when cargo tree is beneficial.
  2. Have a command line flag or envvar flag to turn off using cargo tree.
  3. This function and its callers ripple up any parsing errors (including parts[1]) in a Result, and have a command line flag or envvar flag to allow the user to choose whether to fail or emit warnings when this parser fails.

2 & 3 could be the same config item, likeCARGO_AUDITABLE_USE_CARGO_TREE=off,on,warn

I personally dont mind if these protections are not done. Just ideas to help get this feature across the line.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It actually helps with more than just workspaces. cargo metadata also does somewhat incorrect platform resolution, e.g. with rustix depending on errno on Linux according to cargo metadata but not cargo tree unless you use cargo tree --platforms=all, so it needs to be enabled unconditionally.

@jayvdb
Copy link

jayvdb commented Jun 17, 2025

@Shnatsel , what is the next step with this? How can I help?

@Shnatsel
Copy link
Member Author

The path to shipping this looks like this:

  1. Make parsing cargo tree return a Result instead of panicking. Print a warning and continue if it fails.
  2. Add a rev: 2 field to the generated JSON. It will be a marker that specifies that the dependency tree is accurate. Write it only if parsing cargo tree succeeds.
  3. Do a bunch of housekeeping around adding that field: bump semver where appropriate (in auditable-serde and everything that depends on it), regenerate the JSON schema, anything else that comes up.

You can help by cloning this branch, doing the things from this list and opening a PR with the changes.

@Shnatsel
Copy link
Member Author

Shnatsel commented Jun 27, 2025

We've discovered a significant issue with this approach which is documented in #216, I've updated the tests to demonstrate it

@Shnatsel
Copy link
Member Author

I believe it will be possible to work around by passing through the working directory as well as flags -p/--package, --workspace, --exclude and --all from the original cargo auditable build call on to cargo tree. This will add a nontrivial amount of complexity, but it should be manageable since we already have all the required infrastructure in place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants