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

Add proc-macro to index, and new feature resolver. #8003

Merged
merged 3 commits into from
Mar 18, 2020
Merged
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
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ path = "src/cargo/lib.rs"
atty = "0.2"
bytesize = "1.0"
cargo-platform = { path = "crates/cargo-platform", version = "0.1.1" }
crates-io = { path = "crates/crates-io", version = "0.31" }
crates-io = { path = "crates/crates-io", version = "0.32" }
crossbeam-utils = "0.7"
crypto-hash = "0.3.1"
curl = { version = "0.4.23", features = ["http2"] }
Expand Down
12 changes: 12 additions & 0 deletions crates/cargo-test-support/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ pub struct Package {
local: bool,
alternative: bool,
invalid_json: bool,
proc_macro: bool,
}

#[derive(Clone)]
Expand Down Expand Up @@ -242,6 +243,7 @@ impl Package {
local: false,
alternative: false,
invalid_json: false,
proc_macro: false,
}
}

Expand Down Expand Up @@ -345,6 +347,12 @@ impl Package {
self
}

/// Specifies whether or not this is a proc macro.
pub fn proc_macro(&mut self, proc_macro: bool) -> &mut Package {
self.proc_macro = proc_macro;
self
}

/// Adds an entry in the `[features]` section.
pub fn feature(&mut self, name: &str, deps: &[&str]) -> &mut Package {
let deps = deps.iter().map(|s| s.to_string()).collect();
Expand Down Expand Up @@ -413,6 +421,7 @@ impl Package {
"cksum": cksum,
"features": self.features,
"yanked": self.yanked,
"pm": self.proc_macro,
})
.to_string();

Expand Down Expand Up @@ -498,6 +507,9 @@ impl Package {
manifest.push_str(&format!("registry-index = \"{}\"", alt_registry_url()));
}
}
if self.proc_macro {
manifest.push_str("[lib]\nproc-macro = true\n");
}

let dst = self.archive_dst();
t!(fs::create_dir_all(dst.parent().unwrap()));
Expand Down
2 changes: 1 addition & 1 deletion crates/crates-io/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "crates-io"
version = "0.31.0"
version = "0.32.0"
edition = "2018"
authors = ["Alex Crichton <alex@alexcrichton.com>"]
license = "MIT OR Apache-2.0"
Expand Down
2 changes: 1 addition & 1 deletion crates/crates-io/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ pub struct NewCrate {
pub license_file: Option<String>,
pub repository: Option<String>,
pub badges: BTreeMap<String, BTreeMap<String, String>>,
#[serde(default)]
pub links: Option<String>,
pub proc_macro: bool,
}

