Skip to content

Commit 49fbf52

Browse files
committed
Auto merge of #7216 - ehuss:build-std, r=alexcrichton
Basic standard library support. This is not intended to be useful to anyone. If people want to try it, that's great, but do not rely on this. This is only for experimenting and setting up for future work. This adds a flag `-Zbuild-std` to build the standard library with a project. The flag can also take optional comma-separated crate names, like `-Zbuild-std=core`. Default is `std,core,panic_unwind,compiler_builtins`. Closes rust-lang/wg-cargo-std-aware#10. Note: I can probably break some of the refactoring into smaller PRs if necessary. ## Overview The general concept here is to use two resolvers, and to combine everything in the Unit graph. There are a number of changes to support this: - A synthetic workspace for the standard library is created to set up the patches and members correctly. - Decouple `unit_dependencies` from `Context` to make it easier to manage. - Add `features` to `Unit` to keep it unique and to remove the need to query a resolver. - Add a `UnitDep` struct which encodes the edges between `Unit`s. This removes the need to query a resolver for `extern_crate_name` and `public`. - Remove `Resolver` from `BuildContext` to avoid any confusion and to keep the complexity focused in `unit_dependencies`. - Remove `Links` from `Context` since it used the resolver. Adjusted so that instead of checking links at runtime, they are all checked at once in the beginning. Note that it does not check links for the standard lib, but it should be safe? I think `compiler-rt` is the only `links`? I currently went with a strategy of linking the standard library dependencies using `--extern` (instead of `--sysroot` or `-L`). This has some benefits but some significant drawbacks. See below for some questions. ## For future PRs - Add Cargo.toml support. See rust-lang/wg-cargo-std-aware#5 - Source is not downloaded. It assumes you have run `rustup component add rust-src`. See rust-lang/wg-cargo-std-aware#11 - `cargo metadata` does not include any information about std. I don't know how this should work. - `cargo clean` is not std-aware. - `cargo fetch` does not fetch std dependencies. - `cargo vendor` does not vendor std dependencies. - `cargo pkgid` is not std-aware. - `--target` is required on the command-line. This should default to host-as-target. - `-p` is not std aware. - A synthetic `Cargo.toml` workspace is created which has to know about things like `rustc-std-workspace-core`. Perhaps rust-lang/rust should publish the source with this `Cargo.toml` already created? - `compiler_builtins` uses default features (pure Rust implementation, etc.). See rust-lang/wg-cargo-std-aware#15 - `compiler_builtins` may need to be built without debug assertions, see [this](https://github.com/rust-lang/rust/blob/8e917f48382c6afaf50568263b89d35fba5d98e4/src/bootstrap/bin/rustc.rs#L210-L214). Could maybe use profile overrides. - Panic issues: - `panic_abort` is not yet supported, though it should probably be easy. It could maybe look at the profile to determine which panic implementation to use? This requires more hard-coding in Cargo to know about rustc implementation details. - [This](https://github.com/rust-lang/rust/blob/8e917f48382c6afaf50568263b89d35fba5d98e4/src/bootstrap/bin/rustc.rs#L186-L201) should probably be handled where `panic` is set for `panic_abort` and `compiler_builtins`. I would like to get a test case for it. This can maybe be done with profile overrides? - Using two resolvers is quite messy and causes a lot of complications. It would be ideal if it could only use one, though that may not be possible for the foreseeable future. See rust-lang/wg-cargo-std-aware#12 - Features are hard-coded. See rust-lang/wg-cargo-std-aware#13 - Lots of various platform-specific support is not included (musl, wasi, windows-gnu, etc.). - Default `backtrace` is used with C compiler. See rust-lang/wg-cargo-std-aware#16 - Sanitizers are not built. See rust-lang/wg-cargo-std-aware#17 - proc_macro has some hacky code to synthesize its dependencies. See rust-lang/wg-cargo-std-aware#18. This may not be necessary if this uses `--sysroot` instead. - Profile overrides cause weird linker errors. That is: ```toml [profile.dev.overrides.std] opt-level = 2 ``` Using `[profile.dev.overrides."*"]` works. I tried fiddling with it, but couldn't figure it out. We may also want to consider altering the syntax for profile overrides. Having to repeat the same profile for `std` and `core` and `alloc` and everything else would not be ideal. - ~~`Context::unit_deps` does not handle build overrides, see #7215.~~ FIXED ## Questions for this PR - I went with the strategy of using `--extern` to link the standard lib. This seems to work, and I haven't found any problems, but it seems risky. It also forces Cargo to know about certain implicit dependencies like `compiler_builtins` and `panic_*`. The alternative is to create a sysroot and copy all the crates to that directory and pass `--sysroot`. However, this is complicated by pipelining, which would require special support to copy `.rmeta` files when they are generated. Let me know if you think I should use a different strategy. I'm on the fence here, and I think using `--sysroot` may be safer, but adds more complexity. - As an aside, if rustc ever tries to grab a crate from sysroot that was not passed in via `--extern`, then it results in duplicate lang items. For example, saying `extern crate proc_macro;` without specifying `proc_macro` as a dependency. We could prevent rustc from ever trying by passing `--sysroot=/nonexistent` to prevent it from trying. Or add an equivalent flag to rustc. - How should this be tested? I added a janky integration test, but it has some drawbacks. It requires internet access. It is slow. Since it is slow, it reuses the same target directory for multiple tests which makes it awkward to work with. - What interesting things are there to test? - We may want to disable the test before merging if it seems too annoying to make it the default. It requires rust-src to be downloaded, and takes several minutes to run, and are somewhat platform-dependent. - How to test that it is actually linking the correct standard library? I did tests locally with a modified libcore, but I can't think of a good way to do that in the test suite. - I did not add `__CARGO_DEFAULT_LIB_METADATA` to the hash. I had a hard time coming up with a test case where it would matter. - My only thought is that it is a problem because libstd includes a dylib, which prevents the hash from being added to the filename. It does cause recompiles when switching between compilers, for example, when it normally wouldn't. - Very dumb question: Why exactly does libstd include a dylib? This can cause issues (see rust-lang/rust#56443). - This should probably change, but I want to better understand it first. - The `bin_nostd` test needs to link libc on linux, and I'm not sure I understand why. I'm concerned there is something wrong there. libstd does not do that AFAIK.
2 parents 5da11a5 + 9382be1 commit 49fbf52

