Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Apr 28, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions src/cargo/core/compiler/build_context/mod.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/core/compiler/job_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
9 changes: 5 additions & 4 deletions src/cargo/core/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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!(
Expand Down Expand Up @@ -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");
}
}
Expand Down
3 changes: 1 addition & 2 deletions src/cargo/core/compiler/output_depinfo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;
}
}
Expand Down
9 changes: 7 additions & 2 deletions src/cargo/core/compiler/standard_lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
14 changes: 14 additions & 0 deletions src/cargo/core/compiler/unit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down
12 changes: 8 additions & 4 deletions src/cargo/core/compiler/unit_dependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
3 changes: 2 additions & 1 deletion src/cargo/core/profiles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,7 @@ impl Profiles {
&self,
pkg_id: PackageId,
is_member: bool,
is_local: bool,
unit_for: UnitFor,
mode: CompileMode,
) -> Profile {
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 2 additions & 0 deletions src/cargo/ops/cargo_clean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down
10 changes: 8 additions & 2 deletions src/cargo/ops/cargo_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
7 changes: 4 additions & 3 deletions tests/testsuite/profile_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
28 changes: 28 additions & 0 deletions tests/testsuite/standard_lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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-"));
}