#[derive(Serialize)]
Expand Down
4 changes: 4 additions & 0 deletions crates/resolver-tests/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ pub fn resolve_with_config_raw(
&BTreeMap::<String, Vec<String>>::new(),
None::<&String>,
false,
false,
)
.unwrap();
let opts = ResolveOpts::everything();
Expand Down Expand Up @@ -577,6 +578,7 @@ pub fn pkg_dep<T: ToPkgId>(name: T, dep: Vec<Dependency>) -> Summary {
&BTreeMap::<String, Vec<String>>::new(),
link,
false,
false,
)
.unwrap()
}
Expand Down Expand Up @@ -605,6 +607,7 @@ pub fn pkg_loc(name: &str, loc: &str) -> Summary {
&BTreeMap::<String, Vec<String>>::new(),
link,
false,
false,
)
.unwrap()
}
Expand All @@ -619,6 +622,7 @@ pub fn remove_dep(sum: &Summary, ind: usize) -> Summary {
&BTreeMap::<String, Vec<String>>::new(),
sum.links().map(|a| a.as_str()),
sum.namespaced_features(),
sum.proc_macro(),
)
.unwrap()
}
Expand Down
33 changes: 19 additions & 14 deletions src/cargo/core/compiler/unit_dependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,9 +178,11 @@ fn deps_of_roots<'a, 'cfg>(roots: &[Unit<'a>], mut state: &mut State<'a, 'cfg>)
} else if unit.target.is_custom_build() {
// This normally doesn't happen, except `clean` aggressively
// generates all units.
UnitFor::new_build(false)
UnitFor::new_host(false)
} else if unit.target.proc_macro() {
UnitFor::new_host(true)
} else if unit.target.for_host() {
// Proc macro / plugin should never have panic set.
// Plugin should never have panic set.
UnitFor::new_compiler()
} else {
UnitFor::new_normal()
Expand Down Expand Up @@ -297,7 +299,7 @@ fn compute_deps<'a, 'cfg>(
let dep_unit_for = unit_for
.with_for_host(lib.for_host())
// If it is a custom build script, then it *only* has build dependencies.
.with_build_dep(unit.target.is_custom_build());
.with_host_features(unit.target.is_custom_build() || lib.proc_macro());

if bcx.config.cli_unstable().dual_proc_macros && lib.proc_macro() && !unit.kind.is_host() {
let unit_dep = new_unit_dep(state, unit, pkg, lib, dep_unit_for, unit.kind, mode)?;
Expand Down Expand Up @@ -388,9 +390,10 @@ fn compute_deps_custom_build<'a, 'cfg>(
return Ok(Vec::new());
}
}
// All dependencies of this unit should use profiles for custom
// builds.
let script_unit_for = UnitFor::new_build(unit_for.is_for_build_dep());
// All dependencies of this unit should use profiles for custom builds.
// If this is a build script of a proc macro, make sure it uses host
// features.
let script_unit_for = UnitFor::new_host(unit_for.is_for_host_features());
// When not overridden, then the dependencies to run a build script are:
//
// 1. Compiling the build script itself.
Expand Down Expand Up @@ -445,7 +448,9 @@ fn compute_deps_doc<'a, 'cfg>(
// Rustdoc only needs rmeta files for regular dependencies.
// However, for plugins/proc macros, deps should be built like normal.
let mode = check_or_build_mode(unit.mode, lib);
let dep_unit_for = UnitFor::new_normal().with_for_host(lib.for_host());
let dep_unit_for = UnitFor::new_normal()
.with_for_host(lib.for_host())
.with_host_features(lib.proc_macro());
let lib_unit_dep = new_unit_dep(
state,
unit,
Expand Down Expand Up @@ -528,32 +533,32 @@ fn dep_build_script<'a>(
.bcx
.profiles
.get_profile_run_custom_build(&unit.profile);
// UnitFor::new_build is used because we want the `host` flag set
// UnitFor::new_host is used because we want the `host` flag set
// for all of our build dependencies (so they all get
// build-override profiles), including compiling the build.rs
// script itself.
//
// If `is_for_build_dep` here is `false`, that means we are a
// If `is_for_host_features` here is `false`, that means we are a
// build.rs script for a normal dependency and we want to set the
// CARGO_FEATURE_* environment variables to the features as a
// normal dep.
//
// If `is_for_build_dep` here is `true`, that means that this
// package is being used as a build dependency, and so we only
// want to set CARGO_FEATURE_* variables for the build-dependency
// If `is_for_host_features` here is `true`, that means that this
// package is being used as a build dependency or proc-macro, and
// so we only want to set CARGO_FEATURE_* variables for the host
// side of the graph.
//
// Keep in mind that the RunCustomBuild unit and the Compile
// build.rs unit use the same features. This is because some
// people use `cfg!` and `#[cfg]` expressions to check for enabled
// features instead of just checking `CARGO_FEATURE_*` at runtime.
// In the case with `-Zfeatures=build_dep`, and a shared
// In the case with `-Zfeatures=host_dep`, and a shared
// dependency has different features enabled for normal vs. build,
// then the build.rs script will get compiled twice. I believe it
// is not feasible to only build it once because it would break a
// large number of scripts (they would think they have the wrong
// set of features enabled).
let script_unit_for = UnitFor::new_build(unit_for.is_for_build_dep());
let script_unit_for = UnitFor::new_host(unit_for.is_for_host_features());
new_unit_dep_with_profile(
state,
unit,
Expand Down
85 changes: 45 additions & 40 deletions src/cargo/core/profiles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -768,7 +768,7 @@ pub struct UnitFor {
/// any of its dependencies. This enables `build-override` profiles for
/// these targets.
///
/// An invariant is that if `build_dep` is true, `host` must be true.
/// An invariant is that if `host_features` is true, `host` must be true.
///
/// Note that this is `true` for `RunCustomBuild` units, even though that
/// unit should *not* use build-override profiles. This is a bit of a
Expand All @@ -779,16 +779,16 @@ pub struct UnitFor {
/// sticky (and forced to `true` for all further dependencies) — which is
/// the whole point of `UnitFor`.
host: bool,
/// A target for a build dependency (or any of its dependencies). This is
/// used for computing features of build dependencies independently of
/// other dependency kinds.
/// A target for a build dependency or proc-macro (or any of its
/// dependencies). This is used for computing features of build
/// dependencies and proc-macros independently of other dependency kinds.
///
/// The subtle difference between this and `host` is that the build script
/// for a non-host package sets this to `false` because it wants the
/// features of the non-host package (whereas `host` is true because the
/// build script is being built for the host). `build_dep` becomes `true`
/// for build-dependencies, or any of their dependencies. For example, with
/// this dependency tree:
/// build script is being built for the host). `host_features` becomes
/// `true` for build-dependencies or proc-macros, or any of their
/// dependencies. For example, with this dependency tree:
///
/// ```text
/// foo
Expand All @@ -799,17 +799,18 @@ pub struct UnitFor {
/// └── shared_dep build.rs
/// ```
///
/// In this example, `foo build.rs` is HOST=true, BUILD_DEP=false. This is
/// so that `foo build.rs` gets the profile settings for build scripts
/// (HOST=true) and features of foo (BUILD_DEP=false) because build scripts
/// need to know which features their package is being built with.
/// In this example, `foo build.rs` is HOST=true, HOST_FEATURES=false.
/// This is so that `foo build.rs` gets the profile settings for build
/// scripts (HOST=true) and features of foo (HOST_FEATURES=false) because
/// build scripts need to know which features their package is being built
/// with.
///
/// But in the case of `shared_dep`, when built as a build dependency,
/// both flags are true (it only wants the build-dependency features).
/// When `shared_dep` is built as a normal dependency, then `shared_dep
/// build.rs` is HOST=true, BUILD_DEP=false for the same reasons that
/// build.rs` is HOST=true, HOST_FEATURES=false for the same reasons that
/// foo's build script is set that way.
build_dep: bool,
host_features: bool,
/// How Cargo processes the `panic` setting or profiles. This is done to
/// handle test/benches inheriting from dev/release, as well as forcing
/// `for_host` units to always unwind.
Expand Down Expand Up @@ -837,32 +838,35 @@ impl UnitFor {
pub fn new_normal() -> UnitFor {
UnitFor {
host: false,
build_dep: false,
host_features: false,
panic_setting: PanicSetting::ReadProfile,
}
}

/// A unit for a custom build script or its dependencies.
/// A unit for a custom build script or proc-macro or its dependencies.
///
/// The `build_dep` parameter is whether or not this is for a build
/// dependency. Build scripts for non-host units should use `false`
/// because they want to use the features of the package they are running
/// for.
pub fn new_build(build_dep: bool) -> UnitFor {
/// The `host_features` parameter is whether or not this is for a build
/// dependency or proc-macro (something that requires being built "on the
/// host"). Build scripts for non-host units should use `false` because
/// they want to use the features of the package they are running for.
pub fn new_host(host_features: bool) -> UnitFor {
UnitFor {
host: true,
build_dep,
host_features,
// Force build scripts to always use `panic=unwind` for now to
// maximally share dependencies with procedural macros.
panic_setting: PanicSetting::AlwaysUnwind,
}
}

/// A unit for a proc macro or compiler plugin or their dependencies.
/// A unit for a compiler plugin or their dependencies.
pub fn new_compiler() -> UnitFor {
UnitFor {
host: false,
build_dep: false,
// The feature resolver doesn't know which dependencies are
// plugins, so for now plugins don't split features. Since plugins
// are mostly deprecated, just leave this as false.
host_features: false,
// Force plugins to use `panic=abort` so panics in the compiler do
// not abort the process but instead end with a reasonable error
// message that involves catching the panic in the compiler.
Expand All @@ -879,7 +883,7 @@ impl UnitFor {
pub fn new_test(config: &Config) -> UnitFor {
UnitFor {
host: false,
build_dep: false,
host_features: false,
// We're testing out an unstable feature (`-Zpanic-abort-tests`)
// which inherits the panic setting from the dev/release profile
// (basically avoid recompiles) but historical defaults required
Expand All @@ -902,7 +906,7 @@ impl UnitFor {
pub fn with_for_host(self, for_host: bool) -> UnitFor {
UnitFor {
host: self.host || for_host,
build_dep: self.build_dep,
host_features: self.host_features,
panic_setting: if for_host {
PanicSetting::AlwaysUnwind
} else {
Expand All @@ -911,15 +915,16 @@ impl UnitFor {
}
}

/// Returns a new copy updating it for a build dependency.
/// Returns a new copy updating it whether or not it should use features
/// for build dependencies and proc-macros.
///
/// This is part of the machinery responsible for handling feature
/// decoupling for build dependencies in the new feature resolver.
pub fn with_build_dep(mut self, build_dep: bool) -> UnitFor {
if build_dep {
pub fn with_host_features(mut self, host_features: bool) -> UnitFor {
if host_features {
assert!(self.host);
}
self.build_dep = self.build_dep || build_dep;
self.host_features = self.host_features || host_features;
self
}

Expand All @@ -929,8 +934,8 @@ impl UnitFor {
self.host
}

pub fn is_for_build_dep(&self) -> bool {
self.build_dep
pub fn is_for_host_features(&self) -> bool {
self.host_features
}

/// Returns how `panic` settings should be handled for this profile
Expand All @@ -943,43 +948,43 @@ impl UnitFor {
static ALL: &[UnitFor] = &[
UnitFor {
host: false,
build_dep: false,
host_features: false,
panic_setting: PanicSetting::ReadProfile,
},
UnitFor {
host: true,
build_dep: false,
host_features: false,
panic_setting: PanicSetting::AlwaysUnwind,
},
UnitFor {
host: false,
build_dep: false,
host_features: false,
panic_setting: PanicSetting::AlwaysUnwind,
},
UnitFor {
host: false,
build_dep: false,
host_features: false,
panic_setting: PanicSetting::Inherit,
},
// build_dep=true must always have host=true
// host_features=true must always have host=true
// `Inherit` is not used in build dependencies.
UnitFor {
host: true,
build_dep: true,
host_features: true,
panic_setting: PanicSetting::ReadProfile,
},
UnitFor {
host: true,
build_dep: true,
host_features: true,
panic_setting: PanicSetting::AlwaysUnwind,
},
];
ALL
}

pub(crate) fn map_to_features_for(&self) -> FeaturesFor {
if self.is_for_build_dep() {
FeaturesFor::BuildDep
if self.is_for_host_features() {
FeaturesFor::HostDep
} else {
FeaturesFor::NormalOrDev
}
Expand Down
Loading