Auto merge of #8177 - ehuss:build-std-incremental, r=alexcrichton

build-std: Don't treat std like a "local" package.

This changes it so that build-std will not treat the std crates like a "local" package. This has the following changes:

- std does not build with incremental. This generally shouldn't be needed, and incremental has various bugs with std crates.
- Cargo's dep-info fingerprint tracking will not track the std crate sources, these are tracked via other means.
- Cargo's `.d` dep-info file does not include std crate sources.
- Lints are capped to "allow" for std crates, and warnings are not shown by default.

Closes rust-lang/wg-cargo-std-aware#44
Closes rust-lang/wg-cargo-std-aware#55

Author: bors

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

@@ -1,8 +1,8 @@
 use crate::core::compiler::unit_graph::UnitGraph;
 use crate::core::compiler::{BuildConfig, CompileKind, Unit};
 use crate::core::profiles::Profiles;
+use crate::core::PackageSet;
 use crate::core::{InternedString, Workspace};
-use crate::core::{PackageId, PackageSet};
 use crate::util::config::Config;
 use crate::util::errors::CargoResult;
 use crate::util::Rustc;
@@ -99,10 +99,6 @@ impl<'a, 'cfg> BuildContext<'a, 'cfg> {
         &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 extra_args_for(&self, unit: &Unit) -> Option<&Vec<String>> {
         self.extra_compiler_args.get(unit)
     }

src/cargo/core/compiler/job_queue.rs

@@ -889,7 +889,7 @@ impl<'cfg> DrainState<'cfg> {
         artifact: Artifact,
         cx: &mut Context<'_, '_>,
     ) -> CargoResult<()> {
-        if unit.mode.is_run_custom_build() && cx.bcx.show_warnings(unit.pkg.package_id()) {
+        if unit.mode.is_run_custom_build() && unit.show_warnings(cx.bcx.config) {
             self.emit_warnings(None, unit, cx)?;
         }
         let unlocked = self.queue.finish(unit, &artifact);

src/cargo/core/compiler/mod.rs

@@ -136,7 +136,7 @@ fn compile<'cfg>(
             };
             work.then(link_targets(cx, unit, false)?)
         } else {
-            let work = if cx.bcx.show_warnings(unit.pkg.package_id()) {
+            let work = if unit.show_warnings(bcx.config) {
                 replay_output_cache(
                     unit.pkg.package_id(),
                     &unit.target,
@@ -223,6 +223,7 @@ fn rustc(cx: &mut Context<'_, '_>, unit: &Unit, exec: &Arc<dyn Executor>) -> Car
         .to_path_buf();
     let fingerprint_dir = cx.files().fingerprint_dir(unit);
     let script_metadata = cx.find_build_script_metadata(unit.clone());
+    let is_local = unit.is_local();
 
     return Ok(Work::new(move |state| {
         // Only at runtime have we discovered what the extra -L and -l
@@ -312,7 +313,7 @@ fn rustc(cx: &mut Context<'_, '_>, unit: &Unit, exec: &Arc<dyn Executor>) -> Car
                 &pkg_root,
                 &target_dir,
                 // Do not track source files in the fingerprint for registry dependencies.
-                current_id.source_id().is_path(),
+                is_local,
             )
             .chain_err(|| {
                 internal(format!(
@@ -687,12 +688,12 @@ fn add_path_args(bcx: &BuildContext<'_, '_>, unit: &Unit, cmd: &mut ProcessBuild
 fn add_cap_lints(bcx: &BuildContext<'_, '_>, unit: &Unit, cmd: &mut ProcessBuilder) {
     // If this is an upstream dep we don't want warnings from, turn off all
     // lints.
-    if !bcx.show_warnings(unit.pkg.package_id()) {
+    if !unit.show_warnings(bcx.config) {
         cmd.arg("--cap-lints").arg("allow");
 
     // If this is an upstream dep but we *do* want warnings, make sure that they
     // don't fail compilation.
-    } else if !unit.pkg.package_id().source_id().is_path() {
+    } else if !unit.is_local() {
         cmd.arg("--cap-lints").arg("warn");
     }
 }

src/cargo/core/compiler/output_depinfo.rs

@@ -96,8 +96,7 @@ fn add_deps_for_unit(
     // Recursively traverse all transitive dependencies
     let unit_deps = Vec::from(cx.unit_deps(unit)); // Create vec due to mutable borrow.
     for dep in unit_deps {
-        let source_id = dep.unit.pkg.package_id().source_id();
-        if source_id.is_path() {
+        if unit.is_local() {
             add_deps_for_unit(deps, cx, &dep.unit, visited)?;
         }
     }

src/cargo/core/compiler/standard_lib.rs

@@ -152,8 +152,13 @@ pub fn generate_std_roots(
             // in time is minimal, and the difference in caching is
             // significant.
             let mode = CompileMode::Build;
-            let profile =
-                profiles.get_profile(pkg.package_id(), /*is_member*/ false, unit_for, mode);
+            let profile = profiles.get_profile(
+                pkg.package_id(),
+                /*is_member*/ false,
+                /*is_local*/ false,
+                unit_for,
+                mode,
+            );
             let features =
                 std_features.activated_features(pkg.package_id(), FeaturesFor::NormalOrDev);
             Ok(interner.intern(

src/cargo/core/compiler/unit.rs

@@ -2,6 +2,7 @@ use crate::core::compiler::{CompileKind, CompileMode};
 use crate::core::manifest::{LibKind, Target, TargetKind};
 use crate::core::{profiles::Profile, InternedString, Package};
 use crate::util::hex::short_hash;
+use crate::util::Config;
 use std::cell::RefCell;
 use std::collections::HashSet;
 use std::fmt;
@@ -67,6 +68,19 @@ impl UnitInner {
     pub fn requires_upstream_objects(&self) -> bool {
         self.mode.is_any_test() || self.target.kind().requires_upstream_objects()
     }
+
+    /// Returns whether or not this is a "local" package.
+    ///
+    /// A "local" package is one that the user can likely edit, or otherwise
+    /// wants warnings, etc.
+    pub fn is_local(&self) -> bool {
+        self.pkg.package_id().source_id().is_path() && !self.is_std
+    }
+
+    /// Returns whether or not warnings should be displayed for this unit.
+    pub fn show_warnings(&self, config: &Config) -> bool {
+        self.is_local() || config.extra_verbose()
+    }
 }
 
 impl Unit {

src/cargo/core/compiler/unit_dependencies.rs

@@ -574,10 +574,14 @@ fn new_unit_dep(
     kind: CompileKind,
     mode: CompileMode,
 ) -> CargoResult<UnitDep> {
-    let profile =
-        state
-            .profiles
-            .get_profile(pkg.package_id(), state.ws.is_member(pkg), unit_for, mode);
+    let is_local = pkg.package_id().source_id().is_path() && !state.is_std;
+    let profile = state.profiles.get_profile(
+        pkg.package_id(),
+        state.ws.is_member(pkg),
+        is_local,
+        unit_for,
+        mode,
+    );
     new_unit_dep_with_profile(state, parent, pkg, target, unit_for, kind, mode, profile)
 }
 

src/cargo/core/profiles.rs

@@ -287,6 +287,7 @@ impl Profiles {
         &self,
         pkg_id: PackageId,
         is_member: bool,
+        is_local: bool,
         unit_for: UnitFor,
         mode: CompileMode,
     ) -> Profile {
@@ -360,7 +361,7 @@ impl Profiles {
         // itself (aka crates.io / git dependencies)
         //
         // (see also https://github.com/rust-lang/cargo/issues/3972)
-        if !pkg_id.source_id().is_path() {
+        if !is_local {
             profile.incremental = false;
         }
         profile.name = profile_name;

src/cargo/ops/cargo_clean.rs

@@ -109,13 +109,15 @@ pub fn clean(ws: &Workspace<'_>, opts: &CleanOptions<'_>) -> CargoResult<()> {
                             profiles.get_profile_run_custom_build(&profiles.get_profile(
                                 pkg.package_id(),
                                 ws.is_member(pkg),
+                                /*is_local*/ true,
                                 *unit_for,
                                 CompileMode::Build,
                             ))
                         } else {
                             profiles.get_profile(
                                 pkg.package_id(),
                                 ws.is_member(pkg),
+                                /*is_local*/ true,
                                 *unit_for,
                                 *mode,
                             )

src/cargo/ops/cargo_compile.rs

@@ -760,8 +760,14 @@ fn generate_targets(
             _ => target_mode,
         };
         let kind = default_arch_kind.for_target(target);
-        let profile =
-            profiles.get_profile(pkg.package_id(), ws.is_member(pkg), unit_for, target_mode);
+        let is_local = pkg.package_id().source_id().is_path();
+        let profile = profiles.get_profile(
+            pkg.package_id(),
+            ws.is_member(pkg),
+            is_local,
+            unit_for,
+            target_mode,
+        );
 
         // No need to worry about build-dependencies, roots are never build dependencies.
         let features_for = FeaturesFor::from_for_host(target.proc_macro());

tests/testsuite/profile_config.rs

@@ -405,7 +405,8 @@ fn named_config_profile() {
     let dep_pkg = PackageId::new("dep", "0.1.0", crates_io).unwrap();
 
     // normal package
-    let p = profiles.get_profile(a_pkg, true, UnitFor::new_normal(), CompileMode::Build);
+    let mode = CompileMode::Build;
+    let p = profiles.get_profile(a_pkg, true, true, UnitFor::new_normal(), mode);
     assert_eq!(p.name, "foo");
     assert_eq!(p.codegen_units, Some(2)); // "foo" from config
     assert_eq!(p.opt_level, "1"); // "middle" from manifest
@@ -414,7 +415,7 @@ fn named_config_profile() {
     assert_eq!(p.overflow_checks, true); // "dev" built-in (ignore package override)
 
     // build-override
-    let bo = profiles.get_profile(a_pkg, true, UnitFor::new_host(false), CompileMode::Build);
+    let bo = profiles.get_profile(a_pkg, true, true, UnitFor::new_host(false), mode);
     assert_eq!(bo.name, "foo");
     assert_eq!(bo.codegen_units, Some(6)); // "foo" build override from config
     assert_eq!(bo.opt_level, "1"); // SAME as normal
@@ -423,7 +424,7 @@ fn named_config_profile() {
     assert_eq!(bo.overflow_checks, true); // SAME as normal
 
     // package overrides
-    let po = profiles.get_profile(dep_pkg, false, UnitFor::new_normal(), CompileMode::Build);
+    let po = profiles.get_profile(dep_pkg, false, true, UnitFor::new_normal(), mode);
     assert_eq!(po.name, "foo");
     assert_eq!(po.codegen_units, Some(7)); // "foo" package override from config
     assert_eq!(po.opt_level, "1"); // SAME as normal

tests/testsuite/standard_lib.rs

@@ -576,3 +576,31 @@ fn macro_expanded_shadow() {
 
     p.cargo("build -v").build_std(&setup).target_host().run();
 }
+
+#[cargo_test]
+fn ignores_incremental() {
+    // Incremental is not really needed for std, make sure it is disabled.
+    // Incremental also tends to have bugs that affect std libraries more than
+    // any other crate.
+    let setup = match setup() {
+        Some(s) => s,
+        None => return,
+    };
+    let p = project().file("src/lib.rs", "").build();
+    p.cargo("build")
+        .env("CARGO_INCREMENTAL", "1")
+        .build_std(&setup)
+        .target_host()
+        .run();
+    let incremental: Vec<_> = p
+        .glob(format!("target/{}/debug/incremental/*", rustc_host()))
+        .map(|e| e.unwrap())
+        .collect();
+    assert_eq!(incremental.len(), 1);
+    assert!(incremental[0]
+        .file_name()
+        .unwrap()
+        .to_str()
+        .unwrap()
+        .starts_with("foo-"));
+}