Auto merge of rust-lang#7699 - ehuss:build-std-no-sysroot, r=alexcrichton

Switch build-std to use --extern

Switch `build-std` to use `--extern` flags instead of `--sysroot`.

This is mostly a revert of rust-lang#7421. It uses the new extern flag options introduced in rust-lang/rust#67074. It avoids modifying the extern prelude which was the source of the problem of rust-lang/wg-cargo-std-aware#40.

Closes rust-lang/wg-cargo-std-aware#49
Reopens rust-lang/wg-cargo-std-aware#31

Author: bors

src/cargo/core/compiler/compilation.rs

@@ -1,24 +1,25 @@
 use std::collections::{BTreeSet, HashMap, HashSet};
 use std::env;
-use std::ffi::OsStr;
+use std::ffi::{OsStr, OsString};
 use std::path::PathBuf;
 
 use cargo_platform::CfgExpr;
 use semver::Version;
 
 use super::BuildContext;
 use crate::core::compiler::CompileKind;
-use crate::core::{Edition, InternedString, Package, PackageId, Target};
+use crate::core::{Edition, Package, PackageId, Target};
 use crate::util::{self, join_paths, process, CargoResult, Config, ProcessBuilder};
 
 pub struct Doctest {
     /// The package being doc-tested.
     pub package: Package,
     /// The target being tested (currently always the package's lib).
     pub target: Target,
-    /// Extern dependencies needed by `rustdoc`. The path is the location of
-    /// the compiled lib.
-    pub deps: Vec<(InternedString, PathBuf)>,
+    /// Arguments needed to pass to rustdoc to run this test.
+    pub args: Vec<OsString>,
+    /// Whether or not -Zunstable-options is needed.
+    pub unstable_opts: bool,
 }
 
 /// A structure returning the result of a compilation.

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

@@ -1,14 +1,12 @@
 #![allow(deprecated)]
 use std::collections::{BTreeSet, HashMap, HashSet};
-use std::ffi::OsStr;
 use std::path::PathBuf;
 use std::sync::{Arc, Mutex};
 
 use filetime::FileTime;
 use jobserver::Client;
 
-use crate::core::compiler::compilation;
-use crate::core::compiler::Unit;
+use crate::core::compiler::{self, compilation, Unit};
 use crate::core::PackageId;
 use crate::util::errors::{CargoResult, CargoResultExt};
 use crate::util::{internal, profile, Config};
@@ -18,7 +16,6 @@ use super::custom_build::{self, BuildDeps, BuildScriptOutputs, BuildScripts};
 use super::fingerprint::Fingerprint;
 use super::job_queue::JobQueue;
 use super::layout::Layout;
-use super::standard_lib;
 use super::unit_dependencies::{UnitDep, UnitGraph};
 use super::{BuildContext, Compilation, CompileKind, CompileMode, Executor, FileFlavor};
 
@@ -206,35 +203,13 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
 
             // Collect information for `rustdoc --test`.
             if unit.mode.is_doc_test() {
-                // Note that we can *only* doc-test rlib outputs here. A
-                // staticlib output cannot be linked by the compiler (it just
-                // doesn't do that). A dylib output, however, can be linked by
-                // the compiler, but will always fail. Currently all dylibs are
-                // built as "static dylibs" where the standard library is
-                // statically linked into the dylib. The doc tests fail,
-                // however, for now as they try to link the standard library
-                // dynamically as well, causing problems. As a result we only
-                // pass `--extern` for rlib deps and skip out on all other
-                // artifacts.
-                let mut doctest_deps = Vec::new();
-                for dep in self.unit_deps(unit) {
-                    if dep.unit.target.is_lib() && dep.unit.mode == CompileMode::Build {
-                        let outputs = self.outputs(&dep.unit)?;
-                        let outputs = outputs.iter().filter(|output| {
-                            output.path.extension() == Some(OsStr::new("rlib"))
-                                || dep.unit.target.for_host()
-                        });
-                        for output in outputs {
-                            doctest_deps.push((dep.extern_crate_name, output.path.clone()));
-                        }
-                    }
-                }
-                // Help with tests to get a stable order with renamed deps.
-                doctest_deps.sort();
+                let mut unstable_opts = false;
+                let args = compiler::extern_args(&self, unit, &mut unstable_opts)?;
                 self.compilation.to_doc_test.push(compilation::Doctest {
                     package: unit.pkg.clone(),
                     target: unit.target.clone(),
-                    deps: doctest_deps,
+                    args,
+                    unstable_opts,
                 });
             }
 
@@ -312,7 +287,6 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
         let mut targets = HashMap::new();
         if let CompileKind::Target(target) = self.bcx.build_config.requested_kind {
             let layout = Layout::new(self.bcx.ws, Some(target), &dest)?;
-            standard_lib::prepare_sysroot(&layout)?;
             targets.insert(target, layout);
         }
         self.primary_packages

