Skip to content

Commit

Permalink
Auto merge of #9030 - Ekleog:target-setting, r=alexcrichton
Browse files Browse the repository at this point in the history
Expose build.target .cargo/config setting as packages.target in Cargo.toml

Hey!

I'm trying to do my first cargo contribution by implementing per-crate target settings as per [the irlo thread](https://internals.rust-lang.org/t/proposal-move-some-cargo-config-settings-to-cargo-toml/13336) ; and I think I have a draft that looks good-ish (the root units returned by `generate_targets` have the right kinds set).

Closes #7004

**_Edit: the below problem description is now solved in the latest version of this PR, please ignore_**

But for some reason running on a test project now blocks on `Blocking waiting for file lock on build directory` and I have literally no idea how my changes could trigger this… would anyone have an idea of how the changes could lead to infinitely blocking there? (I already tried cargo clean just in case and it didn't appear to help)

FWIW, the output that looks hopeful to me is, on my testbed workspace:
```
Root units [out of generate_targets] are [...]:
 - package ‘smtp-client’ with kind ‘Target(CompileTarget { name: "x86_64-unknown-linux-gnu" })’
 - package ‘smtp-server’ with kind ‘Target(CompileTarget { name: "x86_64-unknown-linux-gnu" })’
 - package ‘yuubind-config’ with kind ‘Target(CompileTarget { name: "x86_64-unknown-linux-gnu" })’
 - package ‘smtp-message-fuzz’ with kind ‘Target(CompileTarget { name: "x86_64-unknown-linux-gnu" })’
 - package ‘yuubind-rpc’ with kind ‘Target(CompileTarget { name: "x86_64-unknown-linux-gnu" })’
 - package ‘yuubind-config-example’ with kind ‘Target(CompileTarget { name: "x86_64-unknown-linux-gnu" })’
 - package ‘smtp-message-fuzz’ with kind ‘Target(CompileTarget { name: "x86_64-unknown-linux-gnu" })’
 - package ‘yuubind-config-example’ with kind ‘Target(CompileTarget { name: "wasm32-unknown-unknown" })’
 - package ‘smtp-queue’ with kind ‘Target(CompileTarget { name: "x86_64-unknown-linux-gnu" })’
 - package ‘smtp-message-fuzz’ with kind ‘Target(CompileTarget { name: "x86_64-unknown-linux-gnu" })’
 - package ‘smtp-message’ with kind ‘Target(CompileTarget { name: "x86_64-unknown-linux-gnu" })’
 - package ‘smtp-server-fuzz’ with kind ‘Target(CompileTarget { name: "x86_64-unknown-linux-gnu" })’
 - package ‘yuubind-config’ with kind ‘Target(CompileTarget { name: "wasm32-unknown-unknown" })’
 - package ‘yuubind’ with kind ‘Target(CompileTarget { name: "x86_64-unknown-linux-gnu" })’
 - package ‘smtp-queue-fs’ with kind ‘Target(CompileTarget { name: "x86_64-unknown-linux-gnu" })’
```
(where both `yuubind-config` and `yuubind-config-example` are being configured to be `wasm32-unknown-unknown` and the other ones stay as host).

Interestingly enough, if I remove the `target` setting from `yuubind-config` (and leave it on `yuubind-config-example`) then it does no longer block on waiting for file lock on build directory, even though it does not actually compile with `wasm32-unknown-unknown`. And it does appear to correctly build yuubind-config-example as wasm32.

My investigation shows that it appears to happen iff there is a package with `package.target` being set that has dependencies.

This most likely is a bug in my code (eg. I build only the root units and not the whole unit graph maybe?), and am going to keep investigating it as such, but maybe someone would already know how dependency resolution could interact with build lock acquisition and give me hints?

Anyway, thank you all for all you do cargo!
  • Loading branch information
bors committed Apr 26, 2021
2 parents a3ff382 + fcd7617 commit d1c0a9d
Show file tree
Hide file tree
Showing 18 changed files with 393 additions and 36 deletions.
6 changes: 6 additions & 0 deletions crates/cargo-test-support/build.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
fn main() {
println!(
"cargo:rustc-env=NATIVE_ARCH={}",
std::env::var("TARGET").unwrap()
);
}
26 changes: 26 additions & 0 deletions crates/cargo-test-support/src/cross_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,23 @@ rustup does not appear to be installed. Make sure that the appropriate
panic!("{}", message);
}

/// The arch triple of the test-running host.
pub fn native() -> &'static str {
env!("NATIVE_ARCH")
}

pub fn native_arch() -> &'static str {
match native()
.split("-")
.next()
.expect("Target triple has unexpected format")
{
"x86_64" => "x86_64",
"i686" => "x86",
_ => panic!("This test should be gated on cross_compile::disabled."),
}
}

