Skip to content

Commit

Permalink
Auto merge of #8269 - ehuss:tree-all-targets, r=alexcrichton
Browse files Browse the repository at this point in the history
Fix panic with `cargo tree --target=all -Zfeatures=all`

When `cargo tree --target=all` was used with the new feature resolver, this would cause a panic because the feature resolver doesn't know about the "all" behavior, and would filter out packages that don't match.

I don't feel like this is a particularly elegant solution, but I'm uncertain of how to make it better.

Closes #8109
  • Loading branch information
bors committed May 23, 2020
2 parents daf7f2f + d267fac commit 40d566d
Show file tree
Hide file tree
Showing 9 changed files with 61 additions and 5 deletions.
1 change: 1 addition & 0 deletions src/cargo/core/compiler/standard_lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ pub fn resolve_std<'cfg>(
&opts,
&specs,
HasDevUnits::No,
crate::core::resolver::features::ForceAllTargets::No,
)?;
Ok((
resolve.pkg_set,
Expand Down
19 changes: 17 additions & 2 deletions src/cargo/core/resolver/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,13 @@ pub enum HasDevUnits {
No,
}

/// Flag to indicate that target-specific filtering should be disabled.
#[derive(Copy, Clone, PartialEq)]
pub enum ForceAllTargets {
Yes,
No,
}

/// Flag to indicate if features are requested for a build dependency or not.
#[derive(Copy, Clone, Debug, PartialEq)]
pub enum FeaturesFor {
Expand All @@ -110,7 +117,11 @@ impl FeaturesFor {
}

impl FeatureOpts {
fn new(ws: &Workspace<'_>, has_dev_units: HasDevUnits) -> CargoResult<FeatureOpts> {
fn new(
ws: &Workspace<'_>,
has_dev_units: HasDevUnits,
force_all_targets: ForceAllTargets,
) -> CargoResult<FeatureOpts> {
let mut opts = FeatureOpts::default();
let unstable_flags = ws.config().cli_unstable();
opts.package_features = unstable_flags.package_features;
Expand Down Expand Up @@ -155,6 +166,9 @@ impl FeatureOpts {
// Dev deps cannot be decoupled when they are in use.
opts.decouple_dev_deps = false;
}
if let ForceAllTargets::Yes = force_all_targets {
opts.ignore_inactive_targets = false;
}
Ok(opts)
}
}
Expand Down Expand Up @@ -269,11 +283,12 @@ impl<'a, 'cfg> FeatureResolver<'a, 'cfg> {
specs: &[PackageIdSpec],
requested_targets: &[CompileKind],
has_dev_units: HasDevUnits,
force_all_targets: ForceAllTargets,
) -> CargoResult<ResolvedFeatures> {
use crate::util::profile;
let _p = profile::start("resolve features");

let opts = FeatureOpts::new(ws, has_dev_units)?;
let opts = FeatureOpts::new(ws, has_dev_units, force_all_targets)?;
if !opts.new_resolver {
// Legacy mode.
return Ok(ResolvedFeatures {
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/core/resolver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ use self::types::{FeaturesSet, RcVecIter, RemainingDeps, ResolverProgress};
pub use self::encode::Metadata;
pub use self::encode::{EncodableDependency, EncodablePackageId, EncodableResolve};
pub use self::errors::{ActivateError, ActivateResult, ResolveError};
pub use self::features::HasDevUnits;
pub use self::features::{ForceAllTargets, HasDevUnits};
pub use self::resolve::{Resolve, ResolveVersion};
pub use self::types::{ResolveBehavior, ResolveOpts};

Expand Down
1 change: 1 addition & 0 deletions src/cargo/ops/cargo_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,7 @@ pub fn create_bcx<'a, 'cfg>(
&opts,
&specs,
has_dev_units,
crate::core::resolver::features::ForceAllTargets::No,
)?;
let WorkspaceResolve {
mut pkg_set,
Expand Down
1 change: 1 addition & 0 deletions src/cargo/ops/cargo_doc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ pub fn doc(ws: &Workspace<'_>, options: &DocOptions) -> CargoResult<()> {
&opts,
&specs,
HasDevUnits::No,
crate::core::resolver::features::ForceAllTargets::No,
)?;

let ids = ws_resolve.targeted_resolve.specs_to_ids(&specs)?;
Expand Down
1 change: 1 addition & 0 deletions src/cargo/ops/cargo_output_metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ fn build_resolve_graph(
&resolve_opts,
&specs,
HasDevUnits::Yes,
crate::core::resolver::features::ForceAllTargets::No,
)?;
// Download all Packages. This is needed to serialize the information
// for every package. In theory this could honor target filtering,
Expand Down
4 changes: 3 additions & 1 deletion src/cargo/ops/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
use crate::core::compiler::{CompileKind, RustcTargetData};
use crate::core::registry::PackageRegistry;
use crate::core::resolver::features::{FeatureResolver, ResolvedFeatures};
use crate::core::resolver::features::{FeatureResolver, ForceAllTargets, ResolvedFeatures};
use crate::core::resolver::{self, HasDevUnits, Resolve, ResolveOpts};
use crate::core::summary::Summary;
use crate::core::Feature;
Expand Down Expand Up @@ -79,6 +79,7 @@ pub fn resolve_ws_with_opts<'cfg>(
opts: &ResolveOpts,
specs: &[PackageIdSpec],
has_dev_units: HasDevUnits,
force_all_targets: ForceAllTargets,
) -> CargoResult<WorkspaceResolve<'cfg>> {
let mut registry = PackageRegistry::new(ws.config())?;
let mut add_patches = true;
Expand Down Expand Up @@ -148,6 +149,7 @@ pub fn resolve_ws_with_opts<'cfg>(
specs,
requested_targets,
has_dev_units,
force_all_targets,
)?;

Ok(WorkspaceResolve {
Expand Down
8 changes: 7 additions & 1 deletion src/cargo/ops/tree/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
use self::format::Pattern;
use crate::core::compiler::{CompileKind, RustcTargetData};
use crate::core::dependency::DepKind;
use crate::core::resolver::{HasDevUnits, ResolveOpts};
use crate::core::resolver::{ForceAllTargets, HasDevUnits, ResolveOpts};
use crate::core::{Package, PackageId, PackageIdSpec, Workspace};
use crate::ops::{self, Packages};
use crate::util::{CargoResult, Config};
Expand Down Expand Up @@ -150,13 +150,19 @@ pub fn build_and_print(ws: &Workspace<'_>, opts: &TreeOptions) -> CargoResult<()
} else {
HasDevUnits::No
};
let force_all = if opts.target == Target::All {
ForceAllTargets::Yes
} else {
ForceAllTargets::No
};
let ws_resolve = ops::resolve_ws_with_opts(
ws,
&target_data,
&requested_kinds,
&resolve_opts,
&specs,
has_dev,
force_all,
)?;
// Download all Packages. Some display formats need to display package metadata.
let package_map: HashMap<PackageId, &Package> = ws_resolve
Expand Down
29 changes: 29 additions & 0 deletions tests/testsuite/features2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1720,3 +1720,32 @@ resolver = "2"
&[("Cargo.toml", &rewritten_toml)],
);
}

#[cargo_test]
fn tree_all() {
// `cargo tree` with the new feature resolver.
Package::new("log", "0.4.8").feature("serde", &[]).publish();
let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.1.0"
[target.'cfg(whatever)'.dependencies]
log = {version="*", features=["serde"]}
"#,
)
.file("src/lib.rs", "")
.build();
p.cargo("tree --target=all -Zfeatures=all")
.masquerade_as_nightly_cargo()
.with_stdout(
"\
foo v0.1.0 ([..]/foo)
└── log v0.4.8
",
)
.run();
}

0 comments on commit 40d566d

Please sign in to comment.