Skip to content

Commit

Permalink
Auto merge of #7820 - ehuss:features2-split, r=alexcrichton
Browse files Browse the repository at this point in the history
Add new feature resolver.

This adds a new resolver which handles feature unification independently of the main resolver. This can be enabled with the `-Zfeatures` flag which takes a comma-separated list of options to enable new behaviors. See `unstable.md` docs for details.

There are two significant behavior changes:

1. Ignore targets that are not enabled.
2. Do not unify features between build_deps, dev_deps, and normal deps.

The "forks" in the unit graph are handled by adding `DepKind` to `UnitFor`. The feature resolver tracks features independently for the different dependency kinds.

Unfortunately this currently does not support decoupling proc_macro dependencies. This is because at resolve time it does not know which dependencies are proc_macros. Moving feature resolution to after the packages are downloaded would require massive changes, and would make the unit computation much more complex. Nobody to my knowledge has requested this capability, presumably because proc_macros are relatively new, and they tend not to have very many dependencies, and those dependencies tend to be proc-macro specific (like syn/quote). I'd like to lean towards adding proc-macro to the index so that it can be known during resolve time, which would be much easier to implement, but with the downside of needing to add a new field to the index.

I did not update `cargo metadata`, yet. It's not really clear how it should behave. I think I'll need to investigate how people are currently using the feature information and figure out how it should work. Perhaps adding features to "dep_kinds" will be the solution, but I'm not sure.

The goal is to try to gather feedback about how well this new resolver works. There are two important things to check: whether it breaks a project, and how much does it increases compile time (since packages can be built multiple times with different features). I'd like to stabilize it one piece at a time assuming the disruption is not too great. If a project breaks or builds slower, the user can implement a backwards-compatible workaround of sprinkling additional features into `Cargo.toml` dependencies. I think `itarget` is a good candidate to try to stabilize first, since it is less likely to break things or change how things are built. If it does cause too much disruption, then I think we'll need to consider making it optional, enabled *somehow*.

There is an environment variable that can be set which forces Cargo to use the new feature resolver. This can be used in Cargo's own testsuite to explore which tests behave differently with the different features set.
  • Loading branch information
bors committed Feb 20, 2020
2 parents 369991b + 4d0fda7 commit d6fa260
Show file tree
Hide file tree
Showing 36 changed files with 2,304 additions and 416 deletions.
105 changes: 19 additions & 86 deletions src/cargo/core/compiler/build_context/mod.rs
Original file line number Diff line number Diff line change
@@ -1,19 +1,17 @@
use crate::core::compiler::unit::UnitInterner;
use crate::core::compiler::CompileTarget;
use crate::core::compiler::{BuildConfig, BuildOutput, CompileKind, Unit};
use crate::core::profiles::Profiles;
use crate::core::{Dependency, InternedString, Workspace};
use crate::core::{InternedString, Workspace};
use crate::core::{PackageId, PackageSet};
use crate::util::config::{Config, TargetConfig};
use crate::util::config::Config;
use crate::util::errors::CargoResult;
use crate::util::Rustc;
use cargo_platform::Cfg;
use std::collections::HashMap;
use std::path::PathBuf;
use std::str;

mod target_info;
pub use self::target_info::{FileFlavor, TargetInfo};
pub use self::target_info::{FileFlavor, RustcTargetData, TargetInfo};

/// The build context, containing all information about a build task.
///
Expand All @@ -30,26 +28,14 @@ pub struct BuildContext<'a, 'cfg> {
pub build_config: &'a BuildConfig,
/// Extra compiler args for either `rustc` or `rustdoc`.
pub extra_compiler_args: HashMap<Unit<'a>, Vec<String>>,
/// Package downloader.
pub packages: &'a PackageSet<'cfg>,

/// Source of interning new units as they're created.
pub units: &'a UnitInterner<'a>,

/// Information about the compiler that we've detected on the local system.
pub rustc: Rustc,

/// Build information for the "host", which is information about when
/// `rustc` is invoked without a `--target` flag. This is used for
/// procedural macros, build scripts, etc.
host_config: TargetConfig,
host_info: TargetInfo,

/// Build information for targets that we're building for. This will be
/// empty if the `--target` flag is not passed, and currently also only ever
/// has at most one entry, but eventually we'd like to support multi-target
/// builds with Cargo.
target_config: HashMap<CompileTarget, TargetConfig>,
target_info: HashMap<CompileTarget, TargetInfo>,
/// Information about rustc and the target platform.
pub target_data: RustcTargetData,
}

