From 04a1650a5f68276ee13cccfa82c0419f1b788da2 Mon Sep 17 00:00:00 2001 From: Frederick Mayle Date: Wed, 29 Mar 2023 14:04:01 -0700 Subject: [PATCH 1/2] cargo_embargo: use enum for crate type No behavior change intended. This will make a following CL easier. Test: external/crosvm/android-merge-2-cargo-embargo.sh Bug: 275386231 Change-Id: Id7e4f2eb3f25fa12d867852a0ec95ce443949257 --- tools/cargo_embargo/src/cargo_out.rs | 51 ++++++++++++++++++++++------ tools/cargo_embargo/src/main.rs | 36 +++++++++++--------- 2 files changed, 61 insertions(+), 26 deletions(-) diff --git a/tools/cargo_embargo/src/cargo_out.rs b/tools/cargo_embargo/src/cargo_out.rs index 2aa6ad462..c0648416a 100644 --- a/tools/cargo_embargo/src/cargo_out.rs +++ b/tools/cargo_embargo/src/cargo_out.rs @@ -22,6 +22,21 @@ use std::collections::BTreeMap; use std::path::Path; use std::path::PathBuf; +/// Combined representation of --crate-type and --test flags. +#[derive(Debug, PartialEq, Eq)] +pub enum CrateType { + // --crate-type types + Bin, + Lib, + RLib, + DyLib, + CDyLib, + StaticLib, + ProcMacro, + // --test + Test, +} + /// Info extracted from `CargoOut` for a crate. /// /// Note that there is a 1-to-many relationship between a Cargo.toml file and these `Crate` @@ -32,11 +47,7 @@ pub struct Crate { pub name: String, pub package_name: String, pub version: Option, - // cargo calls rustc with multiple --crate-type flags. - // rustc can accept: - // --crate-type [bin|lib|rlib|dylib|cdylib|staticlib|proc-macro] - pub types: Vec, - pub test: bool, // --test + pub types: Vec, pub target: Option, // --target pub features: Vec, // --cfg feature= pub cfgs: Vec, // non-feature --cfg @@ -219,6 +230,21 @@ impl CargoOut { } } +impl CrateType { + fn from_str(s: &str) -> CrateType { + match s { + "bin" => CrateType::Bin, + "lib" => CrateType::Lib, + "rlib" => CrateType::RLib, + "dylib" => CrateType::DyLib, + "cdylib" => CrateType::CDyLib, + "staticlib" => CrateType::StaticLib, + "proc-macro" => CrateType::ProcMacro, + _ => panic!("unexpected --crate-type: {}", s), + } + } +} + impl Crate { fn from_rustc_invocation(rustc: &str, metadata: &WorkspaceMetadata) -> Result { let mut out = Crate::default(); @@ -239,8 +265,10 @@ impl Crate { while let Some(arg) = arg_iter.next() { match arg { "--crate-name" => out.name = arg_iter.next().unwrap().to_string(), - "--crate-type" => out.types.push(arg_iter.next().unwrap().to_string()), - "--test" => out.test = true, + "--crate-type" => out + .types + .push(CrateType::from_str(arg_iter.next().unwrap().to_string().as_str())), + "--test" => out.types.push(CrateType::Test), "--target" => out.target = Some(arg_iter.next().unwrap().to_string()), "--cfg" => { // example: feature=\"sink\" @@ -355,10 +383,13 @@ impl Crate { if out.main_src.as_os_str().is_empty() { bail!("missing main source file"); } - if out.types.is_empty() != out.test { - bail!("expected exactly one of either --crate-type or --test"); + if out.types.is_empty() { + bail!("must specify one of --test or --crate-type"); } - if out.types.iter().any(|x| x == "lib") && out.types.iter().any(|x| x == "rlib") { + if out.types.contains(&CrateType::Test) && out.types.len() != 1 { + bail!("cannot specify both --test and --crate-type"); + } + if out.types.contains(&CrateType::Lib) && out.types.contains(&CrateType::RLib) { bail!("cannot both have lib and rlib crate types"); } diff --git a/tools/cargo_embargo/src/main.rs b/tools/cargo_embargo/src/main.rs index d0243451f..80645985e 100644 --- a/tools/cargo_embargo/src/main.rs +++ b/tools/cargo_embargo/src/main.rs @@ -417,42 +417,39 @@ fn crate_to_bp_modules( extra_srcs: &[String], ) -> Result> { let mut modules = Vec::new(); - let mut types = crate_.types.clone(); - if crate_.test { - types.push("test".to_string()); - } - for crate_type in types { + for crate_type in &crate_.types { let host = if package_cfg.device_supported.unwrap_or(true) { "" } else { "_host" }; let rlib = if package_cfg.force_rlib { "_rlib" } else { "" }; - let (module_type, module_name, stem) = match crate_type.as_str() { - "bin" => ("rust_binary".to_string() + host, crate_.name.clone(), crate_.name.clone()), - "lib" | "rlib" => { + let (module_type, module_name, stem) = match crate_type { + CrateType::Bin => { + ("rust_binary".to_string() + host, crate_.name.clone(), crate_.name.clone()) + } + CrateType::Lib | CrateType::RLib => { let stem = "lib".to_string() + &crate_.name; ("rust_library".to_string() + rlib + host, stem.clone(), stem) } - "dylib" => { + CrateType::DyLib => { let stem = "lib".to_string() + &crate_.name; ("rust_library".to_string() + host + "_dylib", stem.clone() + "_dylib", stem) } - "cdylib" => { + CrateType::CDyLib => { let stem = "lib".to_string() + &crate_.name; ("rust_ffi".to_string() + host + "_shared", stem.clone() + "_shared", stem) } - "staticlib" => { + CrateType::StaticLib => { let stem = "lib".to_string() + &crate_.name; ("rust_ffi".to_string() + host + "_static", stem.clone() + "_static", stem) } - "proc-macro" => { + CrateType::ProcMacro => { let stem = "lib".to_string() + &crate_.name; ("rust_proc_macro".to_string(), stem.clone(), stem) } - "test" => { + CrateType::Test => { let suffix = crate_.main_src.to_string_lossy().into_owned(); let suffix = suffix.replace('/', "_").replace(".rs", ""); let stem = crate_.package_name.clone() + "_test_" + &suffix; ("rust_test".to_string() + host, stem.clone(), stem) } - _ => panic!("unexpected crate type: {}", crate_type), }; let mut m = BpModule::new(module_type.clone()); @@ -483,7 +480,7 @@ fn crate_to_bp_modules( m.props.set("cargo_pkg_version", version.clone()); } - if crate_.test { + if crate_.types.contains(&CrateType::Test) { m.props.set("test_suites", vec!["general-tests"]); m.props.set("auto_gen_config", true); if package_cfg.host_supported.unwrap_or(true) { @@ -567,7 +564,14 @@ fn crate_to_bp_modules( } if !cfg.apex_available.is_empty() - && ["lib", "rlib", "dylib", "staticlib", "cdylib"].contains(&crate_type.as_str()) + && [ + CrateType::Lib, + CrateType::RLib, + CrateType::DyLib, + CrateType::CDyLib, + CrateType::StaticLib, + ] + .contains(crate_type) { m.props.set("apex_available", cfg.apex_available.clone()); } From 5a6dbaae7a9866ea34e21d8ea57e8fcc6029fc0b Mon Sep 17 00:00:00 2001 From: Frederick Mayle Date: Wed, 29 Mar 2023 14:18:44 -0700 Subject: [PATCH 2/2] cargo_embargo: detect and ignore harness-less tests These currently cause cargo_embargo to fail. We should support them eventually, but for now we'll ignore them to unblock crosvm merges. Test: external/crosvm/android-merge-2-cargo-embargo.sh Bug: 275386231 Change-Id: I3e156afa5672ddd6257c7caabd05346e060448fb --- tools/cargo_embargo/src/cargo_out.rs | 9 ++++++++- tools/cargo_embargo/src/main.rs | 18 ++++++++++++++++-- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/tools/cargo_embargo/src/cargo_out.rs b/tools/cargo_embargo/src/cargo_out.rs index c0648416a..fcf467f88 100644 --- a/tools/cargo_embargo/src/cargo_out.rs +++ b/tools/cargo_embargo/src/cargo_out.rs @@ -35,6 +35,8 @@ pub enum CrateType { ProcMacro, // --test Test, + // "--cfg test" without --test. (Assume it is a test with the harness disabled. + TestNoHarness, } /// Info extracted from `CargoOut` for a crate. @@ -383,8 +385,13 @@ impl Crate { if out.main_src.as_os_str().is_empty() { bail!("missing main source file"); } + // Must have at least one type. if out.types.is_empty() { - bail!("must specify one of --test or --crate-type"); + if out.cfgs.contains(&"test".to_string()) { + out.types.push(CrateType::TestNoHarness); + } else { + bail!("failed to detect crate type. did not have --crate-type or --test or '--cfg test'"); + } } if out.types.contains(&CrateType::Test) && out.types.len() != 1 { bail!("cannot specify both --test and --crate-type"); diff --git a/tools/cargo_embargo/src/main.rs b/tools/cargo_embargo/src/main.rs index 80645985e..dd2caf594 100644 --- a/tools/cargo_embargo/src/main.rs +++ b/tools/cargo_embargo/src/main.rs @@ -360,7 +360,14 @@ fn write_android_bp( }; for c in crates { - modules.extend(crate_to_bp_modules(c, cfg, package_cfg, &extra_srcs)?); + modules.extend(crate_to_bp_modules(c, cfg, package_cfg, &extra_srcs).with_context( + || { + format!( + "failed to generate bp module for crate \"{}\" with package name \"{}\"", + c.name, c.package_name + ) + }, + )?); } if modules.is_empty() { return Ok(()); @@ -444,10 +451,17 @@ fn crate_to_bp_modules( let stem = "lib".to_string() + &crate_.name; ("rust_proc_macro".to_string(), stem.clone(), stem) } - CrateType::Test => { + CrateType::Test | CrateType::TestNoHarness => { let suffix = crate_.main_src.to_string_lossy().into_owned(); let suffix = suffix.replace('/', "_").replace(".rs", ""); let stem = crate_.package_name.clone() + "_test_" + &suffix; + if crate_type == &CrateType::TestNoHarness { + eprintln!( + "WARNING: ignoring test \"{}\" with harness=false. not supported yet", + stem + ); + return Ok(Vec::new()); + } ("rust_test".to_string() + host, stem.clone(), stem) } };