diff --git a/tools/cargo_embargo/Android.bp b/tools/cargo_embargo/Android.bp index e88297a6b..dd53b8b81 100644 --- a/tools/cargo_embargo/Android.bp +++ b/tools/cargo_embargo/Android.bp @@ -27,6 +27,7 @@ rust_defaults { "libenv_logger", "libglob", "liblog_rust", + "libnix", "libonce_cell", "libregex", "libserde", diff --git a/tools/cargo_embargo/src/cargo/cargo_out.rs b/tools/cargo_embargo/src/cargo/cargo_out.rs index 1064d0b2b..a30623899 100644 --- a/tools/cargo_embargo/src/cargo/cargo_out.rs +++ b/tools/cargo_embargo/src/cargo/cargo_out.rs @@ -14,6 +14,7 @@ use super::metadata::WorkspaceMetadata; use super::{Crate, CrateType, Extern, ExternType}; +use crate::CargoOutput; use anyhow::anyhow; use anyhow::bail; use anyhow::Context; @@ -23,7 +24,6 @@ use once_cell::sync::Lazy; use regex::Regex; use std::collections::BTreeMap; use std::env; -use std::fs::{read_to_string, File}; use std::path::Path; use std::path::PathBuf; @@ -31,16 +31,14 @@ use std::path::PathBuf; /// the rustc invocations. /// /// Ignores crates outside the current directory and build script crates. -pub fn parse_cargo_out( - cargo_out_path: impl AsRef, - cargo_metadata_path: impl AsRef, -) -> Result> { - let cargo_out = read_to_string(cargo_out_path).context("failed to read cargo.out")?; - let metadata = serde_json::from_reader( - File::open(cargo_metadata_path).context("failed to open cargo.metadata")?, +pub fn parse_cargo_out(cargo_output: &CargoOutput) -> Result> { + let metadata = serde_json::from_str(&cargo_output.cargo_metadata) + .context("failed to parse cargo metadata")?; + parse_cargo_out_str( + &cargo_output.cargo_out, + &metadata, + env::current_dir().unwrap().canonicalize().unwrap(), ) - .context("failed to parse cargo.metadata")?; - parse_cargo_out_str(&cargo_out, &metadata, env::current_dir().unwrap().canonicalize().unwrap()) } /// Parses the given `cargo.out` and `cargo.metadata` file contents and generates a list of crates diff --git a/tools/cargo_embargo/src/cargo/metadata.rs b/tools/cargo_embargo/src/cargo/metadata.rs index d9a0c33e5..5c7315d19 100644 --- a/tools/cargo_embargo/src/cargo/metadata.rs +++ b/tools/cargo_embargo/src/cargo/metadata.rs @@ -19,7 +19,6 @@ use crate::config::VariantConfig; use anyhow::{anyhow, bail, Context, Result}; use serde::Deserialize; use std::collections::BTreeMap; -use std::fs::File; use std::ops::Deref; use std::path::{Path, PathBuf}; @@ -100,14 +99,9 @@ pub enum TargetKind { Test, } -pub fn parse_cargo_metadata_file( - cargo_metadata_path: impl AsRef, - cfg: &VariantConfig, -) -> Result> { - let metadata: WorkspaceMetadata = serde_json::from_reader( - File::open(cargo_metadata_path).context("failed to open cargo.metadata")?, - ) - .context("failed to parse cargo.metadata")?; +pub fn parse_cargo_metadata_str(cargo_metadata: &str, cfg: &VariantConfig) -> Result> { + let metadata = + serde_json::from_str(cargo_metadata).context("failed to parse cargo metadata")?; parse_cargo_metadata(&metadata, &cfg.features, cfg.tests) } @@ -439,7 +433,13 @@ mod tests { .variants .iter() .map(|variant_cfg| { - parse_cargo_metadata_file(&cargo_metadata_path, variant_cfg).unwrap() + parse_cargo_metadata_str( + &read_to_string(&cargo_metadata_path) + .with_context(|| format!("Failed to open {:?}", cargo_metadata_path)) + .unwrap(), + variant_cfg, + ) + .unwrap() }) .collect::>(); assert_eq!(crates, expected_crates); diff --git a/tools/cargo_embargo/src/main.rs b/tools/cargo_embargo/src/main.rs index 14425150b..ec40d24a6 100644 --- a/tools/cargo_embargo/src/main.rs +++ b/tools/cargo_embargo/src/main.rs @@ -40,20 +40,23 @@ use anyhow::Context; use anyhow::Result; use bp::*; use cargo::{ - cargo_out::parse_cargo_out, metadata::parse_cargo_metadata_file, Crate, CrateType, ExternType, + cargo_out::parse_cargo_out, metadata::parse_cargo_metadata_str, Crate, CrateType, ExternType, }; use clap::Parser; use clap::Subcommand; use log::debug; +use nix::fcntl::OFlag; +use nix::unistd::pipe2; use once_cell::sync::Lazy; use std::collections::BTreeMap; use std::collections::VecDeque; use std::env; -use std::fs::{write, File}; -use std::io::Write; +use std::fs::{read_to_string, write, File}; +use std::io::{Read, Write}; +use std::os::fd::{FromRawFd, OwnedFd}; use std::path::Path; use std::path::PathBuf; -use std::process::Command; +use std::process::{Command, Stdio}; // Major TODOs // * handle errors, esp. in cargo.out parsing. they should fail the program with an error code @@ -286,15 +289,22 @@ fn make_crates(args: &Args, cfg: &VariantConfig) -> Result> { let cargo_out_path = "cargo.out"; let cargo_metadata_path = "cargo.metadata"; - if !args.reuse_cargo_out || !Path::new(cargo_out_path).exists() { - generate_cargo_out(cfg, cargo_out_path, cargo_metadata_path) - .context("generate_cargo_out failed")?; - } + let cargo_output = if args.reuse_cargo_out && Path::new(cargo_out_path).exists() { + CargoOutput { + cargo_out: read_to_string(cargo_out_path)?, + cargo_metadata: read_to_string(cargo_metadata_path)?, + } + } else { + let cargo_output = generate_cargo_out(cfg).context("generate_cargo_out failed")?; + write(cargo_out_path, &cargo_output.cargo_out)?; + write(cargo_metadata_path, &cargo_output.cargo_metadata)?; + cargo_output + }; if cfg.run_cargo { - parse_cargo_out(cargo_out_path, cargo_metadata_path).context("parse_cargo_out failed") + parse_cargo_out(&cargo_output).context("parse_cargo_out failed") } else { - parse_cargo_metadata_file(cargo_metadata_path, cfg) + parse_cargo_metadata_str(&cargo_output.cargo_metadata, cfg) } } @@ -375,32 +385,54 @@ fn write_all_bp( Ok(()) } -fn run_cargo(cargo_out: &mut File, cmd: &mut Command) -> Result<()> { - use std::os::unix::io::OwnedFd; - use std::process::Stdio; - let fd: OwnedFd = cargo_out.try_clone()?.into(); +/// Runs the given command, and returns its standard output and standard error as a string. +fn run_cargo(cmd: &mut Command) -> Result { + let (pipe_read, pipe_write) = pipe()?; + cmd.stdout(pipe_write.try_clone()?).stderr(pipe_write).stdin(Stdio::null()); debug!("Running: {:?}\n", cmd); - let output = cmd.stdout(Stdio::from(fd.try_clone()?)).stderr(Stdio::from(fd)).output()?; - if !output.status.success() { - bail!("cargo command failed with exit status: {:?}", output.status); + let mut child = cmd.spawn()?; + + // Unset the stdout and stderr for the command so that they are dropped in this process. + // Otherwise the `read_to_string` below will block forever as there is still an open write file + // descriptor for the pipe even after the child finishes. + cmd.stderr(Stdio::null()).stdout(Stdio::null()); + + let mut output = String::new(); + File::from(pipe_read).read_to_string(&mut output)?; + let status = child.wait()?; + if !status.success() { + bail!( + "cargo command `{:?}` failed with exit status: {:?}.\nOutput: \n------\n{}\n------", + cmd, + status, + output + ); } - Ok(()) + + Ok(output) } -/// Run various cargo commands and save the output to `cargo_out_path`. -fn generate_cargo_out( - cfg: &VariantConfig, - cargo_out_path: &str, - cargo_metadata_path: &str, -) -> Result<()> { - let mut cargo_out_file = std::fs::File::create(cargo_out_path)?; - let mut cargo_metadata_file = std::fs::File::create(cargo_metadata_path)?; +/// Creates a new pipe and returns the file descriptors for each end of it. +fn pipe() -> Result<(OwnedFd, OwnedFd), nix::Error> { + let (a, b) = pipe2(OFlag::O_CLOEXEC)?; + // SAFETY: We just created the file descriptors, so we own them. + unsafe { Ok((OwnedFd::from_raw_fd(a), OwnedFd::from_raw_fd(b))) } +} +/// The raw output from running `cargo metadata`, `cargo build` and other commands. +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct CargoOutput { + cargo_metadata: String, + cargo_out: String, +} + +/// Run various cargo commands and returns the output. +fn generate_cargo_out(cfg: &VariantConfig) -> Result { let verbose_args = ["-v"]; let target_dir_args = ["--target-dir", "target.tmp"]; // cargo clean - run_cargo(&mut cargo_out_file, Command::new("cargo").arg("clean").args(target_dir_args)) + run_cargo(Command::new("cargo").arg("clean").args(target_dir_args)) .context("Running cargo clean")?; let default_target = "x86_64-unknown-linux-gnu"; @@ -428,8 +460,7 @@ fn generate_cargo_out( }; // cargo metadata - run_cargo( - &mut cargo_metadata_file, + let cargo_metadata = run_cargo( Command::new("cargo") .arg("metadata") .arg("-q") // don't output warnings to stderr @@ -439,10 +470,10 @@ fn generate_cargo_out( ) .context("Running cargo metadata")?; + let mut cargo_out = String::new(); if cfg.run_cargo { // cargo build - run_cargo( - &mut cargo_out_file, + cargo_out += &run_cargo( Command::new("cargo") .args(["build", "--target", default_target]) .args(verbose_args) @@ -453,8 +484,7 @@ fn generate_cargo_out( if cfg.tests { // cargo build --tests - run_cargo( - &mut cargo_out_file, + cargo_out += &run_cargo( Command::new("cargo") .args(["build", "--target", default_target, "--tests"]) .args(verbose_args) @@ -463,8 +493,7 @@ fn generate_cargo_out( .args(&feature_args), )?; // cargo test -- --list - run_cargo( - &mut cargo_out_file, + cargo_out += &run_cargo( Command::new("cargo") .args(["test", "--target", default_target]) .args(target_dir_args) @@ -475,7 +504,7 @@ fn generate_cargo_out( } } - Ok(()) + Ok(CargoOutput { cargo_metadata, cargo_out }) } /// Create the Android.bp file for `package_dir`.