From 545d853a8093b48aa7cb843555db44118993785a Mon Sep 17 00:00:00 2001 From: Ian Chamberlain Date: Wed, 15 Nov 2023 23:47:02 -0500 Subject: [PATCH 1/5] Use `cargo --unit-graph` to figure out debuginfo We can check if `ctru-sys` is being built with debuginfo this way, and use that to build the stdlib with the same linker flags as ctru-sys is expected to use. --- Cargo.lock | 5 ++-- Cargo.toml | 1 + src/graph.rs | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++++ src/lib.rs | 46 +++++++++++++++++++++++----- 4 files changed, 128 insertions(+), 9 deletions(-) create mode 100644 src/graph.rs diff --git a/Cargo.lock b/Cargo.lock index 073a874..21ac74a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -74,6 +74,7 @@ dependencies = [ "rustc_version", "semver", "serde", + "serde_json", "shlex", "tee", "toml", @@ -279,9 +280,9 @@ dependencies = [ [[package]] name = "serde_json" -version = "1.0.107" +version = "1.0.108" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6b420ce6e3d8bd882e9b243c6eed35dbc9a6110c9769e74b584e0d68d1f20c65" +checksum = "3d1c7e3eac408d115102c4c24ad393e0821bb3a5df4d506a80f85f7a742a526b" dependencies = [ "itoa", "ryu", diff --git a/Cargo.toml b/Cargo.toml index 6a8f244..2354eff 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -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" diff --git a/src/graph.rs b/src/graph.rs new file mode 100644 index 0000000..e1cf323 --- /dev/null +++ b/src/graph.rs @@ -0,0 +1,85 @@ +use std::error::Error; +use std::process::{Command, Stdio}; + +use cargo_metadata::Target; +use serde::Deserialize; + +use crate::print_command; + +/// In lieu of +/// 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, +} + +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 . + pub fn from_cargo(cargo_cmd: &Command, verbose: bool) -> Result> { + // Since Command isn't Clone, copy it "by hand": + let mut cmd = Command::new(cargo_cmd.get_program()); + + // TODO: this should probably use "build" subcommand for "run", since right + // now there appears to be a crash in cargo when using `run`: + // + // thread 'main' panicked at src/cargo/ops/cargo_run.rs:83:5: + // assertion `left == right` failed + // left: 0 + // right: 1 + + let mut args = cargo_cmd.get_args(); + cmd.arg(args.next().unwrap()) + // 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) + .stdout(Stdio::piped()); + + if verbose { + print_command(&cmd); + } + + let mut proc = cmd.spawn()?; + let stdout = proc.stdout.take().unwrap(); + + let result: Self = serde_json::from_reader(stdout).map_err(|err| { + let _ = proc.wait(); + err + })?; + + let status = proc.wait()?; + 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, +} diff --git a/src/lib.rs b/src/lib.rs index 8311048..e74695c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,4 +1,5 @@ pub mod command; +mod graph; use core::fmt; use std::io::{BufRead, BufReader}; @@ -13,6 +14,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 +24,20 @@ use crate::command::{CargoCmd, Run}; pub fn run_cargo(input: &Input, message_format: Option) -> (ExitStatus, Vec) { let mut command = make_cargo_command(input, &message_format); + let libctru = if should_use_ctru_debuginfo(&command, input.verbose) { + "ctrud" + } else { + "ctru" + }; + + let rust_flags = env::var("RUSTFLAGS").unwrap_or_default() + + &format!( + " -L{}/libctru/lib -l{libctru}", + env::var("DEVKITPRO").expect("DEVKITPRO is not defined as an environment variable") + ); + + command.env("RUSTFLAGS", rust_flags); + if input.verbose { print_command(&command); } @@ -57,6 +73,29 @@ pub fn run_cargo(input: &Input, message_format: Option) -> (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`), @@ -70,14 +109,7 @@ pub fn make_cargo_command(input: &Input, message_format: &Option) -> Com // 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") From e1cf233cde79ab01596142e55cac9041a0372014 Mon Sep 17 00:00:00 2001 From: Ian Chamberlain Date: Sun, 19 Nov 2023 14:11:06 -0500 Subject: [PATCH 2/5] Handle `--unit-graph` errors better --- src/graph.rs | 36 ++++++++++++++++++++---------------- src/lib.rs | 26 +++++++++++++++++++------- 2 files changed, 39 insertions(+), 23 deletions(-) diff --git a/src/graph.rs b/src/graph.rs index e1cf323..57eb0b6 100644 --- a/src/graph.rs +++ b/src/graph.rs @@ -1,4 +1,5 @@ use std::error::Error; +use std::io::Read; use std::process::{Command, Stdio}; use cargo_metadata::Target; @@ -23,24 +24,18 @@ impl UnitGraph { /// /// See . pub fn from_cargo(cargo_cmd: &Command, verbose: bool) -> Result> { - // Since Command isn't Clone, copy it "by hand": + // Since Command isn't Clone, copy it "by hand", by copying its args and envs let mut cmd = Command::new(cargo_cmd.get_program()); - // TODO: this should probably use "build" subcommand for "run", since right - // now there appears to be a crash in cargo when using `run`: - // - // thread 'main' panicked at src/cargo/ops/cargo_run.rs:83:5: - // assertion `left == right` failed - // left: 0 - // right: 1 - let mut args = cargo_cmd.get_args(); - cmd.arg(args.next().unwrap()) + 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) - .stdout(Stdio::piped()); + .envs(cargo_cmd.get_envs().filter_map(|(k, v)| Some((k, v?)))) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); if verbose { print_command(&cmd); @@ -48,16 +43,25 @@ impl UnitGraph { 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(); - err + format!("unable to parse `--unit-graph` json: {err}\nstderr: `{stderr_str}`") })?; - let status = proc.wait()?; - if !status.success() { - return Err(format!("`cargo --unit-graph` exited with status {status:?}").into()); - } + let _status = proc.wait()?; + // TODO: + // `cargo run --unit-graph` panics at src/cargo/ops/cargo_run.rs:83:5 + // I should probably file a bug for that, then return the error here when it's fixed, + // but for now just ignore it since we still get valid JSON from the command. + // + // if !status.success() { + // return Err(format!("`cargo --unit-graph` exited with status {status:?}").into()); + // } if result.version == 1 { Ok(result) diff --git a/src/lib.rs b/src/lib.rs index e74695c..a9254fa 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -2,6 +2,7 @@ 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}; @@ -30,13 +31,16 @@ pub fn run_cargo(input: &Input, message_format: Option) -> (ExitStatus, "ctru" }; - let rust_flags = env::var("RUSTFLAGS").unwrap_or_default() - + &format!( - " -L{}/libctru/lib -l{libctru}", - env::var("DEVKITPRO").expect("DEVKITPRO is not defined as an environment variable") - ); + 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", rust_flags); + command.env("RUSTFLAGS", rustflags); if input.verbose { print_command(&command); @@ -101,10 +105,18 @@ fn should_use_ctru_debuginfo(cargo_cmd: &Command, verbose: bool) -> bool { /// 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) -> 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. From 85ebef86aa490a7c3e30b43a4d082a433233baf7 Mon Sep 17 00:00:00 2001 From: Ian Chamberlain Date: Fri, 24 Nov 2023 17:23:14 -0500 Subject: [PATCH 3/5] Update comment about panic in `cargo --unit-graph` --- src/graph.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/graph.rs b/src/graph.rs index 57eb0b6..67e5cc8 100644 --- a/src/graph.rs +++ b/src/graph.rs @@ -54,10 +54,11 @@ impl UnitGraph { })?; let _status = proc.wait()?; - // TODO: + // 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 - // I should probably file a bug for that, then return the error here when it's fixed, - // but for now just ignore it since we still get valid JSON from the command. + // 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()); From 2baf9cfc781ff1b019e9ec066a1f1aa7d6d8c3b9 Mon Sep 17 00:00:00 2001 From: Ian Chamberlain Date: Fri, 24 Nov 2023 21:48:23 -0500 Subject: [PATCH 4/5] Bump version to 0.1.2 --- Cargo.lock | 2 +- Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 21ac74a..026942f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -67,7 +67,7 @@ dependencies = [ [[package]] name = "cargo-3ds" -version = "0.1.1" +version = "0.1.2" dependencies = [ "cargo_metadata", "clap", diff --git a/Cargo.toml b/Cargo.toml index 2354eff..de30cce 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "cargo-3ds" -version = "0.1.1" +version = "0.1.2" authors = ["Rust3DS Org", "Andrea Ciliberti "] description = "Cargo wrapper for developing Nintendo 3DS homebrew apps" repository = "https://github.com/rust3ds/cargo-3ds" From 7d6ddb2ef89a6f6118526120bb2e99cc32c294f4 Mon Sep 17 00:00:00 2001 From: Ian Chamberlain Date: Fri, 24 Nov 2023 22:24:45 -0500 Subject: [PATCH 5/5] Fix generated main using now-removed panic handler --- src/command.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/command.rs b/src/command.rs index 7ee7256..02f998f 100644 --- a/src/command.rs +++ b/src/command.rs @@ -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();