Fix extraction of package dir from package ID

After rust-lang/cargo#12914, the format of the
package ID changed to be

    path+file:///path/crate_name#version

See https://doc.rust-lang.org/cargo/reference/pkgid-spec.html for
details on the format.

The test files were regenerated with `cargo metadata`. This procedure
is now documented in a `README.md` file for future generations.

Bug: 333808266
Test: atest --host cargo_embargo.test
Test: Ran `cargo_embargo generate cargo_embargo.json` on chrono
Change-Id: I055d19baf6e78f30b708296434082121762573af

tools/cargo_embargo/Android.bp

@@ -43,6 +43,7 @@ rust_binary_host {
 rust_test_host {
     name: "cargo_embargo.test",
     defaults: ["cargo_embargo.defaults"],
+    rustlibs: ["libgoogletest_rust"],
     test_config: "AndroidTest.xml",
     data: ["testdata/**/*"],
 }

tools/cargo_embargo/src/cargo/metadata.rs

@@ -16,7 +16,7 @@
 
 use super::{Crate, CrateType, Extern, ExternType};
 use crate::config::VariantConfig;
-use anyhow::{anyhow, bail, Context, Result};
+use anyhow::{bail, Context, Result};
 use serde::Deserialize;
 use std::collections::BTreeMap;
 use std::ops::Deref;
@@ -268,14 +268,22 @@ fn make_extern(packages: &[PackageMetadata], dependency: &DependencyMetadata) ->
     Ok(Extern { name, lib_name, extern_type })
 }
 
-/// Given a package ID like
-/// `"either 1.8.1 (path+file:///usr/local/google/home/qwandor/aosp/external/rust/crates/either)"`,
-/// returns the path to the package, e.g.
-/// `"/usr/local/google/home/qwandor/aosp/external/rust/crates/either"`.
+/// Given a Cargo package ID, returns the path.
+///
+/// Extracts `"/path/to/crate"` from
+/// `"path+file:///path/to/crate#1.2.3"`. See
+/// https://doc.rust-lang.org/cargo/reference/pkgid-spec.html for
+/// information on Cargo package ID specifications.
 fn package_dir_from_id(id: &str) -> Result<PathBuf> {
-    const URI_MARKER: &str = "(path+file://";
-    let uri_start = id.find(URI_MARKER).ok_or_else(|| anyhow!("Invalid package ID {}", id))?;
-    Ok(PathBuf::from(id[uri_start + URI_MARKER.len()..id.len() - 1].to_string()))
+    const PREFIX: &str = "path+file://";
+    const SEPARATOR: char = '#';
+    let Some(stripped) = id.strip_prefix(PREFIX) else {
+        bail!("Invalid package ID {id:?}, expected it to start with {PREFIX:?}");
+    };
+    let Some(idx) = stripped.rfind(SEPARATOR) else {
+        bail!("Invalid package ID {id:?}, expected it to contain {SEPARATOR:?}");
+    };
+    Ok(PathBuf::from(stripped[..idx].to_string()))
 }
 
 fn split_src_path<'a>(src_path: &'a Path, package_dir: &Path) -> &'a Path {
