Fix: handle `-- --target-dir` arg in `cargo build-sbf` (#33555)

* fix: handle target dir override in build-sbf cargo args

* fix: refactor to canonicalize target arg for workspace absolute paths

* fix: nightly linting

sdk/cargo-build-sbf/src/main.rs

@@ -1,5 +1,6 @@
 use {
     bzip2::bufread::BzDecoder,
+    cargo_metadata::camino::Utf8PathBuf,
     clap::{crate_description, crate_name, crate_version, Arg},
     itertools::Itertools,
     log::*,
@@ -22,7 +23,8 @@ use {
 
 #[derive(Debug)]
 struct Config<'a> {
-    cargo_args: Option<Vec<&'a str>>,
+    cargo_args: Vec<&'a str>,
+    target_directory: Option<Utf8PathBuf>,
     sbf_out_dir: Option<PathBuf>,
     sbf_sdk: PathBuf,
     platform_tools_version: &'a str,
@@ -43,7 +45,8 @@ struct Config<'a> {
 impl Default for Config<'_> {
     fn default() -> Self {
         Self {
-            cargo_args: None,
+            cargo_args: vec![],
+            target_directory: None,
             sbf_sdk: env::current_exe()
                 .expect("Unable to get current executable")
                 .parent()
@@ -721,11 +724,7 @@ fn build_solana_package(
         cargo_build_args.push("--jobs");
         cargo_build_args.push(jobs);
     }
-    if let Some(args) = &config.cargo_args {
+    cargo_build_args.append(&mut config.cargo_args.clone());
-        for arg in args {
-            cargo_build_args.push(arg);
-        }
-    }
     let output = spawn(
         &cargo_build,
         &cargo_build_args,
@@ -864,9 +863,14 @@ fn build_solana(config: Config, manifest_path: Option<PathBuf>) {
         exit(1);
     });
 
+    let target_dir = config
+        .target_directory
+        .clone()
+        .unwrap_or(metadata.target_directory.clone());
+
     if let Some(root_package) = metadata.root_package() {
         if !config.workspace {
-            build_solana_package(&config, metadata.target_directory.as_ref(), root_package);
+            build_solana_package(&config, target_dir.as_ref(), root_package);
             return;
         }
     }
@@ -887,7 +891,7 @@ fn build_solana(config: Config, manifest_path: Option<PathBuf>) {
         .collect::<Vec<_>>();
 
     for package in all_sbf_packages {
-        build_solana_package(&config, metadata.target_directory.as_ref(), package);
+        build_solana_package(&config, target_dir.as_ref(), package);
     }
 }
 
@@ -1050,10 +1054,39 @@ fn main() {
     } else {
         platform_tools_version
     };
+
+    let mut cargo_args = matches
+        .values_of("cargo_args")
+        .map(|vals| vals.collect::<Vec<_>>())
+        .unwrap_or_default();
+
+    let target_dir_string;
+    let target_directory = if let Some(target_dir) = cargo_args
+        .iter_mut()
+        .skip_while(|x| x != &&"--target-dir")
+        .nth(1)
+    {
+        let target_path = Utf8PathBuf::from(*target_dir);
+        // Directory needs to exist in order to canonicalize it
+        fs::create_dir_all(&target_path).unwrap_or_else(|err| {
+            error!("Unable to create target-dir directory {target_dir}: {err}");
+            exit(1);
+        });
+        // Canonicalize the path to avoid issues with relative paths
+        let canonicalized = target_path.canonicalize_utf8().unwrap_or_else(|err| {
+            error!("Unable to canonicalize provided target-dir directory {target_path}: {err}");
+            exit(1);
+        });
+        target_dir_string = canonicalized.to_string();
+        *target_dir = &target_dir_string;
+        Some(canonicalized)
+    } else {
+        None
+    };
+
     let config = Config {
-        cargo_args: matches
+        cargo_args,
-            .values_of("cargo_args")
+        target_directory,
-            .map(|vals| vals.collect::<Vec<_>>()),
         sbf_sdk: fs::canonicalize(&sbf_sdk).unwrap_or_else(|err| {
             error!(
                 "Solana SDK path does not exist: {}: {}",

sdk/cargo-build-sbf/tests/crates.rs

@@ -2,6 +2,8 @@ use {
     predicates::prelude::*,
     std::{
         env, fs,
+        path::PathBuf,
+        str::FromStr,
         sync::atomic::{AtomicBool, Ordering},
     },
 };
@@ -25,7 +27,9 @@ fn run_cargo_build(crate_name: &str, extra_args: &[&str], fail: bool) {
     for arg in extra_args {
         args.push(arg);
     }
-    args.push("--");
+    if !extra_args.contains(&"--") {
+        args.push("--");
+    }
     args.push("-vv");
     let mut cmd = assert_cmd::Command::cargo_bin("cargo-build-sbf").unwrap();
     let assert = cmd.env("RUST_LOG", "debug").args(&args).assert();
@@ -88,6 +92,40 @@ fn test_out_dir() {
     clean_target("noop");
 }
 
+#[test]
+#[serial]
+fn test_target_dir() {
+    let target_dir = "./temp-target-dir";
+    run_cargo_build("noop", &["--", "--target-dir", target_dir], false);
+    let cwd = env::current_dir().expect("Unable to get current working directory");
+    let normal_target_dir = cwd.join("tests").join("crates").join("noop").join("target");
+    assert!(!normal_target_dir.exists());
+    let so_file = PathBuf::from_str(target_dir)
+        .unwrap()
+        .join("deploy")
+        .join("noop.so");
+    assert!(so_file.exists());
+    fs::remove_dir_all(target_dir).expect("Failed to remove custom target dir");
+}
+
+#[test]
+#[serial]
+fn test_target_and_out_dir() {
+    let target_dir = "./temp-target-dir";
+    run_cargo_build(
+        "noop",
+        &["--sbf-out-dir", "tmp_out", "--", "--target-dir", target_dir],
+        false,
+    );
+    let cwd = env::current_dir().expect("Unable to get current working directory");
+    let dir = cwd.join("tmp_out");
+    assert!(dir.exists());
+    fs::remove_dir_all("tmp_out").expect("Failed to remove tmp_out dir");
+    let normal_target_dir = cwd.join("tests").join("crates").join("noop").join("target");
+    assert!(!normal_target_dir.exists());
+    fs::remove_dir_all(target_dir).expect("Failed to remove custom target dir");
+}
+
 #[test]
 #[serial]
 fn test_generate_child_script_on_failure() {