Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/cargo/ops/tree/format/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ enum Chunk {
Repository,
Features,
LibName,
VersionRequirement,
}

pub struct Pattern(Vec<Chunk>);
Expand All @@ -30,6 +31,7 @@ impl Pattern {
RawChunk::Argument("r") => Chunk::Repository,
RawChunk::Argument("f") => Chunk::Features,
RawChunk::Argument("lib") => Chunk::LibName,
RawChunk::Argument("ver-req") => Chunk::VersionRequirement,
RawChunk::Argument(a) => {
bail!("unsupported pattern `{}`", a);
}
Expand Down Expand Up @@ -111,6 +113,13 @@ impl<'a> fmt::Display for Display<'a> {
write!(fmt, "{}", target.crate_name())?;
}
}
Chunk::VersionRequirement => {
if let Some(version_req) =
self.graph.version_req_for_id(package.package_id())
{
write!(fmt, "{}", version_req)?;
}
}
}
}
}
Expand Down
122 changes: 118 additions & 4 deletions src/cargo/ops/tree/format/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
use std::iter;
use std::str;

#[derive(Debug, PartialEq, Eq)]
pub enum RawChunk<'a> {
/// Raw text to include in the output.
Text(&'a str),
Expand All @@ -23,9 +24,9 @@ pub enum RawChunk<'a> {
/// (and optionally source), and the `{l}` will be the license from
/// `Cargo.toml`.
///
/// Substitutions are alphabetic characters between curly braces, like `{p}`
/// or `{foo}`. The actual interpretation of these are done in the `Pattern`
/// struct.
/// Substitutions are alphabetic characters or hyphens between curly braces,
/// like `{p}`, {foo} or `{bar-baz}`. The actual interpretation of these is
/// done in the `Pattern` struct.
///
/// Bare curly braces can be included in the output with double braces like
/// `{{` will include a single `{`, similar to Rust's format strings.
Expand Down Expand Up @@ -67,7 +68,7 @@ impl<'a> Parser<'a> {

loop {
match self.it.peek() {
Some(&(_, ch)) if ch.is_alphanumeric() => {
Some(&(_, ch)) if ch.is_alphanumeric() || ch == '-' => {
self.it.next();
}
Some(&(end, _)) => return &self.s[start..end],
Expand Down Expand Up @@ -121,3 +122,116 @@ impl<'a> Iterator for Parser<'a> {
}
}
}

#[cfg(test)]
mod tests {
use super::{Parser, RawChunk};

#[test]
fn plain_text() {
let chunks: Vec<_> = Parser::new("Hello World").collect();
assert_eq!(chunks, vec![RawChunk::Text("Hello World")]);
}

#[test]
fn basic_argument() {
let chunks: Vec<_> = Parser::new("{pkg}").collect();
assert_eq!(chunks, vec![RawChunk::Argument("pkg")]);
}

#[test]
fn mixed_content() {
let chunks: Vec<_> = Parser::new("Package {p} version:{v}").collect();
assert_eq!(
chunks,
vec![
RawChunk::Text("Package "),
RawChunk::Argument("p"),
RawChunk::Text(" version:"),
RawChunk::Argument("v"),
]
);
}

#[test]
fn escaped_braces() {
let chunks: Vec<_> = Parser::new("{{text}} in {{braces}}").collect();
assert_eq!(
chunks,
vec![
RawChunk::Text("{"),
RawChunk::Text("text"),
RawChunk::Text("}"),
RawChunk::Text(" in "),
RawChunk::Text("{"),
RawChunk::Text("braces"),
RawChunk::Text("}"),
]
);
}

#[test]
fn hyphenated_argument() {
let chunks: Vec<_> = Parser::new("{foo-bar}").collect();
assert_eq!(chunks, vec![RawChunk::Argument("foo-bar")]);
}

#[test]
fn unclosed_brace() {
let chunks: Vec<_> = Parser::new("{unclosed").collect();
assert_eq!(chunks, vec![RawChunk::Error("expected '}'")])
}

#[test]
fn unexpected_close_brace() {
let chunks: Vec<_> = Parser::new("unexpected}").collect();
assert_eq!(
chunks,
vec![
RawChunk::Text("unexpected"),
RawChunk::Error("unexpected '}'"),
]
);
}

#[test]
fn empty_argument() {
let chunks: Vec<_> = Parser::new("{}").collect();
assert_eq!(chunks, vec![RawChunk::Argument("")]);
}

#[test]
fn invalid_argument_chars() {
let chunks: Vec<_> = Parser::new("{a-b} {123}").collect();
assert_eq!(
chunks,
vec![
RawChunk::Argument("a-b"),
RawChunk::Text(" "),
RawChunk::Error("expected '}'"),
]
);
}

#[test]
fn complex_format() {
let format = "Pkg{{name}}: {p} [{v}] (License: {l})";
let chunks: Vec<_> = Parser::new(format).collect();
assert_eq!(
chunks,
vec![
RawChunk::Text("Pkg"),
RawChunk::Text("{"),
RawChunk::Text("name"),
RawChunk::Text("}"),
RawChunk::Text(": "),
RawChunk::Argument("p"),
RawChunk::Text(" ["),
RawChunk::Argument("v"),
RawChunk::Text("] (License: "),
RawChunk::Argument("l"),
RawChunk::Text(")"),
]
);
}
}
17 changes: 16 additions & 1 deletion src/cargo/ops/tree/graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ use crate::core::dependency::DepKind;
use crate::core::resolver::Resolve;
use crate::core::resolver::features::{CliFeatures, FeaturesFor, ResolvedFeatures};
use crate::core::{FeatureMap, FeatureValue, Package, PackageId, PackageIdSpec, Workspace};
use crate::util::CargoResult;
use crate::util::interning::{INTERNED_DEFAULT, InternedString};
use crate::util::{CargoResult, OptVersionReq};
use std::collections::{HashMap, HashSet};

#[derive(Debug, Copy, Clone)]
Expand Down Expand Up @@ -161,6 +161,8 @@ pub struct Graph<'a> {
/// Key is the index of a package node, value is a map of `dep_name` to a
/// set of `(pkg_node_index, is_optional)`.
dep_name_map: HashMap<NodeId, HashMap<InternedString, HashSet<(NodeId, bool)>>>,
/// Map for looking up version requirements for dependency packages.
version_req_map: HashMap<PackageId, OptVersionReq>,
}

impl<'a> Graph<'a> {
Expand All @@ -172,6 +174,7 @@ impl<'a> Graph<'a> {
package_map,
cli_features: HashSet::new(),
dep_name_map: HashMap::new(),
version_req_map: HashMap::new(),
}
}

Expand Down Expand Up @@ -240,6 +243,12 @@ impl<'a> Graph<'a> {
}
}

/// Returns the version requirement for the given package ID. Returns `None`
/// if no version requirement is recorded (e.g., root packages).
pub fn version_req_for_id(&self, package_id: PackageId) -> Option<&OptVersionReq> {
self.version_req_map.get(&package_id)
}

/// Returns `true` if the given feature node index is a feature enabled
/// via the command-line.
pub fn is_cli_feature(&self, index: NodeId) -> bool {
Expand Down Expand Up @@ -523,6 +532,12 @@ fn add_pkg(
requested_kind,
opts,
);
// Store the version requirement for this dependency for
// later use in formatting.
graph.version_req_map.insert(
graph.package_id_for_index(dep_index),
dep.version_req().clone(),
);
let new_edge = Edge {
kind: EdgeKind::Dep(dep.kind()),
node: dep_index,
Expand Down
1 change: 1 addition & 0 deletions src/doc/man/cargo-tree.md
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ strings will be replaced with the corresponding value:
- `{r}` --- The package repository URL.
- `{f}` --- Comma-separated list of package features that are enabled.
- `{lib}` --- The name, as used in a `use` statement, of the package's library.
- `{ver-req}` --- The version requirement resolved for the package.
{{/option}}

{{#option "`--prefix` _prefix_" }}
Expand Down
2 changes: 2 additions & 0 deletions src/doc/man/generated_txt/cargo-tree.txt
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,8 @@ OPTIONS
o {lib} — The name, as used in a use statement, of the
package’s library.

o {ver-req} — The version requirement resolved for the package.

--prefix prefix
Sets how each line is displayed. The prefix value can be one of:

Expand Down
1 change: 1 addition & 0 deletions src/doc/src/commands/cargo-tree.md
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ strings will be replaced with the corresponding value:</p>
<li><code>{r}</code> — The package repository URL.</li>
<li><code>{f}</code> — Comma-separated list of package features that are enabled.</li>
<li><code>{lib}</code> — The name, as used in a <code>use</code> statement, of the package’s library.</li>
<li><code>{ver-req}</code> — The version requirement resolved for the package.</li>
</ul>
</dd>

Expand Down
4 changes: 4 additions & 0 deletions src/etc/man/cargo-tree.1
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,10 @@ strings will be replaced with the corresponding value:
.RS 4
\h'-04'\(bu\h'+03'\fB{lib}\fR \[em] The name, as used in a \fBuse\fR statement, of the package\[cq]s library.
.RE
.sp
.RS 4
\h'-04'\(bu\h'+03'\fB{ver\-req}\fR \[em] The version requirement resolved for the package.
.RE
.RE
.sp
\fB\-\-prefix\fR \fIprefix\fR
Expand Down
46 changes: 42 additions & 4 deletions tests/testsuite/cargo_tree/deps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1128,7 +1128,10 @@ foo v0.1.0 ([ROOT]/foo)
#[cargo_test]
fn format() {
Package::new("dep", "1.0.0").publish();
Package::new("other-dep", "1.0.0").publish();
Package::new("dep", "2.0.0").publish();
Package::new("other-dep", "1.0.0")
.dep("dep", "^1.0")
.publish();

Package::new("dep_that_is_awesome", "1.0.0")
.file(
Expand All @@ -1140,6 +1143,10 @@ fn format() {

[lib]
name = "awesome_dep"

[dependencies]
dep1 = {package="dep", version="<2.0"}
dep2 = {package="dep", version="2.0"}
"#,
)
.file("src/lib.rs", "pub struct Straw;")
Expand All @@ -1156,9 +1163,9 @@ fn format() {
repository = "https://github.com/rust-lang/cargo"

[dependencies]
dep = {version="1.0", optional=true}
dep = {version="=1.0"}
Copy link
Contributor

Choose a reason for hiding this comment

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

This is removing coverage.

I suspect the test changes here should be purely additive rather than mutating existing stuff.

Copy link
Contributor

Choose a reason for hiding this comment

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

I also question our having a single format test and wonder if we should break out your new cases into a separate test or even multiple tests

Copy link
Author

Choose a reason for hiding this comment

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

Yes that's a good callout, I can work on splitting this out.

other-dep = {version="1.0", optional=true}
dep_that_is_awesome = {version="1.0", optional=true}
dep_that_is_awesome = {version=">=1.0, <2", optional=true}


[features]
Expand All @@ -1173,6 +1180,7 @@ fn format() {
p.cargo("tree --format <<<{p}>>>")
.with_stdout_data(str![[r#"
<<<foo v0.1.0 ([ROOT]/foo)>>>
└── <<<dep v1.0.0>>>

"#]])
.run();
Expand All @@ -1191,6 +1199,7 @@ Caused by:
p.cargo("tree --format {p}-{{hello}}")
.with_stdout_data(str![[r#"
foo v0.1.0 ([ROOT]/foo)-{hello}
└── dep v1.0.0-{hello}

"#]])
.run();
Expand All @@ -1199,6 +1208,7 @@ foo v0.1.0 ([ROOT]/foo)-{hello}
.arg("{p} {l} {r}")
.with_stdout_data(str![[r#"
foo v0.1.0 ([ROOT]/foo) MIT https://github.com/rust-lang/cargo
└── dep v1.0.0

"#]])
.run();
Expand All @@ -1207,17 +1217,19 @@ foo v0.1.0 ([ROOT]/foo) MIT https://github.com/rust-lang/cargo
.arg("{p} {f}")
.with_stdout_data(str![[r#"
foo v0.1.0 ([ROOT]/foo) bar,default,foo
└── dep v1.0.0

"#]])
.run();

p.cargo("tree --all-features --format")
.arg("{p} [{f}]")
.with_stdout_data(str![[r#"
foo v0.1.0 ([ROOT]/foo) [bar,default,dep,dep_that_is_awesome,foo,other-dep]
foo v0.1.0 ([ROOT]/foo) [bar,default,dep_that_is_awesome,foo,other-dep]
├── dep v1.0.0 []
├── dep_that_is_awesome v1.0.0 []
└── other-dep v1.0.0 []
└── dep v1.0.0 []

"#]])
.run();
Expand All @@ -1227,8 +1239,34 @@ foo v0.1.0 ([ROOT]/foo) [bar,default,dep,dep_that_is_awesome,foo,other-dep]
.arg("--format={lib}")
.with_stdout_data(str![[r#"

├── dep
├── awesome_dep
└── other_dep
└── dep

"#]])
.run();

p.cargo("tree --all-features")
.arg("--format={p} satisfies {ver-req}")
.with_stdout_data(str![[r#"
foo v0.1.0 ([ROOT]/foo) satisfies
├── dep v1.0.0 satisfies ^1.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Am I misreading things? This says foo has ^1.0 when it instead has:

dep = {version="=1.0"}

Copy link
Author

Choose a reason for hiding this comment

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

You're right, this version requirement seems to belong to other-dep. I think I see what's happening. We don't consider the parent when registering the version requirement in the lookup map, so it's probably replacing previous entries for the same package?

I think using NodeID as index should make this unique.

Copy link
Contributor

Choose a reason for hiding this comment

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

As a forewarning, a problem I suspect we'll see when we have two different, compatible version requirements on the same package and --invert it is that we'll only see one of them.

version requirements are more of a property of edges and not nodes. We don't annotate edges. Is that good enough, a blocker for this feature, or a cause to re-think some base assumptions?

As an example of the latter, maybe for --invert, we put the version requirement on the dependent package and not the dependency.

├── dep_that_is_awesome v1.0.0 satisfies >=1.0, <2
└── other-dep v1.0.0 satisfies ^1.0
└── dep v1.0.0 satisfies ^1.0

"#]])
.run();

p.cargo("tree --all-features")
.arg("--format={p} satisfies {ver-req}")
.arg("--invert=dep")
.with_stdout_data(str![[r#"
dep v1.0.0 satisfies ^1.0
├── foo v0.1.0 ([ROOT]/foo) satisfies
└── other-dep v1.0.0 satisfies ^1.0
└── foo v0.1.0 ([ROOT]/foo) satisfies

"#]])
.run();
Expand Down