src/cargo/core/compiler/job_queue.rs

@@ -17,7 +17,6 @@ use super::job::{
     Freshness::{self, Dirty, Fresh},
     Job,
 };
-use super::standard_lib;
 use super::timings::Timings;
 use super::{BuildContext, BuildPlan, CompileMode, Context, Unit};
 use crate::core::compiler::ProfileKind;
@@ -618,23 +617,6 @@ impl<'a, 'cfg> JobQueue<'a, 'cfg> {
             Artifact::All => self.timings.unit_finished(id, unlocked),
             Artifact::Metadata => self.timings.unit_rmeta_finished(id, unlocked),
         }
-        if unit.is_std && !unit.kind.is_host() && !cx.bcx.build_config.build_plan {
-            // This is a bit of an unusual place to copy files around, and
-            // ideally this would be somewhere like the Work closure
-            // (`link_targets`). The tricky issue is handling rmeta files for
-            // pipelining. Since those are emitted asynchronously, the code
-            // path (like `on_stderr_line`) does not have enough information
-            // to know where the sysroot is, and that it is an std unit. If
-            // possible, it might be nice to eventually move this to the
-            // worker thread, but may be tricky to have the paths available.
-            // Another possibility is to disable pipelining between std ->
-            // non-std. The pipelining opportunities are small, and are not a
-            // huge win (in a full build, only proc_macro overlaps for 2
-            // seconds out of a 90s build on my system). Care must also be
-            // taken to properly copy these artifacts for Fresh units.
-            let rmeta = artifact == Artifact::Metadata;
-            standard_lib::add_sysroot_artifact(cx, unit, rmeta)?;
-        }
         Ok(())
     }
 

src/cargo/core/compiler/layout.rs

@@ -53,11 +53,6 @@
 //!         # incremental is enabled.
 //!         incremental/
 //!
-//!         # The sysroot for -Zbuild-std builds. This only appears in
-//!         # target-triple directories (not host), and only if -Zbuild-std is
-//!         # enabled.
-//!         .sysroot/
-//!
 //!     # This is the location at which the output of all custom build
 //!     # commands are rooted.
 //!     build/
@@ -129,10 +124,6 @@ pub struct Layout {
     examples: PathBuf,
     /// The directory for rustdoc output: `$root/doc`
     doc: PathBuf,
-    /// The local sysroot for the build-std feature.
-    sysroot: Option<PathBuf>,
-    /// The "lib" directory within `sysroot`.
-    sysroot_libdir: Option<PathBuf>,
     /// The lockfile for a build (`.cargo-lock`). Will be unlocked when this
     /// struct is `drop`ped.
     _lock: FileLock,
@@ -176,21 +167,6 @@ impl Layout {
         let root = root.into_path_unlocked();
         let dest = dest.into_path_unlocked();
 
-        // Compute the sysroot path for the build-std feature.
-        let build_std = ws.config().cli_unstable().build_std.as_ref();
-        let (sysroot, sysroot_libdir) = if let Some(target) = build_std.and(target) {
-            // This uses a leading dot to avoid collision with named profiles.
-            let sysroot = dest.join(".sysroot");
-            let sysroot_libdir = sysroot
-                .join("lib")
-                .join("rustlib")
-                .join(target.short_name())
-                .join("lib");
-            (Some(sysroot), Some(sysroot_libdir))
-        } else {
-            (None, None)
-        };
-
         Ok(Layout {
             deps: dest.join("deps"),
             build: dest.join("build"),
@@ -200,8 +176,6 @@ impl Layout {
             doc: root.join("doc"),
             root,
             dest,
-            sysroot,
-            sysroot_libdir,
             _lock: lock,
         })
     }
@@ -249,16 +223,6 @@ impl Layout {
     pub fn build(&self) -> &Path {
         &self.build
     }
-    /// The local sysroot for the build-std feature.
-    ///
-    /// Returns None if build-std is not enabled or this is the Host layout.
-    pub fn sysroot(&self) -> Option<&Path> {
-        self.sysroot.as_ref().map(|p| p.as_ref())
-    }
-    /// The "lib" directory within `sysroot`.
-    pub fn sysroot_libdir(&self) -> Option<&Path> {
-        self.sysroot_libdir.as_ref().map(|p| p.as_ref())
-    }
 }
 
 #[cfg(not(target_os = "macos"))]

src/cargo/core/compiler/mod.rs

@@ -43,8 +43,7 @@ use self::unit_dependencies::UnitDep;
 pub use crate::core::compiler::unit::{Unit, UnitInterner};
 use crate::core::manifest::TargetSourcePath;
 use crate::core::profiles::{Lto, PanicStrategy, Profile};
-use crate::core::Feature;
-use crate::core::{PackageId, Target};
+use crate::core::{Feature, InternedString, PackageId, Target};
 use crate::util::errors::{self, CargoResult, CargoResultExt, Internal, ProcessError};
 use crate::util::machine_message::Message;
 use crate::util::paths;
@@ -894,8 +893,7 @@ fn build_deps_args<'a, 'cfg>(
         });
     }
 
