Merge pull request #47 from rust3ds/feature/link-debuginfo-profile

Use `cargo --unit-graph` to figure out whether to link debuginfo

Author: ian-h-chamberlain

Cargo.lock

@@ -1,6 +1,6 @@
 [package]
 name = "cargo-3ds"
-version = "0.1.1"
+version = "0.1.2"
 authors = ["Rust3DS Org", "Andrea Ciliberti <[email protected]>"]
 description = "Cargo wrapper for developing Nintendo 3DS homebrew apps"
 repository = "https://github.com/rust3ds/cargo-3ds"
@@ -19,3 +19,4 @@ tee = "0.1.0"
 toml = "0.5.6"
 clap = { version = "4.0.15", features = ["derive", "wrap_help"] }
 shlex = "1.1.0"
+serde_json = "1.0.108"

Cargo.toml

@@ -521,8 +521,6 @@ romfs_dir = "romfs"
 const CUSTOM_MAIN_RS: &str = r#"use ctru::prelude::*;
 
 fn main() {
-    ctru::use_panic_handler();
-
     let apt = Apt::new().unwrap();
     let mut hid = Hid::new().unwrap();
     let gfx = Gfx::new().unwrap();

src/command.rs

@@ -0,0 +1,90 @@
+use std::error::Error;
+use std::io::Read;
+use std::process::{Command, Stdio};
+
+use cargo_metadata::Target;
+use serde::Deserialize;
+
+use crate::print_command;
+
+/// In lieu of <https://github.com/oli-obk/cargo_metadata/issues/107>
+/// and to avoid pulling in the real `cargo`
+/// [data structures](https://docs.rs/cargo/latest/cargo/core/compiler/unit_graph/type.UnitGraph.html)
+/// as a dependency, we define the subset of the build graph we care about.
+#[derive(Deserialize)]
+pub struct UnitGraph {
+    pub version: i32,
+    pub units: Vec<Unit>,
+}
+
+impl UnitGraph {
+    /// Collect the unit graph via Cargo's `--unit-graph` flag.
+    /// This runs the same command as the actual build, except nothing is actually
+    /// build and the graph is output instead.
+    ///
+    /// See <https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#unit-graph>.
+    pub fn from_cargo(cargo_cmd: &Command, verbose: bool) -> Result<Self, Box<dyn Error>> {
+        // Since Command isn't Clone, copy it "by hand", by copying its args and envs
+        let mut cmd = Command::new(cargo_cmd.get_program());
+
+        let mut args = cargo_cmd.get_args();
+        cmd.args(args.next())
+            // These options must be added before any possible `--`, so the best
+            // place is to just stick them immediately after the first arg (subcommand)
+            .args(["-Z", "unstable-options", "--unit-graph"])
+            .args(args)
+            .envs(cargo_cmd.get_envs().filter_map(|(k, v)| Some((k, v?))))
+            .stdout(Stdio::piped())
+            .stderr(Stdio::piped());
+
+        if verbose {
+            print_command(&cmd);
+        }
+
+        let mut proc = cmd.spawn()?;
+        let stdout = proc.stdout.take().unwrap();
+        let mut stderr = proc.stderr.take().unwrap();
+
+        let result: Self = serde_json::from_reader(stdout).map_err(|err| {
+            let mut stderr_str = String::new();
+            let _ = stderr.read_to_string(&mut stderr_str);
+
+            let _ = proc.wait();
+            format!("unable to parse `--unit-graph` json: {err}\nstderr: `{stderr_str}`")
+        })?;
+
+        let _status = proc.wait()?;
+        // TODO: with cargo 1.74.0-nightly (b4ddf95ad 2023-09-18),
+        // `cargo run --unit-graph` panics at src/cargo/ops/cargo_run.rs:83:5
+        // It seems to have been fixed as of cargo 1.76.0-nightly (71cd3a926 2023-11-20)
+        // so maybe we can stop ignoring it once we bump the minimum toolchain version,
+        // and certainly we should once `--unit-graph` is ever stabilized.
+        //
+        // if !status.success() {
+        //     return Err(format!("`cargo --unit-graph` exited with status {status:?}").into());
+        // }
+
+        if result.version == 1 {
+            Ok(result)
+        } else {
+            Err(format!(
+                "unknown `cargo --unit-graph` output version {}",
+                result.version
+            ))?
+        }
+    }
+}
+
+#[derive(Deserialize)]
+pub struct Unit {
+    pub target: Target,
+    pub profile: Profile,
+}
+
+/// This struct is very similar to [`cargo_metadata::ArtifactProfile`], but seems
+/// to have some slight differences so we define a different version. We only
+/// really care about `debuginfo` anyway.
+#[derive(Deserialize)]
+pub struct Profile {
+    pub debuginfo: Option<u32>,
+}

src/graph.rs

@@ -1,6 +1,8 @@
 pub mod command;
+mod graph;
 
 use core::fmt;
+use std::ffi::OsStr;
 use std::io::{BufRead, BufReader};
 use std::path::{Path, PathBuf};
 use std::process::{Command, ExitStatus, Stdio};
@@ -13,6 +15,7 @@ use semver::Version;
 use tee::TeeReader;
 
 use crate::command::{CargoCmd, Run};
+use crate::graph::UnitGraph;
 
 /// Build a command using [`make_cargo_build_command`] and execute it,
 /// parsing and returning the messages from the spawned process.
@@ -22,6 +25,23 @@ use crate::command::{CargoCmd, Run};
 pub fn run_cargo(input: &Input, message_format: Option<String>) -> (ExitStatus, Vec<Message>) {
     let mut command = make_cargo_command(input, &message_format);
 
+    let libctru = if should_use_ctru_debuginfo(&command, input.verbose) {
+        "ctrud"
+    } else {
+        "ctru"
+    };
+
+    let rustflags = command
+        .get_envs()
+        .find(|(var, _)| var == &OsStr::new("RUSTFLAGS"))
+        .and_then(|(_, flags)| flags)
+        .unwrap_or_default()
+        .to_string_lossy();
+
+    let rustflags = format!("{rustflags} -l{libctru}");
+
+    command.env("RUSTFLAGS", rustflags);
+
     if input.verbose {
         print_command(&command);
     }
@@ -57,27 +77,51 @@ pub fn run_cargo(input: &Input, message_format: Option<String>) -> (ExitStatus,
     (process.wait().unwrap(), messages)
 }
 
+/// Ensure that we use the same `-lctru[d]` flag that `ctru-sys` is using in its build.
+fn should_use_ctru_debuginfo(cargo_cmd: &Command, verbose: bool) -> bool {
+    match UnitGraph::from_cargo(cargo_cmd, verbose) {
+        Ok(unit_graph) => {
+            let Some(unit) = unit_graph
+                .units
+                .iter()
+                .find(|unit| unit.target.name == "ctru-sys")
+            else {
+                eprintln!("Warning: unable to check if `ctru` debuginfo should be linked: `ctru-sys` not found");
+                return false;
+            };
+
+            let debuginfo = unit.profile.debuginfo.unwrap_or(0);
+            debuginfo > 0
+        }
+        Err(err) => {
+            eprintln!("Warning: unable to check if `ctru` debuginfo should be linked: {err}");
+            false
+        }
+    }
+}
+
 /// Create a cargo command based on the context.
 ///
 /// For "build" commands (which compile code, such as `cargo 3ds build` or `cargo 3ds clippy`),
 /// if there is no pre-built std detected in the sysroot, `build-std` will be used instead.
 pub fn make_cargo_command(input: &Input, message_format: &Option<String>) -> Command {
+    let devkitpro =
+        env::var("DEVKITPRO").expect("DEVKITPRO is not defined as an environment variable");
+    // TODO: should we actually prepend the user's RUSTFLAGS for linking order? not sure
+    let rustflags =
+        env::var("RUSTFLAGS").unwrap_or_default() + &format!(" -L{devkitpro}/libctru/lib");
+
     let cargo_cmd = &input.cmd;
 
     let mut command = cargo(&input.config);
-    command.arg(cargo_cmd.subcommand_name());
+    command
+        .arg(cargo_cmd.subcommand_name())
+        .env("RUSTFLAGS", rustflags);
 
     // Any command that needs to compile code will run under this environment.
     // Even `clippy` and `check` need this kind of context, so we'll just assume any other `Passthrough` command uses it too.
     if cargo_cmd.should_compile() {
-        let rust_flags = env::var("RUSTFLAGS").unwrap_or_default()
-            + &format!(
-                " -L{}/libctru/lib -lctru",
-                env::var("DEVKITPRO").expect("DEVKITPRO is not defined as an environment variable")
-            );
-
         command
-            .env("RUSTFLAGS", rust_flags)
             .arg("--target")
             .arg("armv6k-nintendo-3ds")
             .arg("--message-format")