From d42fcf2e309a421b51f1891a41bbec1b561129b0 Mon Sep 17 00:00:00 2001 From: Dmitri Makarov Date: Wed, 10 Aug 2022 13:17:38 -0700 Subject: [PATCH] Fix rust flags handling in cargo-build-sbf --- Cargo.lock | 19 +++++++++ sdk/cargo-build-sbf/Cargo.toml | 1 + sdk/cargo-build-sbf/src/main.rs | 60 ++++++++++++++++------------- sdk/cargo-build-sbf/tests/crates.rs | 48 ++++++++++++++++++++--- 4 files changed, 96 insertions(+), 32 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 772039418..c803ec632 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1536,6 +1536,15 @@ dependencies = [ "miniz_oxide", ] +[[package]] +name = "float-cmp" +version = "0.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "98de4bbd547a563b716d8dfa9aad1cb19bfab00f4fa09a6a4ed21dbcf44ce9c4" +dependencies = [ + "num-traits", +] + [[package]] name = "fnv" version = "1.0.7" @@ -2743,6 +2752,12 @@ dependencies = [ "version_check", ] +[[package]] +name = "normalize-line-endings" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "61807f77802ff30975e01f4f071c8ba10c022052f98b3294119f3e615d13e5be" + [[package]] name = "ntapi" version = "0.3.6" @@ -3299,8 +3314,11 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5c6ce811d0b2e103743eec01db1c50612221f173084ce2f7941053e94b6bb474" dependencies = [ "difflib", + "float-cmp", "itertools", + "normalize-line-endings", "predicates-core", + "regex", ] [[package]] @@ -4821,6 +4839,7 @@ dependencies = [ "cargo_metadata", "clap 3.1.8", "log", + "predicates", "regex", "serial_test", "solana-download-utils", diff --git a/sdk/cargo-build-sbf/Cargo.toml b/sdk/cargo-build-sbf/Cargo.toml index 2c8359c4d..4070029a9 100644 --- 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 index d6437b043..0a49b9376 100644 --- 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 + 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 index 9b775a19a..227e0d86a 100644 --- 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"); +}