-    // Create Vec since mutable cx is needed in closure below.
-    let deps = Vec::from(cx.unit_deps(unit));
+    let deps = cx.unit_deps(unit);
 
     // If there is not one linkable target but should, rustc fails later
     // on if there is an `extern crate` for it. This may turn into a hard
@@ -922,23 +920,14 @@ fn build_deps_args<'a, 'cfg>(
 
     let mut unstable_opts = false;
 
-    if let Some(sysroot) = cx.files().layout(unit.kind).sysroot() {
-        if !unit.kind.is_host() {
-            cmd.arg("--sysroot").arg(sysroot);
-        }
-    }
-
     for dep in deps {
-        if !unit.is_std && dep.unit.is_std {
-            // Dependency to sysroot crate uses --sysroot.
-            continue;
-        }
         if dep.unit.mode.is_run_custom_build() {
             cmd.env("OUT_DIR", &cx.files().build_script_out_dir(&dep.unit));
         }
-        if dep.unit.target.linkable() && !dep.unit.mode.is_doc() {
-            link_to(cmd, cx, unit, &dep, &mut unstable_opts)?;
-        }
+    }
+
+    for arg in extern_args(cx, unit, &mut unstable_opts)? {
+        cmd.arg(arg);
     }
 
     // This will only be set if we're already using a feature
@@ -948,37 +937,51 @@ fn build_deps_args<'a, 'cfg>(
     }
 
     return Ok(());
+}
 
-    fn link_to<'a, 'cfg>(
-        cmd: &mut ProcessBuilder,
-        cx: &mut Context<'a, 'cfg>,
-        current: &Unit<'a>,
-        dep: &UnitDep<'a>,
-        need_unstable_opts: &mut bool,
-    ) -> CargoResult<()> {
+/// Generates a list of `--extern` arguments.
+pub fn extern_args<'a>(
+    cx: &Context<'a, '_>,
+    unit: &Unit<'a>,
+    unstable_opts: &mut bool,
+) -> CargoResult<Vec<OsString>> {
+    let mut result = Vec::new();
+    let deps = cx.unit_deps(unit);
+
+    // Closure to add one dependency to `result`.
+    let mut link_to = |dep: &UnitDep<'a>,
+                       extern_crate_name: InternedString,
+                       noprelude: bool|
+     -> CargoResult<()> {
         let mut value = OsString::new();
-        value.push(dep.extern_crate_name.as_str());
+        let mut opts = Vec::new();
+        if unit
+            .pkg
+            .manifest()
+            .features()
+            .require(Feature::public_dependency())
+            .is_ok()
+            && !dep.public
+        {
+            opts.push("priv");
+            *unstable_opts = true;
+        }
+        if noprelude {
+            opts.push("noprelude");
+            *unstable_opts = true;
+        }
+        if !opts.is_empty() {
+            value.push(opts.join(","));
+            value.push(":");
+        }
+        value.push(extern_crate_name.as_str());
         value.push("=");
 
         let mut pass = |file| {
             let mut value = value.clone();
             value.push(file);
-
-            if current
-                .pkg
-                .manifest()
-                .features()
-                .require(Feature::public_dependency())
-                .is_ok()
-                && !dep.public
-            {
-                cmd.arg("--extern-private");
-                *need_unstable_opts = true;
-            } else {
-                cmd.arg("--extern");
-            }
-
-            cmd.arg(&value);
+            result.push(OsString::from("--extern"));
+            result.push(value);
         };
 
         let outputs = cx.outputs(&dep.unit)?;
@@ -987,7 +990,7 @@ fn build_deps_args<'a, 'cfg>(
             _ => None,
         });
 
-        if cx.only_requires_rmeta(current, &dep.unit) {
+        if cx.only_requires_rmeta(unit, &dep.unit) {
             let (output, _rmeta) = outputs
                 .find(|(_output, rmeta)| *rmeta)
                 .expect("failed to find rlib dep for pipelined dep");
@@ -1000,7 +1003,15 @@ fn build_deps_args<'a, 'cfg>(
             }
         }
         Ok(())
+    };
+
+    for dep in deps {
+        if dep.unit.target.linkable() && !dep.unit.mode.is_doc() {
+            link_to(&dep, dep.extern_crate_name, dep.noprelude)?;
+        }
     }
+
+    Ok(result)
 }
 
 fn envify(s: &str) -> String {