From b7858c7b97f9ea8b49195df37ecc9e2ae8f80f84 Mon Sep 17 00:00:00 2001 From: Ian Chamberlain Date: Thu, 13 Jun 2024 23:51:18 -0400 Subject: [PATCH 1/8] Deserialize cargo-3ds metadatainto CTRConfig --- src/lib.rs | 114 ++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 78 insertions(+), 36 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 778b06d..74f9ea6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -8,9 +8,10 @@ use std::process::{Command, ExitStatus, Stdio}; use std::{env, fmt, io, process}; use camino::{Utf8Path, Utf8PathBuf}; -use cargo_metadata::{Message, MetadataCommand, Package}; +use cargo_metadata::{Message, MetadataCommand}; use rustc_version::Channel; use semver::Version; +use serde::Deserialize; use tee::TeeReader; use crate::command::{CargoCmd, Input, Run, Test}; @@ -280,14 +281,6 @@ pub fn get_metadata(messages: &[Message]) -> CTRConfig { let (package, artifact) = (package.unwrap(), artifact.unwrap()); - let mut icon_path = Utf8PathBuf::from("./icon.png"); - - if !icon_path.exists() { - icon_path = Utf8PathBuf::from(env::var("DEVKITPRO").unwrap()) - .join("libctru") - .join("default_icon.png"); - } - // for now assume a single "kind" since we only support one output artifact let name = match artifact.target.kind[0].as_ref() { "bin" | "lib" | "rlib" | "dylib" if artifact.target.test => { @@ -299,30 +292,22 @@ pub fn get_metadata(messages: &[Message]) -> CTRConfig { _ => artifact.target.name, }; - let romfs_dir = get_romfs_dir(&package); + let config = package + .metadata + .get("cargo-3ds") + .and_then(|c| CTRConfig::deserialize(c).ok()) + .unwrap_or_default(); CTRConfig { name, - authors: package.authors, - description: package - .description - .unwrap_or_else(|| String::from("Homebrew Application")), - icon_path, - romfs_dir, + authors: config.authors.or(Some(package.authors)), + description: config.description.or(package.description), manifest_dir: package.manifest_path.parent().unwrap().into(), target_path: artifact.executable.unwrap(), + ..config } } -fn get_romfs_dir(package: &Package) -> Option { - package - .metadata - .get("cargo-3ds")? - .get("romfs_dir")? - .as_str() - .map(Utf8PathBuf::from) -} - /// Builds the 3dsx using `3dsxtool`. /// This will fail if `3dsxtool` is not within the running directory or in a directory found in $PATH pub fn build_3dsx(config: &CTRConfig, verbose: bool) { @@ -381,15 +366,39 @@ pub fn link(config: &CTRConfig, run_args: &Run, verbose: bool) { } } -#[derive(Default, Debug)] +#[derive(Default, Debug, Deserialize, PartialEq, Eq)] pub struct CTRConfig { + /// The authors of the application, which will be joined by `", "` to form + /// the `Publisher` field in the SMDH format. If not specified, a single author + /// of "Unspecified Author" will be used. + authors: Option>, + + /// A description of the application, also called `Long Description` in the + /// SMDH format. The following values will be used in order of precedence: + /// - `cargo-3ds` metadata field + /// - `package.description` in Cargo.toml + /// - "Homebrew Application" + description: Option, + + /// The path to the app icon, defaulting to `$CARGO_MANIFEST_DIR/icon.png` + /// if it exists. If not specified, the devkitPro default icon is used. + icon_path: Option, + + /// The path to the romfs directory, defaulting to `$CARGO_MANIFEST_DIR/romfs` + /// if it exists, or unused otherwise. If a path is specified but does not + /// exist, an error occurs. + #[serde(alias = "romfs-dir")] + romfs_dir: Option, + + // Remaining fields come from cargo metadata / build artifact output and + // cannot be customized by users in `package.metadata.cargo-3ds`. I suppose + // in theory we could allow name to be customizable if we wanted... + #[serde(skip)] name: String, - authors: Vec, - description: String, - icon_path: Utf8PathBuf, + #[serde(skip)] target_path: Utf8PathBuf, + #[serde(skip)] manifest_dir: Utf8PathBuf, - romfs_dir: Option, } impl CTRConfig { @@ -409,22 +418,31 @@ impl CTRConfig { .join(self.romfs_dir.as_deref().unwrap_or(Utf8Path::new("romfs"))) } + // as standard with the devkitPRO toolchain + const DEFAULT_AUTHOR: &'static str = "Unspecified Author"; + const DEFAULT_DESCRIPTION: &'static str = "Homebrew Application"; + /// Builds the smdh using `smdhtool`. /// This will fail if `smdhtool` is not within the running directory or in a directory found in $PATH pub fn build_smdh(&self, verbose: bool) { - let author = if self.authors.is_empty() { - String::from("Unspecified Author") // as standard with the devkitPRO toolchain + let description = self + .description + .as_deref() + .unwrap_or(Self::DEFAULT_DESCRIPTION); + + let publisher = if let Some(authors) = self.authors.as_ref() { + authors.join(", ") } else { - self.authors.join(", ") + Self::DEFAULT_AUTHOR.to_string() }; let mut command = Command::new("smdhtool"); command .arg("--create") .arg(&self.name) - .arg(&self.description) - .arg(author) - .arg(&self.icon_path) + .arg(description) + .arg(publisher) + .arg(self.icon_path()) .arg(self.path_smdh()) .stdin(Stdio::inherit()) .stdout(Stdio::inherit()) @@ -444,6 +462,30 @@ impl CTRConfig { process::exit(status.code().unwrap_or(1)); } } + + /// Possible cases: + /// - icon path specified (exit with error if doesn't exist) + /// - icon path unspecified, icon.png exists + /// - icon path unspecified, icon.png does not exist + fn icon_path(&self) -> Utf8PathBuf { + let abs_path = self.manifest_dir.join( + self.icon_path + .as_deref() + .unwrap_or(Utf8Path::new("icon.png")), + ); + + if abs_path.is_file() { + abs_path + } else if self.icon_path.is_some() { + eprintln!("Specified icon path does not exist: {abs_path}"); + process::exit(1); + } else { + // We assume this default icon will always exist as part of the toolchain + Utf8PathBuf::from(env::var("DEVKITPRO").unwrap()) + .join("libctru") + .join("default_icon.png") + } + } } #[derive(Ord, PartialOrd, PartialEq, Eq, Debug)] From fbf77c1db8dfa1160d8cb5a8ae4880a33716fc54 Mon Sep 17 00:00:00 2001 From: Ian Chamberlain Date: Fri, 14 Jun 2024 00:41:57 -0400 Subject: [PATCH 2/8] Run build callbacks once per executable For each executable artifact generated by the build command, run the build callback on it. Then, if there is only one artifact, run the run/test callback on it. --- src/command.rs | 81 ++++++++++++++++++++++++++++++++++++-------------- src/lib.rs | 39 +++++------------------- src/main.rs | 7 ++++- 3 files changed, 72 insertions(+), 55 deletions(-) diff --git a/src/command.rs b/src/command.rs index 4b0dab7..3841b27 100644 --- a/src/command.rs +++ b/src/command.rs @@ -1,12 +1,13 @@ use std::fs; use std::io::Read; +use std::process; use std::process::Stdio; use std::sync::OnceLock; -use cargo_metadata::Message; +use cargo_metadata::{Message, Metadata}; use clap::{Args, Parser, Subcommand}; -use crate::{build_3dsx, cargo, get_metadata, link, print_command, CTRConfig}; +use crate::{build_3dsx, cargo, get_artifact_config, link, print_command, CTRConfig}; #[derive(Parser, Debug)] #[command(name = "cargo", bin_name = "cargo")] @@ -295,23 +296,64 @@ impl CargoCmd { /// /// - `cargo 3ds build` and other "build" commands will use their callbacks to build the final `.3dsx` file and link it. /// - `cargo 3ds new` and other generic commands will use their callbacks to make 3ds-specific changes to the environment. - pub fn run_callback(&self, messages: &[Message]) { - // Process the metadata only for commands that have it/use it - let config = if self.should_build_3dsx() { - eprintln!("Getting metadata"); + pub fn run_callback(&self, messages: &[Message], metadata: &Metadata) { + let mut configs = Vec::new(); - Some(get_metadata(messages)) - } else { - None - }; + // Process the metadata only for commands that have it/use it + if self.should_build_3dsx() { + configs = self.run_build_callbacks(messages, metadata); + } - // Run callback only for commands that use it + // For run + test, we can only run one of the targets. Error if more or less + // than one executable was built, otherwise run the callback for the first target. match self { - Self::Build(cmd) => cmd.callback(&config), - Self::Run(cmd) => cmd.callback(&config), - Self::Test(cmd) => cmd.callback(&config), + Self::Run(cmd) if configs.len() == 1 => cmd.callback(&configs.into_iter().next()), + Self::Test(cmd) if configs.len() == 1 => cmd.callback(&configs.into_iter().next()), + Self::Run(_) | Self::Test(_) => { + let names: Vec<_> = configs.into_iter().map(|c| c.name).collect(); + eprintln!("Error: expected exactly one executable to run, got {names:?}"); + process::exit(1); + } + // New is a special case where we always want to run its callback Self::New(cmd) => cmd.callback(), - _ => (), + _ => {} + } + } + + /// Generate a .3dsx for every executable artifact within the workspace that + /// was built by the cargo command. + fn run_build_callbacks(&self, messages: &[Message], metadata: &Metadata) -> Vec { + let mut configs = Vec::new(); + + for message in messages { + let Message::CompilerArtifact(artifact) = message else { + continue; + }; + + if artifact.executable.is_none() + || !metadata.workspace_members.contains(&artifact.package_id) + { + continue; + } + + let package = &metadata[&artifact.package_id]; + let config = Some(get_artifact_config(package.clone(), artifact.clone())); + + self.run_artifact_callback(&config); + + configs.push(config.unwrap()); + } + + configs + } + + // TODO: can we just swap all signatures to &CTRConfig? Seems more sensible now + fn run_artifact_callback(&self, config: &Option) { + match self { + Self::Build(cmd) => cmd.callback(config), + Self::Run(cmd) => cmd.build_args.callback(config), + Self::Test(cmd) => cmd.run_args.build_args.callback(config), + _ => {} } } } @@ -403,9 +445,6 @@ impl Run { /// /// This callback handles launching the application via `3dslink`. fn callback(&self, config: &Option) { - // Run the normal "build" callback - self.build_args.callback(config); - if !self.use_custom_runner() { if let Some(cfg) = config { eprintln!("Running 3dslink"); @@ -462,11 +501,7 @@ impl Test { /// /// This callback handles launching the application via `3dslink`. fn callback(&self, config: &Option) { - if self.no_run { - // If the tests don't have to run, use the "build" callback - self.run_args.build_args.callback(config); - } else { - // If the tests have to run, use the "run" callback + if !self.no_run { self.run_args.callback(config); } } diff --git a/src/lib.rs b/src/lib.rs index 74f9ea6..76df09c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -8,7 +8,7 @@ use std::process::{Command, ExitStatus, Stdio}; use std::{env, fmt, io, process}; use camino::{Utf8Path, Utf8PathBuf}; -use cargo_metadata::{Message, MetadataCommand}; +use cargo_metadata::{Artifact, Message, Package}; use rustc_version::Channel; use semver::Version; use serde::Deserialize; @@ -252,36 +252,10 @@ pub fn check_rust_version(input: &Input) { /// Parses messages returned by "build" cargo commands (such as `cargo 3ds build` or `cargo 3ds run`). /// The returned [`CTRConfig`] is then used for further building in and execution -/// in [`build_smdh`], [`build_3dsx`], and [`link`]. -pub fn get_metadata(messages: &[Message]) -> CTRConfig { - let metadata = MetadataCommand::new() - .no_deps() - .exec() - .expect("Failed to get cargo metadata"); - - let mut package = None; - let mut artifact = None; - - // Extract the final built executable. We may want to fail in cases where - // multiple executables, or none, were built? - for message in messages.iter().rev() { - if let Message::CompilerArtifact(art) = message { - if art.executable.is_some() { - package = Some(metadata[&art.package_id].clone()); - artifact = Some(art.clone()); - - break; - } - } - } - if package.is_none() || artifact.is_none() { - eprintln!("No executable found from build command output!"); - process::exit(1); - } - - let (package, artifact) = (package.unwrap(), artifact.unwrap()); - - // for now assume a single "kind" since we only support one output artifact +/// in [`CTRConfig::build_smdh`], [`build_3dsx`], and [`link`]. +pub fn get_artifact_config(package: Package, artifact: Artifact) -> CTRConfig { + // For now, assume a single "kind" per artifact. It seems to be the case + // when a single executable is built anyway but maybe not in all cases. let name = match artifact.target.kind[0].as_ref() { "bin" | "lib" | "rlib" | "dylib" if artifact.target.test => { format!("{} tests", artifact.target.name) @@ -292,6 +266,9 @@ pub fn get_metadata(messages: &[Message]) -> CTRConfig { _ => artifact.target.name, }; + // TODO: need to break down by target kind and name, e.g. + // [package.metadata.cargo-3ds.example.hello-world] + // Probably fall back to top level as well. let config = package .metadata .get("cargo-3ds") diff --git a/src/main.rs b/src/main.rs index d43bf57..3e4030e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -18,11 +18,16 @@ fn main() { } }; + let metadata = cargo_metadata::MetadataCommand::new() + .no_deps() + .exec() + .unwrap(); + let (status, messages) = run_cargo(&input, message_format); if !status.success() { process::exit(status.code().unwrap_or(1)); } - input.cmd.run_callback(&messages); + input.cmd.run_callback(&messages, &metadata); } From db3395946e3831416bbee88590ae60f95f0ffe9b Mon Sep 17 00:00:00 2001 From: Ian Chamberlain Date: Fri, 14 Jun 2024 21:53:26 -0400 Subject: [PATCH 3/8] Address some comments and minor cleanups --- Cargo.toml | 1 + src/command.rs | 138 +++++++++++++++++++++++++++++++------------------ src/main.rs | 11 ++-- 3 files changed, 94 insertions(+), 56 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 2d85859..4cd7563 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -9,6 +9,7 @@ categories = ["command-line-utilities", "development-tools::cargo-plugins"] exclude = [".github"] license = "MIT OR Apache-2.0" edition = "2021" +rust-version = "1.79" [dependencies] cargo_metadata = "0.14.0" diff --git a/src/command.rs b/src/command.rs index 3841b27..5deb7ba 100644 --- a/src/command.rs +++ b/src/command.rs @@ -1,7 +1,6 @@ use std::fs; use std::io::Read; -use std::process; -use std::process::Stdio; +use std::process::{self, Stdio}; use std::sync::OnceLock; use cargo_metadata::{Message, Metadata}; @@ -84,6 +83,12 @@ pub struct RemainingArgs { args: Vec, } +#[allow(unused_variables)] +trait Callbacks { + fn build_callback(&self, config: &CTRConfig) {} + fn run_callback(&self, config: &CTRConfig) {} +} + #[derive(Args, Debug)] pub struct Build { #[arg(from_global)] @@ -296,33 +301,43 @@ impl CargoCmd { /// /// - `cargo 3ds build` and other "build" commands will use their callbacks to build the final `.3dsx` file and link it. /// - `cargo 3ds new` and other generic commands will use their callbacks to make 3ds-specific changes to the environment. - pub fn run_callback(&self, messages: &[Message], metadata: &Metadata) { + pub fn run_callbacks(&self, messages: &[Message], metadata: &Option) { let mut configs = Vec::new(); // Process the metadata only for commands that have it/use it if self.should_build_3dsx() { - configs = self.run_build_callbacks(messages, metadata); + // unwrap: we should always have metadata if the command should compile something + configs = self.build_callbacks(messages, metadata.as_ref().unwrap()); } - // For run + test, we can only run one of the targets. Error if more or less - // than one executable was built, otherwise run the callback for the first target. - match self { - Self::Run(cmd) if configs.len() == 1 => cmd.callback(&configs.into_iter().next()), - Self::Test(cmd) if configs.len() == 1 => cmd.callback(&configs.into_iter().next()), - Self::Run(_) | Self::Test(_) => { - let names: Vec<_> = configs.into_iter().map(|c| c.name).collect(); - eprintln!("Error: expected exactly one executable to run, got {names:?}"); + let config = match self { + // If we produced one executable, we will attempt to run that one + _ if configs.len() == 1 => &configs[0], + + // --no-run can produce any number of executables + Self::Test(Test { no_run: true, .. }) => return, + + // Config is ignored by the New callback, using default is fine + Self::New(_) => &CTRConfig::default(), + + // Otherwise, print an error and exit + _ => { + let paths: Vec<_> = configs.into_iter().map(|c| c.path_3dsx()).collect(); + let names: Vec<_> = paths.iter().filter_map(|p| p.file_name()).collect(); + eprintln!( + "Error: expected exactly one (1) executable to run, got {}: {names:?}", + paths.len(), + ); process::exit(1); } - // New is a special case where we always want to run its callback - Self::New(cmd) => cmd.callback(), - _ => {} - } + }; + + self.run_callback(config); } /// Generate a .3dsx for every executable artifact within the workspace that /// was built by the cargo command. - fn run_build_callbacks(&self, messages: &[Message], metadata: &Metadata) -> Vec { + fn build_callbacks(&self, messages: &[Message], metadata: &Metadata) -> Vec { let mut configs = Vec::new(); for message in messages { @@ -337,23 +352,36 @@ impl CargoCmd { } let package = &metadata[&artifact.package_id]; - let config = Some(get_artifact_config(package.clone(), artifact.clone())); + let config = get_artifact_config(package.clone(), artifact.clone()); - self.run_artifact_callback(&config); + self.build_callback(&config); - configs.push(config.unwrap()); + configs.push(config); } configs } - // TODO: can we just swap all signatures to &CTRConfig? Seems more sensible now - fn run_artifact_callback(&self, config: &Option) { - match self { - Self::Build(cmd) => cmd.callback(config), - Self::Run(cmd) => cmd.build_args.callback(config), - Self::Test(cmd) => cmd.run_args.build_args.callback(config), - _ => {} + fn inner_callbacks(&self) -> Option<&dyn Callbacks> { + Some(match self { + Self::Build(cmd) => cmd, + Self::Run(cmd) => cmd, + Self::Test(cmd) => cmd, + _ => return None, + }) + } +} + +impl Callbacks for CargoCmd { + fn build_callback(&self, config: &CTRConfig) { + if let Some(cb) = self.inner_callbacks() { + cb.build_callback(config); + } + } + + fn run_callback(&self, config: &CTRConfig) { + if let Some(cb) = self.inner_callbacks() { + cb.run_callback(config); } } } @@ -384,17 +412,31 @@ impl RemainingArgs { } } -impl Build { +impl Callbacks for Build { /// Callback for `cargo 3ds build`. /// /// This callback handles building the application as a `.3dsx` file. - fn callback(&self, config: &Option) { - if let Some(config) = config { - eprintln!("Building smdh: {}", config.path_smdh()); - config.build_smdh(self.verbose); + fn build_callback(&self, config: &CTRConfig) { + eprintln!("Building smdh: {}", config.path_smdh()); + config.build_smdh(self.verbose); + + eprintln!("Building 3dsx: {}", config.path_3dsx()); + build_3dsx(config, self.verbose); + } +} - eprintln!("Building 3dsx: {}", config.path_3dsx()); - build_3dsx(config, self.verbose); +impl Callbacks for Run { + fn build_callback(&self, config: &CTRConfig) { + self.build_args.build_callback(config); + } + + /// Callback for `cargo 3ds run`. + /// + /// This callback handles launching the application via `3dslink`. + fn run_callback(&self, config: &CTRConfig) { + if !self.use_custom_runner() { + eprintln!("Running 3dslink"); + link(config, self, self.build_args.verbose); } } } @@ -441,18 +483,6 @@ impl Run { args } - /// Callback for `cargo 3ds run`. - /// - /// This callback handles launching the application via `3dslink`. - fn callback(&self, config: &Option) { - if !self.use_custom_runner() { - if let Some(cfg) = config { - eprintln!("Running 3dslink"); - link(cfg, self, self.build_args.verbose); - } - } - } - /// Returns whether the cargo environment has `target.armv6k-nintendo-3ds.runner` /// configured. This will only be checked once during the lifetime of the program, /// and takes into account the usual ways Cargo looks for its @@ -496,16 +526,22 @@ impl Run { } } -impl Test { +impl Callbacks for Test { + fn build_callback(&self, config: &CTRConfig) { + self.run_args.build_callback(config); + } + /// Callback for `cargo 3ds test`. /// /// This callback handles launching the application via `3dslink`. - fn callback(&self, config: &Option) { + fn run_callback(&self, config: &CTRConfig) { if !self.no_run { - self.run_args.callback(config); + self.run_args.run_callback(config); } } +} +impl Test { fn should_run(&self) -> bool { self.run_args.use_custom_runner() && !self.no_run } @@ -575,11 +611,11 @@ fn main() { } "#; -impl New { +impl Callbacks for New { /// Callback for `cargo 3ds new`. /// /// This callback handles the custom environment modifications when creating a new 3DS project. - fn callback(&self) { + fn run_callback(&self, _: &CTRConfig) { // Commmit changes to the project only if is meant to be a binary if self.cargo_args.args.contains(&"--lib".to_string()) { return; diff --git a/src/main.rs b/src/main.rs index 3e4030e..c6011eb 100644 --- a/src/main.rs +++ b/src/main.rs @@ -18,10 +18,11 @@ fn main() { } }; - let metadata = cargo_metadata::MetadataCommand::new() - .no_deps() - .exec() - .unwrap(); + let metadata = if input.cmd.should_build_3dsx() { + cargo_metadata::MetadataCommand::new().no_deps().exec().ok() + } else { + None + }; let (status, messages) = run_cargo(&input, message_format); @@ -29,5 +30,5 @@ fn main() { process::exit(status.code().unwrap_or(1)); } - input.cmd.run_callback(&messages, &metadata); + input.cmd.run_callbacks(&messages, &metadata); } From 1682db45a370fff57693068e34c0eec7a9d87db8 Mon Sep 17 00:00:00 2001 From: Ian Chamberlain Date: Fri, 14 Jun 2024 22:23:01 -0400 Subject: [PATCH 4/8] Address some comments and minor cleanups --- src/lib.rs | 44 ++++++++++++++++++++++++++------------------ 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 76df09c..4f6026f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -413,13 +413,18 @@ impl CTRConfig { Self::DEFAULT_AUTHOR.to_string() }; + let icon_path = self.icon_path().unwrap_or_else(|err| { + eprintln!("Icon at {err} does not exist"); + process::exit(1); + }); + let mut command = Command::new("smdhtool"); command .arg("--create") .arg(&self.name) .arg(description) .arg(publisher) - .arg(self.icon_path()) + .arg(icon_path) .arg(self.path_smdh()) .stdin(Stdio::inherit()) .stdout(Stdio::inherit()) @@ -440,27 +445,30 @@ impl CTRConfig { } } - /// Possible cases: - /// - icon path specified (exit with error if doesn't exist) - /// - icon path unspecified, icon.png exists - /// - icon path unspecified, icon.png does not exist - fn icon_path(&self) -> Utf8PathBuf { - let abs_path = self.manifest_dir.join( - self.icon_path - .as_deref() - .unwrap_or(Utf8Path::new("icon.png")), - ); - - if abs_path.is_file() { - abs_path - } else if self.icon_path.is_some() { - eprintln!("Specified icon path does not exist: {abs_path}"); - process::exit(1); + /// Get the path to the icon to be used for the SMDH output. + /// + /// # Errors + /// + /// Returns an error if the specified (or fallback) path does not exist. + /// The contained path is the path we tried to use. + fn icon_path(&self) -> Result { + let path = if let Some(path) = &self.icon_path { + self.manifest_dir.join(path) } else { - // We assume this default icon will always exist as part of the toolchain + let path = self.manifest_dir.join("icon.png"); + if path.exists() { + return Ok(path); + } + Utf8PathBuf::from(env::var("DEVKITPRO").unwrap()) .join("libctru") .join("default_icon.png") + }; + + if path.exists() { + Ok(path) + } else { + Err(path) } } } From dcf6b69e14458fb1fc1473194b14d01e86961bf0 Mon Sep 17 00:00:00 2001 From: Ian Chamberlain Date: Fri, 14 Jun 2024 22:31:33 -0400 Subject: [PATCH 5/8] Allocate CTRconfigs ahead of time --- src/command.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/command.rs b/src/command.rs index 5deb7ba..c0de0f0 100644 --- a/src/command.rs +++ b/src/command.rs @@ -302,7 +302,13 @@ impl CargoCmd { /// - `cargo 3ds build` and other "build" commands will use their callbacks to build the final `.3dsx` file and link it. /// - `cargo 3ds new` and other generic commands will use their callbacks to make 3ds-specific changes to the environment. pub fn run_callbacks(&self, messages: &[Message], metadata: &Option) { - let mut configs = Vec::new(); + let max_artifact_count = if let Some(metadata) = metadata { + metadata.packages.iter().map(|pkg| pkg.targets.len()).sum() + } else { + 0 + }; + + let mut configs = Vec::with_capacity(max_artifact_count); // Process the metadata only for commands that have it/use it if self.should_build_3dsx() { From 4525191c26651acbe9673dd5b3f02a8fccf851d2 Mon Sep 17 00:00:00 2001 From: Ian Chamberlain Date: Fri, 14 Jun 2024 23:06:47 -0400 Subject: [PATCH 6/8] Workaround needing 1.79 lifetime extension --- Cargo.toml | 1 - src/command.rs | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 4cd7563..2d85859 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -9,7 +9,6 @@ categories = ["command-line-utilities", "development-tools::cargo-plugins"] exclude = [".github"] license = "MIT OR Apache-2.0" edition = "2021" -rust-version = "1.79" [dependencies] cargo_metadata = "0.14.0" diff --git a/src/command.rs b/src/command.rs index c0de0f0..8675fc9 100644 --- a/src/command.rs +++ b/src/command.rs @@ -318,13 +318,13 @@ impl CargoCmd { let config = match self { // If we produced one executable, we will attempt to run that one - _ if configs.len() == 1 => &configs[0], + _ if configs.len() == 1 => configs.remove(0), // --no-run can produce any number of executables Self::Test(Test { no_run: true, .. }) => return, // Config is ignored by the New callback, using default is fine - Self::New(_) => &CTRConfig::default(), + Self::New(_) => CTRConfig::default(), // Otherwise, print an error and exit _ => { @@ -338,7 +338,7 @@ impl CargoCmd { } }; - self.run_callback(config); + self.run_callback(&config); } /// Generate a .3dsx for every executable artifact within the workspace that From a103806015439cead2493440e88c2e4422904b57 Mon Sep 17 00:00:00 2001 From: Ian Chamberlain Date: Fri, 14 Jun 2024 23:20:07 -0400 Subject: [PATCH 7/8] Fix failure for multiple build outputs Only test/run need to fail for multiple executables. We can also handle custom runners in the same logic and pull it out from the callbacks. --- src/command.rs | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/command.rs b/src/command.rs index 8675fc9..d5a5394 100644 --- a/src/command.rs +++ b/src/command.rs @@ -320,14 +320,21 @@ impl CargoCmd { // If we produced one executable, we will attempt to run that one _ if configs.len() == 1 => configs.remove(0), - // --no-run can produce any number of executables + // --no-run may produce any number of executables, and we skip the callback Self::Test(Test { no_run: true, .. }) => return, - // Config is ignored by the New callback, using default is fine + // If using custom runners, they may be able to handle multiple executables, + // and we also want to skip our own callback. `cargo run` also has its own + // logic to disallow multiple executables. + Self::Test(Test { run_args: run, .. }) | Self::Run(run) if run.use_custom_runner() => { + return + } + + // Config is ignored by the New callback, using default is fine. Self::New(_) => CTRConfig::default(), - // Otherwise, print an error and exit - _ => { + // Otherwise (configs.len() != 1) print an error and exit + Self::Test(_) | Self::Run(_) => { let paths: Vec<_> = configs.into_iter().map(|c| c.path_3dsx()).collect(); let names: Vec<_> = paths.iter().filter_map(|p| p.file_name()).collect(); eprintln!( @@ -336,6 +343,8 @@ impl CargoCmd { ); process::exit(1); } + + _ => return, }; self.run_callback(&config); @@ -440,10 +449,8 @@ impl Callbacks for Run { /// /// This callback handles launching the application via `3dslink`. fn run_callback(&self, config: &CTRConfig) { - if !self.use_custom_runner() { - eprintln!("Running 3dslink"); - link(config, self, self.build_args.verbose); - } + eprintln!("Running 3dslink"); + link(config, self, self.build_args.verbose); } } From 59000ef17b17e25d7e8418b32c1c85b5dd2e3a63 Mon Sep 17 00:00:00 2001 From: Ian Chamberlain Date: Sat, 15 Jun 2024 23:12:31 -0400 Subject: [PATCH 8/8] More small cleanups, hopefully last round --- src/command.rs | 41 ++++++++++++++++------------------------- src/lib.rs | 6 +++--- src/main.rs | 10 ++++++++-- 3 files changed, 27 insertions(+), 30 deletions(-) diff --git a/src/command.rs b/src/command.rs index d5a5394..35f2c32 100644 --- a/src/command.rs +++ b/src/command.rs @@ -301,24 +301,14 @@ impl CargoCmd { /// /// - `cargo 3ds build` and other "build" commands will use their callbacks to build the final `.3dsx` file and link it. /// - `cargo 3ds new` and other generic commands will use their callbacks to make 3ds-specific changes to the environment. - pub fn run_callbacks(&self, messages: &[Message], metadata: &Option) { - let max_artifact_count = if let Some(metadata) = metadata { - metadata.packages.iter().map(|pkg| pkg.targets.len()).sum() - } else { - 0 - }; - - let mut configs = Vec::with_capacity(max_artifact_count); - - // Process the metadata only for commands that have it/use it - if self.should_build_3dsx() { - // unwrap: we should always have metadata if the command should compile something - configs = self.build_callbacks(messages, metadata.as_ref().unwrap()); - } + pub fn run_callbacks(&self, messages: &[Message], metadata: Option<&Metadata>) { + let configs = metadata + .map(|metadata| self.build_callbacks(messages, metadata)) + .unwrap_or_default(); let config = match self { // If we produced one executable, we will attempt to run that one - _ if configs.len() == 1 => configs.remove(0), + _ if configs.len() == 1 => configs.into_iter().next().unwrap(), // --no-run may produce any number of executables, and we skip the callback Self::Test(Test { no_run: true, .. }) => return, @@ -353,7 +343,8 @@ impl CargoCmd { /// Generate a .3dsx for every executable artifact within the workspace that /// was built by the cargo command. fn build_callbacks(&self, messages: &[Message], metadata: &Metadata) -> Vec { - let mut configs = Vec::new(); + let max_artifact_count = metadata.packages.iter().map(|pkg| pkg.targets.len()).sum(); + let mut configs = Vec::with_capacity(max_artifact_count); for message in messages { let Message::CompilerArtifact(artifact) = message else { @@ -377,25 +368,25 @@ impl CargoCmd { configs } - fn inner_callbacks(&self) -> Option<&dyn Callbacks> { - Some(match self { - Self::Build(cmd) => cmd, - Self::Run(cmd) => cmd, - Self::Test(cmd) => cmd, - _ => return None, - }) + fn inner_callback(&self) -> Option<&dyn Callbacks> { + match self { + Self::Build(cmd) => Some(cmd), + Self::Run(cmd) => Some(cmd), + Self::Test(cmd) => Some(cmd), + _ => None, + } } } impl Callbacks for CargoCmd { fn build_callback(&self, config: &CTRConfig) { - if let Some(cb) = self.inner_callbacks() { + if let Some(cb) = self.inner_callback() { cb.build_callback(config); } } fn run_callback(&self, config: &CTRConfig) { - if let Some(cb) = self.inner_callbacks() { + if let Some(cb) = self.inner_callback() { cb.run_callback(config); } } diff --git a/src/lib.rs b/src/lib.rs index 4f6026f..43d2753 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -266,7 +266,7 @@ pub fn get_artifact_config(package: Package, artifact: Artifact) -> CTRConfig { _ => artifact.target.name, }; - // TODO: need to break down by target kind and name, e.g. + // TODO(#62): need to break down by target kind and name, e.g. // [package.metadata.cargo-3ds.example.hello-world] // Probably fall back to top level as well. let config = package @@ -413,8 +413,8 @@ impl CTRConfig { Self::DEFAULT_AUTHOR.to_string() }; - let icon_path = self.icon_path().unwrap_or_else(|err| { - eprintln!("Icon at {err} does not exist"); + let icon_path = self.icon_path().unwrap_or_else(|err_path| { + eprintln!("Icon at {err_path} does not exist"); process::exit(1); }); diff --git a/src/main.rs b/src/main.rs index c6011eb..63140ad 100644 --- a/src/main.rs +++ b/src/main.rs @@ -19,7 +19,13 @@ fn main() { }; let metadata = if input.cmd.should_build_3dsx() { - cargo_metadata::MetadataCommand::new().no_deps().exec().ok() + match cargo_metadata::MetadataCommand::new().no_deps().exec() { + Ok(metadata) => Some(metadata), + Err(err) => { + eprintln!("Warning: failed to gather cargo metadata for the project: {err}"); + None + } + } } else { None }; @@ -30,5 +36,5 @@ fn main() { process::exit(status.code().unwrap_or(1)); } - input.cmd.run_callbacks(&messages, &metadata); + input.cmd.run_callbacks(&messages, metadata.as_ref()); }