feat: Improved strip config handling (#22)

Usually in rust you can set `strip=true` or `strip="symbols"` in your Cargo.toml. This doesn't work for Vita, since it strips the relocation information from the `elf`, which causes `vita-elf-create` to fail. Previously we had an additional vita-specific `strip` step configurable via `Cargo.toml`. This PR changes the behavior in the following way: 1. Before calling `cargo build` it now does an additional `cargo build ... -Z unstable-options --unit-graph`. This unstable option does a dry-run of the build emitting a JSON with a graph. This JSON is parsed to check if symbol stripping was enabled, and if the build failed AND we stripping was detected, `cargo-vita` now emits a sensible warning, improving the devx. Since this cargo feature is unstable (for 4 years already), if it fails, the output is ignored and does NOT fail the build. 2. The additional stripping pass is now configurable separately for dev and release builds, being enabled for release builds by default. This pass always uses `--strip-unneeded` which does not break `vita-elf-create` and still yields smaller binaries. 3. Updated the readme to reflect these changes
vita-rust · Apr 7, 2024 · 944718a · 944718a
1 parent 8b1272c
commit 944718a
Show file tree

Hide file tree

Showing 5 changed files with 241 additions and 84 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -51,6 +51,8 @@ jobs:
         uses: actions/checkout@v4
       - name: rust-toolchain
         uses: dtolnay/rust-toolchain@stable
+      - name: "`cargo fmt`"
+        run: cargo fmt --all --check
       - name: "`cargo deny`"
         uses: EmbarkStudios/cargo-deny-action@v1
       - name: "`cargo clippy`"

diff --git a/README.md b/README.md
@@ -6,14 +6,14 @@
 
 Cargo command to work with Sony PlayStation Vita rust project binaries.
 
-For general guidelines see [vita-rust book](https://vita-rust.github.io/book).
+For general guidelines see the [vita-rust book].
 
 ## Requirements
 
-- [VitaSDK](https://vitasdk.org/) must be installed, and `VITASDK` environment variable must point to its location.
-- [vitacompanion](https://github.com/devnoname120/vitacompanion) for ftp and command server (uploading and running artifacts)
-- [PrincessLog](https://github.com/CelesteBlue-dev/PSVita-RE-tools/tree/master/PrincessLog/build) is required for `cargo vita logs`
-- [vita-parse-core](https://github.com/xyzz/vita-parse-core) for `cargo vita coredump parse`
+- [VitaSDK] must be installed, and `VITASDK` environment variable must point to its location.
+- [vitacompanion] for FTP and command server (uploading and running artifacts)
+- [PrincessLog] is required for `cargo vita logs`
+- [vita-parse-core] for `cargo vita coredump parse`
 
 ## Installation
 
@@ -23,7 +23,7 @@ cargo +nightly install cargo-vita
 
 ## Usage
 
-Use the nightly toolchain to build Vita apps (either by using rustup override nightly for the project directory or by adding +nightly in the cargo invocation).
+Use the nightly toolchain to build Vita apps (either by using `rustup override nightly` for the project directory or by adding +nightly in the cargo invocation).
 
 
 ```
@@ -74,14 +74,17 @@ title_name = "My application"
 assets = "static"
 # Optional, this is the default
 build_std = "std,panic_unwind"
-# Optional, true by default. Will strip debug symbols from the resulting elf when enabled.
-strip = true
-# Optional, this is the default
-vita_strip_flags = ["-g"]
 # Optional, this is the default
 vita_make_fself_flags = ["-s"]
 # Optional, this is the default
 vita_mksfoex_flags = ["-d", "ATTRIBUTE2=12"]
+
+[package.metadata.vita.profile.dev]
+# Strips symbols from the vita elf in dev profile. Optional, default is false
+strip_symbols = true
+[package.metadata.vita.profile.release]
+# Strips symbols from the vita elf in release profile. Optional, default is true
+strip_symbols = true
 ```
 
 ## Examples
@@ -105,13 +108,13 @@ cargo vita logs
 
 ## Additional tools
 
-For a better development experience it is recommended to install additional modules on your Vita.
+For a better development experience, it is recommended to install the following modules on your Vita.
 
 ### vitacompanion
 
-When enabled, this module keeps a FTP server on your Vita running on port `1337`, as well as a TCP command server running on port `1338`.
+When enabled, this module keeps an FTP server on your Vita running on port `1337`, as well as a TCP command server running on port `1338`.
 
-- The FTP server allows you to easily upload `vpk` and `eboot` files to your Vita. This is FTP server is used by `cargo-vita` for the following commands and flags:
+- The FTP server allows you to easily upload `vpk` and `eboot` files to your Vita. This FTP server is used by `cargo-vita` for the following commands and flags:
 
   ```sh
   # Builds a eboot.bin, and uploads it to ux0:/app/TITLEID/eboot.bin
@@ -138,7 +141,7 @@ When enabled, this module keeps a FTP server on your Vita running on port `1337`
 ### PrincessLog
 
 This module allows capturing stdout and stderr from your Vita.
-In order to capture the logs you need to start a TCP server on your computer, and configure
+In order to capture the logs you need to start a TCP server on your computer and configure
 PrincessLog to connect to it.
 
 For convenience `cargo-vita` provides two commands to work with logs:
@@ -149,10 +152,10 @@ For convenience `cargo-vita` provides two commands to work with logs:
     # Start a TCP server on 0.0.0.0, and print all bytes received via the socket to stdout
     cargo vita logs
     ```
-  - A command to reconfigure PrincessLog with the new ip/port. This will use
+  - A command to reconfigure PrincessLog with the new IP/port. This will use
     the FTP server provided by `vitacompanion` to upload a new config.
     If an IP address of your machine is not explicitly provided, it will be guessed
-    using [local-ip-address](https://crates.io/crates/local-ip-address) crate.
+    using [local-ip-address] crate.
     When a configuration file is updated, the changes are not applied until Vita is rebooted.
 
     ```sh
@@ -167,10 +170,44 @@ For convenience `cargo-vita` provides two commands to work with logs:
     cargo vita logs configure --host-ip-address 10.10.10.10 --kernel-debug
     ```
 
+## Notes
+
+To produce the actual artifact runnable on the device, `cargo-vita` does multiple steps[^vita-toolchain-readme]:
+
+1. Calls `cargo build` to build the code and link it to a `elf` file (using linker from [VitaSDK])
+2. Calls `vita-elf-create` from [VitaSDK] to transform the `elf` into Vita `elf` (`velf`)
+3. Calls `vita-make-fself` from [VitaSDK] to make an unsigned `self` file (`fself`, aka `eboot`) from the `velf`.
+
+The second step of this process requires relocation segments in the elf.
+This means, that adding `strip=true` or `strip="symbols"` is not supported for Vita target,
+since symbol stripping also strips relocation information.
+
+To counter this issue, `cargo-vita` can do an additional strip step of the `elf` with `--strip-unneeded` flag, which reduces the binary size without interfering with other steps necessary to produce a runnable binary.
+
+This step is enabled for release profile builds and disabled for other profile builds by default, but can be configured per-crate via the following section in `Cargo.toml`:
+
+```toml
+[package.metadata.vita.profile.dev]
+# Strips symbols from the vita elf in dev profile, default is false
+strip_symbols = true
+[package.metadata.vita.profile.release]
+# Strips symbols from the vita elf in release profile, default is true
+strip_symbols = true
+```
+
+
 ## License
 
 Except where noted (below and/or in individual files), all code in this repository is dual-licensed at your option under either:
 
 * MIT License ([LICENSE-MIT](LICENSE-MIT) or [http://opensource.org/licenses/MIT](http://opensource.org/licenses/MIT))
 * Apache License, Version 2.0 ([LICENSE-APACHE](LICENSE-APACHE) or [http://www.apache.org/licenses/LICENSE-2.0](http://www.apache.org/licenses/LICENSE-2.0))
 
+[vita-rust book]: https://vita-rust.github.io/book
+[VitaSDK]: https://vitasdk.org/
+[vitacompanion]: https://github.com/devnoname120/vitacompanion
+[PrincessLog]: https://github.com/CelesteBlue-dev/PSVita-RE-tools/tree/master/PrincessLog/build
+[vita-parse-core]: https://github.com/xyzz/vita-parse-core
+[local-ip-address]: https://crates.io/crates/local-ip-address
+
+[^vita-toolchain-readme]: https://github.com/vitasdk/vita-toolchain/blob/master/README.md
diff --git a/src/commands/build.rs b/src/commands/build.rs
@@ -6,20 +6,22 @@ use std::{
     process::{Command, Stdio},
 };
 
-use crate::{check, ftp};
+use crate::{check, commands::build::unit_graph::try_parse_unit_graph, ftp};
 use anyhow::{bail, Context};
 use cargo_metadata::{camino::Utf8PathBuf, Artifact, Message, Package};
-use clap::{Args, Subcommand};
+use clap::{command, Args, Subcommand};
 use colored::Colorize;
 use either::Either;
-use log::info;
+use log::{info, warn};
 use tee::TeeReader;
 use walkdir::WalkDir;
 
 use crate::meta::{parse_crate_metadata, PackageMetadata, TitleId, VITA_TARGET};
 
 use super::{ConnectionArgs, Executor, OptionalConnectionArgs, Run};
 
+mod unit_graph;
+
 #[derive(Args, Debug)]
 pub struct Build {
     #[command(subcommand)]
@@ -210,91 +212,131 @@ impl<'a> BuildContext<'a> {
         let rust_flags = env::var("RUSTFLAGS").unwrap_or_default()
             + " --cfg mio_unsupported_force_poll_poll --cfg mio_unsupported_force_waker_pipe";
 
-        let mut command = Command::new(cargo);
+        // FIXME: move build-std to .cargo/config.toml, since it is shared by ALL of the crates built,
+        // but the metadata is per-crate. This still works correctly when building only a single workspace crate.
+        let (meta, _, _) = parse_crate_metadata(None)?;
 
-        if let Ok(path) = env::var("PATH") {
-            let sdk_path = Path::new(&self.sdk).join("bin");
-            let path = format!("{}:{path}", sdk_path.display());
-            command.env("PATH", path);
-        }
+        let command = || {
+            let mut command = Command::new(&cargo);
 
-        // FIXME: move build-std to env/config.toml, since it is shared by all of the crates built
-        // This still works correctly when building only a single workspace crate though
-        let (meta, _, _) = parse_crate_metadata(None)?;
+            if let Ok(path) = env::var("PATH") {
+                let sdk_path = Path::new(&self.sdk).join("bin");
+                let path = format!("{}:{path}", sdk_path.display());
+                command.env("PATH", path);
+            }
 
+            command
+                .env("RUSTFLAGS", &rust_flags)
+                .env("TARGET_CC", "arm-vita-eabi-gcc")
+                .env("TARGET_CXX", "arm-vita-eabi-g++")
+                .pass_path_env("OPENSSL_LIB_DIR", || self.sdk("arm-vita-eabi").join("lib"))
+                .pass_path_env("OPENSSL_INCLUDE_DIR", || {
+                    self.sdk("arm-vita-eabi").join("include")
+                })
+                .pass_path_env("PKG_CONFIG_PATH", || {
+                    self.sdk("arm-vita-eabi").join("lib").join("pkgconfig")
+                })
+                .pass_env("PKG_CONFIG_SYSROOT_DIR", || &self.sdk)
+                .env("VITASDK", &self.sdk)
+                .arg("build")
+                .arg("-Z")
+                .arg(format!("build-std={}", &meta.build_std))
+                .arg("--target")
+                .arg(VITA_TARGET)
+                .arg("--message-format=json-render-diagnostics")
+                .args(&self.command.cargo_args);
+
+            command
+        };
+
+        let hints = try_parse_unit_graph(command()).ok();
+
+        let mut command = command();
         command
-            .env("RUSTFLAGS", rust_flags)
-            .env("TARGET_CC", "arm-vita-eabi-gcc")
-            .env("TARGET_CXX", "arm-vita-eabi-g++")
-            .pass_path_env("OPENSSL_LIB_DIR", || self.sdk("arm-vita-eabi").join("lib"))
-            .pass_path_env("OPENSSL_INCLUDE_DIR", || {
-                self.sdk("arm-vita-eabi").join("include")
-            })
-            .pass_path_env("PKG_CONFIG_PATH", || {
-                self.sdk("arm-vita-eabi").join("lib").join("pkgconfig")
-            })
-            .pass_env("PKG_CONFIG_SYSROOT_DIR", || &self.sdk)
-            .env("VITASDK", &self.sdk)
-            .arg("build")
-            .arg("-Z")
-            .arg(format!("build-std={}", meta.build_std))
-            .arg("--target")
-            .arg(VITA_TARGET)
-            .arg("--message-format")
-            .arg("json-render-diagnostics")
-            .args(&self.command.cargo_args)
             .stdin(Stdio::inherit())
             .stdout(Stdio::piped())
             .stderr(Stdio::inherit());
 
         info!("{}: {command:?}", "Running cargo".blue());
 
         let mut process = command.spawn().context("Unable to spawn build process")?;
-        let command_stdout = process.stdout.take().context("Build failed")?;
-
-        let reader = if log::max_level() >= log::LevelFilter::Trace {
-            Either::Left(BufReader::new(TeeReader::new(command_stdout, io::stdout())))
+        let stdout = process.stdout.take().context("Build failed")?;
+        let stdout = if log::max_level() >= log::LevelFilter::Trace {
+            Either::Left(BufReader::new(TeeReader::new(stdout, io::stdout())))
         } else {
-            Either::Right(BufReader::new(command_stdout))
+            Either::Right(BufReader::new(stdout))
         };
 
-        let messages: Vec<Message> = Message::parse_stream(reader)
-            .collect::<io::Result<_>>()
-            .context("Unable to parse build stdout")?;
+        let message_stream = Message::parse_stream(stdout);
 
-        let artifacts = messages
-            .iter()
-            .rev()
-            .filter_map(|m| match m {
-                Message::CompilerArtifact(art) if art.executable.is_some() => Some(art.clone()),
-                _ => None,
-            })
-            .map(ExecutableArtifact::new)
-            .collect::<anyhow::Result<_>>()?;
+        let mut artifacts = Vec::new();
+
+        for message in message_stream {
+            match message.context("Unable to parse cargo output")? {
+                Message::CompilerArtifact(art) if art.executable.is_some() => {
+                    artifacts.push(ExecutableArtifact::new(art)?);
+                }
+                _ => {}
+            }
+        }
 
         if !process.wait_with_output()?.status.success() {
+            if let Some(hints) = hints {
+                if hints.strip_symbols() {
+                    warn!(
+                        "{warn}\n \
+                        Symbols in elf are required by `{velf}` to create a velf file.\n \
+                        Please remove `{strip_true}` or `{strip_symbols}` from your Cargo.toml.\n \
+                        If you want to optimize for the binary size, replace it \
+                        with `{strip_debug}` to strip debug section.\n \
+                        If you want to strip the symbol data from the resulting \
+                        binary, set `{strip_velf}` in `{vita_section}` \
+                        section of your Cargo.toml, this would strip the symbols from the velf.",
+                        warn = "Stripping symbols from ELF is unsupported.".yellow(),
+                        velf = "vita-elf-create".cyan(),
+                        strip_true = "strip=true".cyan(),
+                        strip_symbols = "strip=\"symbols\"".cyan(),
+                        strip_debug = "strip=\"debuginfo\"".cyan(),
+                        strip_velf = "strip_symbols = true".cyan(),
+                        vita_section = format!("[package.metadata.vita.{}]", hints.profile).cyan()
+                    );
+                }
+            }
+
             bail!("cargo build failed")
         }
 
         Ok(artifacts)
     }
 
     fn strip(&self, art: &ExecutableArtifact) -> anyhow::Result<()> {
-        if !art.meta.strip {
-            info!("{}", "Skipping elf strip".yellow());
+        // Try to guess if the elf was built with debug or release profile.
+        // This intentionally uses components() instead of as_str() to
+        // ensure that it works with operating systems that use a reverse slash for paths (Windows),
+        // as well as it works if the path is not normalized.
+        let profile = art
+            .elf
+            .components()
+            .skip_while(|s| s.as_str() != "armv7-sony-vita-newlibeabihf")
+            .nth(1);
+
+        let profile = profile.map_or("dev", |p| p.as_str());
+
+        if !art.meta.strip_symbols(profile) {
+            info!("{}", "Skipping additional elf strip".yellow());
             return Ok(());
         }
 
         let mut command = Command::new(self.sdk_binary("arm-vita-eabi-strip"));
 
         command
-            .args(&art.meta.vita_strip_flags)
+            .arg("--strip-unneeded")
             .arg(&art.elf)
             .stdin(Stdio::inherit())
             .stdout(Stdio::inherit())
             .stderr(Stdio::inherit());
 
-        info!("{}: {command:?}", "Stripping elf".blue());
+        info!("{}: {command:?}", "Stripping symbols from elf".blue());
 
         if !command.status()?.success() {
             bail!("arm-vita-eabi-strip failed");