impl<'a, 'cfg> BuildContext<'a, 'cfg> {
Expand All @@ -61,90 +47,41 @@ impl<'a, 'cfg> BuildContext<'a, 'cfg> {
profiles: Profiles,
units: &'a UnitInterner<'a>,
extra_compiler_args: HashMap<Unit<'a>, Vec<String>>,
target_data: RustcTargetData,
) -> CargoResult<BuildContext<'a, 'cfg>> {
let rustc = config.load_global_rustc(Some(ws))?;

let host_config = config.target_cfg_triple(&rustc.host)?;
let host_info = TargetInfo::new(
config,
build_config.requested_kind,
&rustc,
CompileKind::Host,
)?;
let mut target_config = HashMap::new();
let mut target_info = HashMap::new();
if let CompileKind::Target(target) = build_config.requested_kind {
let tcfg = config.target_cfg_triple(target.short_name())?;
target_config.insert(target, tcfg);
target_info.insert(
target,
TargetInfo::new(
config,
build_config.requested_kind,
&rustc,
CompileKind::Target(target),
)?,
);
}

Ok(BuildContext {
ws,
packages,
config,
rustc,
target_config,
target_info,
host_config,
host_info,
build_config,
profiles,
extra_compiler_args,
units,
target_data,
})
}

/// Whether a dependency should be compiled for the host or target platform,
/// specified by `CompileKind`.
pub fn dep_platform_activated(&self, dep: &Dependency, kind: CompileKind) -> bool {
// If this dependency is only available for certain platforms,
// make sure we're only enabling it for that platform.
let platform = match dep.platform() {
Some(p) => p,
None => return true,
};
let name = kind.short_name(self);
platform.matches(name, self.cfg(kind))
pub fn rustc(&self) -> &Rustc {
&self.target_data.rustc
}

/// Gets the user-specified linker for a particular host or target.
pub fn linker(&self, kind: CompileKind) -> Option<PathBuf> {
self.target_config(kind)
self.target_data
.target_config(kind)
.linker
.as_ref()
.map(|l| l.val.clone().resolve_program(self.config))
}

/// Gets the list of `cfg`s printed out from the compiler for the specified kind.
pub fn cfg(&self, kind: CompileKind) -> &[Cfg] {
self.info(kind).cfg()
}

/// Gets the host architecture triple.
///
/// For example, x86_64-unknown-linux-gnu, would be
/// - machine: x86_64,
/// - hardware-platform: unknown,
/// - operating system: linux-gnu.
pub fn host_triple(&self) -> InternedString {
self.rustc.host
}

/// Gets the target configuration for a particular host or target.
pub fn target_config(&self, kind: CompileKind) -> &TargetConfig {
match kind {
CompileKind::Host => &self.host_config,
CompileKind::Target(s) => &self.target_config[&s],
}
self.target_data.rustc.host
}

/// Gets the number of jobs specified for this build.
Expand All @@ -153,24 +90,17 @@ impl<'a, 'cfg> BuildContext<'a, 'cfg> {
}

pub fn rustflags_args(&self, unit: &Unit<'_>) -> &[String] {
&self.info(unit.kind).rustflags
&self.target_data.info(unit.kind).rustflags
}

pub fn rustdocflags_args(&self, unit: &Unit<'_>) -> &[String] {
&self.info(unit.kind).rustdocflags
&self.target_data.info(unit.kind).rustdocflags
}

pub fn show_warnings(&self, pkg: PackageId) -> bool {
pkg.source_id().is_path() || self.config.extra_verbose()
}

pub fn info(&self, kind: CompileKind) -> &TargetInfo {
match kind {
CompileKind::Host => &self.host_info,
CompileKind::Target(s) => &self.target_info[&s],
}
}

pub fn extra_args_for(&self, unit: &Unit<'a>) -> Option<&Vec<String>> {
self.extra_compiler_args.get(unit)
}
Expand All @@ -180,6 +110,9 @@ impl<'a, 'cfg> BuildContext<'a, 'cfg> {
/// `lib_name` is the `links` library name and `kind` is whether it is for
/// Host or Target.
pub fn script_override(&self, lib_name: &str, kind: CompileKind) -> Option<&BuildOutput> {
self.target_config(kind).links_overrides.get(lib_name)
self.target_data
.target_config(kind)
.links_overrides
.get(lib_name)
}
}
102 changes: 95 additions & 7 deletions src/cargo/core/compiler/build_context/target_info.rs
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
use crate::core::compiler::CompileKind;
use crate::core::compiler::CompileTarget;
use crate::core::{Dependency, TargetKind, Workspace};
use crate::util::config::{Config, StringList, TargetConfig};
use crate::util::{CargoResult, CargoResultExt, ProcessBuilder, Rustc};
use cargo_platform::{Cfg, CfgExpr};
use std::cell::RefCell;
use std::collections::hash_map::{Entry, HashMap};
use std::env;
use std::path::PathBuf;
use std::str::{self, FromStr};

use crate::core::compiler::CompileKind;
use crate::core::TargetKind;
use crate::util::config::StringList;
use crate::util::{CargoResult, CargoResultExt, Config, ProcessBuilder, Rustc};
use cargo_platform::{Cfg, CfgExpr};

/// Information about the platform target gleaned from querying rustc.
///
/// The `BuildContext` keeps two of these, one for the host and one for the
/// `RustcTargetData` keeps two of these, one for the host and one for the
/// target. If no target is specified, it uses a clone from the host.
#[derive(Clone)]
pub struct TargetInfo {
Expand Down Expand Up @@ -468,3 +468,91 @@ fn env_args(

Ok(Vec::new())
}

/// Collection of information about `rustc` and the host and target.
pub struct RustcTargetData {
/// Information about `rustc` itself.
pub rustc: Rustc,
/// Build information for the "host", which is information about when
/// `rustc` is invoked without a `--target` flag. This is used for
/// procedural macros, build scripts, etc.
host_config: TargetConfig,
host_info: TargetInfo,

/// Build information for targets that we're building for. This will be
/// empty if the `--target` flag is not passed, and currently also only ever
/// has at most one entry, but eventually we'd like to support multi-target
/// builds with Cargo.
target_config: HashMap<CompileTarget, TargetConfig>,
target_info: HashMap<CompileTarget, TargetInfo>,
}

impl RustcTargetData {
pub fn new(ws: &Workspace<'_>, requested_kind: CompileKind) -> CargoResult<RustcTargetData> {
let config = ws.config();
let rustc = config.load_global_rustc(Some(ws))?;
let host_config = config.target_cfg_triple(&rustc.host)?;
let host_info = TargetInfo::new(config, requested_kind, &rustc, CompileKind::Host)?;
let mut target_config = HashMap::new();
let mut target_info = HashMap::new();
if let CompileKind::Target(target) = requested_kind {
let tcfg = config.target_cfg_triple(target.short_name())?;
target_config.insert(target, tcfg);
target_info.insert(
target,
TargetInfo::new(config, requested_kind, &rustc, CompileKind::Target(target))?,
);
}

Ok(RustcTargetData {
rustc,
target_config,
target_info,
host_config,
host_info,
})
}

/// Returns a "short" name for the given kind, suitable for keying off
/// configuration in Cargo or presenting to users.
pub fn short_name<'a>(&'a self, kind: &'a CompileKind) -> &'a str {
match kind {
CompileKind::Host => &self.rustc.host,
CompileKind::Target(target) => target.short_name(),
}
}

/// Whether a dependency should be compiled for the host or target platform,
/// specified by `CompileKind`.
pub fn dep_platform_activated(&self, dep: &Dependency, kind: CompileKind) -> bool {
// If this dependency is only available for certain platforms,
// make sure we're only enabling it for that platform.
let platform = match dep.platform() {
Some(p) => p,
None => return true,
};
let name = self.short_name(&kind);
platform.matches(name, self.cfg(kind))
}

/// Gets the list of `cfg`s printed out from the compiler for the specified kind.
pub fn cfg(&self, kind: CompileKind) -> &[Cfg] {
self.info(kind).cfg()
}

/// Information about the given target platform, learned by querying rustc.
pub fn info(&self, kind: CompileKind) -> &TargetInfo {
match kind {
CompileKind::Host => &self.host_info,
CompileKind::Target(s) => &self.target_info[&s],
}
}

/// Gets the target configuration for a particular host or target.
pub fn target_config(&self, kind: CompileKind) -> &TargetConfig {
match kind {
CompileKind::Host => &self.host_config,
CompileKind::Target(s) => &self.target_config[&s],
}
}
}
20 changes: 14 additions & 6 deletions src/cargo/core/compiler/compilation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ impl<'cfg> Compilation<'cfg> {
bcx: &BuildContext<'a, 'cfg>,
default_kind: CompileKind,
) -> CargoResult<Compilation<'cfg>> {
let mut rustc = bcx.rustc.process();
let mut rustc = bcx.rustc().process();

let mut primary_unit_rustc_process = bcx.build_config.primary_unit_rustc.clone();

Expand All @@ -102,8 +102,16 @@ impl<'cfg> Compilation<'cfg> {
root_output: PathBuf::from("/"),
deps_output: PathBuf::from("/"),
host_deps_output: PathBuf::from("/"),
host_dylib_path: bcx.info(CompileKind::Host).sysroot_host_libdir.clone(),
target_dylib_path: bcx.info(default_kind).sysroot_target_libdir.clone(),
host_dylib_path: bcx
.target_data
.info(CompileKind::Host)
.sysroot_host_libdir
.clone(),
target_dylib_path: bcx
.target_data
.info(default_kind)
.sysroot_target_libdir
.clone(),
tests: Vec::new(),
binaries: Vec::new(),
extra_env: HashMap::new(),
Expand All @@ -114,7 +122,7 @@ impl<'cfg> Compilation<'cfg> {
rustc_process: rustc,
primary_unit_rustc_process,
host: bcx.host_triple().to_string(),
target: default_kind.short_name(bcx).to_string(),
target: bcx.target_data.short_name(&default_kind).to_string(),
target_runner: target_runner(bcx, default_kind)?,
})
}
Expand Down Expand Up @@ -286,7 +294,7 @@ fn target_runner(
bcx: &BuildContext<'_, '_>,
kind: CompileKind,
) -> CargoResult<Option<(PathBuf, Vec<String>)>> {
let target = kind.short_name(bcx);
let target = bcx.target_data.short_name(&kind);

// try target.{}.runner
let key = format!("target.{}.runner", target);
Expand All @@ -296,7 +304,7 @@ fn target_runner(
}

// try target.'cfg(...)'.runner
let target_cfg = bcx.info(kind).cfg();
let target_cfg = bcx.target_data.info(kind).cfg();
let mut cfgs = bcx
.config
.target_cfgs()?
Expand Down
10 changes: 0 additions & 10 deletions src/cargo/core/compiler/compile_kind.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use crate::core::compiler::BuildContext;
use crate::core::{InternedString, Target};
use crate::util::errors::{CargoResult, CargoResultExt};
use serde::Serialize;
Expand Down Expand Up @@ -40,15 +39,6 @@ impl CompileKind {
CompileKind::Target(n) => CompileKind::Target(n),
}
}

/// Returns a "short" name for this kind, suitable for keying off
/// configuration in Cargo or presenting to users.
pub fn short_name(&self, bcx: &BuildContext<'_, '_>) -> &str {
match self {
CompileKind::Host => bcx.host_triple().as_str(),
CompileKind::Target(target) => target.short_name(),
}
}
}

/// Abstraction for the representation of a compilation target that Cargo has.
Expand Down
Loading

0 comments on commit d6fa260

Please sign in to comment.