From b6a4b074ed924878fefb631c7d559ca47331e5eb Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Tue, 28 Apr 2020 13:05:55 -0700 Subject: [PATCH] build-std: Don't treat std like a "local" package. --- src/cargo/core/compiler/build_context/mod.rs | 6 +---- src/cargo/core/compiler/job_queue.rs | 2 +- src/cargo/core/compiler/mod.rs | 9 ++++--- src/cargo/core/compiler/output_depinfo.rs | 3 +-- src/cargo/core/compiler/standard_lib.rs | 9 +++++-- src/cargo/core/compiler/unit.rs | 14 ++++++++++ src/cargo/core/compiler/unit_dependencies.rs | 12 ++++++--- src/cargo/core/profiles.rs | 3 ++- src/cargo/ops/cargo_clean.rs | 2 ++ src/cargo/ops/cargo_compile.rs | 10 +++++-- tests/testsuite/profile_config.rs | 7 ++--- tests/testsuite/standard_lib.rs | 28 ++++++++++++++++++++ 12 files changed, 81 insertions(+), 24 deletions(-) diff --git a/src/cargo/core/compiler/build_context/mod.rs b/src/cargo/core/compiler/build_context/mod.rs index 01dc3260615..8dac81cffd2 100644 --- a/src/cargo/core/compiler/build_context/mod.rs +++ b/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> { self.extra_compiler_args.get(unit) } diff --git a/src/cargo/core/compiler/job_queue.rs b/src/cargo/core/compiler/job_queue.rs index 413f255184a..7a103b13d21 100644 --- a/src/cargo/core/compiler/job_queue.rs +++ b/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); diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index ecf64380887..3fa1e7e6bf9 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/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) -> 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) -> 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"); } } diff --git a/src/cargo/core/compiler/output_depinfo.rs b/src/cargo/core/compiler/output_depinfo.rs index 38b422fed91..284cb338cd5 100644 --- a/src/cargo/core/compiler/output_depinfo.rs +++ b/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)?; } } diff --git a/src/cargo/core/compiler/standard_lib.rs b/src/cargo/core/compiler/standard_lib.rs index 8556448e502..821d42f01d6 100644 --- a/src/cargo/core/compiler/standard_lib.rs +++ b/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( diff --git a/src/cargo/core/compiler/unit.rs b/src/cargo/core/compiler/unit.rs index e35ddc0bfba..ebbba387983 100644 --- a/src/cargo/core/compiler/unit.rs +++ b/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 { diff --git a/src/cargo/core/compiler/unit_dependencies.rs b/src/cargo/core/compiler/unit_dependencies.rs index 3fd2ea3301a..d5cbed9a48d 100644 --- a/src/cargo/core/compiler/unit_dependencies.rs +++ b/src/cargo/core/compiler/unit_dependencies.rs @@ -574,10 +574,14 @@ fn new_unit_dep( kind: CompileKind, mode: CompileMode, ) -> CargoResult { - 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) } diff --git a/src/cargo/core/profiles.rs b/src/cargo/core/profiles.rs index 6083842cf99..d4769389abd 100644 --- a/src/cargo/core/profiles.rs +++ b/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; diff --git a/src/cargo/ops/cargo_clean.rs b/src/cargo/ops/cargo_clean.rs index d3f1c9e7c74..132643271c6 100644 --- a/src/cargo/ops/cargo_clean.rs +++ b/src/cargo/ops/cargo_clean.rs @@ -109,6 +109,7 @@ 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, )) @@ -116,6 +117,7 @@ pub fn clean(ws: &Workspace<'_>, opts: &CleanOptions<'_>) -> CargoResult<()> { profiles.get_profile( pkg.package_id(), ws.is_member(pkg), + /*is_local*/ true, *unit_for, *mode, ) diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 31aaf361051..8ad07991526 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/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()); diff --git a/tests/testsuite/profile_config.rs b/tests/testsuite/profile_config.rs index 3617c2fa9e7..cb917b698a0 100644 --- a/tests/testsuite/profile_config.rs +++ b/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 diff --git a/tests/testsuite/standard_lib.rs b/tests/testsuite/standard_lib.rs index 8ebd35502ae..aa33ff3e307 100644 --- a/tests/testsuite/standard_lib.rs +++ b/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-")); +}