From d861dcf79277bd3221c0418e0ea9fe59e961380a Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Mon, 10 Apr 2023 23:59:08 -0500 Subject: [PATCH] Allow named debuginfo options in Cargo.toml Rustc supports these since rust-lang/rust#109808. It's technically possible to set a named debuginfo level through `RUSTFLAGS`, but in practice cargo always passes its own opinion of what the debuginfo level is, so allow it to be configured through cargo too. --- src/cargo/core/compiler/mod.rs | 14 ++++- src/cargo/core/profiles.rs | 39 ++++++------ src/cargo/util/machine_message.rs | 10 ++- src/cargo/util/toml/mod.rs | 101 +++++++++++++++++++++++++++--- tests/testsuite/bad_config.rs | 34 +++++++++- tests/testsuite/config.rs | 8 +-- tests/testsuite/profile_config.rs | 7 ++- 7 files changed, 174 insertions(+), 39 deletions(-) diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index 7b43fd27d99..1c37cc1aa17 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -92,6 +92,7 @@ use crate::core::{Feature, PackageId, Target, Verbosity}; use crate::util::errors::{CargoResult, VerboseError}; use crate::util::interning::InternedString; use crate::util::machine_message::{self, Message}; +use crate::util::toml::TomlDebugInfo; use crate::util::{add_path_args, internal, iter_join_onto, profile}; use cargo_util::{paths, ProcessBuilder, ProcessError}; use rustfix::diagnostics::Applicability; @@ -603,9 +604,20 @@ fn link_targets(cx: &mut Context<'_, '_>, unit: &Unit, fresh: bool) -> CargoResu } if json_messages { + let debuginfo = profile.debuginfo.to_option().map(|d| match d { + TomlDebugInfo::None => machine_message::ArtifactDebuginfo::Int(0), + TomlDebugInfo::Limited => machine_message::ArtifactDebuginfo::Int(1), + TomlDebugInfo::Full => machine_message::ArtifactDebuginfo::Int(2), + TomlDebugInfo::LineDirectivesOnly => { + machine_message::ArtifactDebuginfo::Named("line-directives-only") + } + TomlDebugInfo::LineTablesOnly => { + machine_message::ArtifactDebuginfo::Named("line-tables-only") + } + }); let art_profile = machine_message::ArtifactProfile { opt_level: profile.opt_level.as_str(), - debuginfo: profile.debuginfo.to_option(), + debuginfo, debug_assertions: profile.debug_assertions, overflow_checks: profile.overflow_checks, test: unit_mode.is_any_test(), diff --git a/src/cargo/core/profiles.rs b/src/cargo/core/profiles.rs index 51d19e32e62..a480143a04e 100644 --- a/src/cargo/core/profiles.rs +++ b/src/cargo/core/profiles.rs @@ -26,7 +26,9 @@ use crate::core::dependency::Artifact; use crate::core::resolver::features::FeaturesFor; use crate::core::{PackageId, PackageIdSpec, Resolve, Shell, Target, Workspace}; use crate::util::interning::InternedString; -use crate::util::toml::{ProfilePackageSpec, StringOrBool, TomlProfile, TomlProfiles, U32OrBool}; +use crate::util::toml::{ + ProfilePackageSpec, StringOrBool, TomlDebugInfo, TomlProfile, TomlProfiles, +}; use crate::util::{closest_msg, config, CargoResult, Config}; use anyhow::{bail, Context as _}; use std::collections::{BTreeMap, HashMap, HashSet}; @@ -276,15 +278,13 @@ 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.to_option() { - if profile.split_debuginfo.is_none() && debug > 0 { - let target = match &kind { - CompileKind::Host => self.rustc_host.as_str(), - CompileKind::Target(target) => target.short_name(), - }; - if target.contains("-apple-") { - profile.split_debuginfo = Some(InternedString::new("unpacked")); - } + if profile.debuginfo.is_turned_on() && profile.split_debuginfo.is_none() { + let target = match &kind { + CompileKind::Host => self.rustc_host.as_str(), + CompileKind::Target(target) => target.short_name(), + }; + if target.contains("-apple-") { + profile.split_debuginfo = Some(InternedString::new("unpacked")); } } @@ -528,11 +528,8 @@ fn merge_profile(profile: &mut Profile, toml: &TomlProfile) { if toml.codegen_units.is_some() { profile.codegen_units = toml.codegen_units; } - match toml.debug { - 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(debuginfo) = toml.debug { + profile.debuginfo = DebugInfo::Explicit(debuginfo); } if let Some(debug_assertions) = toml.debug_assertions { profile.debug_assertions = debug_assertions; @@ -683,7 +680,7 @@ impl Profile { Profile { name: InternedString::new("dev"), root: ProfileRoot::Debug, - debuginfo: DebugInfo::Explicit(2), + debuginfo: DebugInfo::Explicit(TomlDebugInfo::Full), debug_assertions: true, overflow_checks: true, incremental: true, @@ -743,7 +740,7 @@ pub enum DebugInfo { /// No debuginfo level was set. None, /// A debuginfo level that is explicitly set, by a profile or a user. - Explicit(u32), + Explicit(TomlDebugInfo), /// For internal purposes: a deferred debuginfo level that can be optimized /// away, but has this value otherwise. /// @@ -753,22 +750,22 @@ pub enum DebugInfo { /// faster to build (see [DebugInfo::weaken]). /// /// In all other situations, this level value will be the one to use. - Deferred(u32), + Deferred(TomlDebugInfo), } impl DebugInfo { /// The main way to interact with this debuginfo level, turning it into an Option. - pub fn to_option(&self) -> Option { + pub fn to_option(self) -> Option { match self { DebugInfo::None => None, - DebugInfo::Explicit(v) | DebugInfo::Deferred(v) => Some(*v), + 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 + !matches!(self.to_option(), None | Some(TomlDebugInfo::None)) } pub(crate) fn is_deferred(&self) -> bool { diff --git a/src/cargo/util/machine_message.rs b/src/cargo/util/machine_message.rs index baef5167b36..f1602ae57c3 100644 --- a/src/cargo/util/machine_message.rs +++ b/src/cargo/util/machine_message.rs @@ -55,12 +55,20 @@ impl<'a> Message for Artifact<'a> { #[derive(Serialize)] pub struct ArtifactProfile { pub opt_level: &'static str, - pub debuginfo: Option, + pub debuginfo: Option, pub debug_assertions: bool, pub overflow_checks: bool, pub test: bool, } +/// Internally this is an enum with different variants, but keep using 0/1/2 as integers for compatibility. +#[derive(Serialize)] +#[serde(untagged)] +pub enum ArtifactDebuginfo { + Int(u32), + Named(&'static str), +} + #[derive(Serialize)] pub struct BuildScript<'a> { pub package_id: PackageId, diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 9e7c6f63e20..a119b343133 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -1,5 +1,5 @@ use std::collections::{BTreeMap, BTreeSet, HashMap}; -use std::fmt; +use std::fmt::{self, Display, Write}; use std::marker::PhantomData; use std::path::{Path, PathBuf}; use std::rc::Rc; @@ -12,8 +12,8 @@ use itertools::Itertools; use lazycell::LazyCell; use log::{debug, trace}; use semver::{self, VersionReq}; -use serde::de; use serde::de::IntoDeserializer as _; +use serde::de::{self, Unexpected}; use serde::ser; use serde::{Deserialize, Serialize}; use url::Url; @@ -442,11 +442,96 @@ impl ser::Serialize for TomlOptLevel { } } -#[derive(Clone, Debug, Deserialize, Serialize, Eq, PartialEq)] -#[serde(untagged, expecting = "expected a boolean or an integer")] -pub enum U32OrBool { - U32(u32), - Bool(bool), +#[derive(Copy, Clone, Debug, Eq, PartialEq, Hash, PartialOrd, Ord)] +pub enum TomlDebugInfo { + None, + LineDirectivesOnly, + LineTablesOnly, + Limited, + Full, +} + +impl ser::Serialize for TomlDebugInfo { + fn serialize(&self, serializer: S) -> Result + where + S: ser::Serializer, + { + match self { + Self::None => 0.serialize(serializer), + Self::LineDirectivesOnly => "line-directives-only".serialize(serializer), + Self::LineTablesOnly => "line-tables-only".serialize(serializer), + Self::Limited => 1.serialize(serializer), + Self::Full => 2.serialize(serializer), + } + } +} + +impl<'de> de::Deserialize<'de> for TomlDebugInfo { + fn deserialize(d: D) -> Result + where + D: de::Deserializer<'de>, + { + struct Visitor; + + impl<'de> de::Visitor<'de> for Visitor { + type Value = TomlDebugInfo; + + fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter + .write_str("a boolean, 0-2, \"line-tables-only\", or \"line-directives-only\"") + } + + fn visit_i64(self, value: i64) -> Result + where + E: de::Error, + { + let debuginfo = match value { + 0 => TomlDebugInfo::None, + 1 => TomlDebugInfo::Limited, + 2 => TomlDebugInfo::Full, + _ => return Err(de::Error::invalid_value(Unexpected::Signed(value), &self)), + }; + Ok(debuginfo) + } + + fn visit_bool(self, v: bool) -> Result + where + E: de::Error, + { + Ok(if v { + TomlDebugInfo::Full + } else { + TomlDebugInfo::None + }) + } + + fn visit_str(self, value: &str) -> Result + where + E: de::Error, + { + let debuginfo = match value { + "line-directives-only" => TomlDebugInfo::LineDirectivesOnly, + "line-tables-only" => TomlDebugInfo::LineTablesOnly, + _ => return Err(de::Error::invalid_value(Unexpected::Str(value), &self)), + }; + Ok(debuginfo) + } + } + + d.deserialize_any(Visitor) + } +} + +impl Display for TomlDebugInfo { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + TomlDebugInfo::None => f.write_char('0'), + TomlDebugInfo::Limited => f.write_char('1'), + TomlDebugInfo::Full => f.write_char('2'), + TomlDebugInfo::LineDirectivesOnly => f.write_str("line-directives-only"), + TomlDebugInfo::LineTablesOnly => f.write_str("line-tables-only"), + } + } } #[derive(Deserialize, Serialize, Clone, Debug, Default, Eq, PartialEq)] @@ -456,7 +541,7 @@ pub struct TomlProfile { pub lto: Option, pub codegen_backend: Option, pub codegen_units: Option, - pub debug: Option, + pub debug: Option, pub split_debuginfo: Option, pub debug_assertions: Option, pub rpath: Option, diff --git a/tests/testsuite/bad_config.rs b/tests/testsuite/bad_config.rs index ca51b101e39..aaf6accc532 100644 --- a/tests/testsuite/bad_config.rs +++ b/tests/testsuite/bad_config.rs @@ -1313,6 +1313,38 @@ fn bad_debuginfo() { .file("src/lib.rs", "") .build(); + p.cargo("check") + .with_status(101) + .with_stderr( + "\ +error: failed to parse manifest [..] + +Caused by: + invalid value: string \"a\", expected a boolean, 0-2, \"line-tables-only\", or \"line-directives-only\" + in `profile.dev.debug` +", + ) + .run(); +} + +#[cargo_test] +fn bad_debuginfo2() { + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.0" + authors = [] + + [profile.dev] + debug = 3.6 + "#, + ) + .file("src/lib.rs", "") + .build(); + p.cargo("check") .with_status(101) .with_stderr( @@ -1320,7 +1352,7 @@ fn bad_debuginfo() { error: failed to parse manifest at `[..]` Caused by: - expected a boolean or an integer + invalid type: floating point `3.6`, expected a boolean, 0-2, \"line-tables-only\", or \"line-directives-only\" in `profile.dev.debug` ", ) diff --git a/tests/testsuite/config.rs b/tests/testsuite/config.rs index 92e1f42643c..7c37106a786 100644 --- a/tests/testsuite/config.rs +++ b/tests/testsuite/config.rs @@ -401,7 +401,7 @@ lto = false opt_level: Some(cargo_toml::TomlOptLevel("s".to_string())), lto: Some(cargo_toml::StringOrBool::Bool(true)), codegen_units: Some(5), - debug: Some(cargo_toml::U32OrBool::Bool(true)), + debug: Some(cargo_toml::TomlDebugInfo::Full), debug_assertions: Some(true), rpath: Some(true), panic: Some("abort".to_string()), @@ -444,7 +444,7 @@ fn profile_env_var_prefix() { .build(); let p: cargo_toml::TomlProfile = config.get("profile.dev").unwrap(); assert_eq!(p.debug_assertions, None); - assert_eq!(p.debug, Some(cargo_toml::U32OrBool::U32(1))); + assert_eq!(p.debug, Some(cargo_toml::TomlDebugInfo::Limited)); let config = ConfigBuilder::new() .env("CARGO_PROFILE_DEV_DEBUG_ASSERTIONS", "false") @@ -452,7 +452,7 @@ fn profile_env_var_prefix() { .build(); let p: cargo_toml::TomlProfile = config.get("profile.dev").unwrap(); assert_eq!(p.debug_assertions, Some(false)); - assert_eq!(p.debug, Some(cargo_toml::U32OrBool::U32(1))); + assert_eq!(p.debug, Some(cargo_toml::TomlDebugInfo::Limited)); } #[cargo_test] @@ -1511,7 +1511,7 @@ fn all_profile_options() { lto: Some(cargo_toml::StringOrBool::String("thin".to_string())), codegen_backend: Some(InternedString::new("example")), codegen_units: Some(123), - debug: Some(cargo_toml::U32OrBool::U32(1)), + debug: Some(cargo_toml::TomlDebugInfo::Limited), split_debuginfo: Some("packed".to_string()), debug_assertions: Some(true), rpath: Some(true), diff --git a/tests/testsuite/profile_config.rs b/tests/testsuite/profile_config.rs index c59ed7a97f5..cf9807964e2 100644 --- a/tests/testsuite/profile_config.rs +++ b/tests/testsuite/profile_config.rs @@ -1,5 +1,6 @@ //! Tests for profiles defined in config files. +use cargo::util::toml::TomlDebugInfo; use cargo_test_support::paths::CargoPathExt; use cargo_test_support::registry::Package; use cargo_test_support::{basic_lib_manifest, paths, project}; @@ -436,7 +437,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.to_option(), Some(1)); // "bar" from config + assert_eq!(p.debuginfo.to_option(), Some(TomlDebugInfo::Limited)); // "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 +446,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.to_option(), Some(1)); // SAME as normal + assert_eq!(bo.debuginfo.to_option(), Some(TomlDebugInfo::Limited)); // 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 +455,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.to_option(), Some(1)); // SAME as normal + assert_eq!(po.debuginfo.to_option(), Some(TomlDebugInfo::Limited)); // SAME as normal assert_eq!(po.debug_assertions, true); // SAME as normal assert_eq!(po.overflow_checks, false); // "middle" package override from manifest }