@@ -344,8 +352,19 @@ mod tests {
     use super::*;
     use crate::config::Config;
     use crate::tests::testdata_directories;
+    use googletest::matchers::eq;
+    use googletest::prelude::assert_that;
     use std::fs::{read_to_string, File};
 
+    #[test]
+    fn extract_package_dir_from_id() -> Result<()> {
+        assert_eq!(
+            package_dir_from_id("path+file:///path/to/crate#1.2.3")?,
+            PathBuf::from("/path/to/crate")
+        );
+        Ok(())
+    }
+
     #[test]
     fn resolve_multi_level_feature_dependencies() {
         let chosen = vec!["default".to_string(), "extra".to_string(), "on_by_default".to_string()];
@@ -419,6 +438,20 @@ mod tests {
 
     #[test]
     fn parse_metadata() {
+        /// Remove anything before "external/rust/crates/" from the
+        /// `package_dir` field. This makes the test robust since you
+        /// can use `cargo metadata` to regenerate the test files and
+        /// you don't have to care about where your AOSP checkout
+        /// lives.
+        fn normalize_package_dir(mut c: Crate) -> Crate {
+            const EXTERNAL_RUST_CRATES: &str = "external/rust/crates/";
+            let package_dir = c.package_dir.to_str().unwrap();
+            if let Some(idx) = package_dir.find(EXTERNAL_RUST_CRATES) {
+                c.package_dir = PathBuf::from(format!(".../{}", &package_dir[idx..]));
+            }
+            c
+        }
+
         for testdata_directory_path in testdata_directories() {
             let cfg = Config::from_json_str(
                 &read_to_string(testdata_directory_path.join("cargo_embargo.json"))
@@ -432,10 +465,13 @@ mod tests {
             )
             .unwrap();
             let cargo_metadata_path = testdata_directory_path.join("cargo.metadata");
-            let expected_crates: Vec<Vec<Crate>> = serde_json::from_reader(
+            let expected_crates: Vec<Vec<Crate>> = serde_json::from_reader::<_, Vec<Vec<Crate>>>(
                 File::open(testdata_directory_path.join("crates.json")).unwrap(),
             )
-            .unwrap();
+            .unwrap()
+            .into_iter()
+            .map(|crates: Vec<Crate>| crates.into_iter().map(normalize_package_dir).collect())
+            .collect();
 
             let crates = cfg
                 .variants
@@ -448,9 +484,12 @@ mod tests {
                         variant_cfg,
                     )
                     .unwrap()
+                    .into_iter()
+                    .map(normalize_package_dir)
+                    .collect::<Vec<Crate>>()
                 })
-                .collect::<Vec<_>>();
-            assert_eq!(crates, expected_crates);
+                .collect::<Vec<Vec<Crate>>>();
+            assert_that!(format!("{crates:#?}"), eq(format!("{expected_crates:#?}")));
         }
     }
 }

tools/cargo_embargo/src/main.rs

@@ -911,6 +911,8 @@ fn crate_to_bp_modules(
 #[cfg(test)]
 mod tests {
     use super::*;
+    use googletest::matchers::eq;
+    use googletest::prelude::assert_that;
     use std::env::{current_dir, set_current_dir};
     use std::fs::{self, read_to_string};
     use std::path::PathBuf;
@@ -992,7 +994,7 @@ mod tests {
                 .unwrap();
             }
 
-            assert_eq!(output, expected_output);
+            assert_that!(output, eq(expected_output));
 
             set_current_dir(old_current_dir).unwrap();
         }

tools/cargo_embargo/testdata/README.md

@@ -0,0 +1,32 @@
+# Test data for `cargo_embargo`
+
+The files here are used for `cargo_embargo` integration tests. Run the tests with
+
+```shell
+atest --host cargo_embargo.test
+```
+
+## Handling changes in Cargo output
+
+When the output of `cargo metadata` changes, you need to update the
+`cargo.metadata` files found in the subdirectories here. Do this with:
+
+```
+for crate in aho-corasick async-trait either plotters rustc-demangle-capi; do
+    pushd $ANDROID_BUILD_TOP/external/rust/crates/$crate
+    cargo metadata --format-version 1 | jq --sort-keys \
+      > $ANDROID_BUILD_TOP/development/tools/cargo_embargo/testdata/$crate/cargo.metadata
+    popd
+done
+```
+
+Run the integration tests again after updating the crate metadata.
+
+Some tests will likely fail because of outdated information in the other test
+files:
+
+- `expected_Android.bp`: Adjust the version numbers to match the current version
+  from `cargo.metadata`.
+- `crates.json`: Adjust version numbers and `package_dir` as necessary.
+- `cargo_embargo.json`: Adjust the list of Cargo features if this has changed
+  since the file was last touched.