/// The alternate target-triple to build with.
///
/// Only use this function on tests that check `cross_compile::disabled`.
Expand All @@ -204,6 +221,15 @@ pub fn alternate_arch() -> &'static str {
}
}

/// A target-triple that is neither the host nor the target.
///
/// Rustc may not work with it and it's alright, apart from being a
/// valid target triple it is supposed to be used only as a
/// placeholder for targets that should not be considered.
pub fn unused() -> &'static str {
"wasm32-unknown-unknown"
}

/// Whether or not the host can run cross-compiled executables.
pub fn can_run_on_host() -> bool {
if disabled() {
Expand Down
4 changes: 2 additions & 2 deletions src/cargo/core/compiler/build_context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ pub struct BuildContext<'a, 'cfg> {
pub packages: PackageSet<'cfg>,

/// Information about rustc and the target platform.
pub target_data: RustcTargetData,
pub target_data: RustcTargetData<'cfg>,

/// The root units of `unit_graph` (units requested on the command-line).
pub roots: Vec<Unit>,
Expand All @@ -58,7 +58,7 @@ impl<'a, 'cfg> BuildContext<'a, 'cfg> {
build_config: &'a BuildConfig,
profiles: Profiles,
extra_compiler_args: HashMap<Unit, Vec<String>>,
target_data: RustcTargetData,
target_data: RustcTargetData<'cfg>,
roots: Vec<Unit>,
unit_graph: UnitGraph,
) -> CargoResult<BuildContext<'a, 'cfg>> {
Expand Down
63 changes: 47 additions & 16 deletions src/cargo/core/compiler/build_context/target_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -655,9 +655,14 @@ fn env_args(
}

/// Collection of information about `rustc` and the host and target.
pub struct RustcTargetData {
pub struct RustcTargetData<'cfg> {
/// Information about `rustc` itself.
pub rustc: Rustc,

/// Config
config: &'cfg Config,
requested_kinds: Vec<CompileKind>,

/// 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.
Expand All @@ -670,27 +675,17 @@ pub struct RustcTargetData {
target_info: HashMap<CompileTarget, TargetInfo>,
}

impl RustcTargetData {
impl<'cfg> RustcTargetData<'cfg> {
pub fn new(
ws: &Workspace<'_>,
ws: &Workspace<'cfg>,
requested_kinds: &[CompileKind],
) -> CargoResult<RustcTargetData> {
) -> CargoResult<RustcTargetData<'cfg>> {
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_kinds, &rustc, CompileKind::Host)?;
let mut target_config = HashMap::new();
let mut target_info = HashMap::new();
for kind in requested_kinds {
if let CompileKind::Target(target) = *kind {
let tcfg = config.target_cfg_triple(target.short_name())?;
target_config.insert(target, tcfg);
target_info.insert(
target,
TargetInfo::new(config, requested_kinds, &rustc, *kind)?,
);
}
}

// This is a hack. The unit_dependency graph builder "pretends" that
// `CompileKind::Host` is `CompileKind::Target(host)` if the
Expand All @@ -703,13 +698,49 @@ impl RustcTargetData {
target_config.insert(ct, host_config.clone());
}

Ok(RustcTargetData {
let mut res = RustcTargetData {
rustc,
config,
requested_kinds: requested_kinds.into(),
host_config,
host_info,
target_config,
target_info,
})
};

// Get all kinds we currently know about.
//
// For now, targets can only ever come from the root workspace
// units as artifact dependencies are not a thing yet, so this
// correctly represents all the kinds that can happen. When we
// have artifact dependencies or other ways for targets to
// appear at places that are not the root units, we may have
// to revisit this.
let all_kinds = requested_kinds
.iter()
.copied()
.chain(ws.members().flat_map(|p| {
p.manifest()
.default_kind()
.into_iter()
.chain(p.manifest().forced_kind())
}));
for kind in all_kinds {
if let CompileKind::Target(target) = kind {
if !res.target_config.contains_key(&target) {
res.target_config
.insert(target, res.config.target_cfg_triple(target.short_name())?);
}
if !res.target_info.contains_key(&target) {
res.target_info.insert(
target,
TargetInfo::new(res.config, &res.requested_kinds, &res.rustc, kind)?,
);
}
}
}

Ok(res)
}

/// Returns a "short" name for the given kind, suitable for keying off
Expand Down
6 changes: 3 additions & 3 deletions src/cargo/core/compiler/compilation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,10 +127,10 @@ impl<'cfg> Compilation<'cfg> {
sysroot_target_libdir: bcx
.all_kinds
.iter()
.map(|kind| {
.map(|&kind| {
(
*kind,
bcx.target_data.info(*kind).sysroot_target_libdir.clone(),
kind,
bcx.target_data.info(kind).sysroot_target_libdir.clone(),
)
})
.collect(),
Expand Down
4 changes: 2 additions & 2 deletions src/cargo/core/compiler/standard_lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ pub fn parse_unstable_flag(value: Option<&str>) -> Vec<String> {
/// Resolve the standard library dependencies.
pub fn resolve_std<'cfg>(
ws: &Workspace<'cfg>,
target_data: &RustcTargetData,
target_data: &RustcTargetData<'cfg>,
requested_targets: &[CompileKind],
crates: &[String],
) -> CargoResult<(PackageSet<'cfg>, Resolve, ResolvedFeatures)> {
Expand Down Expand Up @@ -185,7 +185,7 @@ pub fn generate_std_roots(
Ok(ret)
}

fn detect_sysroot_src_path(target_data: &RustcTargetData) -> CargoResult<PathBuf> {
fn detect_sysroot_src_path(target_data: &RustcTargetData<'_>) -> CargoResult<PathBuf> {
if let Some(s) = env::var_os("__CARGO_TESTS_ONLY_SRC_ROOT") {
return Ok(s.into());
}
Expand Down
4 changes: 2 additions & 2 deletions src/cargo/core/compiler/unit_dependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ struct State<'a, 'cfg> {
/// library.
is_std: bool,
global_mode: CompileMode,
target_data: &'a RustcTargetData,
target_data: &'a RustcTargetData<'cfg>,
profiles: &'a Profiles,
interner: &'a UnitInterner,

Expand All @@ -63,7 +63,7 @@ pub fn build_unit_dependencies<'a, 'cfg>(
roots: &[Unit],
std_roots: &HashMap<CompileKind, Vec<Unit>>,
global_mode: CompileMode,
target_data: &'a RustcTargetData,
target_data: &'a RustcTargetData<'cfg>,
profiles: &'a Profiles,
interner: &'a UnitInterner,
) -> CargoResult<UnitGraph> {
Expand Down
3 changes: 3 additions & 0 deletions src/cargo/core/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,9 @@ features! {

// Support for 2021 edition.
(unstable, edition2021, "", "reference/unstable.html#edition-2021"),

// Allow to specify per-package targets (compile kinds)
(unstable, per_package_target, "", "reference/unstable.html#per-package-target"),
}

const PUBLISH_LOCKFILE_REMOVED: &str = "The publish-lockfile key in Cargo.toml \
Expand Down
23 changes: 22 additions & 1 deletion src/cargo/core/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use serde::ser;
use serde::Serialize;
use url::Url;

use crate::core::compiler::CrateType;
use crate::core::compiler::{CompileKind, CrateType};
use crate::core::resolver::ResolveBehavior;
use crate::core::{Dependency, PackageId, PackageIdSpec, SourceId, Summary};
use crate::core::{Edition, Feature, Features, WorkspaceConfig};
Expand All @@ -32,6 +32,8 @@ pub enum EitherManifest {
pub struct Manifest {
summary: Summary,
targets: Vec<Target>,
default_kind: Option<CompileKind>,
forced_kind: Option<CompileKind>,
links: Option<String>,
warnings: Warnings,
exclude: Vec<String>,
Expand Down Expand Up @@ -366,6 +368,8 @@ compact_debug! {
impl Manifest {
pub fn new(
summary: Summary,
default_kind: Option<CompileKind>,
forced_kind: Option<CompileKind>,
targets: Vec<Target>,
exclude: Vec<String>,
include: Vec<String>,
Expand All @@ -388,6 +392,8 @@ impl Manifest {
) -> Manifest {
Manifest {
summary,
default_kind,
forced_kind,
targets,
warnings: Warnings::new(),
exclude,
Expand All @@ -414,6 +420,12 @@ impl Manifest {
pub fn dependencies(&self) -> &[Dependency] {
self.summary.dependencies()
}
pub fn default_kind(&self) -> Option<CompileKind> {
self.default_kind
}
pub fn forced_kind(&self) -> Option<CompileKind> {
self.forced_kind
}
pub fn exclude(&self) -> &[String] {
&self.exclude
}
Expand Down Expand Up @@ -503,6 +515,15 @@ impl Manifest {
})?;
}

if self.default_kind.is_some() || self.forced_kind.is_some() {
self.unstable_features
.require(Feature::per_package_target())
.with_context(|| {
"the `package.default-target` and `package.forced-target` \
manifest keys are unstable and may not work properly"
})?;
}

Ok(())
}

Expand Down
4 changes: 2 additions & 2 deletions src/cargo/core/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -500,7 +500,7 @@ impl<'cfg> PackageSet<'cfg> {
root_ids: &[PackageId],
has_dev_units: HasDevUnits,
requested_kinds: &[CompileKind],
target_data: &RustcTargetData,
target_data: &RustcTargetData<'cfg>,
force_all_targets: ForceAllTargets,
) -> CargoResult<()> {
fn collect_used_deps(
Expand All @@ -509,7 +509,7 @@ impl<'cfg> PackageSet<'cfg> {
pkg_id: PackageId,
has_dev_units: HasDevUnits,
requested_kinds: &[CompileKind],
target_data: &RustcTargetData,
target_data: &RustcTargetData<'_>,
force_all_targets: ForceAllTargets,
) -> CargoResult<()> {
if !used.insert(pkg_id) {
Expand Down
4 changes: 2 additions & 2 deletions src/cargo/core/resolver/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ pub struct FeatureDifferences {

pub struct FeatureResolver<'a, 'cfg> {
ws: &'a Workspace<'cfg>,
target_data: &'a RustcTargetData,
target_data: &'a RustcTargetData<'cfg>,
/// The platforms to build for, requested by the user.
requested_targets: &'a [CompileKind],
resolve: &'a Resolve,
Expand Down Expand Up @@ -452,7 +452,7 @@ impl<'a, 'cfg> FeatureResolver<'a, 'cfg> {
/// with the result.
pub fn resolve(
ws: &Workspace<'cfg>,
target_data: &RustcTargetData,
target_data: &RustcTargetData<'cfg>,
resolve: &Resolve,
package_set: &'a PackageSet<'cfg>,
cli_features: &CliFeatures,
Expand Down
31 changes: 29 additions & 2 deletions src/cargo/ops/cargo_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -467,11 +467,17 @@ pub fn create_bcx<'a, 'cfg>(
})
.collect();

// Passing `build_config.requested_kinds` instead of
// `explicit_host_kinds` here so that `generate_targets` can do
// its own special handling of `CompileKind::Host`. It will
// internally replace the host kind by the `explicit_host_kind`
// before setting as a unit.
let mut units = generate_targets(
ws,
&to_builds,
filter,
&explicit_host_kinds,
&build_config.requested_kinds,
explicit_host_kind,
build_config.mode,
&resolve,
&workspace_resolve,
Expand Down Expand Up @@ -842,6 +848,7 @@ fn generate_targets(
packages: &[&Package],
filter: &CompileFilter,
requested_kinds: &[CompileKind],
explicit_host_kind: CompileKind,
mode: CompileMode,
resolve: &Resolve,
workspace_resolve: &Option<Resolve>,
Expand Down Expand Up @@ -915,7 +922,27 @@ fn generate_targets(
let features_for = FeaturesFor::from_for_host(target.proc_macro());
let features = resolved_features.activated_features(pkg.package_id(), features_for);

for kind in requested_kinds {
// If `--target` has not been specified, then the unit
// graph is built almost like if `--target $HOST` was
// specified. See `rebuild_unit_graph_shared` for more on
// why this is done. However, if the package has its own
// `package.target` key, then this gets used instead of
// `$HOST`
let explicit_kinds = if let Some(k) = pkg.manifest().forced_kind() {
vec![k]
} else {
requested_kinds
.iter()
.map(|kind| match kind {
CompileKind::Host => {
pkg.manifest().default_kind().unwrap_or(explicit_host_kind)
}
CompileKind::Target(t) => CompileKind::Target(*t),
})
.collect()
};

for kind in explicit_kinds.iter() {
let profile = profiles.get_profile(
pkg.package_id(),
ws.is_member(pkg),
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/ops/cargo_output_metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ fn build_resolve_graph_r(
pkg_id: PackageId,
resolve: &Resolve,
package_map: &BTreeMap<PackageId, Package>,
target_data: &RustcTargetData,
target_data: &RustcTargetData<'_>,
requested_kinds: &[CompileKind],
) {
if node_map.contains_key(&pkg_id) {
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/ops/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ pub fn resolve_ws<'a>(ws: &Workspace<'a>) -> CargoResult<(PackageSet<'a>, Resolv
/// members. In this case, `opts.all_features` must be `true`.
pub fn resolve_ws_with_opts<'cfg>(
ws: &Workspace<'cfg>,
target_data: &RustcTargetData,
target_data: &RustcTargetData<'cfg>,
requested_targets: &[CompileKind],
cli_features: &CliFeatures,
specs: &[PackageIdSpec],
Expand Down
Loading

0 comments on commit d1c0a9d

Please sign in to comment.