Skip to content
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

include version requirements in path for resolver error messages #5452

Closed
wants to merge 4 commits into from
Closed
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
4 changes: 3 additions & 1 deletion src/bin/cargo/command_prelude.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,9 @@ pub trait AppExt: Sized {
}

fn arg_target_dir(self) -> Self {
self._arg(opt("target-dir", "Directory for all generated artifacts").value_name("DIRECTORY"))
self._arg(
opt("target-dir", "Directory for all generated artifacts").value_name("DIRECTORY"),
)
}

fn arg_manifest_path(self) -> Self {
Expand Down
7 changes: 4 additions & 3 deletions src/bin/cargo/commands/new.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
} else {
path
};
config
.shell()
.status("Created", format!("{} `{}` project", opts.kind, project_name))?;
config.shell().status(
"Created",
format!("{} `{}` project", opts.kind, project_name),
)?;
Ok(())
}
14 changes: 8 additions & 6 deletions src/cargo/core/compiler/build_context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,16 +103,18 @@ impl<'a, 'cfg> BuildContext<'a, 'cfg> {
};

let crate_name = dep.target.crate_name();
let mut names = deps.iter()
.map(|d| d.rename().unwrap_or(&crate_name));
let mut names = deps.iter().map(|d| d.rename().unwrap_or(&crate_name));
let name = names.next().unwrap_or(&crate_name);
for n in names {
if n == name {
continue
continue;
}
bail!("multiple dependencies listed for the same crate must \
all have the same name, but the dependency on `{}` \
is listed as having different names", dep.pkg.package_id());
bail!(
"multiple dependencies listed for the same crate must \
all have the same name, but the dependency on `{}` \
is listed as having different names",
dep.pkg.package_id()
);
}
Ok(name.to_string())
}
Expand Down
15 changes: 11 additions & 4 deletions src/cargo/core/compiler/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -479,10 +479,17 @@ impl<'a> Links<'a> {
let pkg = unit.pkg.package_id();

let describe_path = |pkgid: &PackageId| -> String {
let dep_path = resolve.path_to_top(pkgid);
let mut dep_path_desc = format!("package `{}`", dep_path[0]);
for dep in dep_path.iter().skip(1) {
write!(dep_path_desc, "\n ... which is depended on by `{}`", dep).unwrap();
let dep_path = resolve.graph().path_to_top(pkgid);
let mut dep_path_desc = format!("package `{}`", pkgid);
for &(dep, req) in dep_path.iter() {
let req = req.first().unwrap();
write!(
dep_path_desc,
"\n ... selected to fulfill the requirement \
\"{}\" from package `{}`",
req.version_req(),
dep
).unwrap();
}
dep_path_desc
};
Expand Down
11 changes: 9 additions & 2 deletions src/cargo/core/compiler/fingerprint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use util::errors::{CargoResult, CargoResultExt};
use util::paths;
use util::{internal, profile, Dirty, Fresh, Freshness};

use super::{Context, BuildContext, FileFlavor, Unit};
use super::{BuildContext, Context, FileFlavor, Unit};
use super::custom_build::BuildDeps;
use super::job::Work;

Expand Down Expand Up @@ -355,7 +355,14 @@ impl hash::Hash for Fingerprint {
..
} = *self;
(
rustc, features, target, path, profile, local, edition, rustflags,
rustc,
features,
target,
path,
profile,
local,
edition,
rustflags,
).hash(h);

h.write_usize(deps.len());
Expand Down
38 changes: 21 additions & 17 deletions src/cargo/core/resolver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -401,13 +401,7 @@ fn activate_deps_loop(
dep.name(),
candidate.summary.version()
);
let res = activate(
&mut cx,
registry,
Some((&parent, &dep)),
candidate,
&method,
);
let res = activate(&mut cx, registry, Some((&parent, &dep)), candidate, &method);

let successfully_activated = match res {
// Success! We've now activated our `candidate` in our context
Expand Down Expand Up @@ -871,7 +865,7 @@ fn activation_error(
if !candidates.is_empty() {
let mut msg = format!("failed to select a version for `{}`.", dep.name());
msg.push_str("\n ... required by ");
msg.push_str(&describe_path(&graph.path_to_top(parent.package_id())));
msg.push_str(&describe_path(&graph, parent.package_id()));

msg.push_str("\nversions that meet the requirements `");
msg.push_str(&dep.version_req().to_string());
Expand Down Expand Up @@ -900,7 +894,7 @@ fn activation_error(
msg.push_str(link);
msg.push_str("` as well:\n");
}
msg.push_str(&describe_path(&graph.path_to_top(p)));
msg.push_str(&describe_path(&graph, p));
}

let (features_errors, other_errors): (Vec<_>, Vec<_>) = other_errors
Expand Down Expand Up @@ -931,7 +925,7 @@ fn activation_error(

for &(p, _) in other_errors.iter() {
msg.push_str("\n\n previously selected ");
msg.push_str(&describe_path(&graph.path_to_top(p)));
msg.push_str(&describe_path(&graph, p));
}

msg.push_str("\n\nfailed to select a version for `");
Expand Down Expand Up @@ -982,7 +976,7 @@ fn activation_error(
versions
);
msg.push_str("required by ");
msg.push_str(&describe_path(&graph.path_to_top(parent.package_id())));
msg.push_str(&describe_path(&graph, parent.package_id()));

// If we have a path dependency with a locked version, then this may
// indicate that we updated a sub-package and forgot to run `cargo
Expand All @@ -1003,7 +997,7 @@ fn activation_error(
dep.source_id()
);
msg.push_str("required by ");
msg.push_str(&describe_path(&graph.path_to_top(parent.package_id())));
msg.push_str(&describe_path(&graph, parent.package_id()));

msg
};
Expand All @@ -1023,11 +1017,21 @@ fn activation_error(
}

/// Returns String representation of dependency chain for a particular `pkgid`.
fn describe_path(path: &[&PackageId]) -> String {
fn describe_path(
graph: &::util::graph::Graph<PackageId, Vec<Dependency>>,
this: &PackageId,
) -> String {
use std::fmt::Write;
let mut dep_path_desc = format!("package `{}`", path[0]);
for dep in path[1..].iter() {
write!(dep_path_desc, "\n ... which is depended on by `{}`", dep).unwrap();
let path: &[(&PackageId, &Vec<Dependency>)] = &graph.path_to_top(this);
let mut dep_path_desc = format!("package `{}`", this);
for &(dep, req) in path.iter() {
let req = req.first().unwrap();
write!(
dep_path_desc,
"\n ... selected to fulfill the requirement \"{}\" from package `{}`",
req.version_req(),
dep
).unwrap();
}
dep_path_desc
}
Expand Down Expand Up @@ -1061,7 +1065,7 @@ fn check_cycles(resolve: &Resolve, activations: &Activations) -> CargoResult<()>
bail!(
"cyclic package dependency: package `{}` depends on itself. Cycle:\n{}",
id,
describe_path(&resolve.path_to_top(id))
describe_path(&resolve.graph(), id)
);
}

Expand Down
10 changes: 4 additions & 6 deletions src/cargo/core/resolver/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,6 @@ impl Resolve {
}
}

/// Resolves one of the paths from the given dependent package up to
/// the root.
pub fn path_to_top<'a>(&'a self, pkg: &'a PackageId) -> Vec<&'a PackageId> {
self.graph.path_to_top(pkg)
}

pub fn register_used_patches(&mut self, patches: &HashMap<Url, Vec<Summary>>) {
for summary in patches.values().flat_map(|v| v) {
if self.iter().any(|id| id == summary.package_id()) {
Expand Down Expand Up @@ -187,6 +181,10 @@ unable to verify that `{0}` is the same as when the lockfile was generated
&self.replacements
}

pub fn graph(&self) -> &Graph<PackageId, Vec<Dependency>> {
&self.graph
}

pub fn features(&self, pkg: &PackageId) -> &HashSet<String> {
self.features.get(pkg).unwrap_or(&self.empty_features)
}
Expand Down
6 changes: 4 additions & 2 deletions src/cargo/core/summary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,8 @@ fn build_feature_map(
use self::FeatureValue::*;
let mut dep_map = HashMap::new();
for dep in dependencies.iter() {
dep_map.entry(dep.name().as_str())
dep_map
.entry(dep.name().as_str())
.or_insert(Vec::new())
.push(dep);
}
Expand Down Expand Up @@ -198,7 +199,8 @@ fn build_feature_map(
}
}
};
let is_optional_dep = dep_data.iter()
let is_optional_dep = dep_data
.iter()
.flat_map(|d| d.iter())
.any(|d| d.is_optional());
if let FeatureValue::Crate(ref dep_name) = val {
Expand Down
36 changes: 18 additions & 18 deletions src/cargo/ops/cargo_fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,25 +34,25 @@ pub fn fetch<'a>(
}

packages.get(id)?;
let deps = resolve.deps(id)
let deps = resolve
.deps(id)
.filter(|&(_id, deps)| {
deps.iter()
.any(|d| {
// If no target was specified then all dependencies can
// be fetched.
let target = match options.target {
Some(ref t) => t,
None => return true,
};
// If this dependency is only available for certain
// platforms, make sure we're only fetching it for that
// platform.
let platform = match d.platform() {
Some(p) => p,
None => return true,
};
platform.matches(target, target_info.cfg())
})
deps.iter().any(|d| {
// If no target was specified then all dependencies can
// be fetched.
let target = match options.target {
Some(ref t) => t,
None => return true,
};
// If this dependency is only available for certain
// platforms, make sure we're only fetching it for that
// platform.
let platform = match d.platform() {
Some(p) => p,
None => return true,
};
platform.matches(target, target_info.cfg())
})
})
.map(|(id, _deps)| id);
deps_to_fetch.extend(deps);
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/sources/registry/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ impl<'cfg> RegistryIndex<'cfg> {
Err(e) => {
info!("failed to parse `{}` registry package: {}", name, e);
trace!("line: {}", line);
return None
return None;
}
};
if online || load.is_crate_downloaded(summary.package_id()) {
Expand Down
13 changes: 7 additions & 6 deletions src/cargo/util/graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,24 +73,25 @@ impl<N: Eq + Hash + Clone, E: Default> Graph<N, E> {

/// Resolves one of the paths from the given dependent package up to
/// the root.
pub fn path_to_top<'a>(&'a self, mut pkg: &'a N) -> Vec<&'a N> {
pub fn path_to_top<'a>(&'a self, mut pkg: &'a N) -> Vec<(&'a N, &'a E)> {
// Note that this implementation isn't the most robust per se, we'll
// likely have to tweak this over time. For now though it works for what
// it's used for!
let mut result = vec![pkg];
let first_pkg_depending_on = |pkg: &N, res: &[&N]| {
let mut result: Vec<(&'a N, &'a E)> = vec![];
let first_pkg_depending_on = |pkg: &N, res: &[(&'a N, &'a E)]| {
self.nodes
.iter()
.filter(|&(_node, adjacent)| adjacent.contains_key(pkg))
// Note that we can have "cycles" introduced through dev-dependency
// edges, so make sure we don't loop infinitely.
.filter(|&(node, _)| !res.contains(&node))
.filter(|&(node, _)| res.iter().find(|p| p.0 == node).is_none())
// TODO: find_map would be clearer
.next()
.map(|p| p.0)
.map(|(node, adjacent)| (node, adjacent.get(pkg).unwrap()))
};
while let Some(p) = first_pkg_depending_on(pkg, &result) {
result.push(p);
pkg = p;
pkg = p.0;
}
result
}
Expand Down
9 changes: 8 additions & 1 deletion src/cargo/util/toml/targets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,14 @@ fn clean_targets_with_legacy_path(
validate_unique_names(&toml_targets, target_kind)?;
let mut result = Vec::new();
for target in toml_targets {
let path = target_path(&target, inferred, target_kind, package_root, edition, legacy_path);
let path = target_path(
&target,
inferred,
target_kind,
package_root,
edition,
legacy_path,
);
let path = match path {
Ok(path) => path,
Err(e) => {
Expand Down
4 changes: 1 addition & 3 deletions tests/testsuite/alt_registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -419,9 +419,7 @@ fn alt_registry_and_crates_io_deps() {
.with_stderr_contains("[COMPILING] alt_reg_dep v0.1.0 (registry `file://[..]`)")
.with_stderr_contains("[COMPILING] crates_io_dep v0.0.1")
.with_stderr_contains(&format!("[COMPILING] foo v0.0.1 ({})", p.url()))
.with_stderr_contains(
"[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]s",
),
.with_stderr_contains("[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]s"),
)
}

Expand Down
Loading