25 files changed

+1117
-385
lines changed

ci/azure-test-all.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,11 @@ steps:
1616
- bash: rustup component add clippy || echo "clippy not available"
1717
displayName: "Install clippy (maybe)"
1818

19+
# This is needed for standard library tests.
20+
- bash: rustup component add rust-src
21+
condition: and(succeeded(), eq(variables['TOOLCHAIN'], 'nightly'))
22+
displayName: "Install rust-src (maybe)"
23+
1924
# Deny warnings on CI to avoid warnings getting into the codebase, and note the
2025
# `force-system-lib-on-osx` which is intended to fix compile issues on OSX where
2126
# compiling curl from source on OSX yields linker errors on Azure.

src/cargo/core/compiler/build_config.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ pub enum CompileMode {
156156
/// a test.
157157
Check { test: bool },
158158
/// Used to indicate benchmarks should be built. This is not used in
159-
/// `Target`, because it is essentially the same as `Test` (indicating
159+
/// `Unit`, because it is essentially the same as `Test` (indicating
160160
/// `--test` should be passed to rustc) and by using `Test` instead it
161161
/// allows some de-duping of Units to occur.
162162
Bench,
@@ -221,6 +221,14 @@ impl CompileMode {
221221
}
222222
}
223223

224+
/// Returns `true` if this is something that passes `--test` to rustc.
225+
pub fn is_rustc_test(self) -> bool {
226+
match self {
227+
CompileMode::Test | CompileMode::Bench | CompileMode::Check { test: true } => true,
228+
_ => false,
229+
}
230+
}
231+
224232
/// Returns `true` if this is the *execution* of a `build.rs` script.
225233
pub fn is_run_custom_build(self) -> bool {
226234
self == CompileMode::RunCustomBuild

src/cargo/core/compiler/build_context/mod.rs

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use crate::core::compiler::unit::UnitInterner;
88
use crate::core::compiler::{BuildConfig, BuildOutput, Kind, Unit};
99
use crate::core::profiles::Profiles;
1010
use crate::core::{Dependency, Workspace};
11-
use crate::core::{PackageId, PackageSet, Resolve};
11+
use crate::core::{PackageId, PackageSet};
1212
use crate::util::errors::CargoResult;
1313
use crate::util::{profile, Cfg, Config, Rustc};
1414

@@ -26,8 +26,6 @@ pub struct BuildContext<'a, 'cfg> {
2626
pub ws: &'a Workspace<'cfg>,
2727
/// The cargo configuration.
2828
pub config: &'cfg Config,
29-
/// The dependency graph for our build.
30-
pub resolve: &'a Resolve,
3129
pub profiles: &'a Profiles,
3230
pub build_config: &'a BuildConfig,
3331
/// Extra compiler args for either `rustc` or `rustdoc`.
@@ -48,7 +46,6 @@ pub struct BuildContext<'a, 'cfg> {
4846
impl<'a, 'cfg> BuildContext<'a, 'cfg> {
4947
pub fn new(
5048
ws: &'a Workspace<'cfg>,
51-
resolve: &'a Resolve,
5249
packages: &'a PackageSet<'cfg>,
5350
config: &'cfg Config,
5451
build_config: &'a BuildConfig,
@@ -75,7 +72,6 @@ impl<'a, 'cfg> BuildContext<'a, 'cfg> {
7572

7673
Ok(BuildContext {
7774
ws,
78-
resolve,
7975
packages,
8076
config,
8177
rustc,
@@ -90,16 +86,6 @@ impl<'a, 'cfg> BuildContext<'a, 'cfg> {
9086
})
9187
}
9288

93-
pub fn extern_crate_name(&self, unit: &Unit<'a>, dep: &Unit<'a>) -> CargoResult<String> {
94-
self.resolve
95-
.extern_crate_name(unit.pkg.package_id(), dep.pkg.package_id(), dep.target)
96-
}
97-
98-
pub fn is_public_dependency(&self, unit: &Unit<'a>, dep: &Unit<'a>) -> bool {
99-
self.resolve
100-
.is_public_dep(unit.pkg.package_id(), dep.pkg.package_id())
101-
}
102-
10389
/// Whether a dependency should be compiled for the host or target platform,
10490
/// specified by `Kind`.
10591
pub fn dep_platform_activated(&self, dep: &Dependency, kind: Kind) -> bool {

src/cargo/core/compiler/compilation.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use std::path::PathBuf;
66
use semver::Version;
77

88
use super::BuildContext;
9-
use crate::core::{Edition, Package, PackageId, Target};
9+
use crate::core::{Edition, InternedString, Package, PackageId, Target};
1010
use crate::util::{self, join_paths, process, CargoResult, CfgExpr, Config, ProcessBuilder};
1111

1212
pub struct Doctest {
@@ -16,7 +16,7 @@ pub struct Doctest {
1616
pub target: Target,
1717
/// Extern dependencies needed by `rustdoc`. The path is the location of
1818
/// the compiled lib.
19-
pub deps: Vec<(String, PathBuf)>,
19+
pub deps: Vec<(InternedString, PathBuf)>,
2020
}
2121

2222
/// A structure returning the result of a compilation.

src/cargo/core/compiler/context/compilation_files.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -522,9 +522,7 @@ fn compute_metadata<'a, 'cfg>(
522522

523523
// Also mix in enabled features to our metadata. This'll ensure that
524524
// when changing feature sets each lib is separately cached.
525-
bcx.resolve
526-
.features_sorted(unit.pkg.package_id())
527-
.hash(&mut hasher);
525+
unit.features.hash(&mut hasher);
528526

529527
// Mix in the target-metadata of all the dependencies of this target.
530528
{

src/cargo/core/compiler/context/mod.rs

Lines changed: 48 additions & 112 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
#![allow(deprecated)]
2-
use std::collections::{HashMap, HashSet};
2+
use std::collections::{BTreeSet, HashMap, HashSet};
33
use std::ffi::OsStr;
4-
use std::fmt::Write;
54
use std::path::PathBuf;
65
use std::sync::{Arc, Mutex};
76

@@ -10,7 +9,7 @@ use jobserver::Client;
109

1110
use crate::core::compiler::compilation;
1211
use crate::core::compiler::Unit;
13-
use crate::core::{Package, PackageId, Resolve};
12+
use crate::core::PackageId;
1413
use crate::util::errors::{CargoResult, CargoResultExt};
1514
use crate::util::{internal, profile, Config};
1615

@@ -19,11 +18,9 @@ use super::custom_build::{self, BuildDeps, BuildScriptOutputs, BuildScripts};
1918
use super::fingerprint::Fingerprint;
2019
use super::job_queue::JobQueue;
2120
use super::layout::Layout;
21+
use super::unit_dependencies::{UnitDep, UnitGraph};
2222
use super::{BuildContext, Compilation, CompileMode, Executor, FileFlavor, Kind};
2323

24-
mod unit_dependencies;
25-
use self::unit_dependencies::build_unit_dependencies;
26-
2724
mod compilation_files;
2825
use self::compilation_files::CompilationFiles;
2926
pub use self::compilation_files::{Metadata, OutputFile};
@@ -51,23 +48,18 @@ pub struct Context<'a, 'cfg> {
5148
/// Linking information for each `Unit`.
5249
/// See `build_map` for details.
5350
pub build_scripts: HashMap<Unit<'a>, Arc<BuildScripts>>,
54-
/// Used to check the `links` field in the manifest is not duplicated and
55-
/// is used correctly.
56-
pub links: Links,
5751
/// Job server client to manage concurrency with other processes.
5852
pub jobserver: Client,
5953
/// "Primary" packages are the ones the user selected on the command-line
6054
/// with `-p` flags. If no flags are specified, then it is the defaults
6155
/// based on the current directory and the default workspace members.
6256
primary_packages: HashSet<PackageId>,
6357
/// The dependency graph of units to compile.
64-
unit_dependencies: HashMap<Unit<'a>, Vec<Unit<'a>>>,
58+
unit_dependencies: UnitGraph<'a>,
6559
/// An abstraction of the files and directories that will be generated by
6660
/// the compilation. This is `None` until after `unit_dependencies` has
6761
/// been computed.
6862
files: Option<CompilationFiles<'a, 'cfg>>,
69-
/// Cache of packages, populated when they are downloaded.
70-
package_cache: HashMap<PackageId, &'a Package>,
7163

7264
/// A flag indicating whether pipelining is enabled for this compilation
7365
/// session. Pipelining largely only affects the edges of the dependency
@@ -82,7 +74,11 @@ pub struct Context<'a, 'cfg> {
8274
}
8375

8476
impl<'a, 'cfg> Context<'a, 'cfg> {
85-
pub fn new(config: &'cfg Config, bcx: &'a BuildContext<'a, 'cfg>) -> CargoResult<Self> {
77+
pub fn new(
78+
config: &'cfg Config,
79+
bcx: &'a BuildContext<'a, 'cfg>,
80+
unit_dependencies: UnitGraph<'a>,
81+
) -> CargoResult<Self> {
8682
// Load up the jobserver that we'll use to manage our parallelism. This
8783
// is the same as the GNU make implementation of a jobserver, and
8884
// intentionally so! It's hoped that we can interact with GNU make and
@@ -112,12 +108,10 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
112108
compiled: HashSet::new(),
113109
build_scripts: HashMap::new(),
114110
build_explicit_deps: HashMap::new(),
115-
links: Links::new(),
116111
jobserver,
117112
primary_packages: HashSet::new(),
118-
unit_dependencies: HashMap::new(),
113+
unit_dependencies,
119114
files: None,
120-
package_cache: HashMap::new(),
121115
rmeta_required: HashSet::new(),
122116
pipelining,
123117
})
@@ -203,18 +197,15 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
203197
// pass `--extern` for rlib deps and skip out on all other
204198
// artifacts.
205199
let mut doctest_deps = Vec::new();
206-
for dep in self.dep_targets(unit) {
207-
if dep.target.is_lib() && dep.mode == CompileMode::Build {
208-
let outputs = self.outputs(&dep)?;
200+
for dep in self.unit_deps(unit) {
201+
if dep.unit.target.is_lib() && dep.unit.mode == CompileMode::Build {
202+
let outputs = self.outputs(&dep.unit)?;
209203
let outputs = outputs.iter().filter(|output| {
210204
output.path.extension() == Some(OsStr::new("rlib"))
211-
|| dep.target.for_host()
205+
|| dep.unit.target.for_host()
212206
});
213207
for output in outputs {
214-
doctest_deps.push((
215-
self.bcx.extern_crate_name(unit, &dep)?,
216-
output.path.clone(),
217-
));
208+
doctest_deps.push((dep.extern_crate_name, output.path.clone()));
218209
}
219210
}
220211
}
@@ -227,7 +218,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
227218
});
228219
}
229220

230-
let feats = self.bcx.resolve.features(unit.pkg.package_id());
221+
let feats = &unit.features;
231222
if !feats.is_empty() {
232223
self.compilation
233224
.cfgs
@@ -305,7 +296,8 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
305296
self.primary_packages
306297
.extend(units.iter().map(|u| u.pkg.package_id()));
307298

308-
build_unit_dependencies(self, units)?;
299+
self.record_units_requiring_metadata();
300+
309301
let files = CompilationFiles::new(
310302
units,
311303
host_layout,
@@ -357,38 +349,35 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
357349

358350
/// For a package, return all targets which are registered as dependencies
359351
/// for that package.
352+
/// NOTE: This is deprecated, use `unit_deps` instead.
360353
//
361354
// TODO: this ideally should be `-> &[Unit<'a>]`.
362355
pub fn dep_targets(&self, unit: &Unit<'a>) -> Vec<Unit<'a>> {
363-
self.unit_dependencies[unit].clone()
356+
self.unit_dependencies[unit]
357+
.iter()
358+
.map(|dep| dep.unit)
359+
.collect()
364360
}
365361

366-
pub fn is_primary_package(&self, unit: &Unit<'a>) -> bool {
367-
self.primary_packages.contains(&unit.pkg.package_id())
362+
/// Direct dependencies for the given unit.
363+
pub fn unit_deps(&self, unit: &Unit<'a>) -> &[UnitDep<'a>] {
364+
&self.unit_dependencies[unit]
368365
}
369366

370-
/// Gets a package for the given package ID.
371-
pub fn get_package(&self, id: PackageId) -> CargoResult<&'a Package> {
372-
self.package_cache
373-
.get(&id)
374-
.cloned()
375-
.ok_or_else(|| failure::format_err!("failed to find {}", id))
367+
pub fn is_primary_package(&self, unit: &Unit<'a>) -> bool {
368+
self.primary_packages.contains(&unit.pkg.package_id())
376369
}
377370

378371
/// Returns the list of filenames read by cargo to generate the `BuildContext`
379372
/// (all `Cargo.toml`, etc.).
380373
pub fn build_plan_inputs(&self) -> CargoResult<Vec<PathBuf>> {
381-
let mut inputs = Vec::new();
382-
// Note that we're using the `package_cache`, which should have been
383-
// populated by `build_unit_dependencies`, and only those packages are
384-
// considered as all the inputs.
385-
//
386-
// (Notably, we skip dev-deps here if they aren't present.)
387-
for pkg in self.package_cache.values() {
388-
inputs.push(pkg.manifest_path().to_path_buf());
374+
// Keep sorted for consistency.
375+
let mut inputs = BTreeSet::new();
376+
// Note: dev-deps are skipped if they are not present in the unit graph.
377+
for unit in self.unit_dependencies.keys() {
378+
inputs.insert(unit.pkg.manifest_path().to_path_buf());
389379
}
390-
inputs.sort();
391-
Ok(inputs)
380+
Ok(inputs.into_iter().collect())
392381
}
393382

394383
fn check_collistions(&self) -> CargoResult<()> {
@@ -488,6 +477,20 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
488477
Ok(())
489478
}
490479

480+
/// Records the list of units which are required to emit metadata.
481+
///
482+
/// Units which depend only on the metadata of others requires the others to
483+
/// actually produce metadata, so we'll record that here.
484+
fn record_units_requiring_metadata(&mut self) {
485+
for (key, deps) in self.unit_dependencies.iter() {
486+
for dep in deps {
487+
if self.only_requires_rmeta(key, &dep.unit) {
488+
self.rmeta_required.insert(dep.unit);
489+
}
490+
}
491+
}
492+
}
493+
491494
/// Returns whether when `parent` depends on `dep` if it only requires the
492495
/// metadata file from `dep`.
493496
pub fn only_requires_rmeta(&self, parent: &Unit<'a>, dep: &Unit<'a>) -> bool {
@@ -509,70 +512,3 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
509512
self.rmeta_required.contains(unit)
510513
}
511514
}
512-
513-
#[derive(Default)]
514-
pub struct Links {
515-
validated: HashSet<PackageId>,
516-
links: HashMap<String, PackageId>,
517-
}
518-
519-
impl Links {
520-
pub fn new() -> Links {
521-
Links {
522-
validated: HashSet::new(),
523-
links: HashMap::new(),
524-
}
525-
}
526-
527-
pub fn validate(&mut self, resolve: &Resolve, unit: &Unit<'_>) -> CargoResult<()> {
528-
if !self.validated.insert(unit.pkg.package_id()) {
529-
return Ok(());
530-
}
531-
let lib = match unit.pkg.manifest().links() {
532-
Some(lib) => lib,
533-
None => return Ok(()),
534-
};
535-
if let Some(&prev) = self.links.get(lib) {
536-
let pkg = unit.pkg.package_id();
537-
538-
let describe_path = |pkgid: PackageId| -> String {
539-
let dep_path = resolve.path_to_top(&pkgid);
540-
let mut dep_path_desc = format!("package `{}`", dep_path[0]);
541-
for dep in dep_path.iter().skip(1) {
542-
write!(dep_path_desc, "\n ... which is depended on by `{}`", dep).unwrap();
543-
}
544-
dep_path_desc
545-
};
546-
547-
failure::bail!(
548-
"multiple packages link to native library `{}`, \
549-
but a native library can be linked only once\n\
550-
\n\
551-
{}\nlinks to native library `{}`\n\
552-
\n\
553-
{}\nalso links to native library `{}`",
554-
lib,
555-
describe_path(prev),
556-
lib,
557-
describe_path(pkg),
558-
lib
559-
)
560-
}
561-
if !unit
562-
.pkg
563-
.manifest()
564-
.targets()
565-
.iter()
566-
.any(|t| t.is_custom_build())
567-
{
568-
failure::bail!(
569-
"package `{}` specifies that it links to `{}` but does not \
570-
have a custom build script",
571-
unit.pkg.package_id(),
572-
lib
573-
)
574-
}
575-
self.links.insert(lib.to_string(), unit.pkg.package_id());
576-
Ok(())
577-
}
578-
}

src/cargo/core/compiler/custom_build.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoRes
190190

191191
// Be sure to pass along all enabled features for this package, this is the
192192
// last piece of statically known information that we have.
193-
for feat in bcx.resolve.features(unit.pkg.package_id()).iter() {
193+
for feat in &unit.features {
194194
cmd.env(&format!("CARGO_FEATURE_{}", super::envify(feat)), "1");
195195
}
196196

0 commit comments

Comments
 (0)