From e1cf233cde79ab01596142e55cac9041a0372014 Mon Sep 17 00:00:00 2001
From: Ian Chamberlain <ian.h.chamberlain@gmail.com>
Date: Sun, 19 Nov 2023 14:11:06 -0500
Subject: [PATCH] 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 <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":
+        // 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<String>) -> (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<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.