From 79dd5117dac51ab56622a94ca984db50b08625ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Sat, 15 Oct 2022 14:30:37 +0000 Subject: [PATCH 01/17] introduce dedicated `DebugInfo` enum in `Profile`s This enum will be used to model the current Option value in profiles, as the explicitly set value, and also allow to model a deferred value: one that can be ignored for optimization purposes, and used in all other cases. This allows to have a default debuginfo for build dependencies, that can change: - if a dependency is shared between the build and runtime subgraphs, the default value will be used, to that re-use between units will continue. - otherwise, a build dependency is only used in a context where debuginfo is not very useful. We can optimize build times in this situation by not asking rustc to emit debuginfo. --- src/cargo/core/compiler/custom_build.rs | 2 +- src/cargo/core/compiler/job_queue.rs | 2 +- src/cargo/core/compiler/mod.rs | 4 +- src/cargo/core/profiles.rs | 94 +++++++++++++++++++++++-- tests/testsuite/profile_config.rs | 6 +- 5 files changed, 94 insertions(+), 14 deletions(-) diff --git a/src/cargo/core/compiler/custom_build.rs b/src/cargo/core/compiler/custom_build.rs index 630c2119ecc..cac5cb32eea 100644 --- a/src/cargo/core/compiler/custom_build.rs +++ b/src/cargo/core/compiler/custom_build.rs @@ -187,7 +187,7 @@ fn build_work(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult { // carried over. let to_exec = to_exec.into_os_string(); let mut cmd = cx.compilation.host_process(to_exec, &unit.pkg)?; - let debug = unit.profile.debuginfo.unwrap_or(0) != 0; + let debug = unit.profile.debuginfo.is_turned_on(); cmd.env("OUT_DIR", &script_out_dir) .env("CARGO_MANIFEST_DIR", unit.pkg.root()) .env("NUM_JOBS", &bcx.jobs().to_string()) diff --git a/src/cargo/core/compiler/job_queue.rs b/src/cargo/core/compiler/job_queue.rs index 83e25052abb..47b2e769445 100644 --- a/src/cargo/core/compiler/job_queue.rs +++ b/src/cargo/core/compiler/job_queue.rs @@ -985,7 +985,7 @@ impl<'cfg> DrainState<'cfg> { } else { "optimized" }); - if profile.debuginfo.unwrap_or(0) != 0 { + if profile.debuginfo.is_turned_on() { opt_type += " + debuginfo"; } diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index 0978b584bc3..541e6f54dc8 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -550,7 +550,7 @@ fn link_targets(cx: &mut Context<'_, '_>, unit: &Unit, fresh: bool) -> CargoResu if json_messages { let art_profile = machine_message::ArtifactProfile { opt_level: profile.opt_level.as_str(), - debuginfo: profile.debuginfo, + debuginfo: profile.debuginfo.to_option(), debug_assertions: profile.debug_assertions, overflow_checks: profile.overflow_checks, test: unit_mode.is_any_test(), @@ -1003,7 +1003,7 @@ fn build_base_args( cmd.arg("-C").arg(&format!("codegen-units={}", n)); } - if let Some(debuginfo) = debuginfo { + if let Some(debuginfo) = debuginfo.to_option() { cmd.arg("-C").arg(format!("debuginfo={}", debuginfo)); } diff --git a/src/cargo/core/profiles.rs b/src/cargo/core/profiles.rs index 87a992b6dd8..dec3305be0e 100644 --- a/src/cargo/core/profiles.rs +++ b/src/cargo/core/profiles.rs @@ -276,7 +276,7 @@ impl Profiles { // platform which has a stable `-Csplit-debuginfo` option for rustc, // and it's typically much faster than running `dsymutil` on all builds // in incremental cases. - if let Some(debug) = profile.debuginfo { + if let Some(debug) = profile.debuginfo.to_option() { if profile.split_debuginfo.is_none() && debug > 0 { let target = match &kind { CompileKind::Host => self.rustc_host.as_str(), @@ -515,9 +515,9 @@ fn merge_profile(profile: &mut Profile, toml: &TomlProfile) { profile.codegen_units = toml.codegen_units; } match toml.debug { - Some(U32OrBool::U32(debug)) => profile.debuginfo = Some(debug), - Some(U32OrBool::Bool(true)) => profile.debuginfo = Some(2), - Some(U32OrBool::Bool(false)) => profile.debuginfo = None, + Some(U32OrBool::U32(debug)) => profile.debuginfo = DebugInfo::Explicit(debug), + Some(U32OrBool::Bool(true)) => profile.debuginfo = DebugInfo::Explicit(2), + Some(U32OrBool::Bool(false)) => profile.debuginfo = DebugInfo::None, None => {} } if let Some(debug_assertions) = toml.debug_assertions { @@ -578,7 +578,7 @@ pub struct Profile { pub codegen_backend: Option, // `None` means use rustc default. pub codegen_units: Option, - pub debuginfo: Option, + pub debuginfo: DebugInfo, pub split_debuginfo: Option, pub debug_assertions: bool, pub overflow_checks: bool, @@ -600,7 +600,7 @@ impl Default for Profile { lto: Lto::Bool(false), codegen_backend: None, codegen_units: None, - debuginfo: None, + debuginfo: DebugInfo::None, debug_assertions: false, split_debuginfo: None, overflow_checks: false, @@ -669,7 +669,7 @@ impl Profile { Profile { name: InternedString::new("dev"), root: ProfileRoot::Debug, - debuginfo: Some(2), + debuginfo: DebugInfo::Explicit(2), debug_assertions: true, overflow_checks: true, incremental: true, @@ -708,6 +708,86 @@ impl Profile { } } +/// The debuginfo level in a given profile. This is semantically an +/// `Option`, and should be used as so via the `to_option` method for all +/// intents and purposes: +/// - `DebugInfo::None` corresponds to `None` +/// - `DebugInfo::Explicit(u32)` and `DebugInfo::Deferred` correspond to +/// `Option::Some` +/// +/// Internally, it's used to model a debuginfo level whose value can be deferred +/// for optimization purposes: host dependencies usually don't need the same +/// level as target dependencies. For dependencies that are shared between the +/// two however, that value also affects reuse: different debuginfo levels would +/// cause to build a unit twice. By deferring the choice until we know +/// whether to choose the optimized value or the default value, we can make sure +/// the unit is only built once and the unit graph is still optimized. +#[derive(Debug, Copy, Clone)] +pub enum DebugInfo { + /// No debuginfo + None, + /// A debuginfo level that is explicitly set. + Explicit(u32), + /// For internal purposes: a deferred debuginfo level that can be optimized + /// away but has a default value otherwise. + /// Behaves like `Explicit` in all situations except when dependencies are + /// shared between build dependencies and runtime dependencies. The former + /// case can be optimized, in all other situations this level value will be + /// the one to use. + Deferred(u32), +} + +impl DebugInfo { + /// The main way to interact with this debuginfo level, turning it into an Option. + pub fn to_option(&self) -> Option { + match self { + DebugInfo::None => None, + DebugInfo::Explicit(v) | DebugInfo::Deferred(v) => Some(*v), + } + } + + /// Returns true if the debuginfo level is high enough (at least 1). Helper + /// for a common operation on the usual `Option` representation. + pub(crate) fn is_turned_on(&self) -> bool { + self.to_option().unwrap_or(0) != 0 + } +} + +impl serde::Serialize for DebugInfo { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + self.to_option().serialize(serializer) + } +} + +impl PartialEq for DebugInfo { + fn eq(&self, other: &DebugInfo) -> bool { + self.to_option().eq(&other.to_option()) + } +} + +impl Eq for DebugInfo {} + +impl Hash for DebugInfo { + fn hash(&self, state: &mut H) { + self.to_option().hash(state); + } +} + +impl PartialOrd for DebugInfo { + fn partial_cmp(&self, other: &Self) -> Option { + self.to_option().partial_cmp(&other.to_option()) + } +} + +impl Ord for DebugInfo { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + self.to_option().cmp(&other.to_option()) + } +} + /// The link-time-optimization setting. #[derive(Clone, Copy, PartialEq, Eq, Debug, Hash, PartialOrd, Ord)] pub enum Lto { diff --git a/tests/testsuite/profile_config.rs b/tests/testsuite/profile_config.rs index ed12956244e..c59ed7a97f5 100644 --- a/tests/testsuite/profile_config.rs +++ b/tests/testsuite/profile_config.rs @@ -436,7 +436,7 @@ fn named_config_profile() { assert_eq!(p.name, "foo"); assert_eq!(p.codegen_units, Some(2)); // "foo" from config assert_eq!(p.opt_level, "1"); // "middle" from manifest - assert_eq!(p.debuginfo, Some(1)); // "bar" from config + assert_eq!(p.debuginfo.to_option(), Some(1)); // "bar" from config assert_eq!(p.debug_assertions, true); // "dev" built-in (ignore build-override) assert_eq!(p.overflow_checks, true); // "dev" built-in (ignore package override) @@ -445,7 +445,7 @@ fn named_config_profile() { assert_eq!(bo.name, "foo"); assert_eq!(bo.codegen_units, Some(6)); // "foo" build override from config assert_eq!(bo.opt_level, "0"); // default to zero - assert_eq!(bo.debuginfo, Some(1)); // SAME as normal + assert_eq!(bo.debuginfo.to_option(), Some(1)); // SAME as normal assert_eq!(bo.debug_assertions, false); // "foo" build override from manifest assert_eq!(bo.overflow_checks, true); // SAME as normal @@ -454,7 +454,7 @@ fn named_config_profile() { 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 - assert_eq!(po.debuginfo, Some(1)); // SAME as normal + assert_eq!(po.debuginfo.to_option(), Some(1)); // SAME as normal assert_eq!(po.debug_assertions, true); // SAME as normal assert_eq!(po.overflow_checks, false); // "middle" package override from manifest } From 75af7a41fa742e38e73d40ec3d9bb322978fe388 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Sat, 15 Oct 2022 15:00:18 +0000 Subject: [PATCH 02/17] optimize build dependency debuginfo level When a build dependency unit is not within the runtime dependency subgraph, we don't ask for debuginfo to be emitted by default. --- src/cargo/core/profiles.rs | 35 +++++++++++++++++++++++++++ src/cargo/ops/cargo_compile/mod.rs | 39 +++++++++++++++++++++++++++++- 2 files changed, 73 insertions(+), 1 deletion(-) diff --git a/src/cargo/core/profiles.rs b/src/cargo/core/profiles.rs index dec3305be0e..0669cbc9e83 100644 --- a/src/cargo/core/profiles.rs +++ b/src/cargo/core/profiles.rs @@ -438,6 +438,24 @@ impl ProfileMaker { // well as enabling parallelism by not constraining codegen units. profile.opt_level = InternedString::new("0"); profile.codegen_units = None; + + // For build dependencies, we usually don't need debuginfo, and + // removing it will compile faster, much like the default opt-level + // of 0 is chosen for faster builds. However, that can conflict with + // a unit graph optimization, reusing units that are shared between + // build dependencies and runtime dependencies: when the runtime + // target is the same as the build host, we only need to build a + // dependency once and reuse the results, instead of building twice. + // If we then also change the default debuginfo level for build + // dependencies, we can lose the sharing: build dependencies will be + // built without debuginfo, while runtime dependencies require it by + // default. So we allow cargo to weaken the debuginfo level: only if + // there is not sharing already, will we disable debuginfo: if there + // is no sharing, we will use the preferred value, and if there is + // sharing we will use the explicit value set. + if let Some(debuginfo) = profile.debuginfo.to_option() { + profile.debuginfo = DebugInfo::Deferred(debuginfo); + } } // ... and next comes any other sorts of overrides specified in // profiles, such as `[profile.release.build-override]` or @@ -751,6 +769,23 @@ impl DebugInfo { pub(crate) fn is_turned_on(&self) -> bool { self.to_option().unwrap_or(0) != 0 } + + pub(crate) fn is_deferred(&self) -> bool { + matches!(self, DebugInfo::Deferred(_)) + } + + /// Force the deferred, preferred, debuginfo level to a finalized explicit value. + pub(crate) fn finalize(self) -> Self { + match self { + DebugInfo::Deferred(v) => DebugInfo::Explicit(v), + _ => self, + } + } + + /// Reset to the lowest level: no debuginfo. + pub(crate) fn weaken(self) -> Self { + DebugInfo::None + } } impl serde::Serialize for DebugInfo { diff --git a/src/cargo/ops/cargo_compile/mod.rs b/src/cargo/ops/cargo_compile/mod.rs index a25c8f7c7b9..abedf802121 100644 --- a/src/cargo/ops/cargo_compile/mod.rs +++ b/src/cargo/ops/cargo_compile/mod.rs @@ -626,10 +626,47 @@ fn traverse_and_share( Some(to_host) if to_host == unit.kind => CompileKind::Host, _ => unit.kind, }; + + let mut profile = unit.profile.clone(); + + // If this is a build dependency, and it's not shared with runtime dependencies, we can weaken + // its debuginfo level to optimize build times. + if unit.kind.is_host() && to_host.is_some() && profile.debuginfo.is_deferred() { + // We create a "probe" test to see if a unit with the same explicit debuginfo level exists + // in the graph. This is the level we'd expect if it was set manually, or a default set by + // a profile for a runtime dependency: its canonical value. + let canonical_debuginfo = profile.debuginfo.finalize(); + let mut canonical_profile = profile.clone(); + canonical_profile.debuginfo = canonical_debuginfo; + let explicit_unit_probe = interner.intern( + &unit.pkg, + &unit.target, + canonical_profile, + to_host.unwrap(), + unit.mode, + unit.features.clone(), + unit.is_std, + unit.dep_hash, + unit.artifact, + unit.artifact_target_for_features, + ); + + // We can now turn the deferred value into its actual final value. + profile.debuginfo = if unit_graph.contains_key(&explicit_unit_probe) { + // The unit is present in both build time and runtime subgraphs: we canonicalize its + // level to the other unit's, thus ensuring reuse between the two to optimize build times. + canonical_debuginfo + } else { + // The unit is only present in the build time subgraph, we can weaken its debuginfo + // level to optimize build times. + canonical_debuginfo.weaken() + } + } + let new_unit = interner.intern( &unit.pkg, &unit.target, - unit.profile.clone(), + profile, new_kind, unit.mode, unit.features.clone(), From 6848006cbe829462c458dc08214d6b854cbb7fbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Sat, 15 Oct 2022 15:02:14 +0000 Subject: [PATCH 03/17] update tests for build deps debuginfo optimization --- tests/testsuite/build.rs | 2 +- tests/testsuite/build_script.rs | 4 ++-- tests/testsuite/freshness.rs | 2 +- tests/testsuite/profile_targets.rs | 36 +++++++++++++++--------------- 4 files changed, 22 insertions(+), 22 deletions(-) diff --git a/tests/testsuite/build.rs b/tests/testsuite/build.rs index 8ad9f70ac52..42238f2cff0 100644 --- a/tests/testsuite/build.rs +++ b/tests/testsuite/build.rs @@ -3796,7 +3796,7 @@ fn compiler_json_error_format() { }, "profile": { "debug_assertions": true, - "debuginfo": 2, + "debuginfo": null, "opt_level": "0", "overflow_checks": true, "test": false diff --git a/tests/testsuite/build_script.rs b/tests/testsuite/build_script.rs index d16bac6acdf..91a20dc6f82 100644 --- a/tests/testsuite/build_script.rs +++ b/tests/testsuite/build_script.rs @@ -1651,14 +1651,14 @@ fn build_cmd_with_a_build_cmd() { [RUNNING] `rustc [..] a/build.rs [..] --extern b=[..]` [RUNNING] `[..]/a-[..]/build-script-build` [RUNNING] `rustc --crate-name a [..]lib.rs [..]--crate-type lib \ - --emit=[..]link[..]-C debuginfo=2 \ + --emit=[..]link[..] \ -C metadata=[..] \ --out-dir [..]target/debug/deps \ -L [..]target/debug/deps` [COMPILING] foo v0.5.0 ([CWD]) [RUNNING] `rustc --crate-name build_script_build build.rs [..]--crate-type bin \ --emit=[..]link[..]\ - -C debuginfo=2 -C metadata=[..] --out-dir [..] \ + -C metadata=[..] --out-dir [..] \ -L [..]target/debug/deps \ --extern a=[..]liba[..].rlib` [RUNNING] `[..]/foo-[..]/build-script-build` diff --git a/tests/testsuite/freshness.rs b/tests/testsuite/freshness.rs index 793ebb94c60..6853af13eed 100644 --- a/tests/testsuite/freshness.rs +++ b/tests/testsuite/freshness.rs @@ -1477,7 +1477,7 @@ fn reuse_panic_pm() { .with_stderr_unordered( "\ [COMPILING] bar [..] -[RUNNING] `rustc --crate-name bar bar/src/lib.rs [..]--crate-type lib --emit=[..]link[..]-C debuginfo=2 [..] +[RUNNING] `rustc --crate-name bar bar/src/lib.rs [..]--crate-type lib --emit=[..]link[..] [RUNNING] `rustc --crate-name bar bar/src/lib.rs [..]--crate-type lib --emit=[..]link -C panic=abort[..]-C debuginfo=2 [..] [COMPILING] somepm [..] [RUNNING] `rustc --crate-name somepm [..] diff --git a/tests/testsuite/profile_targets.rs b/tests/testsuite/profile_targets.rs index 1d1a45af752..c86c2dbfc43 100644 --- a/tests/testsuite/profile_targets.rs +++ b/tests/testsuite/profile_targets.rs @@ -85,11 +85,11 @@ fn profile_selection_build() { p.cargo("build -vv").with_stderr_unordered("\ [COMPILING] bar [..] [RUNNING] `[..] rustc --crate-name bar bar/src/lib.rs [..]--crate-type lib --emit=[..]link -C panic=abort[..]-C codegen-units=1 -C debuginfo=2 [..] -[RUNNING] `[..] rustc --crate-name bar bar/src/lib.rs [..]--crate-type lib --emit=[..]link[..]-C codegen-units=5 -C debuginfo=2 [..] +[RUNNING] `[..] rustc --crate-name bar bar/src/lib.rs [..]--crate-type lib --emit=[..]link[..]-C codegen-units=5 [..] [COMPILING] bdep [..] -[RUNNING] `[..] rustc --crate-name bdep bdep/src/lib.rs [..]--crate-type lib --emit=[..]link[..]-C codegen-units=5 -C debuginfo=2 [..] +[RUNNING] `[..] rustc --crate-name bdep bdep/src/lib.rs [..]--crate-type lib --emit=[..]link[..]-C codegen-units=5 [..] [COMPILING] foo [..] -[RUNNING] `[..] rustc --crate-name build_script_build build.rs [..]--crate-type bin --emit=[..]link[..]-C codegen-units=5 -C debuginfo=2 [..] +[RUNNING] `[..] rustc --crate-name build_script_build build.rs [..]--crate-type bin --emit=[..]link[..]-C codegen-units=5 [..] [RUNNING] `[..]/target/debug/build/foo-[..]/build-script-build` [foo 0.0.1] foo custom build PROFILE=debug DEBUG=true OPT_LEVEL=0 [RUNNING] `[..] rustc --crate-name foo src/lib.rs [..]--crate-type lib --emit=[..]link -C panic=abort[..]-C codegen-units=1 -C debuginfo=2 [..] @@ -173,11 +173,11 @@ fn profile_selection_build_all_targets() { [COMPILING] bar [..] [RUNNING] `[..] rustc --crate-name bar bar/src/lib.rs [..]--crate-type lib --emit=[..]link[..]-C codegen-units=1 -C debuginfo=2 [..] [RUNNING] `[..] rustc --crate-name bar bar/src/lib.rs [..]--crate-type lib --emit=[..]link -C panic=abort[..]-C codegen-units=1 -C debuginfo=2 [..] -[RUNNING] `[..] rustc --crate-name bar bar/src/lib.rs [..]--crate-type lib --emit=[..]link[..]-C codegen-units=5 -C debuginfo=2 [..] +[RUNNING] `[..] rustc --crate-name bar bar/src/lib.rs [..]--crate-type lib --emit=[..]link[..]-C codegen-units=5 [..] [COMPILING] bdep [..] -[RUNNING] `[..] rustc --crate-name bdep bdep/src/lib.rs [..]--crate-type lib --emit=[..]link[..]-C codegen-units=5 -C debuginfo=2 [..] +[RUNNING] `[..] rustc --crate-name bdep bdep/src/lib.rs [..]--crate-type lib --emit=[..]link[..]-C codegen-units=5 [..] [COMPILING] foo [..] -[RUNNING] `[..] rustc --crate-name build_script_build build.rs [..]--crate-type bin --emit=[..]link[..]-C codegen-units=5 -C debuginfo=2 [..] +[RUNNING] `[..] rustc --crate-name build_script_build build.rs [..]--crate-type bin --emit=[..]link[..]-C codegen-units=5 [..] [RUNNING] `[..]/target/debug/build/foo-[..]/build-script-build` [foo 0.0.1] foo custom build PROFILE=debug DEBUG=true OPT_LEVEL=0 [RUNNING] `[..] rustc --crate-name foo src/lib.rs [..]--crate-type lib --emit=[..]link -C panic=abort[..]-C codegen-units=1 -C debuginfo=2 [..]` @@ -295,12 +295,12 @@ fn profile_selection_test() { p.cargo("test -vv").with_stderr_unordered("\ [COMPILING] bar [..] [RUNNING] `[..] rustc --crate-name bar bar/src/lib.rs [..]--crate-type lib --emit=[..]link[..]-C codegen-units=3 -C debuginfo=2 [..] -[RUNNING] `[..] rustc --crate-name bar bar/src/lib.rs [..]--crate-type lib --emit=[..]link[..]-C codegen-units=5 -C debuginfo=2 [..] +[RUNNING] `[..] rustc --crate-name bar bar/src/lib.rs [..]--crate-type lib --emit=[..]link[..]-C codegen-units=5 [..] [RUNNING] `[..] rustc --crate-name bar bar/src/lib.rs [..]--crate-type lib --emit=[..]link -C panic=abort[..]-C codegen-units=3 -C debuginfo=2 [..] [COMPILING] bdep [..] -[RUNNING] `[..] rustc --crate-name bdep bdep/src/lib.rs [..]--crate-type lib --emit=[..]link[..]-C codegen-units=5 -C debuginfo=2 [..] +[RUNNING] `[..] rustc --crate-name bdep bdep/src/lib.rs [..]--crate-type lib --emit=[..]link[..]-C codegen-units=5 [..] [COMPILING] foo [..] -[RUNNING] `[..] rustc --crate-name build_script_build build.rs [..]--crate-type bin --emit=[..]link[..]-C codegen-units=5 -C debuginfo=2 [..] +[RUNNING] `[..] rustc --crate-name build_script_build build.rs [..]--crate-type bin --emit=[..]link[..]-C codegen-units=5 [..] [RUNNING] `[..]/target/debug/build/foo-[..]/build-script-build` [foo 0.0.1] foo custom build PROFILE=debug DEBUG=true OPT_LEVEL=0 [RUNNING] `[..] rustc --crate-name foo src/lib.rs [..]--crate-type lib --emit=[..]link -C panic=abort[..]-C codegen-units=3 -C debuginfo=2 [..] @@ -492,13 +492,13 @@ fn profile_selection_check_all_targets() { // p.cargo("check --all-targets -vv").with_stderr_unordered("\ [COMPILING] bar [..] -[RUNNING] `[..] rustc --crate-name bar bar/src/lib.rs [..]--crate-type lib --emit=[..]link[..]-C codegen-units=5 -C debuginfo=2 [..] +[RUNNING] `[..] rustc --crate-name bar bar/src/lib.rs [..]--crate-type lib --emit=[..]link[..]-C codegen-units=5 [..] [RUNNING] `[..] rustc --crate-name bar bar/src/lib.rs [..]--crate-type lib --emit=[..]metadata[..]-C codegen-units=1 -C debuginfo=2 [..] [RUNNING] `[..] rustc --crate-name bar bar/src/lib.rs [..]--crate-type lib --emit=[..]metadata -C panic=abort[..]-C codegen-units=1 -C debuginfo=2 [..] [COMPILING] bdep[..] -[RUNNING] `[..] rustc --crate-name bdep bdep/src/lib.rs [..]--crate-type lib --emit=[..]link[..]-C codegen-units=5 -C debuginfo=2 [..] +[RUNNING] `[..] rustc --crate-name bdep bdep/src/lib.rs [..]--crate-type lib --emit=[..]link[..]-C codegen-units=5 [..] [COMPILING] foo [..] -[RUNNING] `[..] rustc --crate-name build_script_build build.rs [..]--crate-type bin --emit=[..]link[..]-C codegen-units=5 -C debuginfo=2 [..] +[RUNNING] `[..] rustc --crate-name build_script_build build.rs [..]--crate-type bin --emit=[..]link[..]-C codegen-units=5 [..] [RUNNING] `[..]target/debug/build/foo-[..]/build-script-build` [foo 0.0.1] foo custom build PROFILE=debug DEBUG=true OPT_LEVEL=0 [RUNNING] `[..] rustc --crate-name foo src/lib.rs [..]--crate-type lib --emit=[..]metadata -C panic=abort[..]-C codegen-units=1 -C debuginfo=2 [..] @@ -596,12 +596,12 @@ fn profile_selection_check_all_targets_test() { // p.cargo("check --all-targets --profile=test -vv").with_stderr_unordered("\ [COMPILING] bar [..] -[RUNNING] `[..] rustc --crate-name bar bar/src/lib.rs [..]--crate-type lib --emit=[..]link[..]-C codegen-units=5 -C debuginfo=2 [..] +[RUNNING] `[..] rustc --crate-name bar bar/src/lib.rs [..]--crate-type lib --emit=[..]link[..]-C codegen-units=5 [..] [RUNNING] `[..] rustc --crate-name bar bar/src/lib.rs [..]--crate-type lib --emit=[..]metadata[..]-C codegen-units=3 -C debuginfo=2 [..] [COMPILING] bdep[..] -[RUNNING] `[..] rustc --crate-name bdep bdep/src/lib.rs [..]--crate-type lib --emit=[..]link[..]-C codegen-units=5 -C debuginfo=2 [..] +[RUNNING] `[..] rustc --crate-name bdep bdep/src/lib.rs [..]--crate-type lib --emit=[..]link[..]-C codegen-units=5 [..] [COMPILING] foo [..] -[RUNNING] `[..] rustc --crate-name build_script_build build.rs [..]--crate-type bin --emit=[..]link[..]-C codegen-units=5 -C debuginfo=2 [..] +[RUNNING] `[..] rustc --crate-name build_script_build build.rs [..]--crate-type bin --emit=[..]link[..]-C codegen-units=5 [..] [RUNNING] `[..]target/debug/build/foo-[..]/build-script-build` [foo 0.0.1] foo custom build PROFILE=debug DEBUG=true OPT_LEVEL=0 [RUNNING] `[..] rustc --crate-name foo src/lib.rs [..]--crate-type lib --emit=[..]metadata[..]-C codegen-units=3 -C debuginfo=2 [..] @@ -642,13 +642,13 @@ fn profile_selection_doc() { p.cargo("doc -vv").with_stderr_unordered("\ [COMPILING] bar [..] [DOCUMENTING] bar [..] -[RUNNING] `[..] rustc --crate-name bar bar/src/lib.rs [..]--crate-type lib --emit=[..]link[..]-C codegen-units=5 -C debuginfo=2 [..] +[RUNNING] `[..] rustc --crate-name bar bar/src/lib.rs [..]--crate-type lib --emit=[..]link[..]-C codegen-units=5 [..] [RUNNING] `rustdoc [..]--crate-name bar bar/src/lib.rs [..] [RUNNING] `[..] rustc --crate-name bar bar/src/lib.rs [..]--crate-type lib --emit=[..]metadata -C panic=abort[..]-C codegen-units=1 -C debuginfo=2 [..] [COMPILING] bdep [..] -[RUNNING] `[..] rustc --crate-name bdep bdep/src/lib.rs [..]--crate-type lib --emit=[..]link[..]-C codegen-units=5 -C debuginfo=2 [..] +[RUNNING] `[..] rustc --crate-name bdep bdep/src/lib.rs [..]--crate-type lib --emit=[..]link[..]-C codegen-units=5 [..] [COMPILING] foo [..] -[RUNNING] `[..] rustc --crate-name build_script_build build.rs [..]--crate-type bin --emit=[..]link[..]-C codegen-units=5 -C debuginfo=2 [..] +[RUNNING] `[..] rustc --crate-name build_script_build build.rs [..]--crate-type bin --emit=[..]link[..]-C codegen-units=5 [..] [RUNNING] `[..]target/debug/build/foo-[..]/build-script-build` [foo 0.0.1] foo custom build PROFILE=debug DEBUG=true OPT_LEVEL=0 [DOCUMENTING] foo [..] From 7fb97c51607f02c16a5fb2d26c8b020c095f1f7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Sat, 15 Oct 2022 15:08:01 +0000 Subject: [PATCH 04/17] clean up destructuring assignment Removes a fixme --- src/cargo/ops/cargo_compile/mod.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/cargo/ops/cargo_compile/mod.rs b/src/cargo/ops/cargo_compile/mod.rs index abedf802121..c1489cafbc9 100644 --- a/src/cargo/ops/cargo_compile/mod.rs +++ b/src/cargo/ops/cargo_compile/mod.rs @@ -428,17 +428,13 @@ pub fn create_bcx<'a, 'cfg>( // Rebuild the unit graph, replacing the explicit host targets with // CompileKind::Host, removing `artifact_target_for_features` and merging any dependencies // shared with build and artifact dependencies. - let new_graph = rebuild_unit_graph_shared( + (units, scrape_units, unit_graph) = rebuild_unit_graph_shared( interner, unit_graph, &units, &scrape_units, host_kind_requested.then_some(explicit_host_kind), ); - // This would be nicer with destructuring assignment. - units = new_graph.0; - scrape_units = new_graph.1; - unit_graph = new_graph.2; } let mut extra_compiler_args = HashMap::new(); From cec12456c824eeeb3b30d99616cb6cd0689c3af2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Sat, 15 Oct 2022 15:30:41 +0000 Subject: [PATCH 05/17] slightly clarify unit graph sharing --- src/cargo/ops/cargo_compile/mod.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/cargo/ops/cargo_compile/mod.rs b/src/cargo/ops/cargo_compile/mod.rs index c1489cafbc9..d9e387b6dd4 100644 --- a/src/cargo/ops/cargo_compile/mod.rs +++ b/src/cargo/ops/cargo_compile/mod.rs @@ -617,8 +617,17 @@ fn traverse_and_share( } }) .collect(); + // Here, we have recursively traversed this unit's dependencies, and hashed them: we can + // finalize the dep hash. let new_dep_hash = dep_hash.finish(); - let new_kind = match to_host { + + // This is the key part of the sharing process: if the unit is a runtime dependency, whose + // target is the same as the host, we canonicalize the compile kind to `CompileKind::Host`. + // A possible host dependency counterpart to this unit would have that kind, and if such a unit + // exists in the current `unit_graph`, they will unify in the new unit graph map `new_graph`. + // The resulting unit graph will have be optimized with less units, thanks to sharing these host + // dependencies. + let canonical_kind = match to_host { Some(to_host) if to_host == unit.kind => CompileKind::Host, _ => unit.kind, }; @@ -663,7 +672,7 @@ fn traverse_and_share( &unit.pkg, &unit.target, profile, - new_kind, + canonical_kind, unit.mode, unit.features.clone(), unit.is_std, From 937a08cabfdf34b61e1d511ab9df71ca31de9a61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Mon, 17 Oct 2022 16:41:24 +0000 Subject: [PATCH 06/17] improve comments --- src/cargo/core/profiles.rs | 38 +++++++++++++++--------------- src/cargo/ops/cargo_compile/mod.rs | 10 ++++---- 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/src/cargo/core/profiles.rs b/src/cargo/core/profiles.rs index 0669cbc9e83..a7e3f91e0ae 100644 --- a/src/cargo/core/profiles.rs +++ b/src/cargo/core/profiles.rs @@ -440,19 +440,15 @@ impl ProfileMaker { profile.codegen_units = None; // For build dependencies, we usually don't need debuginfo, and - // removing it will compile faster, much like the default opt-level - // of 0 is chosen for faster builds. However, that can conflict with + // removing it will compile faster. However, that can conflict with // a unit graph optimization, reusing units that are shared between // build dependencies and runtime dependencies: when the runtime // target is the same as the build host, we only need to build a // dependency once and reuse the results, instead of building twice. - // If we then also change the default debuginfo level for build - // dependencies, we can lose the sharing: build dependencies will be - // built without debuginfo, while runtime dependencies require it by - // default. So we allow cargo to weaken the debuginfo level: only if - // there is not sharing already, will we disable debuginfo: if there - // is no sharing, we will use the preferred value, and if there is - // sharing we will use the explicit value set. + // We defer the choice of the debuginfo level until we can check if + // a unit is shared. If that's the case, we'll use the deferred value + // below so the unit can be reused, otherwise we can avoid emitting + // the unit's debuginfo. if let Some(debuginfo) = profile.debuginfo.to_option() { profile.debuginfo = DebugInfo::Deferred(debuginfo); } @@ -726,9 +722,10 @@ impl Profile { } } -/// The debuginfo level in a given profile. This is semantically an -/// `Option`, and should be used as so via the `to_option` method for all -/// intents and purposes: +/// The debuginfo level setting. +/// +/// This is semantically an `Option`, and should be used as so via the +/// [DebugInfo::to_option] method for all intents and purposes: /// - `DebugInfo::None` corresponds to `None` /// - `DebugInfo::Explicit(u32)` and `DebugInfo::Deferred` correspond to /// `Option::Some` @@ -742,16 +739,19 @@ impl Profile { /// the unit is only built once and the unit graph is still optimized. #[derive(Debug, Copy, Clone)] pub enum DebugInfo { - /// No debuginfo + /// No debuginfo level was set. None, - /// A debuginfo level that is explicitly set. + /// A debuginfo level that is explicitly set, by a profile or a user. Explicit(u32), /// For internal purposes: a deferred debuginfo level that can be optimized - /// away but has a default value otherwise. - /// Behaves like `Explicit` in all situations except when dependencies are - /// shared between build dependencies and runtime dependencies. The former - /// case can be optimized, in all other situations this level value will be - /// the one to use. + /// away, but has this value otherwise. + /// + /// Behaves like `Explicit` in all situations except for the default build + /// dependencies profile: whenever a build dependency is not shared with + /// runtime dependencies, this level is weakened to a lower level that is + /// faster to build (see [DebugInfo::weaken]). + /// + /// In all other situations, this level value will be the one to use. Deferred(u32), } diff --git a/src/cargo/ops/cargo_compile/mod.rs b/src/cargo/ops/cargo_compile/mod.rs index d9e387b6dd4..d183bf6666a 100644 --- a/src/cargo/ops/cargo_compile/mod.rs +++ b/src/cargo/ops/cargo_compile/mod.rs @@ -625,7 +625,7 @@ fn traverse_and_share( // target is the same as the host, we canonicalize the compile kind to `CompileKind::Host`. // A possible host dependency counterpart to this unit would have that kind, and if such a unit // exists in the current `unit_graph`, they will unify in the new unit graph map `new_graph`. - // The resulting unit graph will have be optimized with less units, thanks to sharing these host + // The resulting unit graph will be optimized with less units, thanks to sharing these host // dependencies. let canonical_kind = match to_host { Some(to_host) if to_host == unit.kind => CompileKind::Host, @@ -638,12 +638,12 @@ fn traverse_and_share( // its debuginfo level to optimize build times. if unit.kind.is_host() && to_host.is_some() && profile.debuginfo.is_deferred() { // We create a "probe" test to see if a unit with the same explicit debuginfo level exists - // in the graph. This is the level we'd expect if it was set manually, or a default set by - // a profile for a runtime dependency: its canonical value. + // in the graph. This is the level we'd expect if it was set manually or the default value + // set by a profile for a runtime dependency: its canonical value. let canonical_debuginfo = profile.debuginfo.finalize(); let mut canonical_profile = profile.clone(); canonical_profile.debuginfo = canonical_debuginfo; - let explicit_unit_probe = interner.intern( + let unit_probe = interner.intern( &unit.pkg, &unit.target, canonical_profile, @@ -657,7 +657,7 @@ fn traverse_and_share( ); // We can now turn the deferred value into its actual final value. - profile.debuginfo = if unit_graph.contains_key(&explicit_unit_probe) { + profile.debuginfo = if unit_graph.contains_key(&unit_probe) { // The unit is present in both build time and runtime subgraphs: we canonicalize its // level to the other unit's, thus ensuring reuse between the two to optimize build times. canonical_debuginfo From 881533b590f340b4e551c9182f4143100149866c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Mon, 21 Mar 2022 15:15:15 +0100 Subject: [PATCH 07/17] display note to increase debuginfo level when build deps fail it's only displayed when backtraces are requested --- src/cargo/core/compiler/custom_build.rs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/cargo/core/compiler/custom_build.rs b/src/cargo/core/compiler/custom_build.rs index cac5cb32eea..eafd6f6180b 100644 --- a/src/cargo/core/compiler/custom_build.rs +++ b/src/cargo/core/compiler/custom_build.rs @@ -405,7 +405,21 @@ fn build_work(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult { }, true, ) - .with_context(|| format!("failed to run custom build command for `{}`", pkg_descr)); + .with_context(|| { + let mut build_error_context = format!("failed to run custom build command for `{}`", pkg_descr); + + // If we're opting into backtraces, mention that build dependencies' backtraces can + // be improved by setting a higher debuginfo level. + if let Ok(show_backtraces) = std::env::var("RUST_BACKTRACE") { + if show_backtraces != "0" { + build_error_context.push_str("\n\ + note: To improve backtraces for build dependencies, make sure full debug info is turned on. \ + More details at https://doc.rust-lang.org/cargo/reference/profiles.html#build-dependencies"); + } + } + + build_error_context + }); if let Err(error) = output { insert_warnings_in_build_outputs( From 7dfabdc681512f8a8031e67720a9dd5c275cfe7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Mon, 21 Mar 2022 15:17:37 +0100 Subject: [PATCH 08/17] add build script failure test when requesting backtraces it displays an additional message on how to improve these backtraces, now that debuginfo is turned off most of the time in `dev.build-override`. --- tests/testsuite/build_script.rs | 38 +++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/tests/testsuite/build_script.rs b/tests/testsuite/build_script.rs index 91a20dc6f82..ffb66a0f61d 100644 --- a/tests/testsuite/build_script.rs +++ b/tests/testsuite/build_script.rs @@ -47,6 +47,44 @@ Caused by: .run(); } +#[cargo_test] +fn custom_build_script_failed_backtraces_message() { + // In this situation (no dependency sharing), debuginfo is turned off in + // `dev.build-override`. However, if an error occurs running e.g. a build + // script, and backtraces are opted into: a message explaining how to + // improve backtraces is also displayed. + let p = project() + .file( + "Cargo.toml", + r#" + [package] + + name = "foo" + version = "0.5.0" + authors = ["wycats@example.com"] + build = "build.rs" + "#, + ) + .file("src/main.rs", "fn main() {}") + .file("build.rs", "fn main() { std::process::exit(101); }") + .build(); + p.cargo("build -v") + .env("RUST_BACKTRACE", "1") + .with_status(101) + .with_stderr( + "\ +[COMPILING] foo v0.5.0 ([CWD]) +[RUNNING] `rustc --crate-name build_script_build build.rs [..]--crate-type bin [..]` +[RUNNING] `[..]/build-script-build` +[ERROR] failed to run custom build command for `foo v0.5.0 ([CWD])` +note: To improve backtraces for build dependencies[..] + +Caused by: + process didn't exit successfully: `[..]/build-script-build` (exit [..]: 101)", + ) + .run(); +} + #[cargo_test] fn custom_build_env_vars() { let p = project() From 4362b1b7aa2718e6c9c080f4336af235717f5371 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Mon, 21 Mar 2022 14:55:06 +0100 Subject: [PATCH 09/17] update build dependencies profiles documentation This describes the new defaults for build-overrides, and how to make sure backtraces have the usual debug info, when needed. --- src/doc/src/reference/profiles.md | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/doc/src/reference/profiles.md b/src/doc/src/reference/profiles.md index 326be46fab2..5cc2a6f7f57 100644 --- a/src/doc/src/reference/profiles.md +++ b/src/doc/src/reference/profiles.md @@ -298,20 +298,29 @@ The `bench` profile inherits the settings from the [`release`](#release) profile #### Build Dependencies -All profiles, by default, do not optimize build dependencies (build scripts, -proc macros, and their dependencies). The default settings for build overrides -are: +To compile quickly, all profiles, by default, do not optimize build +dependencies (build scripts, proc macros, and their dependencies), and avoid +computing debug info when a build dependency is not used as a runtime +dependency. The default settings for build overrides are: ```toml [profile.dev.build-override] opt-level = 0 codegen-units = 256 +debug = false # when possible [profile.release.build-override] opt-level = 0 codegen-units = 256 ``` +However, if errors occur while running build dependencies, turning full debug +info on will improve backtraces and debuggability when needed: + +```toml +debug = true +``` + Build dependencies otherwise inherit settings from the active profile in use, as described in [Profile selection](#profile-selection). From facdb30c03689c7d67c405e57e45c577a5488111 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Mon, 21 Nov 2022 20:06:02 +0000 Subject: [PATCH 10/17] add assertions checking the absence of debuginfo Add some assertions to ensure that debuginfo is not used to compile build dependencies, in a way that differs between the old and new defaults: some of the assert elision could match the previous defaults with debuginfo. These new assertions break if `-C debuginfo` is present in the commands cargo ran. --- tests/testsuite/profile_targets.rs | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/tests/testsuite/profile_targets.rs b/tests/testsuite/profile_targets.rs index c86c2dbfc43..b3235972c7c 100644 --- a/tests/testsuite/profile_targets.rs +++ b/tests/testsuite/profile_targets.rs @@ -82,7 +82,10 @@ fn profile_selection_build() { // NOTES: // - bdep `panic` is not set because it thinks `build.rs` is a plugin. // - build_script_build is built without panic because it thinks `build.rs` is a plugin. - p.cargo("build -vv").with_stderr_unordered("\ + // - We make sure that the build dependencies bar, bdep, and build.rs + // are built without debuginfo. + p.cargo("build -vv") + .with_stderr_unordered("\ [COMPILING] bar [..] [RUNNING] `[..] rustc --crate-name bar bar/src/lib.rs [..]--crate-type lib --emit=[..]link -C panic=abort[..]-C codegen-units=1 -C debuginfo=2 [..] [RUNNING] `[..] rustc --crate-name bar bar/src/lib.rs [..]--crate-type lib --emit=[..]link[..]-C codegen-units=5 [..] @@ -95,7 +98,12 @@ fn profile_selection_build() { [RUNNING] `[..] rustc --crate-name foo src/lib.rs [..]--crate-type lib --emit=[..]link -C panic=abort[..]-C codegen-units=1 -C debuginfo=2 [..] [RUNNING] `[..] rustc --crate-name foo src/main.rs [..]--crate-type bin --emit=[..]link -C panic=abort[..]-C codegen-units=1 -C debuginfo=2 [..] [FINISHED] dev [unoptimized + debuginfo] [..] -").run(); +" + ) + .with_stderr_line_without(&["[RUNNING] `[..] rustc --crate-name bar bar/src/lib.rs [..]-C codegen-units=5 [..]"], &["-C debuginfo"]) + .with_stderr_line_without(&["[RUNNING] `[..] rustc --crate-name bdep bdep/src/lib.rs [..]-C codegen-units=5 [..]"], &["-C debuginfo"]) + .with_stderr_line_without(&["[RUNNING] `[..] rustc --crate-name build_script_build build.rs [..]-C codegen-units=5 [..]"], &["-C debuginfo"]) + .run(); p.cargo("build -vv") .with_stderr_unordered( "\ @@ -149,6 +157,8 @@ fn profile_selection_build_all_targets() { // `build.rs` is a plugin. // - Benchmark dependencies are compiled in `dev` mode, which may be // surprising. See issue rust-lang/cargo#4929. + // - We make sure that the build dependencies bar, bdep, and build.rs + // are built without debuginfo. // // - Dependency profiles: // Pkg Target Profile Reason @@ -169,7 +179,8 @@ fn profile_selection_build_all_targets() { // bin dev dev // bin dev build // example dev build - p.cargo("build --all-targets -vv").with_stderr_unordered("\ + p.cargo("build --all-targets -vv") + .with_stderr_unordered("\ [COMPILING] bar [..] [RUNNING] `[..] rustc --crate-name bar bar/src/lib.rs [..]--crate-type lib --emit=[..]link[..]-C codegen-units=1 -C debuginfo=2 [..] [RUNNING] `[..] rustc --crate-name bar bar/src/lib.rs [..]--crate-type lib --emit=[..]link -C panic=abort[..]-C codegen-units=1 -C debuginfo=2 [..] @@ -189,7 +200,12 @@ fn profile_selection_build_all_targets() { [RUNNING] `[..] rustc --crate-name foo src/main.rs [..]--crate-type bin --emit=[..]link -C panic=abort[..]-C codegen-units=1 -C debuginfo=2 [..]` [RUNNING] `[..] rustc --crate-name ex1 examples/ex1.rs [..]--crate-type bin --emit=[..]link -C panic=abort[..]-C codegen-units=1 -C debuginfo=2 [..]` [FINISHED] dev [unoptimized + debuginfo] [..] -").run(); +" + ) + .with_stderr_line_without(&["[RUNNING] `[..] rustc --crate-name bar bar/src/lib.rs [..]-C codegen-units=5 [..]"], &["-C debuginfo"]) + .with_stderr_line_without(&["[RUNNING] `[..] rustc --crate-name bdep bdep/src/lib.rs [..]-C codegen-units=5 [..]"], &["-C debuginfo"]) + .with_stderr_line_without(&["[RUNNING] `[..] rustc --crate-name build_script_build build.rs [..]-C codegen-units=5 [..]"], &["-C debuginfo"]) + .run(); p.cargo("build -vv") .with_stderr_unordered( "\ From 6c4305355bb157574954785812860462d88e71a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Mon, 21 Nov 2022 21:24:57 +0000 Subject: [PATCH 11/17] derive `Serialize` for `DebugInfo` --- src/cargo/core/profiles.rs | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/cargo/core/profiles.rs b/src/cargo/core/profiles.rs index a7e3f91e0ae..80bc2e7ce95 100644 --- a/src/cargo/core/profiles.rs +++ b/src/cargo/core/profiles.rs @@ -737,7 +737,8 @@ impl Profile { /// cause to build a unit twice. By deferring the choice until we know /// whether to choose the optimized value or the default value, we can make sure /// the unit is only built once and the unit graph is still optimized. -#[derive(Debug, Copy, Clone)] +#[derive(Debug, Copy, Clone, serde::Serialize)] +#[serde(untagged)] pub enum DebugInfo { /// No debuginfo level was set. None, @@ -788,15 +789,6 @@ impl DebugInfo { } } -impl serde::Serialize for DebugInfo { - fn serialize(&self, serializer: S) -> Result - where - S: serde::Serializer, - { - self.to_option().serialize(serializer) - } -} - impl PartialEq for DebugInfo { fn eq(&self, other: &DebugInfo) -> bool { self.to_option().eq(&other.to_option()) From d7732e4d1a9cacae233b305c21b9e019017b78e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Mon, 21 Nov 2022 23:16:55 +0000 Subject: [PATCH 12/17] don't optimize debuginfo for artifact dependencies --- src/cargo/ops/cargo_compile/mod.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/cargo/ops/cargo_compile/mod.rs b/src/cargo/ops/cargo_compile/mod.rs index d183bf6666a..db0d6d2e4b2 100644 --- a/src/cargo/ops/cargo_compile/mod.rs +++ b/src/cargo/ops/cargo_compile/mod.rs @@ -635,8 +635,13 @@ fn traverse_and_share( let mut profile = unit.profile.clone(); // If this is a build dependency, and it's not shared with runtime dependencies, we can weaken - // its debuginfo level to optimize build times. - if unit.kind.is_host() && to_host.is_some() && profile.debuginfo.is_deferred() { + // its debuginfo level to optimize build times. We do nothing if it's an artifact dependency, + // as it and its debuginfo may end up embedded in the main program. + if unit.kind.is_host() + && to_host.is_some() + && profile.debuginfo.is_deferred() + && !unit.artifact.is_true() + { // We create a "probe" test to see if a unit with the same explicit debuginfo level exists // in the graph. This is the level we'd expect if it was set manually or the default value // set by a profile for a runtime dependency: its canonical value. From 49bd6acb31179d8f516cf457a53cc193b32fd7e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Mon, 21 Nov 2022 23:23:41 +0000 Subject: [PATCH 13/17] use `UnitFor::is_for_host` to detect build deps Although `CompileKind::is_host` is currently used for build dependencies prior to unit graph sharing, it's not a guarantee. So we use `UnitFor::is_for_host` to make detection more future-proof. --- src/cargo/ops/cargo_compile/mod.rs | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/src/cargo/ops/cargo_compile/mod.rs b/src/cargo/ops/cargo_compile/mod.rs index db0d6d2e4b2..b0aba266d2e 100644 --- a/src/cargo/ops/cargo_compile/mod.rs +++ b/src/cargo/ops/cargo_compile/mod.rs @@ -574,7 +574,15 @@ fn rebuild_unit_graph_shared( let new_roots = roots .iter() .map(|root| { - traverse_and_share(interner, &mut memo, &mut result, &unit_graph, root, to_host) + traverse_and_share( + interner, + &mut memo, + &mut result, + &unit_graph, + root, + false, + to_host, + ) }) .collect(); // If no unit in the unit graph ended up having scrape units attached as dependencies, @@ -598,6 +606,7 @@ fn traverse_and_share( new_graph: &mut UnitGraph, unit_graph: &UnitGraph, unit: &Unit, + unit_is_for_host: bool, to_host: Option, ) -> Unit { if let Some(new_unit) = memo.get(unit) { @@ -608,8 +617,15 @@ fn traverse_and_share( let new_deps: Vec<_> = unit_graph[unit] .iter() .map(|dep| { - let new_dep_unit = - traverse_and_share(interner, memo, new_graph, unit_graph, &dep.unit, to_host); + let new_dep_unit = traverse_and_share( + interner, + memo, + new_graph, + unit_graph, + &dep.unit, + dep.unit_for.is_for_host(), + to_host, + ); new_dep_unit.hash(&mut dep_hash); UnitDep { unit: new_dep_unit, @@ -637,7 +653,7 @@ fn traverse_and_share( // If this is a build dependency, and it's not shared with runtime dependencies, we can weaken // its debuginfo level to optimize build times. We do nothing if it's an artifact dependency, // as it and its debuginfo may end up embedded in the main program. - if unit.kind.is_host() + if unit_is_for_host && to_host.is_some() && profile.debuginfo.is_deferred() && !unit.artifact.is_true() From 3be6c93a2210a97b74fa8484d09ca2c6957a76e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Tue, 31 Jan 2023 20:57:29 +0000 Subject: [PATCH 14/17] update `optional_build_dep_and_required_normal_dep` test expectations This test dynamically enables a shared build/runtime dependency, and therefore doesn't trigger the build/runtime sharing reuse optimization, as the build dep is initially built without debuginfo for optimization purposes. --- tests/testsuite/build_script.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/testsuite/build_script.rs b/tests/testsuite/build_script.rs index ffb66a0f61d..2ddb6853a88 100644 --- a/tests/testsuite/build_script.rs +++ b/tests/testsuite/build_script.rs @@ -4566,6 +4566,7 @@ fn optional_build_dep_and_required_normal_dep() { .with_stdout("1") .with_stderr( "\ +[COMPILING] bar v0.5.0 ([..]) [COMPILING] foo v0.1.0 ([..]) [FINISHED] dev [unoptimized + debuginfo] target(s) in [..] [RUNNING] `[..]foo[EXE]`", From 2bfc813b9f39266d38ceb9ac144c5fac11781971 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Tue, 31 Jan 2023 21:00:11 +0000 Subject: [PATCH 15/17] update test `proc_macro_ws` This non-regression test didn't pass as-is because of the new debuginfo build deps optimization. This restores the original intent of the test. --- tests/testsuite/features2.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/testsuite/features2.rs b/tests/testsuite/features2.rs index 6103e4bd5d4..1e0e62f2304 100644 --- a/tests/testsuite/features2.rs +++ b/tests/testsuite/features2.rs @@ -1050,6 +1050,11 @@ fn decouple_proc_macro() { #[cargo_test] fn proc_macro_ws() { // Checks for bug with proc-macro in a workspace with dependency (shouldn't panic). + // + // Note, debuginfo is explicitly requested here to preserve the intent of this non-regression + // test: that will disable the debuginfo build dependencies optimization. Otherwise, it would + // initially trigger when the crates are built independently, but rebuild them with debuginfo + // when it sees the shared build/runtime dependency when checking the complete workspace. let p = project() .file( "Cargo.toml", @@ -1057,6 +1062,9 @@ fn proc_macro_ws() { [workspace] members = ["foo", "pm"] resolver = "2" + + [profile.dev.build-override] + debug = true "#, ) .file( From 9d9b7fef2d0f7d3259c89451ff3233883ad1a432 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Tue, 31 Jan 2023 21:38:30 +0000 Subject: [PATCH 16/17] improve debuginfo error message when build script fails --- src/cargo/core/compiler/custom_build.rs | 14 ++++++++++---- tests/testsuite/build_script.rs | 19 ++++++++++++++++++- 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/cargo/core/compiler/custom_build.rs b/src/cargo/core/compiler/custom_build.rs index eafd6f6180b..97764ab25d3 100644 --- a/src/cargo/core/compiler/custom_build.rs +++ b/src/cargo/core/compiler/custom_build.rs @@ -332,6 +332,8 @@ fn build_work(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult { // Need a separate copy for the fresh closure. let targets_fresh = targets.clone(); + let env_profile_name = unit.profile.name.to_uppercase(); + // Prepare the unit of "dirty work" which will actually run the custom build // command. // @@ -406,15 +408,19 @@ fn build_work(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult { true, ) .with_context(|| { - let mut build_error_context = format!("failed to run custom build command for `{}`", pkg_descr); + let mut build_error_context = + format!("failed to run custom build command for `{}`", pkg_descr); // If we're opting into backtraces, mention that build dependencies' backtraces can // be improved by setting a higher debuginfo level. if let Ok(show_backtraces) = std::env::var("RUST_BACKTRACE") { if show_backtraces != "0" { - build_error_context.push_str("\n\ - note: To improve backtraces for build dependencies, make sure full debug info is turned on. \ - More details at https://doc.rust-lang.org/cargo/reference/profiles.html#build-dependencies"); + build_error_context.push_str(&format!( + "\n\ + note: To improve backtraces for build dependencies, set the \ + CARGO_PROFILE_{env_profile_name}_BUILD_OVERRIDE_DEBUG=true environment \ + variable to enable debug information generation.", + )); } } diff --git a/tests/testsuite/build_script.rs b/tests/testsuite/build_script.rs index 2ddb6853a88..783a86bdde6 100644 --- a/tests/testsuite/build_script.rs +++ b/tests/testsuite/build_script.rs @@ -77,7 +77,24 @@ fn custom_build_script_failed_backtraces_message() { [RUNNING] `rustc --crate-name build_script_build build.rs [..]--crate-type bin [..]` [RUNNING] `[..]/build-script-build` [ERROR] failed to run custom build command for `foo v0.5.0 ([CWD])` -note: To improve backtraces for build dependencies[..] +note: To improve backtraces for build dependencies, set the \ +CARGO_PROFILE_DEV_BUILD_OVERRIDE_DEBUG=true environment variable [..] + +Caused by: + process didn't exit successfully: `[..]/build-script-build` (exit [..]: 101)", + ) + .run(); + + p.cargo("check -v") + .env("RUST_BACKTRACE", "1") + .with_status(101) + .with_stderr( + "\ +[COMPILING] foo v0.5.0 ([CWD]) +[RUNNING] `[..]/build-script-build` +[ERROR] failed to run custom build command for `foo v0.5.0 ([CWD])` +note: To improve backtraces for build dependencies, set the \ +CARGO_PROFILE_DEV_BUILD_OVERRIDE_DEBUG=true environment variable [..] Caused by: process didn't exit successfully: `[..]/build-script-build` (exit [..]: 101)", From 7ccda69f700afa70ca0dffa658ac0307379e9299 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Tue, 31 Jan 2023 22:09:54 +0000 Subject: [PATCH 17/17] do not display message to enable debuginfo when it's already enabled --- src/cargo/core/compiler/custom_build.rs | 12 ++++++-- tests/testsuite/build_script.rs | 37 +++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/src/cargo/core/compiler/custom_build.rs b/src/cargo/core/compiler/custom_build.rs index 97764ab25d3..11c3f1a24e4 100644 --- a/src/cargo/core/compiler/custom_build.rs +++ b/src/cargo/core/compiler/custom_build.rs @@ -333,6 +333,13 @@ fn build_work(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult { let targets_fresh = targets.clone(); let env_profile_name = unit.profile.name.to_uppercase(); + let built_with_debuginfo = cx + .bcx + .unit_graph + .get(unit) + .and_then(|deps| deps.iter().find(|dep| dep.unit.target == unit.target)) + .map(|dep| dep.unit.profile.debuginfo.is_turned_on()) + .unwrap_or(false); // Prepare the unit of "dirty work" which will actually run the custom build // command. @@ -412,9 +419,10 @@ fn build_work(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult { format!("failed to run custom build command for `{}`", pkg_descr); // If we're opting into backtraces, mention that build dependencies' backtraces can - // be improved by setting a higher debuginfo level. + // be improved by requesting debuginfo to be built, if we're not building with + // debuginfo already. if let Ok(show_backtraces) = std::env::var("RUST_BACKTRACE") { - if show_backtraces != "0" { + if !built_with_debuginfo && show_backtraces != "0" { build_error_context.push_str(&format!( "\n\ note: To improve backtraces for build dependencies, set the \ diff --git a/tests/testsuite/build_script.rs b/tests/testsuite/build_script.rs index 783a86bdde6..d5ee1f99c9a 100644 --- a/tests/testsuite/build_script.rs +++ b/tests/testsuite/build_script.rs @@ -102,6 +102,43 @@ Caused by: .run(); } +#[cargo_test] +fn custom_build_script_failed_backtraces_message_with_debuginfo() { + // This is the same test as `custom_build_script_failed_backtraces_message` above, this time + // ensuring that the message dedicated to improving backtraces by requesting debuginfo is not + // shown when debuginfo is already turned on. + let p = project() + .file( + "Cargo.toml", + r#" + [package] + + name = "foo" + version = "0.5.0" + authors = ["wycats@example.com"] + build = "build.rs" + "#, + ) + .file("src/main.rs", "fn main() {}") + .file("build.rs", "fn main() { std::process::exit(101); }") + .build(); + p.cargo("build -v") + .env("RUST_BACKTRACE", "1") + .env("CARGO_PROFILE_DEV_BUILD_OVERRIDE_DEBUG", "true") + .with_status(101) + .with_stderr( + "\ +[COMPILING] foo v0.5.0 ([CWD]) +[RUNNING] `rustc --crate-name build_script_build build.rs [..]--crate-type bin [..]` +[RUNNING] `[..]/build-script-build` +[ERROR] failed to run custom build command for `foo v0.5.0 ([CWD])` + +Caused by: + process didn't exit successfully: `[..]/build-script-build` (exit [..]: 101)", + ) + .run(); +} + #[cargo_test] fn custom_build_env_vars() { let p = project()