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 16 commits into
base: master
Choose a base branch
from
Draft

More precise dependency list #210

wants to merge 16 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.

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.

2 participants