Fix rust flags handling in cargo-build-sbf

jeffwashington · Aug 11, 2022 · d42fcf2 · d42fcf2
1 parent 1dcf8cf
commit d42fcf2
Show file tree

Hide file tree

Showing 4 changed files with 96 additions and 32 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/sdk/cargo-build-sbf/Cargo.toml b/sdk/cargo-build-sbf/Cargo.toml
@@ -22,6 +22,7 @@ tar = "0.4.38"
 
 [dev-dependencies]
 assert_cmd = "*"
+predicates = "2.0"
 serial_test = "*"
 
 [features]

diff --git a/sdk/cargo-build-sbf/src/main.rs b/sdk/cargo-build-sbf/src/main.rs
@@ -564,25 +564,6 @@ fn build_sbf_package(config: &Config, target_directory: &Path, package: &cargo_m
     env::set_var("OBJDUMP", llvm_bin.join("llvm-objdump"));
     env::set_var("OBJCOPY", llvm_bin.join("llvm-objcopy"));
 
-    let rustflags = env::var("RUSTFLAGS").ok();
-    let mut rustflags = Cow::Borrowed(rustflags.as_deref().unwrap_or_default());
-    if config.remap_cwd {
-        rustflags = Cow::Owned(format!("{} -Zremap-cwd-prefix=", &rustflags));
-    }
-    if config.debug {
-        // Replace with -Zsplit-debuginfo=packed when stabilized.
-        rustflags = Cow::Owned(format!("{} -g", &rustflags));
-    }
-    if let Cow::Owned(flags) = rustflags {
-        env::set_var("RUSTFLAGS", &flags);
-    }
-    if config.verbose {
-        debug!(
-            "RUSTFLAGS=\"{}\"",
-            env::var("RUSTFLAGS").ok().unwrap_or_default()
-        );
-    }
-
     // RUSTC variable overrides cargo +<toolchain> mechanism of
     // selecting the rust compiler and makes cargo run a rust compiler
     // other than the one linked in BPF toolchain. We have to prevent
@@ -593,15 +574,40 @@ fn build_sbf_package(config: &Config, target_directory: &Path, package: &cargo_m
         );
         env::remove_var("RUSTC")
     }
-
-    let mut target_rustflags = env::var("CARGO_TARGET_SBF_SOLANA_SOLANA_RUSTFLAGS")
-        .ok()
-        .unwrap_or_default();
+    let cargo_target = if config.arch == "bpf" {
+        "CARGO_TARGET_BPFEL_UNKNOWN_UNKNOWN_RUSTFLAGS"
+    } else {
+        "CARGO_TARGET_SBF_SOLANA_SOLANA_RUSTFLAGS"
+    };
+    let rustflags = env::var("RUSTFLAGS").ok().unwrap_or_default();
+    if env::var("RUSTFLAGS").is_ok() {
+        warn!(
+            "Removed RUSTFLAGS from cargo environment, because it overrides {}.",
+            cargo_target,
+        );
+        env::remove_var("RUSTFLAGS")
+    }
+    let target_rustflags = env::var(cargo_target).ok();
+    let mut target_rustflags = Cow::Borrowed(target_rustflags.as_deref().unwrap_or_default());
+    target_rustflags = Cow::Owned(format!("{} {}", &rustflags, &target_rustflags));
+    if config.remap_cwd {
+        target_rustflags = Cow::Owned(format!("{} -Zremap-cwd-prefix=", &target_rustflags));
+    }
+    if config.debug {
+        // Replace with -Zsplit-debuginfo=packed when stabilized.
+        target_rustflags = Cow::Owned(format!("{} -g", &target_rustflags));
+    }
     if config.arch == "sbfv2" {
-        target_rustflags = format!("{} {}", "-C target_cpu=sbfv2", target_rustflags);
-        env::set_var(
-            "CARGO_TARGET_SBF_SOLANA_SOLANA_RUSTFLAGS",
-            &target_rustflags,
+        target_rustflags = Cow::Owned(format!("{} -C target_cpu=sbfv2", &target_rustflags));
+    }
+    if let Cow::Owned(flags) = target_rustflags {
+        env::set_var(cargo_target, &flags);
+    }
+    if config.verbose {
+        debug!(
+            "{}=\"{}\"",
+            cargo_target,
+            env::var(cargo_target).ok().unwrap_or_default(),
         );
     }
 

diff --git a/sdk/cargo-build-sbf/tests/crates.rs b/sdk/cargo-build-sbf/tests/crates.rs
@@ -1,4 +1,7 @@
-use std::{env, fs, process};
+use {
+    predicates::prelude::*,
+    std::{env, fs},
+};
 
 #[macro_use]
 extern crate serial_test;
@@ -47,11 +50,10 @@ fn test_build() {
 #[serial]
 fn test_dump() {
     // This test requires rustfilt.
-    assert!(process::Command::new("cargo")
+    assert_cmd::Command::new("cargo")
         .args(&["install", "-f", "rustfilt"])
-        .status()
-        .expect("Unable to install rustfilt required for --dump option")
-        .success());
+        .assert()
+        .success();
     run_cargo_build("noop", &["--dump"], false);
     let cwd = env::current_dir().expect("Unable to get current working directory");
     let dump = cwd
@@ -90,3 +92,39 @@ fn test_generate_child_script_on_failre() {
     fs::remove_file(scr).expect("Failed to remove script");
     clean_target("fail");
 }
+
+#[test]
+#[serial]
+fn test_sbfv2() {
+    run_cargo_build("noop", &["--arch", "sbfv2"], false);
+    let cwd = env::current_dir().expect("Unable to get current working directory");
+    let bin = cwd
+        .join("tests")
+        .join("crates")
+        .join("noop")
+        .join("target")
+        .join("deploy")
+        .join("noop.so");
+    let bin = bin.to_str().unwrap();
+    let root = cwd
+        .parent()
+        .expect("Unable to get parent directory of current working dir")
+        .parent()
+        .expect("Unable to get ../.. of current working dir");
+    let readelf = root
+        .join("sdk")
+        .join("bpf")
+        .join("dependencies")
+        .join("sbf-tools")
+        .join("llvm")
+        .join("bin")
+        .join("llvm-readelf");
+    assert_cmd::Command::new(readelf)
+        .args(&["-h", bin])
+        .assert()
+        .stdout(predicate::str::contains(
+            "Flags:                             0x20",
+        ))
+        .success();
+    clean_target("noop");
+}