Merge "Add test for generating Android.bp from list of crates." into main am: 4266276d42

Original change: https://android-review.googlesource.com/c/platform/development/+/2695088

Change-Id: Idf315debd29afe266b4be40ca7a70a68f3995bca
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

tools/cargo_embargo/Android.bp

@@ -40,4 +40,5 @@ rust_binary_host {
 rust_test_host {
     name: "cargo_embargo.test",
     defaults: ["cargo_embargo.defaults"],
+    data: ["testdata/**/*"],
 }

tools/cargo_embargo/src/cargo_out.rs

@@ -18,12 +18,14 @@ use anyhow::Context;
 use anyhow::Result;
 use once_cell::sync::Lazy;
 use regex::Regex;
+use serde::{Deserialize, Serialize};
 use std::collections::BTreeMap;
 use std::path::Path;
 use std::path::PathBuf;
 
 /// Combined representation of --crate-type and --test flags.
-#[derive(Copy, Clone, Debug, PartialEq, Eq)]
+#[derive(Copy, Clone, Debug, Deserialize, PartialEq, Eq, Serialize)]
+#[serde[rename_all = "lowercase"]]
 pub enum CrateType {
     // --crate-type types
     Bin,
@@ -51,7 +53,7 @@ impl CrateType {
 /// Note that there is a 1-to-many relationship between a Cargo.toml file and these `Crate`
 /// objects. For example, a Cargo.toml file might have a bin, a lib, and various tests. Each of
 /// those will be a separate `Crate`. All of them will have the same `package_name`.
-#[derive(Debug, Default)]
+#[derive(Debug, Default, Deserialize, Eq, PartialEq, Serialize)]
 pub struct Crate {
     pub name: String,
     pub package_name: String,
@@ -103,12 +105,12 @@ pub fn parse_cargo_out(cargo_out_path: &str, cargo_metadata_path: &str) -> Resul
 }
 
 /// `cargo metadata` output.
-#[derive(serde::Deserialize)]
+#[derive(Debug, Deserialize)]
 struct WorkspaceMetadata {
     packages: Vec<PackageMetadata>,
 }
 
-#[derive(serde::Deserialize)]
+#[derive(Debug, Deserialize)]
 struct PackageMetadata {
     name: String,
     version: String,

tools/cargo_embargo/src/config.rs

@@ -14,7 +14,7 @@
 
 //! Code for reading configuration json files.
 //!
-//! These are usually called `cargo_embargo.json` or `cargo2android.json`.
+//! These are usually called `cargo_embargo.json`.
 
 use serde::Deserialize;
 use std::collections::BTreeMap;

tools/cargo_embargo/src/main.rs

@@ -36,7 +36,7 @@ use anyhow::bail;
 use anyhow::Context;
 use anyhow::Result;
 use bp::*;
-use cargo_out::*;
+use cargo_out::{parse_cargo_out, Crate, CrateType};
 use clap::Parser;
 use once_cell::sync::Lazy;
 use regex::Regex;
@@ -126,15 +126,29 @@ fn main() -> Result<()> {
         }
     }
 
-    // Group by package.
+    write_all_bp(&cfg, crates, &package_out_files)
+}
+
+fn group_by_package(crates: Vec<Crate>) -> BTreeMap<PathBuf, Vec<Crate>> {
     let mut module_by_package: BTreeMap<PathBuf, Vec<Crate>> = BTreeMap::new();
     for c in crates {
         module_by_package.entry(c.package_dir.clone()).or_default().push(c);
     }
+    module_by_package
+}
+
+fn write_all_bp(
+    cfg: &Config,
+    crates: Vec<Crate>,
+    package_out_files: &BTreeMap<String, Vec<PathBuf>>,
+) -> Result<()> {
+    // Group by package.
+    let module_by_package = group_by_package(crates);
+
     // Write an Android.bp file per package.
     for (package_dir, crates) in module_by_package {
         write_android_bp(
-            &cfg,
+            cfg,
             package_dir,
             &crates,
             package_out_files.get(&crates[0].package_name),
@@ -250,6 +264,38 @@ fn write_android_bp(
         Err(e) => bail!("error when reading {bp_path:?}: {e}"),
     };
 
+    // If `copy_out` is enabled and there are any generated out files for the package, copy them to
+    // the appropriate directory.
+    if let (true, Some(out_files)) = (package_cfg.copy_out, out_files) {
+        let out_dir = package_dir.join("out");
+        if !out_dir.exists() {
+            std::fs::create_dir(&out_dir).expect("failed to create out dir");
+        }
+
+        for f in out_files.iter() {
+            let dest = out_dir.join(f.file_name().unwrap());
+            std::fs::copy(f, dest).expect("failed to copy out file");
+        }
+    }
+
+    if let Some(bp_contents) =
+        generate_android_bp(&license_section, cfg, package_cfg, &package_name, crates, out_files)?
+    {
+        write_format_android_bp(&bp_path, &bp_contents, package_cfg.patch.as_deref())?;
+    }
+
+    Ok(())
+}
+
+/// Generates and returns a Soong Blueprint for the given set of crates.
+fn generate_android_bp(
+    license_section: &str,
+    cfg: &Config,
+    package_cfg: &PackageConfig,
+    package_name: &str,
+    crates: &[Crate],
+    out_files: Option<&Vec<PathBuf>>,
+) -> Result<Option<String>> {
     let mut bp_contents = String::new();
     bp_contents += "// This file is generated by cargo_embargo.\n";
     bp_contents += "// Do not modify this file as changes will be overridden on upgrade.\n\n";
@@ -258,31 +304,23 @@ fn write_android_bp(
 
     let mut modules = Vec::new();
 
-    let extra_srcs = match (package_cfg.copy_out, out_files) {
-        (true, Some(out_files)) => {
-            let out_dir = package_dir.join("out");
-            if !out_dir.exists() {
-                std::fs::create_dir(&out_dir).expect("failed to create out dir");
-            }
+    let extra_srcs = if let (true, Some(out_files)) = (package_cfg.copy_out, out_files) {
+        let outs: Vec<String> = out_files
+            .iter()
+            .map(|f| f.file_name().unwrap().to_str().unwrap().to_string())
+            .collect();
 
-            let mut outs: Vec<String> = Vec::new();
-            for f in out_files.iter() {
-                let dest = out_dir.join(f.file_name().unwrap());
-                std::fs::copy(f, dest).expect("failed to copy out file");
-                outs.push(f.file_name().unwrap().to_str().unwrap().to_string());
-            }
+        let mut m = BpModule::new("genrule".to_string());
+        let module_name = format!("copy_{}_build_out", package_name);
+        m.props.set("name", module_name.clone());
+        m.props.set("srcs", vec!["out/*"]);
+        m.props.set("cmd", "cp $(in) $(genDir)");
+        m.props.set("out", outs);
+        modules.push(m);
 
-            let mut m = BpModule::new("genrule".to_string());
-            let module_name = format!("copy_{}_build_out", package_name);
-            m.props.set("name", module_name.clone());
-            m.props.set("srcs", vec!["out/*"]);
-            m.props.set("cmd", "cp $(in) $(genDir)");
-            m.props.set("out", outs);
-            modules.push(m);
-
-            vec![":".to_string() + &module_name]
-        }
-        _ => vec![],
+        vec![":".to_string() + &module_name]
+    } else {
+        vec![]
     };
 
     for c in crates {
@@ -296,7 +334,7 @@ fn write_android_bp(
         )?);
     }
     if modules.is_empty() {
-        return Ok(());
+        return Ok(None);
     }
 
     // In some cases there are nearly identical rustc invocations that that get processed into
@@ -315,9 +353,19 @@ fn write_android_bp(
             &std::fs::read_to_string(path).with_context(|| format!("failed to read {path:?}"))?;
         bp_contents += "\n";
     }
-    File::create(&bp_path)?.write_all(bp_contents.as_bytes())?;
+    Ok(Some(bp_contents))
+}
 
-    let bpfmt_output = Command::new("bpfmt").arg("-w").arg(&bp_path).output()?;
+/// Writes the given contents to the given `Android.bp` file, formats it with `bpfmt`, and applies
+/// the patch if there is one.
+fn write_format_android_bp(
+    bp_path: &Path,
+    bp_contents: &str,
+    patch_path: Option<&Path>,
+) -> Result<()> {
+    File::create(bp_path)?.write_all(bp_contents.as_bytes())?;
+
+    let bpfmt_output = Command::new("bpfmt").arg("-w").arg(bp_path).output()?;
     if !bpfmt_output.status.success() {
         eprintln!(
             "WARNING: bpfmt -w {:?} failed before patch: {}",
@@ -326,14 +374,13 @@ fn write_android_bp(
         );
     }
 
-    if let Some(patch_path) = &package_cfg.patch {
-        let patch_output =
-            Command::new("patch").arg("-s").arg(&bp_path).arg(patch_path).output()?;
+    if let Some(patch_path) = patch_path {
+        let patch_output = Command::new("patch").arg("-s").arg(bp_path).arg(patch_path).output()?;
         if !patch_output.status.success() {
             eprintln!("WARNING: failed to apply patch {:?}", patch_path);
         }
         // Re-run bpfmt after the patch so
-        let bpfmt_output = Command::new("bpfmt").arg("-w").arg(&bp_path).output()?;
+        let bpfmt_output = Command::new("bpfmt").arg("-w").arg(bp_path).output()?;
         if !bpfmt_output.status.success() {
             eprintln!(
                 "WARNING: bpfmt -w {:?} failed after patch: {}",
@@ -539,6 +586,54 @@ fn crate_to_bp_modules(
 #[cfg(test)]
 mod tests {
     use super::*;
+    use std::env::{current_dir, set_current_dir};
+    use std::fs::{self, read_to_string};
+    use std::path::PathBuf;
+
+    const TESTDATA_PATH: &str = "testdata";
+
+    #[test]
+    fn generate_bp() {
+        for testdata_directory_path in testdata_directories() {
+            let cfg: Config = serde_json::from_reader(
+                File::open(testdata_directory_path.join("cargo_embargo.json"))
+                    .expect("Failed to open cargo_embargo.json"),
+            )
+            .unwrap();
+            let crates: Vec<Crate> = serde_json::from_reader(
+                File::open(testdata_directory_path.join("crates.json"))
+                    .expect("Failed to open crates.json"),
+            )
+            .unwrap();
+            let expected_output =
+                read_to_string(testdata_directory_path.join("expected_Android.bp")).unwrap();
+
+            let module_by_package = group_by_package(crates);
+            assert_eq!(module_by_package.len(), 1);
+            let crates = module_by_package.into_values().next().unwrap();
+            let package_name = &crates[0].package_name;
+            let def = PackageConfig::default();
+            let package_cfg = cfg.package.get(package_name).unwrap_or(&def);
+
+            let old_current_dir = current_dir().unwrap();
+            set_current_dir(&testdata_directory_path).unwrap();
+
+            let output = generate_android_bp(
+                "// License section.\n",
+                &cfg,
+                package_cfg,
+                package_name,
+                &crates,
+                None,
+            )
+            .unwrap()
+            .unwrap();
+
+            assert_eq!(output, expected_output);
+
+            set_current_dir(old_current_dir).unwrap();
+        }
+    }
 
     #[test]
     fn crate_to_bp_empty() {
@@ -589,4 +684,25 @@ mod tests {
             }]
         );
     }
+
+    /// Returns a list of directories containing test data.
+    ///
+    /// Each directory under `testdata/` contains a single test case.
+    pub fn testdata_directories() -> Vec<PathBuf> {
+        fs::read_dir(TESTDATA_PATH)
+            .expect("Failed to read testdata directory")
+            .filter_map(|entry| {
+                let entry = entry.expect("Error reading testdata directory entry");
+                if entry
+                    .file_type()
+                    .expect("Error getting metadata for testdata subdirectory")
+                    .is_dir()
+                {
+                    Some(entry.path())
+                } else {
+                    None
+                }
+            })
+            .collect()
+    }
 }

tools/cargo_embargo/testdata/either/cargo_embargo.json

@@ -0,0 +1,8 @@
+{
+  "tests": true,
+  "package": {
+    "either": {
+      "device_supported": true
+    }
+  }
+}

tools/cargo_embargo/testdata/either/crates.json

@@ -0,0 +1,36 @@
+[
+  {
+    "name": "either",
+    "package_name": "either",
+    "version": "1.8.1",
+    "types": ["lib"],
+    "target": "x86_64-unknown-linux-gnu",
+    "features": ["default", "use_std"],
+    "cfgs": [],
+    "externs": [],
+    "codegens": [],
+    "cap_lints": "",
+    "static_libs": [],
+    "shared_libs": [],
+    "edition": "2018",
+    "package_dir": ".",
+    "main_src": "src/lib.rs"
+  },
+  {
+    "name": "either",
+    "package_name": "either",
+    "version": "1.8.1",
+    "types": ["test"],
+    "target": "x86_64-unknown-linux-gnu",
+    "features": ["default", "use_std"],
+    "cfgs": [],
+    "externs": [["serde_json", "libserde_json-a330ac0e36ca324c.rlib"]],
+    "codegens": [],
+    "cap_lints": "",
+    "static_libs": [],
+    "shared_libs": [],
+    "edition": "2018",
+    "package_dir": ".",
+    "main_src": "src/lib.rs"
+  }
+]

tools/cargo_embargo/testdata/either/expected_Android.bp

@@ -0,0 +1,35 @@
+// This file is generated by cargo_embargo.
+// Do not modify this file as changes will be overridden on upgrade.
+
+// License section.
+rust_test {
+name: "either_test_src_lib",
+host_supported: true,
+crate_name: "either",
+cargo_env_compat: true,
+cargo_pkg_version: "1.8.1",
+srcs: ["src/lib.rs"],
+test_suites: ["general-tests"],
+auto_gen_config: true,
+test_options: {
+unit_test: true,
+},
+edition: "2018",
+features: ["default", "use_std"],
+rustlibs: ["libserde_json"],
+}
+
+rust_library {
+name: "libeither",
+host_supported: true,
+crate_name: "either",
+cargo_env_compat: true,
+cargo_pkg_version: "1.8.1",
+srcs: ["src/lib.rs"],
+edition: "2018",
+features: ["default", "use_std"],
+apex_available: ["//apex_available:platform", "//apex_available:anyapex"],
+product_available: true,
+vendor_available: true,
+}
+

tools/cargo_embargo/testdata/plotters/cargo_embargo.json

@@ -0,0 +1,9 @@
+{
+  "features": ["svg_backend", "area_series", "line_series"],
+  "tests": false,
+  "package": {
+    "plotters": {
+      "device_supported": true
+    }
+  }
+}

tools/cargo_embargo/testdata/plotters/crates.json

@@ -0,0 +1,23 @@
+[
+  {
+    "name": "plotters",
+    "package_name": "plotters",
+    "version": "0.3.4",
+    "types": ["lib"],
+    "target": "x86_64-unknown-linux-gnu",
+    "features": ["area_series", "line_series", "plotters-svg", "svg_backend"],
+    "cfgs": [],
+    "externs": [
+      ["num_traits", "libnum_traits-93a69c224ed38166.rmeta"],
+      ["plotters_backend", "libplotters_backend-4110c43be89cf520.rmeta"],
+      ["plotters_svg", "libplotters_svg-e0f639cce7c4701c.rmeta"]
+    ],
+    "codegens": [],
+    "cap_lints": "",
+    "static_libs": [],
+    "shared_libs": [],
+    "edition": "2018",
+    "package_dir": ".",
+    "main_src": "src/lib.rs"
+  }
+]

tools/cargo_embargo/testdata/plotters/expected_Android.bp

@@ -0,0 +1,19 @@
+// This file is generated by cargo_embargo.
+// Do not modify this file as changes will be overridden on upgrade.
+
+// License section.
+rust_library {
+name: "libplotters",
+host_supported: true,
+crate_name: "plotters",
+cargo_env_compat: true,
+cargo_pkg_version: "0.3.4",
+srcs: ["src/lib.rs"],
+edition: "2018",
+features: ["area_series", "line_series", "plotters-svg", "svg_backend"],
+rustlibs: ["libnum_traits", "libplotters_backend", "libplotters_svg"],
+apex_available: ["//apex_available:platform", "//apex_available:anyapex"],
+product_available: true,
+vendor_available: true,
+}
+